[release-4.14] OCPBUGS-26573: Improve troubleshooting IC upgrades #2076

ricky-rav · 2023-10-20T09:17:03Z

If ever a cluster is not making progress during phase 1 or phase 2 of the upgrade to OVN IC, a cluster admin can now edit the interconnect configmap and add a new key (fast-forward-to-multizone) to bypass the two-phase upgrade and let CNO apply directly the multizone YAMLs.

The current code only allows the cluster to move forward when it's in phase 1 by setting zone-mode=multizone and temporary=false:

Let's improve that by allowing to jump to multizone in either phase of the upgrade. At the same time, remove the temporary field from the configmap, since the ongoing-upgrade field already tracks that CNO is going through an upgrade:

openshift-ci · 2023-10-20T09:17:11Z

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

ricky-rav · 2023-10-23T09:34:01Z

/retest

ricky-rav · 2023-10-23T15:13:27Z

/retest

Signed-off-by: Riccardo Ravaioli <rravaiol@redhat.com> (cherry picked from commit 77bff544f331e14db3b7a4b5245af335c5d16a19)

The "temporary" field became redundant when "ongoing-upgrade" was added to the IC upgrade logic. Let's only keep the "ongoing-upgrade" to keep track of a configmap pushed by CNO for IC upgrade. Signed-off-by: Riccardo Ravaioli <rravaiol@redhat.com> (cherry picked from commit f3a4bff911baa5a0d33780e2cf4347712c3e6026)

ricky-rav · 2023-10-24T13:57:26Z

/retest

openshift-ci-robot · 2023-10-25T09:54:30Z

@ricky-rav: This pull request references SDN-4154 which is a valid jira issue.

In response to this:

If ever a cluster is not making progress during phase 1 or phase 2 of the upgrade to OVN IC, a cluster admin can now edit the interconnect configmap and add a new key (fast-forward-to-multizone) to bypass the two-phase upgrade and let CNO apply directly the multizone YAMLs.

The current code only allows the cluster to move forward when it's in phase 1 by setting zone-mode=multizone and temporary=false:

Let's improve that by allowing to jump to multizone in either phase of the upgrade. At the same time, remove the temporary field from the configmap, since the ongoing-upgrade field already tracks that CNO is going through an upgrade:

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.

ricky-rav · 2023-11-20T11:21:04Z

/retest

ricky-rav · 2023-11-21T10:05:08Z

/retest

tssurya

Overall logic is clear where you are trying to add a fast-forward knob. Left some inline questions...
Do we need some unit tests?
Did you do live testing and results were ok? (asking because we are kinda going in blind here...)

tssurya · 2023-12-05T10:10:27Z

pkg/network/ovn_kubernetes.go

-	// "temporary", if true, indicates that the target zone mode is only temporary;
-	// it is used along with zoneMode=singlezone in order to temporarily switch to single-zone mode when upgrading
-	// from versions with no interconnect support (<=4.13). It has no effect if zoneMode=multizone.
-	temporary bool


so this variable is now removed in 4.14.7 or whatever z stream this lands in right?
how will the new upgrades from nonIC to 4.14.z where this change lands look like? I see
remove the temporary field from the configmap, since the ongoing-upgrade field already tracks that CNO is going through an upgrade in the PR message so that means we no longer need to know where the phase is temporary or not, just need to know if upgrade is ongoing or not? why did we need the temporary flag originally? seems like it was redundant with ongoingUpgrade?

I guess my TL;DR question is is there a difference between going from 4.13.0 to 4.14.0 and 4.13.0 to 4.14.z where this lands functionally for the end user if this field is going away? Do we need to make updates to the original enhancement?

I could have simplified the logic earlier on, but I wanted to leave a window open in case we were determined to allow single zone...

Here's how it went.

The temporary field was how I envisaged to distinguish between being permanently in single zone (temporary=false) or just momentarily during the upgrade to 4.14 (temporary=true). The idea was that a configmap would track an upgrade to IC or a voluntary switch to single zone.

Then I needed another flag in the configmap, ongoing-upgrade, so that the CNO status manager could know whether an upgrade to IC was ongoing: it would only report version=4.14 when the upgrade was done, which was signaled by ongoingUpgrade getting removed from the configmap.

After 4.14 GA, we decided not to enable switching back to single zone ~~, so now we can finally get rid of temporary.~~

Well, I told a nice story, but with hindsight temporary was indeed redundant also before :-D

To finish answering your questions, the phases in the upgrade are exactly the same as before, there's no change in behaviour. I can take care of updating the enhancement, once this PR merges :)

tssurya · 2023-12-05T10:16:38Z

pkg/network/ovn_kubernetes.go

+		targetZoneMode.fastForwardToMultiZone = true
+		if targetZoneMode.zoneMode != zoneModeMultiZone {
+			klog.Warningf("Forcing interconnect zone mode to multizone due to 'fast-forward-to-multizone' being set")
+			targetZoneMode.zoneMode = zoneModeMultiZone


so this is sort of a disruptive change in case ongoingUpgrade=true and someone sets this right? example, single zone is rolling out and stuck and now we set fastForwardToMultiZone to true to unblock and move forward with multizone.. will the single zone pods be left behind or will cleanup happen?

Also I guess we can add something to the PR description or enhancement update around this option to say "at your own risk - to be used only with guidance from eng or something"

Yes, using the new flag is disruptive: CNO will push the final multizone YAMLs regardless of where the cluster is during the 4.13->4.14 upgrade. After this PR merges, my idea was to write a document about this for support. Either that or I update the enhancement...

As for the single-zone pods, they will be removed as a consequence of the new ones being pushed.

tssurya · 2023-12-05T10:20:48Z

pkg/network/ovn_kubernetes.go

-	// "temporary", if true, indicates that the target zone mode is only temporary;
-	// it is used along with zoneMode=singlezone in order to temporarily switch to single-zone mode when upgrading
-	// from versions with no interconnect support (<=4.13). It has no effect if zoneMode=multizone.
-	temporary bool
 	// "configMapFound" indicates whether the interconnect configmap was found; when not found,
 	// the zone mode defaults to multizone.
 	configMapFound bool
 	// ongoingUpgrade is true when the configmap was pushed by CNO itself; it is used by status manager


do we need to make updates to what ongoingUpgrade means? It seems like its also taking over the role that "temporary" was doing...

Actually, since we've decided that we're not allowing a cluster to be permanently in single zone, I might as well remove "ongoingUpgrade" and just refer to configMapFound, since only CNO is supposed to push the configmap now... let me try it out!

ack did you try it? all ok on this PR?

yes, I retested everything after pushing the current changes: #2076 (comment)

tssurya · 2023-12-05T10:22:11Z

pkg/network/ovn_kubernetes.go

-				"op":    "replace",
-				"path":  "/data/temporary",
-				"value": "false",
-			},
 		}


hmm maybe stupid question, but why isn't ongoing-upgrade also patched?

It's set already at the beginning of the upgrade, in the first "if" block inside prepareUpgradeToInterConnect:

cluster-network-operator/pkg/network/ovn_kubernetes.go

Line 2449 in ab2547c

"ongoing-upgrade": "",

Single-zone mode is only needed during a 4.13->4.14 upgrade. Switching back to single zone is not supported. As a consequence, we can further simplify the code and remove the "ongoing-upgrade" flag, whose purpose was to distinguish between a configmap pushed by a cluster admin to switch to single / multi zone and the configmap pushed by CNO to track the upgrade progress. Signed-off-by: Riccardo Ravaioli <rravaiol@redhat.com>

ricky-rav · 2023-12-05T18:45:53Z

I'll test the new changes tomorrow morning...

ricky-rav · 2023-12-06T08:51:58Z

/retest

ricky-rav · 2023-12-06T13:46:56Z

/retest

ricky-rav · 2023-12-12T12:53:37Z

/retest

ricky-rav · 2023-12-12T12:54:54Z

I've manually tested the new changes to fast-forward to the multizone yamls when the cluster is in phase 1 and when it's in phase 2. No surprises, the cluster converges to the new YAMLs as expected.

ricky-rav · 2023-12-19T10:43:27Z

/retest

ricky-rav · 2024-01-02T09:18:29Z

/retest

openshift-ci · 2024-02-14T20:11:56Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ricky-rav, trozet, tssurya

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

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [trozet]

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

asood-rh · 2024-02-16T13:00:21Z

/label cherry-pick-approved

openshift-ci-robot · 2024-02-16T14:25:42Z

/retest-required

Remaining retests: 0 against base HEAD acd8754 and 2 for PR HEAD 5983b44 in total

ricky-rav · 2024-02-16T16:04:15Z

/test e2e-hypershift-ovn

ricky-rav · 2024-02-16T17:31:54Z

/retest

openshift-ci · 2024-02-16T19:38:59Z

@ricky-rav: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/e2e-azure-ovn-dualstack	`5983b44`	link	false	`/test e2e-azure-ovn-dualstack`
ci/prow/e2e-openstack-kuryr	`5983b44`	link	false	`/test e2e-openstack-kuryr`
ci/prow/e2e-vsphere-ovn-dualstack-primaryv6	`5983b44`	link	false	`/test e2e-vsphere-ovn-dualstack-primaryv6`
ci/prow/security	`5983b44`	link	false	`/test security`

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/test-infra repository. I understand the commands that are listed here.

ricky-rav · 2024-02-19T08:57:08Z

/test e2e-hypershift-ovn

openshift-ci-robot · 2024-02-19T10:23:38Z

@ricky-rav: Jira Issue OCPBUGS-26573: All pull requests linked via external trackers have merged:

openshift/cluster-network-operator#2076

Jira Issue OCPBUGS-26573 has been moved to the MODIFIED state.

In response to this:

If ever a cluster is not making progress during phase 1 or phase 2 of the upgrade to OVN IC, a cluster admin can now edit the interconnect configmap and add a new key (fast-forward-to-multizone) to bypass the two-phase upgrade and let CNO apply directly the multizone YAMLs.

The current code only allows the cluster to move forward when it's in phase 1 by setting zone-mode=multizone and temporary=false:

Let's improve that by allowing to jump to multizone in either phase of the upgrade. At the same time, remove the temporary field from the configmap, since the ongoing-upgrade field already tracks that CNO is going through an upgrade:

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.

openshift-bot · 2024-02-19T11:01:30Z

[ART PR BUILD NOTIFIER]

This PR has been included in build cluster-network-operator-container-v4.14.0-202402191039.p0.g8a28156.assembly.stream.el8 for distgit cluster-network-operator.
All builds following this will include this PR.

With openshift/cluster-network-operator#2076 we added the possibility to skip the 4.13->4.14 two-phase OVNK upgrade for toubleshooting purposes and fast forward to final YAMLs for 4.14 OVNK. Also, the ConfigMap pushed by CNO got simplified. Signed-off-by: Riccardo Ravaioli <rravaiol@redhat.com>

openshift-merge-robot · 2024-02-19T22:52:00Z

Fix included in accepted release 4.14.0-0.nightly-2024-02-19-170131

With openshift/cluster-network-operator#2076 we added the possibility to skip the 4.13->4.14 two-phase OVNK upgrade for toubleshooting purposes and fast forward to final YAMLs for 4.14 OVNK. Also, the ConfigMap pushed by CNO got simplified. Signed-off-by: Riccardo Ravaioli <rravaiol@redhat.com>

ricky-rav · 2024-02-22T17:40:10Z

@anuragthehatter for the final diagrams & configmap please refer to openshift/enhancements#1567

With openshift/cluster-network-operator#2076 we added the possibility to skip the 4.13->4.14 two-phase OVNK upgrade for toubleshooting purposes and fast forward to final YAMLs for 4.14 OVNK. Also, the ConfigMap pushed by CNO got simplified. Signed-off-by: Riccardo Ravaioli <rravaiol@redhat.com>

openshift-ci-robot · 2024-03-05T10:52:34Z

@ricky-rav: Jira Issue OCPBUGS-26573 is in an unrecognized state (Closed) and will not be moved to the MODIFIED state.

In response to this:

If ever a cluster is not making progress during phase 1 or phase 2 of the upgrade to OVN IC, a cluster admin can now edit the interconnect configmap and add a new key (fast-forward-to-multizone) to bypass the two-phase upgrade and let CNO apply directly the multizone YAMLs.

The current code only allows the cluster to move forward when it's in phase 1 by setting zone-mode=multizone and temporary=false:

Let's improve that by allowing to jump to multizone in either phase of the upgrade. At the same time, remove the temporary field from the configmap, since the ongoing-upgrade field already tracks that CNO is going through an upgrade:

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.

openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Oct 20, 2023

ricky-rav marked this pull request as ready for review October 20, 2023 09:19

openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Oct 20, 2023

openshift-ci bot requested review from abhat and danwinship October 20, 2023 09:20

ricky-rav changed the title ~~[release-4.14][WIP] Improve troubleshooting IC upgrades~~ [release-4.14] Improve troubleshooting IC upgrades Oct 23, 2023

ricky-rav added 2 commits October 23, 2023 18:51

Enable fast-forwarding to multizone for troubleshooting IC upgrades

c9b793b

Signed-off-by: Riccardo Ravaioli <rravaiol@redhat.com> (cherry picked from commit 77bff544f331e14db3b7a4b5245af335c5d16a19)

ricky-rav force-pushed the debugICupgrade414 branch from a51f388 to ab2547c Compare October 23, 2023 16:51

ricky-rav changed the title ~~[release-4.14] Improve troubleshooting IC upgrades~~ [release-4.14] SDN-4154: Improve troubleshooting IC upgrades Oct 25, 2023

openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Oct 25, 2023

tssurya reviewed Dec 5, 2023

View reviewed changes

ricky-rav force-pushed the debugICupgrade414 branch from 0364ebe to 5983b44 Compare December 5, 2023 18:44

ricky-rav changed the title ~~[release-4.14] SDN-4154: Improve troubleshooting IC upgrades~~ [release-4.14] OCPBUGS-26573: Improve troubleshooting IC upgrades Jan 10, 2024

openshift-ci-robot added the jira/valid-bug Indicates that a referenced Jira bug is valid for the branch this PR is targeting. label Jan 10, 2024

openshift-ci bot added the backport-risk-assessed Indicates a PR to a release branch has been evaluated and considered safe to accept. label Feb 14, 2024

openshift-ci bot assigned anuragthehatter, asood-rh, eurijon, huiran0826, jechen0648, mffiedler, qiowang721, rbbratta and rlobillo Feb 14, 2024

openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 14, 2024

openshift-ci bot added the cherry-pick-approved Indicates a cherry-pick PR into a release branch has been approved by the release branch manager. label Feb 16, 2024

openshift-merge-bot bot merged commit 8a28156 into openshift:release-4.14 Feb 19, 2024
40 of 43 checks passed

[release-4.14] OCPBUGS-26573: Improve troubleshooting IC upgrades #2076

[release-4.14] OCPBUGS-26573: Improve troubleshooting IC upgrades #2076

Conversation

ricky-rav commented Oct 20, 2023 • edited

openshift-ci bot commented Oct 20, 2023

ricky-rav commented Oct 23, 2023

ricky-rav commented Oct 23, 2023

ricky-rav commented Oct 24, 2023

openshift-ci-robot commented Oct 25, 2023 • edited by openshift-ci bot

ricky-rav commented Nov 20, 2023

ricky-rav commented Nov 21, 2023

tssurya left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ricky-rav Dec 5, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ricky-rav commented Dec 5, 2023

ricky-rav commented Dec 6, 2023

ricky-rav commented Dec 6, 2023

ricky-rav commented Dec 12, 2023

ricky-rav commented Dec 12, 2023

ricky-rav commented Dec 19, 2023

ricky-rav commented Jan 2, 2024

openshift-ci bot commented Feb 14, 2024

asood-rh commented Feb 16, 2024

openshift-ci-robot commented Feb 16, 2024

ricky-rav commented Feb 16, 2024

ricky-rav commented Feb 16, 2024

openshift-ci bot commented Feb 16, 2024 • edited

ricky-rav commented Feb 19, 2024

openshift-ci-robot commented Feb 19, 2024

openshift-bot commented Feb 19, 2024

openshift-merge-robot commented Feb 19, 2024

ricky-rav commented Feb 22, 2024

openshift-ci-robot commented Mar 5, 2024

ricky-rav commented Oct 20, 2023 •

edited

openshift-ci-robot commented Oct 25, 2023 •

edited by openshift-ci bot

ricky-rav Dec 5, 2023 •

edited

openshift-ci bot commented Feb 16, 2024 •

edited