Skip to content

CNV-7522 - nmstate more config example#26524

Merged
aburdenthehand merged 1 commit intoopenshift:masterfrom
aburdenthehand:cnv-7521-nmstate-examples
Nov 6, 2020
Merged

CNV-7522 - nmstate more config example#26524
aburdenthehand merged 1 commit intoopenshift:masterfrom
aburdenthehand:cnv-7521-nmstate-examples

Conversation

@aburdenthehand
Copy link
Contributor

@aburdenthehand aburdenthehand commented Oct 16, 2020

Adding some new nmstate configuration examples and snippets, which required a bit of rejigging of current content
And it looks like I name my branch the epic ID. Oh well.

https://issues.redhat.com/browse/CNV-7522

Build link: https://cnv-7521-nmstate-examples--ocpdocs.netlify.app/openshift-enterprise/latest/virt/node_network/virt-updating-node-network-config.html

@openshift-ci-robot openshift-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Oct 16, 2020
@openshift-docs-preview-bot

The preview will be available shortly at:

@aburdenthehand aburdenthehand changed the title WIP CNV-7522 - nmstate more config example CNV-7522 - nmstate more config example Oct 23, 2020
@openshift-ci-robot openshift-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Oct 23, 2020
@RamLavi
Copy link
Contributor

RamLavi commented Oct 25, 2020

/lgtm

@openshift-ci-robot
Copy link

@RamLavi: changing LGTM is restricted to collaborators

Details

In response to this:

/lgtm

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

Copy link

@yossisegev yossisegev left a comment

Choose a reason for hiding this comment

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

Besides the single typo - LGTM

Copy link
Contributor

@sjhala-ccs sjhala-ccs left a comment

Choose a reason for hiding this comment

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

A few minor things, but otherwise LGTM!

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think that "snippet" is necessary when you're already saying "example".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Despite what these changes may suggest, I actually don't much like the term 'snippet'. However I do think it's important to be very specific here between what is a complete example configuration (also featured in the assembly), and what is only a part of a configuration. nmstate is pretty powerful and these policies can alter the network of the whole cluster so I guess this is my prudence showing :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Cleaned up some of the flagrant usage. Now only used in leading para specific to the config

Copy link
Contributor

Choose a reason for hiding this comment

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

Just curious, what is the purpose of the [discrete] label?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It prevents the title from showing in the TOC.
I used it here so that I could xref to the different policy examples after the early procedure where it's relevant, but having additional resources turn up as entry 3 in the TOC didn't look right (and those elements are already in the TOC, so it's only relevant to people who are looking at that particular procedure.

Copy link
Member

@ousleyp ousleyp left a comment

Choose a reason for hiding this comment

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

Some nits, but overall this is looking good!

Copy link
Member

Choose a reason for hiding this comment

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

Question that I have asked you before, but that might have a new answer now that the formatting guidelines have been updated: should Policy be capitalized throughout this document? If yes, should it be in backticks?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good question. Not in backticks, as 'Policy' is only part of the full object name. I'm reluctant to drop it down to 'policy' though because we also have other policies that impact VMs. Maybe using the acronym (NNCP) makes the most sense but runs into the danger of blurring with NNCE, and that distinction should remain clear.
I'd like to keep capitalised for now as I think that makes the most sense for now, but I've raised an item for our next meeting for addressing the decapitalisation of objects in our docs en masse.

…quired a bit of rejigging of current content
@aburdenthehand aburdenthehand force-pushed the cnv-7521-nmstate-examples branch from cb64cbe to fca6c95 Compare November 6, 2020 12:42
@aburdenthehand aburdenthehand merged commit 71e73ed into openshift:master Nov 6, 2020
@aburdenthehand
Copy link
Contributor Author

aburdenthehand commented Nov 6, 2020

/cherry-pick enterprise-4.5

@aburdenthehand
Copy link
Contributor Author

aburdenthehand commented Nov 6, 2020

/cherry-pick enterprise-4.6

@openshift-cherrypick-robot
Copy link

openshift-cherrypick-robot commented Nov 6, 2020

@aburdenthehand: new pull request created: #27144

Details

In response to this:

/cherry-pick enterprise-4.5

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@aburdenthehand
Copy link
Contributor Author

aburdenthehand commented Nov 6, 2020

/cherry-pick enterprise-4.7

@openshift-cherrypick-robot
Copy link

openshift-cherrypick-robot commented Nov 6, 2020

@aburdenthehand: new pull request created: #27145

Details

In response to this:

/cherry-pick enterprise-4.6

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@openshift-cherrypick-robot
Copy link

openshift-cherrypick-robot commented Nov 6, 2020

@aburdenthehand: new pull request created: #27146

Details

In response to this:

/cherry-pick enterprise-4.7

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

branch/enterprise-4.5 branch/enterprise-4.6 branch/enterprise-4.7 CNV Label for all CNV PRs size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants