Bug 1937594: Split SDN migration into 2 phase

pkg/controller/operconfig/operconfig_controller.go

@@ -175,17 +175,14 @@ func (r *ReconcileOperConfig) Reconcile(request reconcile.Request) (reconcile.Re
 	// Compare against previous applied configuration to see if this change
 	// is safe.
 	if prev != nil {
-		// Check if the operator is put in the 'Network Migration' mode.
-		if _, ok := operConfig.GetAnnotations()[names.NetworkMigrationAnnotation]; !ok {
-			// We may need to fill defaults here -- sort of as a poor-man's
-			// upconversion scheme -- if we add additional fields to the config.
-			err = network.IsChangeSafe(prev, &operConfig.Spec)
-			if err != nil {
-				log.Printf("Not applying unsafe change: %v", err)
-				r.status.SetDegraded(statusmanager.OperatorConfig, "InvalidOperatorConfig",
-					fmt.Sprintf("Not applying unsafe configuration change: %v. Use 'oc edit network.operator.openshift.io cluster' to undo the change.", err))
-				return reconcile.Result{}, err
-			}
+		// We may need to fill defaults here -- sort of as a poor-man's
+		// upconversion scheme -- if we add additional fields to the config.
+		err = network.IsChangeSafe(prev, &operConfig.Spec)
+		if err != nil {
+			log.Printf("Not applying unsafe change: %v", err)
+			r.status.SetDegraded(statusmanager.OperatorConfig, "InvalidOperatorConfig",
+				fmt.Sprintf("Not applying unsafe configuration change: %v. Use 'oc edit network.operator.openshift.io cluster' to undo the change.", err))
+			return reconcile.Result{}, err
 		}
 	}
 

pkg/network/cluster_config.go

@@ -164,5 +164,11 @@ func StatusFromOperatorConfig(operConf *operv1.NetworkSpec, oldStatus *configv1.
 		status.ClusterNetworkMTU = int(*operConf.DefaultNetwork.KuryrConfig.MTU)
 	}
 
+	// Set migration in the config status
+	if operConf.Migration != nil {
+		status.Migration = &configv1.NetworkMigration{NetworkType: string(operConf.Migration.NetworkType)}
+	} else {
+		status.Migration = nil
+	}
 	return &status
 }

pkg/network/render.go

@@ -232,6 +232,9 @@ func IsChangeSafe(prev, next *operv1.NetworkSpec) error {
 		errs = append(errs, err)
 	}
 
+	// Check the network migration
+	errs = append(errs, isMigrationChangeSafe(prev, next)...)
+
 	// Check the default network
 	errs = append(errs, isDefaultNetworkChangeSafe(prev, next)...)
 
@@ -456,20 +459,37 @@ func fillDefaultNetworkDefaults(conf, previous *operv1.NetworkSpec, hostMTU int)
 }
 
 func isDefaultNetworkChangeSafe(prev, next *operv1.NetworkSpec) []error {
+
 	if prev.DefaultNetwork.Type != next.DefaultNetwork.Type {
-		return []error{errors.Errorf("cannot change default network type")}
+		if prev.Migration == nil {
+			return []error{errors.Errorf("cannot change default network type when not doing migration")}
+		} else {
+			if prev.Migration.NetworkType != next.DefaultNetwork.Type {
+				return []error{errors.Errorf("can only change default network type to the target migration network type")}
+			}
+		}
 	}
 
-	switch prev.DefaultNetwork.Type {
-	case operv1.NetworkTypeOpenShiftSDN:
-		return isOpenShiftSDNChangeSafe(prev, next)
-	case operv1.NetworkTypeOVNKubernetes:
-		return isOVNKubernetesChangeSafe(prev, next)
-	case operv1.NetworkTypeKuryr:
-		return isKuryrChangeSafe(prev, next)
-	default:
-		return nil
+	if prev.Migration == nil {
+		switch prev.DefaultNetwork.Type {
+		case operv1.NetworkTypeOpenShiftSDN:
+			return isOpenShiftSDNChangeSafe(prev, next)
+		case operv1.NetworkTypeOVNKubernetes:
+			return isOVNKubernetesChangeSafe(prev, next)
+		case operv1.NetworkTypeKuryr:
+			return isKuryrChangeSafe(prev, next)
+		default:
+			return nil
+		}
+	}
+	return nil
+}
+
+func isMigrationChangeSafe(prev, next *operv1.NetworkSpec) []error {
+	if prev.Migration != nil && next.Migration != nil && prev.Migration.NetworkType != next.Migration.NetworkType {
+		return []error{errors.Errorf("cannot change migration network type after migration is start")}
 	}
+	return nil
 }
 
 // ValidateAdditionalNetworks validates additional networks configs

pkg/network/render_test.go

@@ -46,7 +46,7 @@ func TestIsChangeSafe(t *testing.T) {
 	FillDefaults(next, nil)
 	next.DefaultNetwork.Type = "Kuryr"
 	err = IsChangeSafe(prev, next)
-	g.Expect(err).To(MatchError(ContainSubstring("cannot change default network type")))
+	g.Expect(err).To(MatchError(ContainSubstring("cannot change default network type when not doing migration")))
 
 	// You can change a single-stack config to dual-stack
 	next = OpenShiftSDNConfig.Spec.DeepCopy()
@@ -112,6 +112,22 @@ func TestIsChangeSafe(t *testing.T) {
 	)
 	err = IsChangeSafe(prev, next)
 	g.Expect(err).To(MatchError(ContainSubstring("cannot change ClusterNetwork")))
+
+	// You can't change default network type to non-target migration network type
+	next = OpenShiftSDNConfig.Spec.DeepCopy()
+	FillDefaults(next, nil)
+	prev.Migration = &operv1.NetworkMigration{NetworkType: "OVNKubernetes"}
+	next.DefaultNetwork.Type = "Kuryr"
+	err = IsChangeSafe(prev, next)
+	g.Expect(err).To(MatchError(ContainSubstring("can only change default network type to the target migration network type")))
+
+	// You can't change the migration network type when it is not null.
+	next = OpenShiftSDNConfig.Spec.DeepCopy()
+	FillDefaults(next, nil)
+	next.Migration = &operv1.NetworkMigration{NetworkType: "OVNKubernetes"}
+	prev.Migration = &operv1.NetworkMigration{NetworkType: "Kuryr"}
+	err = IsChangeSafe(prev, next)
+	g.Expect(err).To(MatchError(ContainSubstring("cannot change migration network type after migration is start")))
 }
 
 func TestRenderUnknownNetwork(t *testing.T) {