OCPBUGS-64825: Add MachineConfigNode informer to trigger MCP machine count syncs

cmd/machine-config-controller/start.go

@@ -244,6 +244,7 @@ func createControllers(ctx *ctrlcommon.ControllerContext) []ctrlcommon.Controlle
 			ctx.KubeInformerFactory.Core().V1().Pods(),
 			ctx.OCLInformerFactory.Machineconfiguration().V1().MachineOSConfigs(),
 			ctx.OCLInformerFactory.Machineconfiguration().V1().MachineOSBuilds(),
+			ctx.InformerFactory.Machineconfiguration().V1().MachineConfigNodes(),
 			ctx.ConfigInformerFactory.Config().V1().Schedulers(),
 			ctx.ClientBuilder.KubeClientOrDie("node-update-controller"),
 			ctx.ClientBuilder.MachineConfigClientOrDie("node-update-controller"),

pkg/controller/node/node_controller.go

@@ -95,6 +95,7 @@ type Controller struct {
 	mosbLister mcfglistersv1.MachineOSBuildLister
 	nodeLister corelisterv1.NodeLister
 	podLister  corelisterv1.PodLister
+	mcnLister  mcfglistersv1.MachineConfigNodeLister
 
 	ccListerSynced   cache.InformerSynced
 	mcListerSynced   cache.InformerSynced
@@ -124,6 +125,7 @@ func New(
 	podInformer coreinformersv1.PodInformer,
 	moscInformer mcfginformersv1.MachineOSConfigInformer,
 	mosbInformer mcfginformersv1.MachineOSBuildInformer,
+	mcnInformer mcfginformersv1.MachineConfigNodeInformer,
 	schedulerInformer cligoinformersv1.SchedulerInformer,
 	kubeClient clientset.Interface,
 	mcfgClient mcfgclientset.Interface,
@@ -137,6 +139,7 @@ func New(
 		mosbInformer,
 		nodeInformer,
 		podInformer,
+		mcnInformer,
 		schedulerInformer,
 		kubeClient,
 		mcfgClient,
@@ -153,6 +156,7 @@ func NewWithCustomUpdateDelay(
 	podInformer coreinformersv1.PodInformer,
 	moscInformer mcfginformersv1.MachineOSConfigInformer,
 	mosbInformer mcfginformersv1.MachineOSBuildInformer,
+	mcnInformer mcfginformersv1.MachineConfigNodeInformer,
 	schedulerInformer cligoinformersv1.SchedulerInformer,
 	kubeClient clientset.Interface,
 	mcfgClient mcfgclientset.Interface,
@@ -167,6 +171,7 @@ func NewWithCustomUpdateDelay(
 		mosbInformer,
 		nodeInformer,
 		podInformer,
+		mcnInformer,
 		schedulerInformer,
 		kubeClient,
 		mcfgClient,
@@ -184,6 +189,7 @@ func newController(
 	mosbInformer mcfginformersv1.MachineOSBuildInformer,
 	nodeInformer coreinformersv1.NodeInformer,
 	podInformer coreinformersv1.PodInformer,
+	mcnInformer mcfginformersv1.MachineConfigNodeInformer,
 	schedulerInformer cligoinformersv1.SchedulerInformer,
 	kubeClient clientset.Interface,
 	mcfgClient mcfgclientset.Interface,
@@ -229,6 +235,11 @@ func newController(
 		UpdateFunc: ctrl.checkMasterNodesOnUpdate,
 		DeleteFunc: ctrl.checkMasterNodesOnDelete,
 	})
+	mcnInformer.Informer().AddEventHandler(cache.ResourceEventHandlerFuncs{
+		AddFunc:    ctrl.addMachineConfigNode,
+		UpdateFunc: ctrl.updateMachineConfigNode,
+		DeleteFunc: ctrl.deleteMachineConfigNode,
+	})
 
 	ctrl.syncHandler = ctrl.syncMachineConfigPool
 	ctrl.enqueueMachineConfigPool = ctrl.enqueueDefault
@@ -240,12 +251,14 @@ func newController(
 	ctrl.mosbLister = mosbInformer.Lister()
 	ctrl.nodeLister = nodeInformer.Lister()
 	ctrl.podLister = podInformer.Lister()
+	ctrl.mcnLister = mcnInformer.Lister()
 	ctrl.ccListerSynced = ccInformer.Informer().HasSynced
 	ctrl.mcListerSynced = mcInformer.Informer().HasSynced
 	ctrl.mcpListerSynced = mcpInformer.Informer().HasSynced
 	ctrl.moscListerSynced = moscInformer.Informer().HasSynced
 	ctrl.mosbListerSynced = mosbInformer.Informer().HasSynced
 	ctrl.nodeListerSynced = nodeInformer.Informer().HasSynced
+	ctrl.mcnListerSynced = mcnInformer.Informer().HasSynced
 
 	ctrl.schedulerList = schedulerInformer.Lister()
 	ctrl.schedulerListerSynced = schedulerInformer.Informer().HasSynced
@@ -537,6 +550,81 @@ func (ctrl *Controller) updateMachineOSBuild(old, cur interface{}) {
 	ctrl.enqueueMachineConfigPool(mcp)
 }
 
+func (ctrl *Controller) addMachineConfigNode(obj interface{}) {
+	curMCN := obj.(*mcfgv1.MachineConfigNode)
+	klog.V(4).Infof("Adding MachineConfigNode %s", curMCN.Name)
+
+	// Find the associated MachineConfigPool from the MachineConfigNode. If the pool value is
+	// "not-yet-set" it means that the MCN does not have an associated MCP yet.
+	poolName := curMCN.Spec.Pool.Name
+	if poolName == upgrademonitor.NotYetSet {
+		return
+	}
+
+	mcp, err := ctrl.mcpLister.Get(poolName)
+	if err != nil {
+		utilruntime.HandleError(fmt.Errorf("Couldn't get MachineConfigPool from MachineConfigNode %v: %v", curMCN, err))
+		return
+	}
+	klog.V(4).Infof("MachineConfigNode %s affects MachineConfigPool %s", curMCN.Name, mcp.Name)
+	ctrl.enqueueMachineConfigPool(mcp)
+}
+
+func (ctrl *Controller) updateMachineConfigNode(old, cur interface{}) {
+	oldMCN := old.(*mcfgv1.MachineConfigNode)
+	curMCN := cur.(*mcfgv1.MachineConfigNode)
+
+	// Only process if the MCN conditions, desired config, or pool association has changed. If the
+	// pool name value is "not-yet-set" it means that the MCN does not have an associated MCP yet,
+	// which is also a condition we should skip on.
+	curPoolName := curMCN.Spec.Pool.Name
+	if curPoolName == upgrademonitor.NotYetSet || (oldMCN.Spec.Pool.Name == curPoolName &&
+		oldMCN.Spec.ConfigVersion.Desired == curMCN.Spec.ConfigVersion.Desired &&
+		equality.Semantic.DeepEqual(oldMCN.Status.Conditions, curMCN.Status.Conditions)) {
+		return
+	}
+
+	klog.V(4).Infof("Updating MachineConfigNode %s", curMCN.Name)
+
+	mcp, err := ctrl.mcpLister.Get(curPoolName)
+	if err != nil {
+		utilruntime.HandleError(fmt.Errorf("Couldn't get MachineConfigPool from MachineConfigNode %v: %v", curMCN.Name, err))
+		return
+	}
+	klog.V(4).Infof("MachineConfigNode %s status changed for MachineConfigPool %s", curMCN.Name, mcp.Name)
+	ctrl.enqueueMachineConfigPool(mcp)
+}
+
+func (ctrl *Controller) deleteMachineConfigNode(obj interface{}) {
+	curMCN, ok := obj.(*mcfgv1.MachineConfigNode)
+	if !ok {
+		tombstone, ok := obj.(cache.DeletedFinalStateUnknown)
+		if !ok {
+			utilruntime.HandleError(fmt.Errorf("couldn't get object from tombstone %#v", obj))
+			return
+		}
+		curMCN, ok = tombstone.Obj.(*mcfgv1.MachineConfigNode)
+		if !ok {
+			utilruntime.HandleError(fmt.Errorf("tombstone contained object that is not a MCN %#v", obj))
+			return
+		}
+	}
+	klog.V(4).Infof("Deleting MachineConfigNode %s", curMCN.Name)
+
+	// Find the associated MachineConfigPool from the MachineConfigNode. If the pool value is
+	// "not-yet-set" it means that the MCN does not have an associated MCP yet.
+	mcpName := curMCN.Spec.Pool.Name
+	if mcpName == upgrademonitor.NotYetSet {
+		return
+	}
+	mcp, err := ctrl.mcpLister.Get(mcpName)
+	if err != nil {
+		utilruntime.HandleError(fmt.Errorf("Couldn't get MachineConfigPool from MachineConfigNode %v", curMCN.Name))
+		return
+	}
+	ctrl.enqueueMachineConfigPool(mcp)
+}
+
 func (ctrl *Controller) addMachineConfigPool(obj interface{}) {
 	pool := obj.(*mcfgv1.MachineConfigPool)
 	klog.V(4).Infof("Adding MachineConfigPool %s", pool.Name)

pkg/controller/node/node_controller_test.go

@@ -107,7 +107,7 @@ func (f *fixture) newControllerWithStopChan(stopCh <-chan struct{}) *Controller
 	k8sI := kubeinformers.NewSharedInformerFactory(f.kubeclient, noResyncPeriodFunc())
 	ci := configv1informer.NewSharedInformerFactory(f.schedulerClient, noResyncPeriodFunc())
 	c := NewWithCustomUpdateDelay(i.Machineconfiguration().V1().ControllerConfigs(), i.Machineconfiguration().V1().MachineConfigs(), i.Machineconfiguration().V1().MachineConfigPools(), k8sI.Core().V1().Nodes(),
-		k8sI.Core().V1().Pods(), i.Machineconfiguration().V1().MachineOSConfigs(), i.Machineconfiguration().V1().MachineOSBuilds(), ci.Config().V1().Schedulers(), f.kubeclient, f.client, time.Millisecond, f.fgHandler)
+		k8sI.Core().V1().Pods(), i.Machineconfiguration().V1().MachineOSConfigs(), i.Machineconfiguration().V1().MachineOSBuilds(), i.Machineconfiguration().V1().MachineConfigNodes(), ci.Config().V1().Schedulers(), f.kubeclient, f.client, time.Millisecond, f.fgHandler)
 
 	c.ccListerSynced = alwaysReady
 	c.mcpListerSynced = alwaysReady
@@ -264,7 +264,9 @@ func filterInformerActions(actions []core.Action) []core.Action {
 				action.Matches("list", "machineosbuilds") ||
 				action.Matches("watch", "machineosbuilds") ||
 				action.Matches("list", "machineosconfigs") ||
-				action.Matches("watch", "machineosconfigs")) {
+				action.Matches("watch", "machineosconfigs") ||
+				action.Matches("list", "machineconfignodes") ||
+				action.Matches("watch", "machineconfignodes")) {
 			continue
 		}
 		ret = append(ret, action)

pkg/controller/node/status_test.go

@@ -7,7 +7,7 @@ import (
 
 	features "github.com/openshift/api/features"
 
-	apicfgv1 "github.com/openshift/api/config/v1"
+	configv1 "github.com/openshift/api/config/v1"
 	mcfgv1 "github.com/openshift/api/machineconfiguration/v1"
 	"github.com/openshift/machine-config-operator/pkg/apihelpers"
 	ctrlcommon "github.com/openshift/machine-config-operator/pkg/controller/common"
@@ -431,11 +431,18 @@ func assertExpectedNodes(t *testing.T, expected []string, actual []*corev1.Node)
 }
 
 func TestCalculateStatus(t *testing.T) {
+	defaultFGs := []configv1.FeatureGateName{
+		features.FeatureGateMachineConfigNodes,
+		features.FeatureGatePinnedImages,
+	}
+
 	t.Parallel()
 	tests := []struct {
 		nodes         []*corev1.Node
+		mcns          []*mcfgv1.MachineConfigNode
 		currentConfig string
 		paused        bool
+		featureGates  []configv1.FeatureGateName
 
 		verify func(mcfgv1.MachineConfigPoolStatus, *testing.T)
 	}{{
@@ -445,6 +452,7 @@ func TestCalculateStatus(t *testing.T) {
 			helpers.NewNodeWithReady("node-2", machineConfigV0, machineConfigV0, corev1.ConditionTrue),
 		},
 		currentConfig: machineConfigV1,
+		featureGates:  defaultFGs,
 		verify: func(status mcfgv1.MachineConfigPoolStatus, t *testing.T) {
 			if got, want := status.MachineCount, int32(3); got != want {
 				t.Fatalf("mismatch MachineCount: got %d want: %d", got, want)
@@ -487,6 +495,7 @@ func TestCalculateStatus(t *testing.T) {
 			helpers.NewNodeWithReady("node-2", machineConfigV0, machineConfigV0, corev1.ConditionTrue),
 		},
 		currentConfig: machineConfigV1,
+		featureGates:  defaultFGs,
 		verify: func(status mcfgv1.MachineConfigPoolStatus, t *testing.T) {
 			if got, want := status.MachineCount, int32(3); got != want {
 				t.Fatalf("mismatch MachineCount: got %d want: %d", got, want)
@@ -530,6 +539,7 @@ func TestCalculateStatus(t *testing.T) {
 		},
 		currentConfig: machineConfigV1,
 		paused:        true,
+		featureGates:  defaultFGs,
 		verify: func(status mcfgv1.MachineConfigPoolStatus, t *testing.T) {
 			condupdated := apihelpers.GetMachineConfigPoolCondition(status, mcfgv1.MachineConfigPoolUpdated)
 			if condupdated == nil {
@@ -554,6 +564,7 @@ func TestCalculateStatus(t *testing.T) {
 			helpers.NewNodeWithReady("node-2", machineConfigV0, machineConfigV0, corev1.ConditionTrue),
 		},
 		currentConfig: machineConfigV1,
+		featureGates:  defaultFGs,
 		verify: func(status mcfgv1.MachineConfigPoolStatus, t *testing.T) {
 			if got, want := status.MachineCount, int32(3); got != want {
 				t.Fatalf("mismatch MachineCount: got %d want: %d", got, want)
@@ -596,6 +607,7 @@ func TestCalculateStatus(t *testing.T) {
 			helpers.NewNodeWithReady("node-2", machineConfigV0, machineConfigV0, corev1.ConditionTrue),
 		},
 		currentConfig: machineConfigV1,
+		featureGates:  defaultFGs,
 		verify: func(status mcfgv1.MachineConfigPoolStatus, t *testing.T) {
 			if got, want := status.MachineCount, int32(3); got != want {
 				t.Fatalf("mismatch MachineCount: got %d want: %d", got, want)
@@ -638,6 +650,7 @@ func TestCalculateStatus(t *testing.T) {
 			helpers.NewNodeWithReady("node-2", machineConfigV0, machineConfigV0, corev1.ConditionTrue),
 		},
 		currentConfig: machineConfigV1,
+		featureGates:  defaultFGs,
 		verify: func(status mcfgv1.MachineConfigPoolStatus, t *testing.T) {
 			if got, want := status.MachineCount, int32(3); got != want {
 				t.Fatalf("mismatch MachineCount: got %d want: %d", got, want)
@@ -680,6 +693,7 @@ func TestCalculateStatus(t *testing.T) {
 			helpers.NewNodeWithReady("node-2", machineConfigV0, machineConfigV1, corev1.ConditionTrue),
 		},
 		currentConfig: machineConfigV1,
+		featureGates:  defaultFGs,
 		verify: func(status mcfgv1.MachineConfigPoolStatus, t *testing.T) {
 			if got, want := status.MachineCount, int32(3); got != want {
 				t.Fatalf("mismatch MachineCount: got %d want: %d", got, want)
@@ -722,6 +736,55 @@ func TestCalculateStatus(t *testing.T) {
 			helpers.NewNodeWithReady("node-2", machineConfigV1, machineConfigV1, corev1.ConditionTrue),
 		},
 		currentConfig: machineConfigV1,
+		featureGates:  defaultFGs,
+		verify: func(status mcfgv1.MachineConfigPoolStatus, t *testing.T) {
+			if got, want := status.MachineCount, int32(3); got != want {
+				t.Fatalf("mismatch MachineCount: got %d want: %d", got, want)
+			}
+
+			if got, want := status.UpdatedMachineCount, int32(3); got != want {
+				t.Fatalf("mismatch UpdatedMachineCount: got %d want: %d", got, want)
+			}
+
+			if got, want := status.ReadyMachineCount, int32(3); got != want {
+				t.Fatalf("mismatch ReadyMachineCount: got %d want: %d", got, want)
+			}
+
+			if got, want := status.UnavailableMachineCount, int32(0); got != want {
+				t.Fatalf("mismatch UnavailableMachineCount: got %d want: %d", got, want)
+			}
+
+			condupdated := apihelpers.GetMachineConfigPoolCondition(status, mcfgv1.MachineConfigPoolUpdated)
+			if condupdated == nil {
+				t.Fatal("updated condition not found")
+			}
+
+			condupdating := apihelpers.GetMachineConfigPoolCondition(status, mcfgv1.MachineConfigPoolUpdating)
+			if condupdating == nil {
+				t.Fatal("updated condition not found")
+			}
+
+			if got, want := condupdated.Status, corev1.ConditionTrue; got != want {
+				t.Fatalf("mismatch condupdated.Status: got %s want: %s", got, want)
+			}
+
+			if got, want := condupdating.Status, corev1.ConditionFalse; got != want {
+				t.Fatalf("mismatch condupdating.Status: got %s want: %s", got, want)
+			}
+		},
+	}, { // For the `ImageModeStatusReporting` feature gate, MCP machine counts are populated from MCN conditions, so test those cases
+		nodes: []*corev1.Node{
+			helpers.NewNodeWithReady("node-0", machineConfigV1, machineConfigV1, corev1.ConditionTrue),
+			helpers.NewNodeWithReady("node-1", machineConfigV1, machineConfigV1, corev1.ConditionTrue),
+			helpers.NewNodeWithReady("node-2", machineConfigV1, machineConfigV1, corev1.ConditionTrue),
+		},
+		mcns: []*mcfgv1.MachineConfigNode{
+			helpers.NewMachineConfigNode("node-0", "worker", machineConfigV1, true, false),
+			helpers.NewMachineConfigNode("node-1", "worker", machineConfigV1, true, false),
+			helpers.NewMachineConfigNode("node-2", "worker", machineConfigV1, true, false),
+		},
+		currentConfig: machineConfigV1,
+		featureGates:  append(defaultFGs, features.FeatureGateImageModeStatusReporting),
 		verify: func(status mcfgv1.MachineConfigPoolStatus, t *testing.T) {
 			if got, want := status.MachineCount, int32(3); got != want {
 				t.Fatalf("mismatch MachineCount: got %d want: %d", got, want)
@@ -770,14 +833,14 @@ func TestCalculateStatus(t *testing.T) {
 				},
 			}
 			f := newFixtureWithFeatureGates(t,
-				[]apicfgv1.FeatureGateName{
+				[]configv1.FeatureGateName{
 					features.FeatureGateMachineConfigNodes,
 					features.FeatureGatePinnedImages,
 				},
-				[]apicfgv1.FeatureGateName{},
+				[]configv1.FeatureGateName{},
 			)
 			c := f.newController()
-			status := c.calculateStatus([]*mcfgv1.MachineConfigNode{}, nil, pool, test.nodes, nil, nil)
+			status := c.calculateStatus(test.mcns, nil, pool, test.nodes, nil, nil)
 			test.verify(status, t)
 		})
 	}

test/e2e-bootstrap/bootstrap_test.go

@@ -525,6 +525,7 @@ func createControllers(ctx *ctrlcommon.ControllerContext) []ctrlcommon.Controlle
 			ctx.KubeInformerFactory.Core().V1().Pods(),
 			ctx.InformerFactory.Machineconfiguration().V1().MachineOSConfigs(),
 			ctx.InformerFactory.Machineconfiguration().V1().MachineOSBuilds(),
+			ctx.InformerFactory.Machineconfiguration().V1().MachineConfigNodes(),
 			ctx.ConfigInformerFactory.Config().V1().Schedulers(),
 			ctx.ClientBuilder.KubeClientOrDie("node-update-controller"),
 			ctx.ClientBuilder.MachineConfigClientOrDie("node-update-controller"),

test/helpers/helpers.go

@@ -308,3 +308,46 @@ func GetNamesFromNodes(nodes []*corev1.Node) []string {
 	}
 	return names
 }
+
+// NewMachineConfigNode creates a new MachineConfigNode with the specified configuration and ready status
+func NewMachineConfigNode(name, pool, desiredConfig string, isUpdated, isDegraded bool) *mcfgv1.MachineConfigNode {
+	updatedStatus := metav1.ConditionFalse
+	if isUpdated {
+		updatedStatus = metav1.ConditionTrue
+	}
+	degradedStatus := metav1.ConditionFalse
+	if isDegraded {
+		degradedStatus = metav1.ConditionTrue
+	}
+	return &mcfgv1.MachineConfigNode{
+		Spec: mcfgv1.MachineConfigNodeSpec{
+			Node: mcfgv1.MCOObjectReference{
+				Name: name,
+			},
+			Pool: mcfgv1.MCOObjectReference{
+				Name: pool,
+			},
+			ConfigVersion: mcfgv1.MachineConfigNodeSpecMachineConfigVersion{
+				Desired: desiredConfig,
+			},
+		},
+		Status: mcfgv1.MachineConfigNodeStatus{
+			Conditions: []metav1.Condition{
+				metav1.Condition{
+					Type:               string(mcfgv1.MachineConfigNodeUpdated),
+					Message:            "test",
+					Reason:             "test",
+					LastTransitionTime: metav1.Now(),
+					Status:             updatedStatus,
+				},
+				metav1.Condition{
+					Type:               string(mcfgv1.MachineConfigNodeNodeDegraded),
+					Message:            "test",
+					Reason:             "test",
+					LastTransitionTime: metav1.Now(),
+					Status:             degradedStatus,
+				},
+			},
+		},
+	}
+}