From 0e0599e17427588c98e04b406e71e9f47b7adf38 Mon Sep 17 00:00:00 2001 From: Isabella Janssen Date: Mon, 17 Nov 2025 15:34:35 -0500 Subject: [PATCH 1/2] controller: add MachineConfigNode informer so changes trigger MachineConfigPool syncs --- cmd/machine-config-controller/start.go | 1 + pkg/controller/node/node_controller.go | 88 +++++++++++++++++++++ pkg/controller/node/node_controller_test.go | 6 +- test/e2e-bootstrap/bootstrap_test.go | 1 + 4 files changed, 94 insertions(+), 2 deletions(-) diff --git a/cmd/machine-config-controller/start.go b/cmd/machine-config-controller/start.go index dc191b6c33..f1db55c5f0 100644 --- a/cmd/machine-config-controller/start.go +++ b/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"), diff --git a/pkg/controller/node/node_controller.go b/pkg/controller/node/node_controller.go index b0cfa0eb6e..8a97410cc8 100644 --- a/pkg/controller/node/node_controller.go +++ b/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) diff --git a/pkg/controller/node/node_controller_test.go b/pkg/controller/node/node_controller_test.go index f9b42152f5..fae5d35f70 100644 --- a/pkg/controller/node/node_controller_test.go +++ b/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) diff --git a/test/e2e-bootstrap/bootstrap_test.go b/test/e2e-bootstrap/bootstrap_test.go index 1e90d421ca..c8ceb05354 100644 --- a/test/e2e-bootstrap/bootstrap_test.go +++ b/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"), From 6e1f8ab14db8d356abd30dd7e9fabde7b0d7020b Mon Sep 17 00:00:00 2001 From: Isabella Janssen Date: Wed, 26 Nov 2025 14:31:01 -0500 Subject: [PATCH 2/2] one unit test --- pkg/controller/node/status_test.go | 71 ++++++++++++++++++++++++++++-- test/helpers/helpers.go | 43 ++++++++++++++++++ 2 files changed, 110 insertions(+), 4 deletions(-) diff --git a/pkg/controller/node/status_test.go b/pkg/controller/node/status_test.go index 9304d34f6f..da011ace82 100644 --- a/pkg/controller/node/status_test.go +++ b/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) }) } diff --git a/test/helpers/helpers.go b/test/helpers/helpers.go index cf327dfb90..4b09cf34cd 100644 --- a/test/helpers/helpers.go +++ b/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, + }, + }, + }, + } +}