feature gate: remove references to MachineConfigNodes and PinnedImages feature gates

Author: isabella-janssen

cmd/machine-config-controller/start.go

@@ -112,6 +112,14 @@ func runStartCmd(_ *cobra.Command, _ []string) {
 			klog.Fatalf("unable to start cert rotation controller: %v", err)
 		}
 
+		pinnedImageSet := pinnedimageset.New(
+			ctrlctx.InformerFactory.Machineconfiguration().V1().PinnedImageSets(),
+			ctrlctx.InformerFactory.Machineconfiguration().V1().MachineConfigPools(),
+			ctrlctx.ClientBuilder.KubeClientOrDie("pinned-image-set-controller"),
+			ctrlctx.ClientBuilder.MachineConfigClientOrDie("pinned-image-set-controller"),
+		)
+		go pinnedImageSet.Run(2, ctrlctx.Stop)
+
 		// Start the shared factory informers that you need to use in your controller
 		ctrlctx.InformerFactory.Start(ctrlctx.Stop)
 		ctrlctx.KubeInformerFactory.Start(ctrlctx.Stop)
@@ -124,20 +132,6 @@ func runStartCmd(_ *cobra.Command, _ []string) {
 
 		close(ctrlctx.InformersStarted)
 
-		if ctrlctx.FeatureGatesHandler.Enabled(features.FeatureGatePinnedImages) && ctrlctx.FeatureGatesHandler.Enabled(features.FeatureGateMachineConfigNodes) {
-			pinnedImageSet := pinnedimageset.New(
-				ctrlctx.InformerFactory.Machineconfiguration().V1().PinnedImageSets(),
-				ctrlctx.InformerFactory.Machineconfiguration().V1().MachineConfigPools(),
-				ctrlctx.ClientBuilder.KubeClientOrDie("pinned-image-set-controller"),
-				ctrlctx.ClientBuilder.MachineConfigClientOrDie("pinned-image-set-controller"),
-			)
-
-			go pinnedImageSet.Run(2, ctrlctx.Stop)
-			// start the informers again to enable feature gated types.
-			// see comments in SharedInformerFactory interface.
-			ctrlctx.InformerFactory.Start(ctrlctx.Stop)
-		}
-
 		if ctrlctx.FeatureGatesHandler.Enabled(features.FeatureGateNoRegistryClusterInstall) {
 			iriController := internalreleaseimage.New(
 				ctrlctx.InformerFactory.Machineconfiguration().V1alpha1().InternalReleaseImages(),

cmd/machine-config-daemon/start.go

@@ -9,8 +9,6 @@ import (
 	"os"
 	"time"
 
-	"github.com/openshift/api/features"
-
 	"k8s.io/apimachinery/pkg/api/resource"
 	"k8s.io/client-go/tools/clientcmd"
 
@@ -213,43 +211,35 @@ func runStartCmd(_ *cobra.Command, _ []string) {
 		klog.Fatalf("Failed to initialize: %v", err)
 	}
 
+	// Create CRI client and pinned image set manager before starting informers
+	criClient, err := cri.NewClient(ctx, constants.DefaultCRIOSocketPath)
+	if err != nil {
+		klog.Fatalf("Failed to initialize CRI client: %v", err)
+	}
+	prefetchTimeout := 2 * time.Minute
+	pinnedImageSetManager := daemon.NewPinnedImageSetManager(
+		startOpts.nodeName,
+		criClient,
+		ctrlctx.ClientBuilder.MachineConfigClientOrDie(componentName),
+		ctrlctx.InformerFactory.Machineconfiguration().V1().PinnedImageSets(),
+		nodeScopedInformer,
+		ctrlctx.InformerFactory.Machineconfiguration().V1().MachineConfigPools(),
+		resource.MustParse(constants.MinFreeStorageAfterPrefetch),
+		constants.DefaultCRIOSocketPath,
+		constants.KubeletAuthFile,
+		constants.ContainerRegistryConfPath,
+		prefetchTimeout,
+		ctrlctx.FeatureGatesHandler,
+	)
+	go pinnedImageSetManager.Run(2, stopCh)
+
 	ctrlctx.KubeInformerFactory.Start(stopCh)
 	ctrlctx.KubeNamespacedInformerFactory.Start(stopCh)
 	ctrlctx.InformerFactory.Start(stopCh)
 	ctrlctx.OperatorInformerFactory.Start(stopCh)
 	nodeScopedInformerStartFunc(ctrlctx.Stop)
 	close(ctrlctx.InformersStarted)
 
-	// ok to start the rest of the informers now that we have observed the initial feature gates
-	if ctrlctx.FeatureGatesHandler.Enabled(features.FeatureGatePinnedImages) && ctrlctx.FeatureGatesHandler.Enabled(features.FeatureGateMachineConfigNodes) {
-		klog.Infof("Feature enabled: %s", features.FeatureGatePinnedImages)
-		criClient, err := cri.NewClient(ctx, constants.DefaultCRIOSocketPath)
-		if err != nil {
-			klog.Fatalf("Failed to initialize CRI client: %v", err)
-		}
-
-		prefetchTimeout := 2 * time.Minute
-		pinnedImageSetManager := daemon.NewPinnedImageSetManager(
-			startOpts.nodeName,
-			criClient,
-			ctrlctx.ClientBuilder.MachineConfigClientOrDie(componentName),
-			ctrlctx.InformerFactory.Machineconfiguration().V1().PinnedImageSets(),
-			nodeScopedInformer,
-			ctrlctx.InformerFactory.Machineconfiguration().V1().MachineConfigPools(),
-			resource.MustParse(constants.MinFreeStorageAfterPrefetch),
-			constants.DefaultCRIOSocketPath,
-			constants.KubeletAuthFile,
-			constants.ContainerRegistryConfPath,
-			prefetchTimeout,
-			ctrlctx.FeatureGatesHandler,
-		)
-
-		go pinnedImageSetManager.Run(2, stopCh)
-		// start the informers for the pinned image set again after the feature gate is enabled this is allowed.
-		// see comments in SharedInformerFactory interface.
-		ctrlctx.InformerFactory.Start(stopCh)
-	}
-
 	if err := dn.Run(stopCh, exitCh, errCh); err != nil {
 		ctrlcommon.WriteTerminationError(err)
 		if errors.Is(err, daemon.ErrAuxiliary) {

pkg/controller/node/node_controller.go

@@ -13,7 +13,6 @@ import (
 	"github.com/openshift/machine-config-operator/pkg/upgrademonitor"
 
 	configv1 "github.com/openshift/api/config/v1"
-	features "github.com/openshift/api/features"
 	mcfgv1 "github.com/openshift/api/machineconfiguration/v1"
 
 	cligoinformersv1 "github.com/openshift/client-go/config/informers/externalversions/config/v1"
@@ -892,11 +891,9 @@ func (ctrl *Controller) updateNode(old, cur interface{}) {
 		return
 	}
 
-	if ctrl.fgHandler.Enabled(features.FeatureGatePinnedImages) {
-		for _, pool := range pools {
-			if isPinnedImageSetsInProgressForPool(pool) {
-				changed = true
-			}
+	for _, pool := range pools {
+		if isPinnedImageSetsInProgressForPool(pool) {
+			changed = true
 		}
 	}
 

pkg/controller/node/node_controller_test.go

@@ -8,7 +8,6 @@ import (
 	"testing"
 	"time"
 
-	features "github.com/openshift/api/features"
 	"github.com/openshift/library-go/pkg/operator/configobserver/featuregates"
 
 	"github.com/openshift/machine-config-operator/pkg/apihelpers"
@@ -95,7 +94,7 @@ func newFixtureWithFeatureGates(t *testing.T, enabled, disabled []configv1.Featu
 }
 
 func newFixture(t *testing.T) *fixture {
-	return newFixtureWithFeatureGates(t, []configv1.FeatureGateName{features.FeatureGatePinnedImages}, []configv1.FeatureGateName{})
+	return newFixtureWithFeatureGates(t, []configv1.FeatureGateName{}, []configv1.FeatureGateName{})
 }
 
 func (f *fixture) newControllerWithStopChan(stopCh <-chan struct{}) *Controller {
@@ -265,6 +264,7 @@ func filterInformerActions(actions []core.Action) []core.Action {
 				action.Matches("watch", "machineosbuilds") ||
 				action.Matches("list", "machineosconfigs") ||
 				action.Matches("watch", "machineosconfigs") ||
+				action.Matches("get", "machineconfignodes") ||
 				action.Matches("list", "machineconfignodes") ||
 				action.Matches("watch", "machineconfignodes")) {
 			continue

pkg/controller/node/status.go

@@ -31,15 +31,13 @@ func (ctrl *Controller) syncStatusOnly(pool *mcfgv1.MachineConfigPool) error {
 	}
 
 	machineConfigStates := []*mcfgv1.MachineConfigNode{}
-	if ctrl.fgHandler.Enabled(features.FeatureGateMachineConfigNodes) {
-		for _, node := range nodes {
-			ms, err := ctrl.client.MachineconfigurationV1().MachineConfigNodes().Get(context.TODO(), node.Name, metav1.GetOptions{})
-			if err != nil {
-				klog.Errorf("Could not find our MachineConfigNode for node. %s: %v", node.Name, err)
-				continue
-			}
-			machineConfigStates = append(machineConfigStates, ms)
+	for _, node := range nodes {
+		ms, err := ctrl.client.MachineconfigurationV1().MachineConfigNodes().Get(context.TODO(), node.Name, metav1.GetOptions{})
+		if err != nil {
+			klog.Errorf("Could not find our MachineConfigNode for node. %s: %v", node.Name, err)
+			continue
 		}
+		machineConfigStates = append(machineConfigStates, ms)
 	}
 
 	// Get fresh copy of MCP from lister to ensure we have the latest status
@@ -100,7 +98,6 @@ func (ctrl *Controller) calculateStatus(mcns []*mcfgv1.MachineConfigNode, cconfi
 
 	// Determine if pool is layered and get enabled feature gates
 	isLayeredPool := ctrl.isLayeredPool(mosc, mosb)
-	pisIsEnabled := ctrl.fgHandler.Enabled(features.FeatureGatePinnedImages)
 	imageModeReportingIsEnabled := ctrl.fgHandler.Enabled(features.FeatureGateImageModeStatusReporting)
 
 	// Update the number of degraded and updated machines from conditions in the MCNs for the nodes
@@ -128,11 +125,9 @@ func (ctrl *Controller) calculateStatus(mcns []*mcfgv1.MachineConfigNode, cconfi
 			break
 		}
 
-		// If the PIS feature gate is enabled, update the PIS reference in the PoolSynchronizer object
-		if pisIsEnabled {
-			if isPinnedImageSetsUpdated(mcn) {
-				poolSynchronizer.SetUpdated(mcfgv1.PinnedImageSets)
-			}
+		// Update the PIS reference in the PoolSynchronizer object
+		if isPinnedImageSetsUpdated(mcn) {
+			poolSynchronizer.SetUpdated(mcfgv1.PinnedImageSets)
 		}
 
 		// Loop through the MCN conditions to determine if the associated node is updating, updated, or degraded
@@ -220,18 +215,16 @@ func (ctrl *Controller) calculateStatus(mcns []*mcfgv1.MachineConfigNode, cconfi
 	}
 
 	// Update synchronizer status for pinned image sets
-	if pisIsEnabled {
-		syncStatus := poolSynchronizer.GetStatus(mcfgv1.PinnedImageSets)
-		status.PoolSynchronizersStatus = []mcfgv1.PoolSynchronizerStatus{
-			{
-				PoolSynchronizerType:    mcfgv1.PinnedImageSets,
-				MachineCount:            syncStatus.MachineCount,
-				UpdatedMachineCount:     syncStatus.UpdatedMachineCount,
-				ReadyMachineCount:       int64(readyMachineCount),
-				UnavailableMachineCount: int64(unavailableMachineCount),
-				AvailableMachineCount:   int64(totalMachineCount - unavailableMachineCount),
-			},
-		}
+	syncStatus := poolSynchronizer.GetStatus(mcfgv1.PinnedImageSets)
+	status.PoolSynchronizersStatus = []mcfgv1.PoolSynchronizerStatus{
+		{
+			PoolSynchronizerType:    mcfgv1.PinnedImageSets,
+			MachineCount:            syncStatus.MachineCount,
+			UpdatedMachineCount:     syncStatus.UpdatedMachineCount,
+			ReadyMachineCount:       int64(readyMachineCount),
+			UnavailableMachineCount: int64(unavailableMachineCount),
+			AvailableMachineCount:   int64(totalMachineCount - unavailableMachineCount),
+		},
 	}
 
 	// Update MCP status configuation & conditions

pkg/controller/node/status_test.go

@@ -921,8 +921,6 @@ func TestCalculateStatus(t *testing.T) {
 			}
 			f := newFixtureWithFeatureGates(t,
 				[]apicfgv1.FeatureGateName{
-					features.FeatureGateMachineConfigNodes,
-					features.FeatureGatePinnedImages,
 					features.FeatureGateOSStreams,
 				},
 				[]apicfgv1.FeatureGateName{},
@@ -944,8 +942,6 @@ func TestCalculateStatusWithImageModeReporting(t *testing.T) {
 	// This simulates a DevPreview environment where this feature gate is available
 	fgHandler := ctrlcommon.NewFeatureGatesHardcodedHandler(
 		[]apicfgv1.FeatureGateName{
-			features.FeatureGateMachineConfigNodes,
-			features.FeatureGatePinnedImages,
 			features.FeatureGateImageModeStatusReporting, // Enable ImageModeStatusReporting directly
 		},
 		[]apicfgv1.FeatureGateName{},
@@ -1342,8 +1338,6 @@ func TestCalculateStatusWithImageModeReporting(t *testing.T) {
 			// Create fixture with our ImageModeStatusReporting feature gate handler
 			f := newFixtureWithFeatureGates(t,
 				[]apicfgv1.FeatureGateName{
-					features.FeatureGateMachineConfigNodes,
-					features.FeatureGatePinnedImages,
 					features.FeatureGateImageModeStatusReporting,
 				},
 				[]apicfgv1.FeatureGateName{},

pkg/daemon/daemon.go

@@ -427,7 +427,7 @@ func (dn *Daemon) ClusterConnect(
 
 	// If the IrreconcilableMachineConfig FG is enabled turn on the reporting of irreconcilable differences
 	// MCN is required too, as the irreconcilable diffs report is stored as part of the MCN status
-	if dn.fgHandler.Enabled(features.FeatureGateIrreconcilableMachineConfig) && dn.fgHandler.Enabled(features.FeatureGateMachineConfigNodes) {
+	if dn.fgHandler.Enabled(features.FeatureGateIrreconcilableMachineConfig) {
 		dn.irreconcilableReporter = NewIrreconcilableReporter(dn.mcfgClient)
 	}
 

pkg/daemon/update.go

@@ -2738,10 +2738,9 @@ func (dn *Daemon) updateLayeredOS(config *mcfgv1.MachineConfig) error {
 }
 
 func (dn *Daemon) isPinnedImageSetConfigured() (bool, error) {
-	if dn.fgHandler == nil || !dn.fgHandler.Enabled(features.FeatureGatePinnedImages) || dn.node == nil || dn.mcpLister == nil {
-		// Two options:
-		// - PIS is not enabled
-		// - MCD first boot run: No connection to the cluster and node not populated -> Cannot check PIS config
+	if dn.node == nil || dn.mcpLister == nil {
+		// If we are here it means we are in the MCD first boot run. No connection to the cluster
+		// and the node not being populated means we cannot check the PIS config.
 		return false, nil
 	}
 

pkg/operator/status.go

@@ -510,25 +510,25 @@ func (optr *Operator) allMachineConfigPoolStatus() (map[string]string, error) {
 
 	ret := map[string]string{}
 	for _, pool := range pools {
-		ret[pool.GetName()] = machineConfigPoolStatus(optr.fgHandler, pool)
+		ret[pool.GetName()] = machineConfigPoolStatus(pool)
 	}
 	return ret, nil
 }
 
 // isMachineConfigPoolConfigurationValid returns nil, or error when the configuration of a `pool` is created by the controller at version `version`,
 // when the osImageURL does not match what's in the configmap or when the rendered-config-xxx does not match the OCP release version.
-func isMachineConfigPoolConfigurationValid(fgHandler ctrlcommon.FeatureGatesHandler, pool *mcfgv1.MachineConfigPool, version, releaseVersion, osURL string, machineConfigGetter func(string) (*mcfgv1.MachineConfig, error)) error {
+func isMachineConfigPoolConfigurationValid(pool *mcfgv1.MachineConfigPool, version, releaseVersion, osURL string, machineConfigGetter func(string) (*mcfgv1.MachineConfig, error)) error {
 	// both .status.configuration.name and .status.configuration.source must be set.
 	if pool.Spec.Configuration.Name == "" {
-		return fmt.Errorf("configuration spec for pool %s is empty: %v", pool.GetName(), machineConfigPoolStatus(fgHandler, pool))
+		return fmt.Errorf("configuration spec for pool %s is empty: %v", pool.GetName(), machineConfigPoolStatus(pool))
 	}
 	if pool.Status.Configuration.Name == "" {
 		// if status is empty, it means the node controller hasn't seen any node at the target configuration
 		// we bubble up any error from the pool to make the info more visible
-		return fmt.Errorf("configuration status for pool %s is empty: %s", pool.GetName(), machineConfigPoolStatus(fgHandler, pool))
+		return fmt.Errorf("configuration status for pool %s is empty: %s", pool.GetName(), machineConfigPoolStatus(pool))
 	}
 	if len(pool.Status.Configuration.Source) == 0 {
-		return fmt.Errorf("list of MachineConfigs that were used to generate configuration for pool %s is empty: %v", pool.GetName(), machineConfigPoolStatus(fgHandler, pool))
+		return fmt.Errorf("list of MachineConfigs that were used to generate configuration for pool %s is empty: %v", pool.GetName(), machineConfigPoolStatus(pool))
 	}
 	mcs := []string{pool.Status.Configuration.Name}
 	for _, fragment := range pool.Status.Configuration.Source {
@@ -547,13 +547,13 @@ func isMachineConfigPoolConfigurationValid(fgHandler ctrlcommon.FeatureGatesHand
 		// The bootstrapped MCs fragments have this annotation, however, we don't fail (???) if they don't have
 		// the annotation for some reason.
 		if !ok && pool.Status.Configuration.Name == mcName {
-			return fmt.Errorf("%s must be created by controller version %s: %v", mcName, version, machineConfigPoolStatus(fgHandler, pool))
+			return fmt.Errorf("%s must be created by controller version %s: %v", mcName, version, machineConfigPoolStatus(pool))
 		}
 		// user provided MC fragments do not have the annotation, so we just skip the version check there.
 		// The check below is for: 1) the generated MC for the pool, and 2) the bootstrapped fragments
 		// that do have this annotation set with a version.
 		if ok && v != version {
-			return fmt.Errorf("controller version mismatch for %s expected %s has %s: %v", mcName, version, v, machineConfigPoolStatus(fgHandler, pool))
+			return fmt.Errorf("controller version mismatch for %s expected %s has %s: %v", mcName, version, v, machineConfigPoolStatus(pool))
 		}
 	}
 	// all MCs were generated by correct controller, but osImageURL is not a source, so let's double check here
@@ -588,7 +588,7 @@ func isMachineConfigPoolConfigurationValid(fgHandler ctrlcommon.FeatureGatesHand
 	return nil
 }
 
-func machineConfigPoolStatus(fgHandler ctrlcommon.FeatureGatesHandler, pool *mcfgv1.MachineConfigPool) string {
+func machineConfigPoolStatus(pool *mcfgv1.MachineConfigPool) string {
 	switch {
 	case apihelpers.IsMachineConfigPoolConditionTrue(pool.Status.Conditions, mcfgv1.MachineConfigPoolRenderDegraded):
 		cond := apihelpers.GetMachineConfigPoolCondition(pool.Status, mcfgv1.MachineConfigPoolRenderDegraded)
@@ -601,11 +601,9 @@ func machineConfigPoolStatus(fgHandler ctrlcommon.FeatureGatesHandler, pool *mcf
 	case apihelpers.IsMachineConfigPoolConditionTrue(pool.Status.Conditions, mcfgv1.MachineConfigPoolUpdating):
 		return fmt.Sprintf("%d (ready %d) out of %d nodes are updating to latest configuration %s", pool.Status.UpdatedMachineCount, pool.Status.ReadyMachineCount, pool.Status.MachineCount, pool.Spec.Configuration.Name)
 	default:
-		if fgHandler.Enabled(features.FeatureGatePinnedImages) {
-			if apihelpers.IsMachineConfigPoolConditionTrue(pool.Status.Conditions, mcfgv1.MachineConfigPoolPinnedImageSetsDegraded) {
-				cond := apihelpers.GetMachineConfigPoolCondition(pool.Status, mcfgv1.MachineConfigPoolPinnedImageSetsDegraded)
-				return fmt.Sprintf("pool is degraded because pinned image sets failed with %q: %q", cond.Reason, cond.Message)
-			}
+		if apihelpers.IsMachineConfigPoolConditionTrue(pool.Status.Conditions, mcfgv1.MachineConfigPoolPinnedImageSetsDegraded) {
+			cond := apihelpers.GetMachineConfigPoolCondition(pool.Status, mcfgv1.MachineConfigPoolPinnedImageSetsDegraded)
+			return fmt.Sprintf("pool is degraded because pinned image sets failed with %q: %q", cond.Reason, cond.Message)
 		}
 		return "<unknown>"
 	}

pkg/operator/status_test.go

@@ -19,9 +19,7 @@ import (
 	"github.com/stretchr/testify/assert"
 	"k8s.io/apimachinery/pkg/util/uuid"
 
-	apicfgv1 "github.com/openshift/api/config/v1"
 	configv1 "github.com/openshift/api/config/v1"
-	features "github.com/openshift/api/features"
 	mcfgv1 "github.com/openshift/api/machineconfiguration/v1"
 	opv1 "github.com/openshift/api/operator/v1"
 	fakeconfigclientset "github.com/openshift/client-go/config/clientset/versioned/fake"
@@ -194,14 +192,7 @@ func TestIsMachineConfigPoolConfigurationValid(t *testing.T) {
 				source = append(source, corev1.ObjectReference{Name: s})
 			}
 
-			fgHandler := ctrlcommon.NewFeatureGatesHardcodedHandler(
-				[]apicfgv1.FeatureGateName{
-					features.FeatureGateMachineConfigNodes,
-					features.FeatureGatePinnedImages,
-				},
-				[]apicfgv1.FeatureGateName{},
-			)
-			err := isMachineConfigPoolConfigurationValid(fgHandler, &mcfgv1.MachineConfigPool{
+			err := isMachineConfigPoolConfigurationValid(&mcfgv1.MachineConfigPool{
 				ObjectMeta: metav1.ObjectMeta{
 					Name: "dummy-pool",
 				},
@@ -639,7 +630,7 @@ func TestOperatorSyncStatus(t *testing.T) {
 		optr := &Operator{
 			eventRecorder: &record.FakeRecorder{},
 			fgHandler: ctrlcommon.NewFeatureGatesHardcodedHandler(
-				[]configv1.FeatureGateName{features.FeatureGatePinnedImages}, []configv1.FeatureGateName{},
+				[]configv1.FeatureGateName{}, []configv1.FeatureGateName{},
 			),
 		}
 		optr.vStore = newVersionStore()
@@ -729,7 +720,7 @@ func TestInClusterBringUpStayOnErr(t *testing.T) {
 	optr := &Operator{
 		eventRecorder: &record.FakeRecorder{},
 		fgHandler: ctrlcommon.NewFeatureGatesHardcodedHandler(
-			[]configv1.FeatureGateName{features.FeatureGatePinnedImages}, []configv1.FeatureGateName{},
+			[]configv1.FeatureGateName{}, []configv1.FeatureGateName{},
 		),
 	}
 	optr.vStore = newVersionStore()