-
Notifications
You must be signed in to change notification settings - Fork 1.8k
OCPBUGS-56925: Updated the creating-manifest-file-customized-br-ex-br… #94273
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
base: main
Are you sure you want to change the base?
Conversation
@dfitzmau: This pull request references Jira Issue OCPBUGS-56925, which is invalid:
Comment The bug has been updated to refer to the pull request using the external bug tracker. In response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
e9a8bdd
to
e20c9f4
Compare
The This is because your PR targets the If the update in your PR does NOT apply to version 4.20 onward, please re-target this PR to go directly into the appropriate version branch or branches (enterprise-4.x) instead of main. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
<3> If you have a single global configuration specified in an `/etc/nmstate/openshift/cluster.yml` configuration file that you want to apply to all nodes in your cluster, you do not need to specify the hostname path for each node, such as `/etc/nmstate/openshift/<node_hostname>.yml`. The `worker` role is the default role for nodes in your cluster. The `.yaml` extension does not work when specifying the hostname path for each node or all nodes in the `MachineConfig` manifest file. | ||
<4> For each node in your cluster, specify the hostname path to your node and the base-64 encoded Ignition configuration file data for the machine type. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might be nice if you could could show an example of not specifying a hostname path for one of the two example.
Also, I find it a bit confusing, in <3> and <4> are the same thing (right), yet the wording is completely different.
<3> If you have a single global configuration specified in an `/etc/nmstate/openshift/cluster.yml` configuration file that you want to apply to all nodes in your cluster, you do not need to specify the hostname path for each node, such as `/etc/nmstate/openshift/<node_hostname>.yml`. The `worker` role is the default role for nodes in your cluster. The `.yaml` extension does not work when specifying the hostname path for each node or all nodes in the `MachineConfig` manifest file. | |
<4> For each node in your cluster, specify the hostname path to your node and the base-64 encoded Ignition configuration file data for the machine type. | |
<3> For each node in your cluster, specify the hostname path to your node and the base-64 encoded Ignition configuration file data for the machine type. The `.yaml` extension does not work when specifying the hostname path. | |
+ | |
Alternatively, If you have a single global configuration specified in an `/etc/nmstate/openshift/cluster.yml` configuration file that you want to apply to all nodes in your cluster, you do not need to specify the hostname path for each node. The `worker` role is the default role for nodes in your cluster. For example: | |
+ | |
[source,yaml] | |
---- | |
- contents: | |
source: data:text/plain;charset=utf-8;base64,<base64_encoded_nmstate_configuration> | |
mode: 0644 | |
overwrite: true | |
---- |
(or whatever that ^^ is supposed to look like.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great suggestion, @mburke5678 . I implemented what you suggestion. Fresh eyes are always good thing!
@dfitzmau Just a couple random thoughts. Maybe I am missing something. Otherwise LGTM |
…idge.adoc for individual hostname paths
New changes are detected. LGTM label has been removed. |
@dfitzmau: all tests passed! Full PR test history. Your PR dashboard. 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-sigs/prow repository. I understand the commands that are listed here. |
Version(s):
4.16+
Issue:
OCPBUGS-56925
Link to docs preview: