Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove helm NicClusterPolicy install from docs #67

Merged
merged 1 commit into from
Jul 11, 2024

Conversation

killianmuldoon
Copy link
Contributor

Remove the instructions for installing the NicClusterPolicy through helm from the documentation and replace it with a two step process that involves applying the NicClusterPolicy directly using kubectl.

This feature is deprecated and is going to be removed from the Network Operator helm chart in a future release

@killianmuldoon killianmuldoon force-pushed the pr-remove-helm-cr-from-docs branch 5 times, most recently from 858a708 to 2650643 Compare July 9, 2024 15:38
@killianmuldoon killianmuldoon force-pushed the pr-remove-helm-cr-from-docs branch from 2650643 to d8aa53c Compare July 9, 2024 15:45
@killianmuldoon killianmuldoon changed the title [WIP] Remove helm NicClusterPolicy install from docs Remove helm NicClusterPolicy install from docs Jul 9, 2024
deploy: false
Note: You may need to change the interface names in the NicClusterPolicy to those used by your target nodes.

.. parsed-literal::
Copy link
Member

@tobiasgiese tobiasgiese Jul 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
.. parsed-literal::
.. code-block:: yaml

shouldn't this be rather a yaml code block? And all other occurrences

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately no - as we need to substitute the version numbers here. That functionality doesn't work with code-block. So here we're trading off the variable substitution for syntax highlighting.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#28

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, wasn't aware of that. Thanks

@rollandf rollandf merged commit aee680e into Mellanox:main Jul 11, 2024
1 check passed
@noama-nv
Copy link
Collaborator

@rollandf @adrianchiris please add me as reviewer for upcoming doc changes.

here few comments we should get address in upcoming changed:

  • Adjust doc to NFD install externally to Network Operator.
  • We set NV IPAM as our default IPAM but documentation still vastly refer to whereabouts

@adrianchiris
Copy link
Collaborator

@noama-nv please star the project to get notifications. you can then get notified on changes.

@noama-nv
Copy link
Collaborator

@adrianchiris i am, but in this case it was reviewed and merge at the speed of light :)
think you should include me in default reviewers

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants