Upgrade changes for OVN IC #2656

vthapar · 2023-08-23T12:12:03Z

When a submariner installation upgrades from legacy non-OVN-IC version to an OVN-IC version, cleanup resources created by legacy installation.

Fixes submariner-io/enhancements#193

submariner-bot · 2023-08-23T12:12:07Z

🤖 Created branch: z_pr2656/vthapar/ovn-ic-upgrades
🚀 Full E2E won't run until the "ready-to-test" label is applied. I will add it automatically once the PR has 2 approvals, or you can add it manually.

sridhargaddam · 2023-08-24T07:09:56Z

pkg/routeagent_driver/handlers/ovn/handler.go

@@ -88,6 +88,8 @@ func (ovn *Handler) GetNetworkPlugins() []string {
 }

 func (ovn *Handler) Init() error {
+	ovn.LegacyCleanup()


LegacyCleanup is deleting the ovn-k8s-sub0 and br-submariner interfaces.
What about the flows like Logical Router policies that are programmed in the ovn_cluster_router and submariner_router ?

@aswinsuryan Can correct if my understanding is wrong, but those exist only on node where NetworkPluginSyncer was running and will be removed as part of Cleanup when NetworkPluginSyncer is "stopped" as part of upgrade process.

This LegacyCleanup is for resources that are created on all nodes as part of RouteAgent and is not covered by NetowrkPluginSyncer's cleanup.

ya that is right , the changes done here submariner-io/submariner-operator#2759 , will delete any networkplugin-syncer deployment if found and that should cleanup the routes added to ovn routers.

@aswinsuryan Are you assuming the operator will initiate the uninstall process for the networkplugin-syncer? If so, this is not (currently) the case - the operator just deletes the networkplugin-syncer deployment.

oh ok , I was thinking on delete Stop would be called. If that is not the case we will have to add it here.

Also it is not sure if OVN is going to preserve these flows that submariner have created when it changes from non-ic to ic, as it is not OVN created flows.

Good point. We have to understand how OVN-IC upgrade happens, whether it will read the etcd and change the state of the system to match it or if it uses the info from the NBDB/SBDB.

We may have to test it to see the state after an upgrade.

Normally for a regular linux interface, when its deleted the associated routes/ips on that interface are deleted. In case of OVN, when br-submariner is deleted, does the associated openflow rules get deleted or remain as stale rules?

Another way is to use a logic similar to here on startup to delete the flows. Like if the CIDR and priority matches but nethop does not delete the flow.

Are you suggesting that we do this while programming the required flows or as part of init() operation?

Ok so it sounds like you think we should do whatever legacy NPS cleanup is needed here.

Yes that is right

Also it is not sure if OVN is going to preserve these flows that submariner have created when it changes from non-ic to ic, as it is not OVN created flows.

Good point. We have to understand how OVN-IC upgrade happens, whether it will read the etcd and change the state of the system to match it or if it uses the info from the NBDB/SBDB.

We may have to test it to see the state after an upgrade.

Normally for a regular linux interface, when its deleted the associated routes/ips on that interface are deleted. In case of OVN, when br-submariner is deleted, does the associated openflow rules get deleted or remain as stale rules?

Ya right it should get deleted when we remove ovn-k8s-sub0

Another way is to use a logic similar to here on startup to delete the flows. Like if the CIDR and priority matches but nethop does not delete the flow.

Are you suggesting that we do this while programming the required flows or as part of init() operation?

As a part of init().

When I gave more thought on it , I think calling it from nps may not work too. If the OVN changes from non-IC to IC , nps cannot connect to ovn db the way it does now . Also it is not sure if OVN is going to preserve these flows that submariner have created when it changes from non-ic to ic, as it is not OVN created flows.

Is that a valid upgrade path? Upgrading from non-ic to ic before upgrading means deployment will be using an unsupported deployment of Submariner [< 0.16 with ic]. Proper way to upgrade should be to upgrade Submariner to 0.16+ before upgrading to IC coz 0.16 is only one that supports IC and non IC both.

We may have to test it to see the state after an upgrade.

But for a submariner upgrade for Opeshift < 4.14 this would work.

So, requiring a Submariner upgrade before upgrading OCP should take care of it. I am sure other projects will also have a similar requirement, upgrade them before switching OCP tp 4.14 coz older code will likely be incompatible with OCP 4.14.

Another way is to use a logic similar to here on startup to delete the flows. Like if the CIDR and priority matches but nethop does not delete the flow.

https://github.com/submariner-io/submariner/blob/devel/pkg/routeagent_driver/handlers/ovn/ovn_logical_routes.go#L78

Will this be still needed if we require submariner upgrade before OCP?

needed if we require submariner upgrade before OCP?

This should solve the cleanup issue if we add the code to remove flows during netwrok plugin uninstall. However the operator needs to discover the CNI again to move from non-IC to IC. Currently, we do not support on-the-fly change from non-ic mode to IC mode.

Needs to handle route delete too.

vthapar · 2023-08-25T03:56:41Z

@aswinsuryan Delete ip routes or logicalrouteer policies?

aswinsuryan · 2023-08-25T13:03:08Z

@aswinsuryan Delete ip routes or logicalrouteer policies?

Logical router policies and static routes in Ovn cluster router I mean

When a submariner installation upgrades from legacy non-OVN-IC version to an OVN-IC version, cleanup resources created by legacy installation. Fixes submariner-io/enhancements#193 Signed-off-by: Vishal Thapar <[email protected]>

vthapar · 2023-08-30T14:03:38Z

@aswinsuryan OVN tests failing. Failed to deploy kind with OVN.

2023-08-30T13:53:59.2282190Z �[36m[13:53:59.227] [dir=submariner]$ exit_error Failed to create clusters using 'kind'.�[0m
2023-08-30T13:53:59.2294776Z ERROR: Failed to create clusters using 'kind'.
2023-08-30T13:53:59.2321587Z �[36m[13:53:59.231] [dir=submariner; fn=exit_error]$ exit 1�[0m
2023-08-30T13:53:59.2346922Z make[1]: *** [/opt/shipyard/Makefile.inc:188: clusters] Error 1
2023-08-30T13:53:59.2347631Z make[1]: Leaving directory '/go/src/github.com/submariner-io/submariner'

aswinsuryan · 2023-08-30T16:04:56Z

@aswinsuryan OVN tests failing. Failed to deploy kind with OVN.

2023-08-30T13:53:59.2282190Z �[36m[13:53:59.227] [dir=submariner]$ exit_error Failed to create clusters using 'kind'.�[0m
2023-08-30T13:53:59.2294776Z ERROR: Failed to create clusters using 'kind'.
2023-08-30T13:53:59.2321587Z �[36m[13:53:59.231] [dir=submariner; fn=exit_error]$ exit 1�[0m
2023-08-30T13:53:59.2346922Z make[1]: *** [/opt/shipyard/Makefile.inc:188: clusters] Error 1
2023-08-30T13:53:59.2347631Z make[1]: Leaving directory '/go/src/github.com/submariner-io/submariner'

This should fix it

submariner-io/shipyard#1351

submariner-bot · 2023-08-30T18:36:28Z

🤖 Closed branches: [z_pr2656/vthapar/ovn-ic-upgrades]

vthapar requested review from Oats87, skitt, sridhargaddam and tpantelis as code owners August 23, 2023 12:12

vthapar force-pushed the ovn-ic-upgrades branch from 439302b to eba8298 Compare August 23, 2023 12:12

vthapar requested a review from aswinsuryan August 23, 2023 12:14

vthapar self-assigned this Aug 23, 2023

vthapar force-pushed the ovn-ic-upgrades branch 2 times, most recently from 3384efa to 36ef528 Compare August 23, 2023 12:48

aswinsuryan previously approved these changes Aug 23, 2023

View reviewed changes

sridhargaddam reviewed Aug 24, 2023

View reviewed changes

Upgrade changes for OVN IC

af45983

When a submariner installation upgrades from legacy non-OVN-IC version to an OVN-IC version, cleanup resources created by legacy installation. Fixes submariner-io/enhancements#193 Signed-off-by: Vishal Thapar <[email protected]>

vthapar force-pushed the ovn-ic-upgrades branch from 36ef528 to af45983 Compare August 30, 2023 03:08

vthapar requested review from sridhargaddam and aswinsuryan August 30, 2023 03:08

aswinsuryan approved these changes Aug 30, 2023

View reviewed changes

sridhargaddam approved these changes Aug 30, 2023

View reviewed changes

submariner-bot added the ready-to-test When a PR is ready for full E2E testing label Aug 30, 2023

skitt approved these changes Aug 30, 2023

View reviewed changes

skitt enabled auto-merge (rebase) August 30, 2023 13:27

skitt merged commit a2fe53d into submariner-io:devel Aug 30, 2023
39 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Upgrade changes for OVN IC #2656

Upgrade changes for OVN IC #2656

vthapar commented Aug 23, 2023

submariner-bot commented Aug 23, 2023

sridhargaddam Aug 24, 2023

vthapar Aug 24, 2023

aswinsuryan Aug 24, 2023

tpantelis Aug 24, 2023 •

edited

Loading

aswinsuryan Aug 24, 2023

sridhargaddam Aug 24, 2023

aswinsuryan Aug 25, 2023

aswinsuryan Aug 25, 2023

vthapar Aug 27, 2023

aswinsuryan Aug 29, 2023

vthapar commented Aug 25, 2023

aswinsuryan commented Aug 25, 2023

vthapar commented Aug 30, 2023

aswinsuryan commented Aug 30, 2023

submariner-bot commented Aug 30, 2023

Upgrade changes for OVN IC #2656

Upgrade changes for OVN IC #2656

Conversation

vthapar commented Aug 23, 2023

submariner-bot commented Aug 23, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tpantelis Aug 24, 2023 • edited Loading

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

vthapar commented Aug 25, 2023

aswinsuryan commented Aug 25, 2023

vthapar commented Aug 30, 2023

aswinsuryan commented Aug 30, 2023

submariner-bot commented Aug 30, 2023

tpantelis Aug 24, 2023 •

edited

Loading