Skip to content

Conversation

@juanluisvaladas
Copy link

For SDN-1365. Proposes changes to CNO to make it possible for customers to change the MTU, geneve port, and vxlan port in both OVN Kubernetes, and openshift SDN.

/cc @mccv1r0
For MTU changes
/cc @trozet
For OVN kubernetes
/cc @danwinship @knobunc @abhat
For general thoughts


`ovs-vsctl --if-exists del-port br0 vxlan0`

And create it again with no delay at all:
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this procedure actually already tested or is this just a guess/placeholder? If the latter, you don't need to specify it in that much detail here.

Copy link
Author

Choose a reason for hiding this comment

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

I tested this manually. Should I make it less detailed anyway?

Copy link

Choose a reason for hiding this comment

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

@juanluisvaladas I'd like to leave detailed steps where possible. Could be helpful for QE to understand the flow of the things. Right now it seems useful to understand what's being planned.

@juanluisvaladas juanluisvaladas force-pushed the change-mtu-port branch 2 times, most recently from 7080c3f to 209a65e Compare February 3, 2021 14:30
@juanluisvaladas
Copy link
Author

Just fixed a problem with markdownlint, the actual content doesn't change at all.

- Detail when will the tests will be run
- Do the preconditions in a separate daemonset.
- Make all the API calls in CNO instead of doing them in the pods ran on
  each node.
@russellb
Copy link
Contributor

I'm good with the direction of this. I'll leave it to someone from the SDN team to lgtm the details of the plan.

/approve

@openshift-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: russellb

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 15, 2021
Copy link

@kedark3 kedark3 left a comment

Choose a reason for hiding this comment

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

Hi @juanluisvaladas thanks for providing detailed info on enhancements. Some minor spelling nitpicks. Hope you don't mind. Thanks.

approvers:
- TBD
creation-date: 2021-01-25
last-updated: 2022-02-012
Copy link

Choose a reason for hiding this comment

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

Request: s/012/12/


Customers may need to change the MTU, or the ports used for VXLAN or Geneve
tunnels post-installation. However these changes aren't trivial and may cause
downtime, hence the CNO forbids currently them.
Copy link

Choose a reason for hiding this comment

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

Request: s/the CNO forbids currently them/forbids them currently/

* Allow to change the overlay network ports on the underlay in both OpenShift
SDN and OVN Kubernetes.

* Allow to change both VXLAN and Geneve ports porst install.
Copy link

Choose a reason for hiding this comment

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

Request: s/ ports porst install./ ports post install./

1. Set the `clusteroperator/network` conditions:
- Progressing: true
- Upgradeable: false
2. Deploy a deaemonset with `restartPolicy: Mever` which is responsible for
Copy link

Choose a reason for hiding this comment

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

Request: s/restartPolicy: Mever/restartPolicy: Never


Once the preconditions are met the steps to change the MTU and the ports are
different, for MTU changes the CNO will:
1. Cordon every node, we don't want pods created during the process.
Copy link

Choose a reason for hiding this comment

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

Sorry for naive question, is it a rolling cordon or all at once ?

1. Cordon every node, we don't want pods created during the process.
2. Deploy a new daemonset that will run on every node which will apply the
changes that must be done manually in that node in particular.
3. So that we don't have nodes doing things a t different times and we have
Copy link

Choose a reason for hiding this comment

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

s/things a t different/things at different/

1. Cordon every node, we don't want pods created during the process.
2. Deploy a new daemonset that will run on every node which will apply the
changes that must be done manually in that node in particular.
3. So that we don't have nodes doing things a t different times and we have
Copy link

Choose a reason for hiding this comment

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

s/things a t different/things at different/


`ovs-vsctl --if-exists del-port br0 vxlan0`

And create it again with no delay at all:
Copy link

Choose a reason for hiding this comment

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

@juanluisvaladas I'd like to leave detailed steps where possible. Could be helpful for QE to understand the flow of the things. Right now it seems useful to understand what's being planned.

kept alive, and the short lived connections get established.

Packet loss, TCP retransmissions, increased latency, and reduced bandwidth are
considered acceptable while the chane is happening.
Copy link

Choose a reason for hiding this comment

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

s/the chane/the change/

@kedark3
Copy link

kedark3 commented Apr 12, 2021

Maybe a good idea to close this as a new one #730 is now opened.

@russellb
Copy link
Contributor

Maybe a good idea to close this as a new one #730 is now opened.

can you clarify why a new enhancement was opened instead of updating this one?

@russellb
Copy link
Contributor

original author of this enhancement is no longer working on this

/close

@openshift-ci-robot
Copy link

@russellb: Closed this PR.

Details

In response to this:

original author of this enhancement is no longer working on this

/close

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

approved Indicates a PR has been approved by an approver from all required OWNERS files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants