Merge pull request #4060 from dkhater-redhat/ocb-feature-gate

MCO-831: added feature gate to mco for on cluster builds

pkg/controller/drain/drain_controller.go

@@ -326,7 +326,7 @@ func (ctrl *Controller) syncNode(key string) error {
 			if nErr != nil {
 				klog.Errorf("Error making MCN for Uncordon failure: %v", err)
 			}
-			return fmt.Errorf("failed to uncordon node %v: %w", node.Name, err)
+			return fmt.Errorf("failed to uncordon node %v: %v", node.Name, err)
 
 		}
 
@@ -359,7 +359,7 @@ func (ctrl *Controller) syncNode(key string) error {
 		daemonconsts.LastAppliedDrainerAnnotationKey: desiredState,
 	}
 	if err := ctrl.setNodeAnnotations(node.Name, annotations); err != nil {
-		return fmt.Errorf("node %s: failed to set node uncordoned annotation: %w", node.Name, err)
+		return fmt.Errorf("node %s: failed to set node uncordoned annotation: %v", node.Name, err)
 	}
 	ctrlcommon.UpdateStateMetric(ctrlcommon.MCCSubControllerState, "machine-config-controller-drain", desiredVerb, node.Name)
 	return nil
@@ -404,7 +404,7 @@ func (ctrl *Controller) drainNode(node *corev1.Node, drainer *drain.Helper) erro
 			if Nerr != nil {
 				klog.Errorf("Error making MCN for Cordon Failure: %v", Nerr)
 			}
-			return fmt.Errorf("node %s: failed to cordon: %w", node.Name, err)
+			return fmt.Errorf("node %s: failed to cordon: %v", node.Name, err)
 		}
 		ctrl.ongoingDrains[node.Name] = time.Now()
 		err := upgrademonitor.GenerateAndApplyMachineConfigNodes(&upgrademonitor.Condition{State: v1alpha1.MachineConfigNodeUpdateExecuted, Reason: string(v1alpha1.MachineConfigNodeUpdateCordoned), Message: fmt.Sprintf("Cordoned Node as part of update executed phase")},
@@ -513,14 +513,14 @@ func (ctrl *Controller) setNodeAnnotations(nodeName string, annotations map[stri
 
 		patchBytes, err := strategicpatch.CreateTwoWayMergePatch(oldNode, newNode, corev1.Node{})
 		if err != nil {
-			return fmt.Errorf("node %s: failed to create patch for: %w", nodeName, err)
+			return fmt.Errorf("node %s: failed to create patch for: %v", nodeName, err)
 		}
 
 		_, err = ctrl.kubeClient.CoreV1().Nodes().Patch(context.TODO(), nodeName, types.StrategicMergePatchType, patchBytes, metav1.PatchOptions{})
 		return err
 	}); err != nil {
 		// may be conflict if max retries were hit
-		return fmt.Errorf("node %s: unable to update: %w", nodeName, err)
+		return fmt.Errorf("node %s: unable to update: %v", nodeName, err)
 	}
 	return nil
 }
@@ -564,9 +564,9 @@ func (ctrl *Controller) cordonOrUncordonNode(desired bool, node *corev1.Node, dr
 	}); err != nil {
 		if wait.Interrupted(err) {
 			errs := kubeErrs.NewAggregate([]error{err, lastErr})
-			return fmt.Errorf("node %s: failed to %s (%d tries): %w", node.Name, verb, ctrl.cfg.CordonOrUncordonBackoff.Steps, errs)
+			return fmt.Errorf("node %s: failed to %s (%d tries): %v", node.Name, verb, ctrl.cfg.CordonOrUncordonBackoff.Steps, errs)
 		}
-		return fmt.Errorf("node %s: failed to %s: %w", node.Name, verb, err)
+		return fmt.Errorf("node %s: failed to %s: %v", node.Name, verb, err)
 	}
 
 	return nil

pkg/controller/render/render_controller.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"fmt"
 	"reflect"
+	"sort"
 	"time"
 
 	mcfgv1 "github.com/openshift/api/machineconfiguration/v1"
@@ -431,6 +432,8 @@ func (ctrl *Controller) syncMachineConfigPool(key string) error {
 	if err != nil {
 		return err
 	}
+	sort.SliceStable(mcs, func(i, j int) bool { return mcs[i].Name < mcs[j].Name })
+
 	if len(mcs) == 0 {
 		return ctrl.syncFailingStatus(pool, fmt.Errorf("no MachineConfigs found matching selector %v", selector))
 	}

pkg/operator/sync.go

@@ -1040,6 +1040,21 @@ func (optr *Operator) syncMachineOSBuilder(config *renderConfig) error {
 // Determines if the Machine OS Builder deployment is in the correct state
 // based upon whether we have opted-in pools or not.
 func (optr *Operator) reconcileMachineOSBuilder(mob *appsv1.Deployment) error {
+	// Access current feature gates
+	fg, err := optr.fgAccessor.CurrentFeatureGates()
+	if err != nil {
+		return fmt.Errorf("could not get feature gates: %w", err)
+	}
+
+	if fg == nil {
+		return fmt.Errorf("received nil feature gates")
+	}
+
+	// Check if OnClusterBuild feature gate is enabled
+	if !fg.Enabled(configv1.FeatureGateOnClusterBuild) {
+		return nil
+	}
+
 	// First, check if we have any MachineConfigPools opted in.
 	layeredMCPs, err := optr.getLayeredMachineConfigPools()
 	if err != nil {

test/e2e/mob_test.go

@@ -71,6 +71,24 @@ func TestMachineOSBuilder(t *testing.T) {
 
 	t.Cleanup(createConfigMapForTest(t, cs))
 
+	// get the feature gates because we're gating this for now
+	featureGates, err := cs.ConfigV1Interface.FeatureGates().Get(context.TODO(), "cluster", metav1.GetOptions{})
+	require.NoError(t, err, "Failed to retrieve feature gates")
+
+	// TODO(jkyros): this should be a helper or we should use whatever the "best practice" way is
+	// for retrieving the gates during a test, but this works for now
+	var featureGateEnabled bool
+	for _, featureGateDetails := range featureGates.Status.FeatureGates {
+		for _, enabled := range featureGateDetails.Enabled {
+			if enabled.Name == "OnClusterBuild" {
+				featureGateEnabled = true
+
+			}
+		}
+	}
+
+	t.Logf("Feature gate OnClusterBuiild enabled: %t", featureGateEnabled)
+
 	cleanup := helpers.MakeIdempotent(helpers.CreateMCP(t, cs, mcpName))
 	t.Cleanup(cleanup)
 	time.Sleep(10 * time.Second) // Wait a bit to ensure MCP is fully created
@@ -94,33 +112,38 @@ func TestMachineOSBuilder(t *testing.T) {
 
 	// assertion to see if the deployment object is present after setting the label
 	ctx := context.TODO()
-	err := wait.PollUntilContextTimeout(ctx, 2*time.Second, time.Minute, true, func(ctx context.Context) (bool, error) {
-		exists, err := helpers.CheckDeploymentExists(cs, "machine-os-builder", namespace)
-		return exists, err
-	})
-	require.NoError(t, err, "Failed to check the existence of the Machine OS Builder deployment")
 
-	// wait for Machine OS Builder pod to start
-	err = helpers.WaitForPodStart(cs, mobPodNamePrefix, namespace)
-	require.NoError(t, err, "Failed to start the Machine OS Builder pod")
-	t.Logf("machine-os-builder deployment exists")
-
-	// delete the MachineConfigPool
-	cleanup()
-	time.Sleep(20 * time.Second)
-
-	// assertion to see if the deployment object is absent after deleting the MCP
 	err = wait.PollUntilContextTimeout(ctx, 2*time.Second, time.Minute, true, func(ctx context.Context) (bool, error) {
 		exists, err := helpers.CheckDeploymentExists(cs, "machine-os-builder", namespace)
-		return !exists, err
+		return exists, err
 	})
-	require.NoError(t, err, "Failed to check the absence of the Machine OS Builder deployment")
-
-	// wait for Machine OS Builder pod to stop
-	err = helpers.WaitForPodStop(cs, mobPodNamePrefix, namespace)
-	require.NoError(t, err, "Failed to stop the Machine OS Builder pod")
-	t.Logf("machine-os-builder deployment no longer exists")
-
-	_, err = cs.AppsV1Interface.Deployments(ctrlcommon.MCONamespace).Get(context.TODO(), "machine-os-builder", metav1.GetOptions{})
-	assert.True(t, apierrs.IsNotFound(err), "machine-os-builder deployment still present")
+	if featureGateEnabled {
+		require.NoError(t, err, "Failed to check the existence of the Machine OS Builder deployment")
+
+		// wait for Machine OS Builder pod to start
+		err = helpers.WaitForPodStart(cs, mobPodNamePrefix, namespace)
+		require.NoError(t, err, "Failed to start the Machine OS Builder pod")
+		t.Logf("machine-os-builder deployment exists")
+
+		// delete the MachineConfigPool
+		cleanup()
+		time.Sleep(20 * time.Second)
+
+		// assertion to see if the deployment object is absent after deleting the MCP
+		err = wait.PollUntilContextTimeout(ctx, 2*time.Second, time.Minute, true, func(ctx context.Context) (bool, error) {
+			exists, err := helpers.CheckDeploymentExists(cs, "machine-os-builder", namespace)
+			return !exists, err
+		})
+		require.NoError(t, err, "Failed to check the absence of the Machine OS Builder deployment")
+
+		// wait for Machine OS Builder pod to stop
+		err = helpers.WaitForPodStop(cs, mobPodNamePrefix, namespace)
+		require.NoError(t, err, "Failed to stop the Machine OS Builder pod")
+		t.Logf("machine-os-builder deployment no longer exists")
+
+		_, err = cs.AppsV1Interface.Deployments(ctrlcommon.MCONamespace).Get(context.TODO(), "machine-os-builder", metav1.GetOptions{})
+		assert.True(t, apierrs.IsNotFound(err), "machine-os-builder deployment still present")
+	} else {
+		require.Error(t, err, "Machine OS Builder deployment exists and it should not, because the feature gate is disabled")
+	}
 }