diff --git a/pkg/controller/operconfig/operconfig_controller.go b/pkg/controller/operconfig/operconfig_controller.go index 77b2f90ef4..5da9d5a4eb 100644 --- a/pkg/controller/operconfig/operconfig_controller.go +++ b/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 } } diff --git a/pkg/network/cluster_config.go b/pkg/network/cluster_config.go index 057a0bd376..ce1614f8f9 100644 --- a/pkg/network/cluster_config.go +++ b/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 } diff --git a/pkg/network/render.go b/pkg/network/render.go index d2bb6f2dc1..b93b2257cb 100644 --- a/pkg/network/render.go +++ b/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 diff --git a/pkg/network/render_test.go b/pkg/network/render_test.go index 00bfa9d08a..ca9fbc2140 100644 --- a/pkg/network/render_test.go +++ b/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) {