-
Notifications
You must be signed in to change notification settings - Fork 1.8k
OCPBUGS-51174: Updated the changing-cluster-network-mtu file with NIC… #89757
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: No Jira issue with key OCPBUGS-55174 exists in the tracker at https://issues.redhat.com/. 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. |
@dfitzmau: No Jira issue with key OCPBUGS-55174 exists in the tracker at https://issues.redhat.com/. 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. |
769f37d
to
b5eccd9
Compare
🤖 Fri Mar 21 16:56:58 - Prow CI generated the docs preview: |
@dfitzmau: No Jira issue with key OCPBUGS-55174 exists in the tracker at https://issues.redhat.com/. 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. |
c087ec1
to
5a171ac
Compare
ada3169
to
7c95b15
Compare
modules/nw-cluster-mtu-change.adoc
Outdated
[source,ini] | ||
---- | ||
[connection-<primary-NIC-bond-interface>-mtu] | ||
match-device=interface-name:<bond-iface-name> |
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.
There are normally at least 2 interfaces assigned to a bond, I think it would be more clear to show two.
This solution article How to change bond MTU shows 2 slave interfaces.
To make this truly be an "Example", it might be more helpful to use eth0 and eth1:
[bond0-mtu]
match-device=interface-name:bond0
ethernet.mtu=9000
[connection-eth0-mtu]
match-device=interface-name:eth0
ethernet.mtu=9000
[connection-eth1-mtu]
match-device=interface-name:eth1
ethernet.mtu=9000
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.
Thanks for that info. Updated!
@dfitzmau: This pull request references Jira Issue OCPBUGS-51174, 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. |
LGTM |
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.
Mostly LGTM! some comments and suggestions below
modules/nw-cluster-mtu-change.adoc
Outdated
`<machine_to>`:: Specifies the MTU for the primary network interface on the underlying host network. | ||
-- | ||
<1> Where `<overlay_from>` specifies the current cluster network MTU value. | ||
<2> Where `<overlay_to>` specifies the target MTU for the cluster network. This value is set relative to the value of |
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.
This callout is cut off
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.
✔️
/retest |
Looks good to me. I would like @jcaamano to take a look as well |
+ | ||
ifndef::outposts[] | ||
.Example that increases the cluster MTU | ||
[source,terminal] | ||
---- | ||
$ oc patch Network.operator.openshift.io cluster --type=merge --patch \ | ||
'{"spec": { "migration": { "mtu": { "network": { "from": 1400, "to": 9000 } , "machine": { "to" : 9100} } } } }' | ||
'{"spec": { "migration": { "mtu": { "network": { "from": 1400, "to": 8900 } , "machine": { "to" : 9000} } } } }' |
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.
This change stemmed from https://issues.redhat.com/browse/OSDOCS-13689.
@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. |
---- | ||
<1> Where `<node_name>` specifies the name of a node in your cluster. | ||
<2> Where `ovs-if-phys0` is the primary network interface. For nodes that use multiple NIC bonds, append `bond-sub0` for the primary NIC bond interface and `bond-sub1` for the secondary NIC bond interface. |
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.
Append to what? Where does bond-sub0
and bond-sub1
come from?
where: | ||
[NOTE] | ||
==== | ||
For nodes that use a network interface controller (NIC) bond interface, list the bond interface and any sub-interfaces in the `<bond-interface>-mtu.conf` file. |
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.
Where does this information come from? The MTU of the sub-interfaces should automatically acquire the same MTU of the bond interface.
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.
I don't see the <bond-interface>-mtu.conf
mentioned before in this document.
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.
@jcaamano There are 2 things that come to mind with respect to the procedure as written:
-
The docs does not address bonded NICs or VLANs at all. It is left as an exercise for the reader to extrapolate the documented steps, covering a single NIC, to multiple NICs under a bond
-
The OpenShift agent based installer for bare metal systems allows for the MTU to be specified on each "interface"; sub NICs, bond, VLAN. If, during install, we set the MTU at all 3 levels, for some reason, and then we apply the MTU change procedure only to the sub NICs of the bond, will the bond and the vlan interfaces inherit the new sub NIC configuration, or, will the fact that I specifically set the MTU level at all 3 places during install mean that the sub NICs have the new MTU but the bond and vlan interfaces have the old MTU because there was an actual setting provided and not just inherited?
If on the other hand, changing the sub NIC interfaces' MTU would propagate then item 1 is really the primary concern. Leaving the extrapolation of the procedure as an exercise for the reader is risky and actually did impact one of our systems.
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.
- What do you feel this doc need to address about bonds that is missing? To my knowledge, the procedure would be exactly the same with bonds.
- You don't need to specify the MTU of the subordinates of the bond. They automatically acquire the MTU of the parent. Maybe this is what is being the problem but unknown without taking a deeper look. In any case, it would be impossible for this procedure to cover, or be aware of, all possible network configurations. I had already submitted a PR to add that disclaimer to this doc, I will dig that PR up.
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.
Disclaimer Jira: https://issues.redhat.com/browse/OCPBUGS-17933
<2> Specify the local filename for the updated NetworkManager configuration file from the previous step. | ||
|
||
.... Create the following Butane config in the `worker-interface.bu` file: | ||
<2> Specify the local filename for the updated NetworkManager configuration file from the previous step. For NIC bonds, specify the name for the `<bond-interface>-mtu.conf` file. |
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.
I don't see why <interface>
would be different than <bond-interface>
Version(s):
4.12+
Issue:
OCPBUGS-51174 and OSDOCS-13689
Link to docs preview:
More clarifications needed.