MCO-1282: Remove all code related to the Image Registry workaround Config Map

install/0000_90_machine-config_01_prometheus-rules.yaml

@@ -1,30 +1,5 @@
 apiVersion: monitoring.coreos.com/v1
 kind: PrometheusRule
-metadata:
-  name: machine-config-operator
-  namespace: openshift-machine-config-operator
-  labels:
-    k8s-app: machine-config-operator
-  annotations:
-    include.release.openshift.io/ibm-cloud-managed: "true"
-    include.release.openshift.io/self-managed-high-availability: "true"
-    include.release.openshift.io/single-node-developer: "true"
-spec:
-  groups:
-    - name: drain-override-configmap-present
-      rules:
-        - alert: MCODrainOverrideConfigMapAlert
-          expr: |
-            mco_image_registry_drain_override_exists > 0
-          labels:
-            namespace: openshift-machine-config-operator
-            severity: warning
-          annotations:
-            summary: "Alerts the user to the presence of a drain override configmap that is being deprecated and removed in a future release."
-            description: "Image Registry Drain Override configmap has been detected. Please use the Node Disruption Policy feature to control the cluster's drain behavior as the configmap method is currently deprecated and will be removed in a future release."
----
-apiVersion: monitoring.coreos.com/v1
-kind: PrometheusRule
 metadata:
   name: machine-config-controller
   namespace: openshift-machine-config-operator

pkg/daemon/constants/constants.go

@@ -141,10 +141,6 @@ const (
 	// changes to this path. Note that other files added to the parent directory will not be handled specially
 	GPGNoRebootPath = "/etc/machine-config-daemon/no-reboot/containers-gpg.pub"
 
-	// ImageRegistryDrainOverrideConfigmap is the name of the Configmap a user can apply to force all
-	// image registry changes to not drain
-	ImageRegistryDrainOverrideConfigmap = "image-registry-override-drain"
-
 	// rpm-ostree command arguments
 	RPMOSTreeUpdateArg    = "update"
 	RPMOSTreeInstallArg   = "--install"

pkg/daemon/daemon.go

@@ -1139,7 +1139,7 @@ func (dn *Daemon) syncNodeHypershift(key string) error {
 	}
 
 	// Check and perform node drain if required
-	drain, err := isDrainRequired(actions, diffFileSet, oldIgnConfig, newIgnConfig, false)
+	drain, err := isDrainRequired(actions, diffFileSet, oldIgnConfig, newIgnConfig)
 	if err != nil {
 		return err
 	}

pkg/daemon/drain.go

@@ -128,17 +128,13 @@ func (dn *Daemon) performDrain() error {
 }
 
 // isDrainRequiredForNodeDisruptionActions determines whether node drain is required or not to apply config changes for this set of NodeDisruptionActions
-func isDrainRequiredForNodeDisruptionActions(actions []opv1.NodeDisruptionPolicyStatusAction, oldIgnConfig, newIgnConfig ign3types.Config, overrideImageRegistryDrain bool) (bool, error) {
+func isDrainRequiredForNodeDisruptionActions(actions []opv1.NodeDisruptionPolicyStatusAction, oldIgnConfig, newIgnConfig ign3types.Config) (bool, error) {
 	klog.Infof("Checking drain required for node disruption actions")
 	if apihelpers.CheckNodeDisruptionActionsForTargetActions(actions, opv1.RebootStatusAction, opv1.DrainStatusAction) {
 		// We definitely want to perform drain for these cases
 		return true, nil
 	} else if apihelpers.CheckNodeDisruptionActionsForTargetActions(actions, opv1.SpecialStatusAction) {
 		// This is a specially reserved action for "/etc/containers/registries.conf" and for this action, drain may or may not be necessary
-		if overrideImageRegistryDrain {
-			klog.Warningf("Drain was skipped for this image registry update due to the configmap %s being present. This may not be a safe change", constants.ImageRegistryDrainOverrideConfigmap)
-			return false, nil
-		}
 		isSafe, err := isSafeContainerRegistryConfChanges(oldIgnConfig, newIgnConfig)
 		if err != nil {
 			return false, err
@@ -150,18 +146,14 @@ func isDrainRequiredForNodeDisruptionActions(actions []opv1.NodeDisruptionPolicy
 }
 
 // isDrainRequired determines whether node drain is required or not to apply config changes.
-func isDrainRequired(actions, diffFileSet []string, oldIgnConfig, newIgnConfig ign3types.Config, overrideImageRegistryDrain bool) (bool, error) {
+func isDrainRequired(actions, diffFileSet []string, oldIgnConfig, newIgnConfig ign3types.Config) (bool, error) {
 	switch {
 	case ctrlcommon.InSlice(postConfigChangeActionReboot, actions):
 		// Node is going to reboot, we definitely want to perform drain
 		return true, nil
 	case ctrlcommon.InSlice(postConfigChangeActionReloadCrio, actions), ctrlcommon.InSlice(postConfigChangeActionRestartCrio, actions):
 		// Drain may or may not be necessary in case of container registry config changes.
 		if ctrlcommon.InSlice(constants.ContainerRegistryConfPath, diffFileSet) {
-			if overrideImageRegistryDrain {
-				klog.Warningf("Drain was skipped for this image registry update due to the configmap %s being present. This may not be a safe change", constants.ImageRegistryDrainOverrideConfigmap)
-				return false, nil
-			}
 			isSafe, err := isSafeContainerRegistryConfChanges(oldIgnConfig, newIgnConfig)
 			if err != nil {
 				return false, err

pkg/daemon/drain_test.go

@@ -468,7 +468,7 @@ location = "mirror.com/repo/test-img-14"
 				t.Errorf("parsing new Ignition config failed: %v", err)
 			}
 			diffFileSet := ctrlcommon.CalculateConfigFileDiffs(&oldIgnConfig, &newIgnConfig)
-			drain, err := isDrainRequired(test.actions, diffFileSet, oldIgnConfig, newIgnConfig, false)
+			drain, err := isDrainRequired(test.actions, diffFileSet, oldIgnConfig, newIgnConfig)
 			if !reflect.DeepEqual(test.expectedAction, drain) {
 				t.Errorf("Failed determining drain behavior: expected: %v but result is: %v. Error: %v", test.expectedAction, drain, err)
 			}

pkg/daemon/update.go

@@ -22,7 +22,6 @@ import (
 	"github.com/coreos/go-semver/semver"
 	ign3types "github.com/coreos/ignition/v2/config/v3_5/types"
 	corev1 "k8s.io/api/core/v1"
-	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	kubeErrs "k8s.io/apimachinery/pkg/util/errors"
@@ -1191,21 +1190,17 @@ func (dn *Daemon) update(oldConfig, newConfig *mcfgv1.MachineConfig, skipCertifi
 	}
 
 	var drain bool
-	crioOverrideConfigmapExists, err := dn.hasImageRegistryDrainOverrideConfigMap()
-	if err != nil {
-		return err
-	}
 	// Node Disruption Policies cannot be used during firstboot as API is not accessible.
 	if !firstBoot {
 		// Check actions list and perform node drain if required
-		drain, err = isDrainRequiredForNodeDisruptionActions(nodeDisruptionActions, oldIgnConfig, newIgnConfig, crioOverrideConfigmapExists)
+		drain, err = isDrainRequiredForNodeDisruptionActions(nodeDisruptionActions, oldIgnConfig, newIgnConfig)
 		if err != nil {
 			return err
 		}
 		klog.Infof("Drain calculated for node disruption: %v for config %s", drain, newConfigName)
 	} else {
 		// Check and perform node drain if required
-		drain, err = isDrainRequired(actions, diffFileSet, oldIgnConfig, newIgnConfig, crioOverrideConfigmapExists)
+		drain, err = isDrainRequired(actions, diffFileSet, oldIgnConfig, newIgnConfig)
 		if err != nil {
 			return err
 		}
@@ -2970,23 +2965,6 @@ func (dn *CoreOSDaemon) applyLayeredOSChanges(mcDiff machineConfigDiff, oldConfi
 	return dn.applyExtensions(oldConfig, newConfig)
 }
 
-func (dn *Daemon) hasImageRegistryDrainOverrideConfigMap() (bool, error) {
-	if dn.kubeClient == nil {
-		return false, nil
-	}
-
-	_, err := dn.kubeClient.CoreV1().ConfigMaps(ctrlcommon.MCONamespace).Get(context.TODO(), constants.ImageRegistryDrainOverrideConfigmap, metav1.GetOptions{})
-	if err == nil {
-		return true, nil
-	}
-
-	if apierrors.IsNotFound(err) {
-		return false, nil
-	}
-
-	return false, fmt.Errorf("Error fetching image registry drain override configmap: %w", err)
-}
-
 // Enables the revert layering systemd unit.
 //
 // To enable the unit, we perform the following operations:

pkg/operator/metrics.go

@@ -45,13 +45,6 @@ var (
 			Name: "mco_unavailable_machine_count",
 			Help: "total number of unavailable machines in specified pool",
 		}, []string{"pool"})
-	// mcoImageRegistryDrainOverrideConfigmapExists tracks the presence of the image registry drain override configmap
-	mcoImageRegistryDrainOverrideConfigmapExists = prometheus.NewGauge(
-		prometheus.GaugeOpts{
-			Name: "mco_image_registry_drain_override_exists",
-			Help: "tracks presence of the image registry drain override configmap",
-		},
-	)
 )
 
 func RegisterMCOMetrics() error {
@@ -61,7 +54,6 @@ func RegisterMCOMetrics() error {
 		mcoUpdatedMachineCount,
 		mcoDegradedMachineCount,
 		mcoUnavailableMachineCount,
-		mcoImageRegistryDrainOverrideConfigmapExists,
 	})
 
 	if err != nil {

pkg/operator/status.go

@@ -27,7 +27,6 @@ import (
 	"github.com/openshift/machine-config-operator/pkg/apihelpers"
 	ctrlcommon "github.com/openshift/machine-config-operator/pkg/controller/common"
 	kcc "github.com/openshift/machine-config-operator/pkg/controller/kubelet-config"
-	daemonconsts "github.com/openshift/machine-config-operator/pkg/daemon/constants"
 	"github.com/openshift/machine-config-operator/pkg/helpers"
 )
 
@@ -332,19 +331,6 @@ func (optr *Operator) syncMetrics() error {
 		mcoDegradedMachineCount.WithLabelValues(pool.Name).Set(float64(pool.Status.DegradedMachineCount))
 		mcoUnavailableMachineCount.WithLabelValues(pool.Name).Set(float64(pool.Status.UnavailableMachineCount))
 	}
-
-	// Attempt to fetch image-registry-override-drain configmap
-	_, err = optr.mcoCmLister.ConfigMaps(ctrlcommon.MCONamespace).Get(daemonconsts.ImageRegistryDrainOverrideConfigmap)
-	if err != nil && !apierrors.IsNotFound(err) {
-		return fmt.Errorf("error fetching configmap %s during metrics sync", daemonconsts.ImageRegistryDrainOverrideConfigmap)
-	}
-	// If the configmap is present, fire an alert warning admin of deprecation
-	if apierrors.IsNotFound(err) {
-		mcoImageRegistryDrainOverrideConfigmapExists.Set(0)
-	} else {
-		mcoImageRegistryDrainOverrideConfigmapExists.Set(1)
-	}
-
 	return nil
 }