diff --git a/pkg/controller/drain/drain_controller.go b/pkg/controller/drain/drain_controller.go index a5fab9a324..5cfd8d7533 100644 --- a/pkg/controller/drain/drain_controller.go +++ b/pkg/controller/drain/drain_controller.go @@ -315,7 +315,7 @@ func (ctrl *Controller) syncNode(key string) error { } // Get MCP associated with node - mcpName, desiredConfigVersion, err := helpers.GetPrimaryPoolDetailsForMCN(ctrl.mcpLister, node) + pool, err := helpers.GetPrimaryPoolNameForMCN(ctrl.mcpLister, node) if err != nil { return err } @@ -333,8 +333,7 @@ func (ctrl *Controller) syncNode(key string) error { node, ctrl.client, ctrl.fgHandler, - mcpName, - desiredConfigVersion, + pool, ) if nErr != nil { klog.Errorf("Error making MCN for Uncordon failure: %v", err) @@ -350,8 +349,7 @@ func (ctrl *Controller) syncNode(key string) error { node, ctrl.client, ctrl.fgHandler, - mcpName, - desiredConfigVersion, + pool, ) if err != nil { klog.Errorf("Error making MCN for UnCordon success: %v", err) @@ -407,7 +405,7 @@ func (ctrl *Controller) drainNode(node *corev1.Node, drainer *drain.Helper) erro } // Get MCP associated with node - mcpName, desiredConfigVersion, err := helpers.GetPrimaryPoolDetailsForMCN(ctrl.mcpLister, node) + pool, err := helpers.GetPrimaryPoolNameForMCN(ctrl.mcpLister, node) if err != nil { return err } @@ -423,8 +421,7 @@ func (ctrl *Controller) drainNode(node *corev1.Node, drainer *drain.Helper) erro node, ctrl.client, ctrl.fgHandler, - mcpName, - desiredConfigVersion, + pool, ) if Nerr != nil { klog.Errorf("Error making MCN for Cordon Failure: %v", Nerr) @@ -439,8 +436,7 @@ func (ctrl *Controller) drainNode(node *corev1.Node, drainer *drain.Helper) erro node, ctrl.client, ctrl.fgHandler, - mcpName, - desiredConfigVersion, + pool, ) if err != nil { klog.Errorf("Error making MCN for Cordon Success: %v", err) @@ -457,8 +453,7 @@ func (ctrl *Controller) drainNode(node *corev1.Node, drainer *drain.Helper) erro node, ctrl.client, ctrl.fgHandler, - mcpName, - desiredConfigVersion, + pool, ) if err != nil { klog.Errorf("Error making MCN for Drain beginning: %v", err) @@ -488,8 +483,7 @@ func (ctrl *Controller) drainNode(node *corev1.Node, drainer *drain.Helper) erro node, ctrl.client, ctrl.fgHandler, - mcpName, - desiredConfigVersion, + pool, ) if nErr != nil { klog.Errorf("Error making MCN for Drain failure: %v", nErr) @@ -506,8 +500,7 @@ func (ctrl *Controller) drainNode(node *corev1.Node, drainer *drain.Helper) erro node, ctrl.client, ctrl.fgHandler, - mcpName, - desiredConfigVersion, + pool, ) if err != nil { klog.Errorf("Error making MCN for Drain success: %v", err) diff --git a/pkg/controller/node/node_controller.go b/pkg/controller/node/node_controller.go index 5b1a1989c9..1eb93fc68b 100644 --- a/pkg/controller/node/node_controller.go +++ b/pkg/controller/node/node_controller.go @@ -9,7 +9,6 @@ import ( "time" helpers "github.com/openshift/machine-config-operator/pkg/helpers" - "github.com/openshift/machine-config-operator/pkg/upgrademonitor" configv1 "github.com/openshift/api/config/v1" features "github.com/openshift/api/features" @@ -1222,21 +1221,14 @@ func (ctrl *Controller) updateCandidateNode(mosc *mcfgv1.MachineOSConfig, mosb * } lns := ctrlcommon.NewLayeredNodeState(oldNode) - var desiredConfig string if !layered { lns.SetDesiredStateFromPool(pool) - desiredConfig = pool.Spec.Configuration.Name } else { lns.SetDesiredStateFromMachineOSConfig(mosc, mosb) - desiredConfig = mosb.Spec.MachineConfig.Name - } - // Update the desired config version in the node's MCN - err = upgrademonitor.GenerateAndApplyMachineConfigNodeSpec(ctrl.fgHandler, pool.Name, desiredConfig, oldNode, ctrl.client) - if err != nil { - klog.Errorf("error updating MCN spec for node %s: %v", oldNode.Name, err) } // Set the desired state to match the pool. + newData, err := json.Marshal(lns.Node()) if err != nil { return err diff --git a/pkg/controller/node/node_controller_test.go b/pkg/controller/node/node_controller_test.go index 3800f39d2c..57288abdfb 100644 --- a/pkg/controller/node/node_controller_test.go +++ b/pkg/controller/node/node_controller_test.go @@ -95,7 +95,7 @@ func newFixtureWithFeatureGates(t *testing.T, enabled, disabled []configv1.Featu } func newFixture(t *testing.T) *fixture { - return newFixtureWithFeatureGates(t, []configv1.FeatureGateName{features.FeatureGateMachineConfigNodes, features.FeatureGatePinnedImages, features.FeatureGateOnClusterBuild}, []configv1.FeatureGateName{}) + return newFixtureWithFeatureGates(t, []configv1.FeatureGateName{features.FeatureGatePinnedImages, features.FeatureGateOnClusterBuild}, []configv1.FeatureGateName{}) } func (f *fixture) newControllerWithStopChan(stopCh <-chan struct{}) *Controller { @@ -144,10 +144,18 @@ func (f *fixture) newController() *Controller { return f.newControllerWithStopChan(stopCh) } +func (f *fixture) newControllerWithContext(ctx context.Context) *Controller { + return f.newControllerWithStopChan(ctx.Done()) +} + func (f *fixture) run(pool string) { f.runController(pool, false) } +func (f *fixture) runExpectError(pool string) { + f.runController(pool, true) +} + func (f *fixture) runController(pool string, expectError bool) { c := f.newController() @@ -256,9 +264,7 @@ 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("get", "machineconfignodes") || - action.Matches("create", "machineconfignodes")) { + action.Matches("watch", "machineosconfigs")) { continue } ret = append(ret, action) @@ -1144,7 +1150,7 @@ func TestShouldMakeProgress(t *testing.T) { expectTaintsAddPatch: false, }, { - description: "node not at desired config, patch on annotation and taints", + description: "node not at desired config, patch on annotation and taints", // failing node: newNodeWithLabel("nodeNeedingUpdates", machineConfigV0, machineConfigV0, map[string]string{"node-role/worker": "", "node-role/infra": ""}), expectAnnotationPatch: true, expectTaintsAddPatch: true, @@ -1433,11 +1439,9 @@ func TestCertStatus(t *testing.T) { mcp := helpers.NewMachineConfigPool("test-cluster-infra", nil, helpers.InfraSelector, "v1") mcpWorker := helpers.NewMachineConfigPool("worker", nil, helpers.WorkerSelector, "v1") mcp.Spec.MaxUnavailable = intStrPtr(intstr.FromInt(1)) - nodeName0 := "node-0" - nodeName1 := "node-1" nodes := []*corev1.Node{ - newNodeWithLabel(nodeName0, "v1", "v1", map[string]string{"node-role/worker": "", "node-role/infra": ""}), - newNodeWithLabel(nodeName1, "v1", "v1", map[string]string{"node-role/worker": "", "node-role/infra": ""}), + newNodeWithLabel("node-0", "v1", "v1", map[string]string{"node-role/worker": "", "node-role/infra": ""}), + newNodeWithLabel("node-1", "v1", "v1", map[string]string{"node-role/worker": "", "node-role/infra": ""}), } f.ccLister = append(f.ccLister, cc) diff --git a/pkg/daemon/daemon.go b/pkg/daemon/daemon.go index d8cfc9bace..830db6e744 100644 --- a/pkg/daemon/daemon.go +++ b/pkg/daemon/daemon.go @@ -712,7 +712,7 @@ func (dn *Daemon) syncNode(key string) error { } // Get MCP associated with node - mcpName, desiredConfigVersion, err := helpers.GetPrimaryPoolDetailsForMCN(dn.mcpLister, node) + pool, err := helpers.GetPrimaryPoolNameForMCN(dn.mcpLister, node) if err != nil { return err } @@ -727,8 +727,7 @@ func (dn *Daemon) syncNode(key string) error { node, dn.mcfgClient, dn.fgHandler, - mcpName, - desiredConfigVersion, + pool, ) if err != nil { klog.Errorf("Error making MCN for Rebooted: %v", err) @@ -804,8 +803,7 @@ func (dn *Daemon) syncNode(key string) error { node, dn.mcfgClient, dn.fgHandler, - mcpName, - desiredConfigVersion, + pool, ) if err != nil { klog.Errorf("Error making MCN for Resumed true: %v", err) @@ -844,8 +842,7 @@ func (dn *Daemon) syncNode(key string) error { dn.node, dn.mcfgClient, dn.fgHandler, - mcpName, - desiredConfigVersion, + pool, ) if err != nil { klog.Errorf("Error making MCN for Updated false: %v", err) @@ -870,8 +867,7 @@ func (dn *Daemon) syncNode(key string) error { dn.node, dn.mcfgClient, dn.fgHandler, - mcpName, - desiredConfigVersion, + pool, ) if err != nil { klog.Errorf("Error making MCN for Updated: %v", err) @@ -2322,7 +2318,7 @@ func (dn *Daemon) updateConfigAndState(state *stateAndConfigs) (bool, bool, erro // let's mark it done! // Get MCP associated with node - mcpName, desiredConfigVersion, err := helpers.GetPrimaryPoolDetailsForMCN(dn.mcpLister, dn.node) + pool, err := helpers.GetPrimaryPoolNameForMCN(dn.mcpLister, dn.node) if err != nil { return missingODC, inDesiredConfig, err } @@ -2335,8 +2331,7 @@ func (dn *Daemon) updateConfigAndState(state *stateAndConfigs) (bool, bool, erro dn.node, dn.mcfgClient, dn.fgHandler, - mcpName, - desiredConfigVersion, + pool, ) if err != nil { klog.Errorf("Error making MCN for Resumed true: %v", err) diff --git a/pkg/daemon/drain.go b/pkg/daemon/drain.go index 5b84501b67..23cd1ec20c 100644 --- a/pkg/daemon/drain.go +++ b/pkg/daemon/drain.go @@ -43,7 +43,7 @@ func (dn *Daemon) performDrain() error { logSystem("Drain not required, skipping") // Get MCP associated with node - mcpName, desiredConfigVersion, err := helpers.GetPrimaryPoolDetailsForMCN(dn.mcpLister, dn.node) + pool, err := helpers.GetPrimaryPoolNameForMCN(dn.mcpLister, dn.node) if err != nil { return err } @@ -56,8 +56,7 @@ func (dn *Daemon) performDrain() error { dn.node, dn.mcfgClient, dn.fgHandler, - mcpName, - desiredConfigVersion, + pool, ) if err != nil { klog.Errorf("Error making MCN for Drain not required: %v", err) diff --git a/pkg/daemon/pinned_image_set.go b/pkg/daemon/pinned_image_set.go index f9a8bd9649..f90b1a3e78 100644 --- a/pkg/daemon/pinned_image_set.go +++ b/pkg/daemon/pinned_image_set.go @@ -546,7 +546,7 @@ func (p *PinnedImageSetManager) updateStatusProgressing(pools []*mcfgv1.MachineC } // Get MCP associated with node - mcpName, desiredConfigVersion, err := helpers.GetPrimaryPoolDetailsForMCN(p.mcpLister, node) + pool, err := helpers.GetPrimaryPoolNameForMCN(p.mcpLister, node) if err != nil { return err } @@ -564,8 +564,7 @@ func (p *PinnedImageSetManager) updateStatusProgressing(pools []*mcfgv1.MachineC p.mcfgClient, applyCfg, p.fgHandler, - mcpName, - desiredConfigVersion, + pool, ) } @@ -582,7 +581,7 @@ func (p *PinnedImageSetManager) updateStatusProgressingComplete(pools []*mcfgv1. } // Get MCP associated with node - mcpName, desiredConfigVersion, err := helpers.GetPrimaryPoolDetailsForMCN(p.mcpLister, node) + pool, err := helpers.GetPrimaryPoolNameForMCN(p.mcpLister, node) if err != nil { return err } @@ -600,8 +599,7 @@ func (p *PinnedImageSetManager) updateStatusProgressingComplete(pools []*mcfgv1. p.mcfgClient, applyCfg, p.fgHandler, - mcpName, - desiredConfigVersion, + pool, ) if err != nil { klog.Errorf("Failed to update machine config node: %v", err) @@ -621,8 +619,7 @@ func (p *PinnedImageSetManager) updateStatusProgressingComplete(pools []*mcfgv1. p.mcfgClient, nil, p.fgHandler, - mcpName, - desiredConfigVersion, + pool, ) } @@ -639,7 +636,7 @@ func (p *PinnedImageSetManager) updateStatusError(pools []*mcfgv1.MachineConfigP } // Get MCP associated with node - mcpName, desiredConfigVersion, err := helpers.GetPrimaryPoolDetailsForMCN(p.mcpLister, node) + pool, err := helpers.GetPrimaryPoolNameForMCN(p.mcpLister, node) if err != nil { return err } @@ -657,8 +654,7 @@ func (p *PinnedImageSetManager) updateStatusError(pools []*mcfgv1.MachineConfigP p.mcfgClient, applyCfg, p.fgHandler, - mcpName, - desiredConfigVersion, + pool, ) } diff --git a/pkg/daemon/update.go b/pkg/daemon/update.go index 9b5b4380e5..425a3254f2 100644 --- a/pkg/daemon/update.go +++ b/pkg/daemon/update.go @@ -123,7 +123,7 @@ func (dn *Daemon) executeReloadServiceNodeDisruptionAction(serviceName string, r } // Get MCP associated with node - mcpName, desiredConfigVersion, err := helpers.GetPrimaryPoolDetailsForMCN(dn.mcpLister, dn.node) + pool, err := helpers.GetPrimaryPoolNameForMCN(dn.mcpLister, dn.node) if err != nil { return err } @@ -136,8 +136,7 @@ func (dn *Daemon) executeReloadServiceNodeDisruptionAction(serviceName string, r dn.node, dn.mcfgClient, dn.fgHandler, - mcpName, - desiredConfigVersion, + pool, ) if err != nil { klog.Errorf("Error making MCN for Reloading success: %v", err) @@ -165,7 +164,7 @@ func (dn *Daemon) performPostConfigChangeNodeDisruptionAction(postConfigChangeAc logSystem("Performing post config change action: %v for config %s", action.Type, configName) // Get MCP associated with node - mcpName, desiredConfigVersion, err := helpers.GetPrimaryPoolDetailsForMCN(dn.mcpLister, dn.node) + pool, err := helpers.GetPrimaryPoolNameForMCN(dn.mcpLister, dn.node) if err != nil { return err } @@ -180,8 +179,7 @@ func (dn *Daemon) performPostConfigChangeNodeDisruptionAction(postConfigChangeAc dn.node, dn.mcfgClient, dn.fgHandler, - mcpName, - desiredConfigVersion, + pool, ) if err != nil { klog.Errorf("Error making MCN for rebooting: %v", err) @@ -201,8 +199,7 @@ func (dn *Daemon) performPostConfigChangeNodeDisruptionAction(postConfigChangeAc dn.node, dn.mcfgClient, dn.fgHandler, - mcpName, - desiredConfigVersion, + pool, ) if err != nil { klog.Errorf("Error making MCN for no post config change action: %v", err) @@ -271,7 +268,7 @@ func (dn *Daemon) performPostConfigChangeNodeDisruptionAction(postConfigChangeAc // If at any point an error occurs, we reboot the node so that node has correct configuration. func (dn *Daemon) performPostConfigChangeAction(postConfigChangeActions []string, configName string) error { // Get MCP associated with node - mcpName, desiredConfigVersion, err := helpers.GetPrimaryPoolDetailsForMCN(dn.mcpLister, dn.node) + pool, err := helpers.GetPrimaryPoolNameForMCN(dn.mcpLister, dn.node) if err != nil { return err } @@ -285,8 +282,7 @@ func (dn *Daemon) performPostConfigChangeAction(postConfigChangeActions []string dn.node, dn.mcfgClient, dn.fgHandler, - mcpName, - desiredConfigVersion, + pool, ) if err != nil { klog.Errorf("Error making MCN for rebooting: %v", err) @@ -307,8 +303,7 @@ func (dn *Daemon) performPostConfigChangeAction(postConfigChangeActions []string dn.node, dn.mcfgClient, dn.fgHandler, - mcpName, - desiredConfigVersion, + pool, ) if err != nil { klog.Errorf("Error making MCN for no post config change action: %v", err) @@ -334,8 +329,7 @@ func (dn *Daemon) performPostConfigChangeAction(postConfigChangeActions []string dn.node, dn.mcfgClient, dn.fgHandler, - mcpName, - desiredConfigVersion, + pool, ) if err != nil { klog.Errorf("Error making MCN for Reloading success: %v", err) @@ -960,14 +954,14 @@ func (dn *Daemon) update(oldConfig, newConfig *mcfgv1.MachineConfig, skipCertifi }() // Get MCP associated with node - mcpName, desiredConfigVersion, err := helpers.GetPrimaryPoolDetailsForMCN(dn.mcpLister, dn.node) + pool, err := helpers.GetPrimaryPoolNameForMCN(dn.mcpLister, dn.node) if err != nil { return err } // Update the MCN's NodeNodeDegraded condition with the update result defer func() { - dn.reportMachineNodeDegradeStatus(retErr, mcpName, desiredConfigVersion) + dn.reportMachineNodeDegradeStatus(retErr, pool) }() oldConfigName := oldConfig.GetName() @@ -996,8 +990,7 @@ func (dn *Daemon) update(oldConfig, newConfig *mcfgv1.MachineConfig, skipCertifi dn.node, dn.mcfgClient, dn.fgHandler, - mcpName, - desiredConfigVersion, + pool, ) if Nerr != nil { klog.Errorf("Error making MCN for Preparing update failed: %v", err) @@ -1033,8 +1026,7 @@ func (dn *Daemon) update(oldConfig, newConfig *mcfgv1.MachineConfig, skipCertifi dn.node, dn.mcfgClient, dn.fgHandler, - mcpName, - desiredConfigVersion, + pool, ) if Nerr != nil { klog.Errorf("Error making MCN for Preparing update failed: %v", err) @@ -1066,13 +1058,16 @@ func (dn *Daemon) update(oldConfig, newConfig *mcfgv1.MachineConfig, skipCertifi dn.node, dn.mcfgClient, dn.fgHandler, - mcpName, - desiredConfigVersion, + pool, ) if err != nil { klog.Errorf("Error making MCN for Update Compatible: %v", err) } + err = upgrademonitor.GenerateAndApplyMachineConfigNodeSpec(dn.fgHandler, pool, dn.node, dn.mcfgClient) + if err != nil { + klog.Errorf("Error making MCN spec for Update Compatible: %v", err) + } if drain { if err := dn.performDrain(); err != nil { return err @@ -1087,8 +1082,7 @@ func (dn *Daemon) update(oldConfig, newConfig *mcfgv1.MachineConfig, skipCertifi dn.node, dn.mcfgClient, dn.fgHandler, - mcpName, - desiredConfigVersion, + pool, ) if err != nil { klog.Errorf("Error making MCN for Drain not required: %v", err) @@ -1116,8 +1110,7 @@ func (dn *Daemon) update(oldConfig, newConfig *mcfgv1.MachineConfig, skipCertifi dn.node, dn.mcfgClient, dn.fgHandler, - mcpName, - desiredConfigVersion, + pool, ) if err != nil { klog.Errorf("Error making MCN for Updating Files and OS: %v", err) @@ -1239,8 +1232,7 @@ func (dn *Daemon) update(oldConfig, newConfig *mcfgv1.MachineConfig, skipCertifi dn.node, dn.mcfgClient, dn.fgHandler, - mcpName, - desiredConfigVersion, + pool, ) if err != nil { klog.Errorf("Error making MCN for Updated Files and OS: %v", err) @@ -2920,7 +2912,7 @@ func canonicalizeMachineConfigImage(img string, mc *mcfgv1.MachineConfig) *mcfgv // If the error is not nil the condition status is set to [metav1.ConditionTrue] and the condition // message is formatted accordingly to include the error message. The condition is otherwise set to // [metav1.ConditionFalse]. -func (dn *Daemon) reportMachineNodeDegradeStatus(err error, mcpName, desiredConfigVersion string) { +func (dn *Daemon) reportMachineNodeDegradeStatus(err error, pool string) { if dn.node == nil { return } @@ -2944,8 +2936,7 @@ func (dn *Daemon) reportMachineNodeDegradeStatus(err error, mcpName, desiredConf dn.node, dn.mcfgClient, dn.fgHandler, - mcpName, - desiredConfigVersion, + pool, ); applyErr != nil { klog.Errorf("Error updating MCN degraded status condition %v", applyErr) } diff --git a/pkg/daemon/upgrade_monitor_test.go b/pkg/daemon/upgrade_monitor_test.go index 4ba4cfc61e..42d38cd88d 100644 --- a/pkg/daemon/upgrade_monitor_test.go +++ b/pkg/daemon/upgrade_monitor_test.go @@ -2,9 +2,8 @@ package daemon import ( "context" - "testing" - ctrlcommon "github.com/openshift/machine-config-operator/pkg/controller/common" + "testing" apicfgv1 "github.com/openshift/api/config/v1" "github.com/openshift/machine-config-operator/pkg/upgrademonitor" @@ -155,12 +154,12 @@ func (tc upgradeMonitorTestCase) run(t *testing.T) { for _, n := range f.nodeLister { // Get MCP associated with node - mcpName, desiredConfigVersion, err := helpers.GetPrimaryPoolDetailsForMCN(d.mcpLister, n) + pool, err := helpers.GetPrimaryPoolNameForMCN(d.mcpLister, n) if err != nil { f.t.Fatalf("Error getting primary pool for node: %v", n.Name) } - err = upgrademonitor.GenerateAndApplyMachineConfigNodes(tc.parentCondition, tc.childCondition, tc.parentStatus, tc.childStatus, n, d.mcfgClient, d.fgHandler, mcpName, desiredConfigVersion) + err = upgrademonitor.GenerateAndApplyMachineConfigNodes(tc.parentCondition, tc.childCondition, tc.parentStatus, tc.childStatus, n, d.mcfgClient, d.fgHandler, pool) if err != nil { f.t.Fatalf("Could not generate and apply MCN %v", err) } diff --git a/pkg/helpers/helpers.go b/pkg/helpers/helpers.go index 23054e6a20..0b0ad484d8 100644 --- a/pkg/helpers/helpers.go +++ b/pkg/helpers/helpers.go @@ -51,32 +51,30 @@ func GetNodesForPool(mcpLister v1.MachineConfigPoolLister, nodeLister corev1list return nodes, nil } -// GetPrimaryPoolDetailsForMCN gets the MCP details needed to populate a node's MachineConfigNode object, -// which are the MCP name and desired config version. -// - When the node does not yet exist (is nil) or the node does not yet have annotations, the MCP name -// and desried config version will temporarily be set to `not-yet-set` (the default not yet set -// placeholder value for upgrade monitor functions). -// - Once the node exists (is not nil) and the node annotations are properly set, the node will update -// again and the MCP name and desired config version will be updated accordingly. -func GetPrimaryPoolDetailsForMCN(mcpLister v1.MachineConfigPoolLister, node *corev1.Node) (mcpName, desiredConfig string, err error) { +// GetPrimaryPoolNameForMCN gets the MCP pool name value that is used in a node's MachineConfigNode object. +// When the node does not yet exist (is nil) or the node does not yet have annotations, the pool name will +// temporarily be set to `not-yet-set` (the default not yet set placeholder value for upgrade monitor +// functions). Once the node exists (is not nil) and the annotations are properly set, the node will update +// again and the pool name will be updated. +func GetPrimaryPoolNameForMCN(mcpLister v1.MachineConfigPoolLister, node *corev1.Node) (string, error) { // Handle case of nil node if node == nil { - klog.Error("node object is nil, setting associated MCP values to 'not-yet-set'") - return upgrademonitor.NotYetSet, upgrademonitor.NotYetSet, nil + klog.Error("node object is nil, setting associated MCP to unknown") + return upgrademonitor.NotYetSet, nil } // Use `GetPrimaryPoolForNode` to get primary MCP associated with node primaryPool, err := GetPrimaryPoolForNode(mcpLister, node) if err != nil { - klog.Errorf("error getting primary MCP for node: %v", node.Name) - return "", "", err + klog.Errorf("error getting primary pool for node: %v", node.Name) + return "", err } else if primaryPool == nil { // On first provisioning, the node may not have annoatations and, thus, will not be associated with a pool. // In this case, the pool value will be set to a temporary dummy value. - klog.Infof("No primary MCP is associated with node: %v", node.Name) - return upgrademonitor.NotYetSet, upgrademonitor.NotYetSet, nil + klog.Infof("No primary pool is associated with node: %v", node.Name) + return upgrademonitor.NotYetSet, nil } - return primaryPool.Name, primaryPool.Spec.Configuration.Name, nil + return primaryPool.Name, nil } func GetPrimaryPoolForNode(mcpLister v1.MachineConfigPoolLister, node *corev1.Node) (*mcfgv1.MachineConfigPool, error) { diff --git a/pkg/operator/sync.go b/pkg/operator/sync.go index 411c9bcd14..0d37d800b9 100644 --- a/pkg/operator/sync.go +++ b/pkg/operator/sync.go @@ -769,7 +769,7 @@ func (optr *Operator) syncMachineConfigNodes(_ *renderConfig, _ *configv1.Cluste } // Get MCP associated with node - mcpName, mcpDesiredConfig, err := helpers.GetPrimaryPoolDetailsForMCN(optr.mcpLister, node) + pool, err := helpers.GetPrimaryPoolNameForMCN(optr.mcpLister, node) if err != nil { return err } @@ -780,7 +780,7 @@ func (optr *Operator) syncMachineConfigNodes(_ *renderConfig, _ *configv1.Cluste Name: node.Name, }, Pool: mcfgv1.MCOObjectReference{ - Name: mcpName, + Name: pool, }, ConfigVersion: mcfgv1.MachineConfigNodeSpecMachineConfigVersion{ Desired: upgrademonitor.NotYetSet, @@ -814,7 +814,7 @@ func (optr *Operator) syncMachineConfigNodes(_ *renderConfig, _ *configv1.Cluste } // if this is the first time we are applying the MCN and the node is ready, set the config version probably if mcn.Spec.ConfigVersion.Desired == upgrademonitor.NotYetSet { - err = upgrademonitor.GenerateAndApplyMachineConfigNodeSpec(optr.fgHandler, mcpName, mcpDesiredConfig, node, optr.client) + err = upgrademonitor.GenerateAndApplyMachineConfigNodeSpec(optr.fgHandler, pool, node, optr.client) if err != nil { klog.Errorf("Error making MCN spec for Update Compatible: %v", err) } diff --git a/pkg/upgrademonitor/upgrade_monitor.go b/pkg/upgrademonitor/upgrade_monitor.go index 3fb3194f4d..d58542b8bb 100644 --- a/pkg/upgrademonitor/upgrade_monitor.go +++ b/pkg/upgrademonitor/upgrade_monitor.go @@ -42,10 +42,9 @@ func GenerateAndApplyMachineConfigNodes( node *corev1.Node, mcfgClient mcfgclientset.Interface, fgHandler ctrlcommon.FeatureGatesHandler, - mcpName string, - desiredConfigVersion string, + pool string, ) error { - return generateAndApplyMachineConfigNodes(parentCondition, childCondition, parentStatus, childStatus, node, mcfgClient, nil, fgHandler, mcpName, desiredConfigVersion) + return generateAndApplyMachineConfigNodes(parentCondition, childCondition, parentStatus, childStatus, node, mcfgClient, nil, fgHandler, pool) } func UpdateMachineConfigNodeStatus( @@ -57,10 +56,9 @@ func UpdateMachineConfigNodeStatus( mcfgClient mcfgclientset.Interface, imageSetApplyConfig []*machineconfigurationv1.MachineConfigNodeStatusPinnedImageSetApplyConfiguration, fgHandler ctrlcommon.FeatureGatesHandler, - mcpName string, - desiredConfigVersion string, + pool string, ) error { - return generateAndApplyMachineConfigNodes(parentCondition, childCondition, parentStatus, childStatus, node, mcfgClient, imageSetApplyConfig, fgHandler, mcpName, desiredConfigVersion) + return generateAndApplyMachineConfigNodes(parentCondition, childCondition, parentStatus, childStatus, node, mcfgClient, imageSetApplyConfig, fgHandler, pool) } // Helper function to convert metav1.Condition to ConditionApplyConfiguration @@ -93,8 +91,7 @@ func generateAndApplyMachineConfigNodes( mcfgClient mcfgclientset.Interface, imageSetApplyConfig []*machineconfigurationv1.MachineConfigNodeStatusPinnedImageSetApplyConfiguration, fgHandler ctrlcommon.FeatureGatesHandler, - mcpName string, - desiredConfigVersion string, + pool string, ) error { if fgHandler == nil || node == nil || parentCondition == nil || mcfgClient == nil { return nil @@ -305,7 +302,7 @@ func generateAndApplyMachineConfigNodes( newMCNode.Spec.ConfigVersion.Desired = NotYetSet } newMCNode.Name = node.Name - newMCNode.Spec.Pool = mcfgv1.MCOObjectReference{Name: mcpName} + newMCNode.Spec.Pool = mcfgv1.MCOObjectReference{Name: pool} newMCNode.Spec.Node = mcfgv1.MCOObjectReference{Name: node.Name} _, err := mcfgClient.MachineconfigurationV1().MachineConfigNodes().Create(context.TODO(), newMCNode, metav1.CreateOptions{}) @@ -316,7 +313,7 @@ func generateAndApplyMachineConfigNodes( } // if this is the first time we are applying the MCN and the node is ready, set the config version probably if node.Status.Phase != corev1.NodePending && node.Status.Phase != corev1.NodePhase("Provisioning") && newMCNode.Spec.ConfigVersion.Desired == "NotYetSet" { - err := GenerateAndApplyMachineConfigNodeSpec(fgHandler, mcpName, desiredConfigVersion, node, mcfgClient) + err := GenerateAndApplyMachineConfigNodeSpec(fgHandler, pool, node, mcfgClient) if err != nil { klog.Errorf("Error making MCN spec for Update Compatible: %v", err) } @@ -338,22 +335,21 @@ func isSingletonCondition(singletonConditionTypes []mcfgv1.StateProgress, condit return false } -// GenerateAndApplyMachineConfigNodeSpec generates and applies a new MCN spec based off node and MCP properties -func GenerateAndApplyMachineConfigNodeSpec(fgHandler ctrlcommon.FeatureGatesHandler, mcpName, desiredConfigVersion string, node *corev1.Node, mcfgClient mcfgclientset.Interface) error { +// GenerateAndApplyMachineConfigNodeSpec generates and applies a new MCN spec based off the node state +func GenerateAndApplyMachineConfigNodeSpec(fgHandler ctrlcommon.FeatureGatesHandler, pool string, node *corev1.Node, mcfgClient mcfgclientset.Interface) error { if fgHandler == nil || node == nil { return nil } + if !fgHandler.Enabled(features.FeatureGateMachineConfigNodes) { - klog.V(4).Infof("MachineConfigNode feature gate is not enabled. Please enable the featureset to use the MCN resource.") + klog.Infof("MCN Featuregate is not enabled. Please enable the TechPreviewNoUpgrade featureset to use MachineConfigNodes") return nil } - - // Get the existing MCN for the node or, if one does not exist, create one later in the flow - mcn, needNewMCN := createOrGetMachineConfigNode(mcfgClient, node) - newMCN := mcn.DeepCopy() - - // Set the MCN metadata - newMCN.ObjectMeta.OwnerReferences = []metav1.OwnerReference{ + // get the existing MCN, or if it DNE create one below + mcNode, needNewMCNode := createOrGetMachineConfigNode(mcfgClient, node) + newMCNode := mcNode.DeepCopy() + // set the spec config version + newMCNode.ObjectMeta.OwnerReferences = []metav1.OwnerReference{ { APIVersion: "v1", Name: node.ObjectMeta.Name, @@ -362,37 +358,38 @@ func GenerateAndApplyMachineConfigNodeSpec(fgHandler ctrlcommon.FeatureGatesHand }, } - // Set MCN.Spec values - newMCN.Spec.ConfigVersion = mcfgv1.MachineConfigNodeSpecMachineConfigVersion{ - Desired: desiredConfigVersion, + newMCNode.Spec.ConfigVersion = mcfgv1.MachineConfigNodeSpecMachineConfigVersion{ + Desired: node.Annotations[daemonconsts.DesiredMachineConfigAnnotationKey], } - newMCN.Spec.Pool = mcfgv1.MCOObjectReference{ - Name: mcpName, + // Set desired config to NotYetSet if the annotation is empty to satisfy API validation + if newMCNode.Spec.ConfigVersion.Desired == "" { + newMCNode.Spec.ConfigVersion.Desired = NotYetSet } - newMCN.Spec.Node = mcfgv1.MCOObjectReference{ + + newMCNode.Spec.Pool = mcfgv1.MCOObjectReference{ + Name: pool, + } + newMCNode.Spec.Node = mcfgv1.MCOObjectReference{ Name: node.Name, } - - // Update the existing MCN if one exists - if !needNewMCN { - nodeRefApplyConfig := machineconfigurationv1.MCOObjectReference().WithName(newMCN.Spec.Node.Name) - poolRefApplyConfig := machineconfigurationv1.MCOObjectReference().WithName(newMCN.Spec.Pool.Name) - specConfigVersionApplyConfig := machineconfigurationv1.MachineConfigNodeSpecMachineConfigVersion().WithDesired(newMCN.Spec.ConfigVersion.Desired) - specApplyConfig := machineconfigurationv1.MachineConfigNodeSpec().WithNode(nodeRefApplyConfig).WithPool(poolRefApplyConfig).WithConfigVersion(specConfigVersionApplyConfig) - mcnApplyConfig := machineconfigurationv1.MachineConfigNode(newMCN.Name).WithSpec(specApplyConfig) - _, err := mcfgClient.MachineconfigurationV1().MachineConfigNodes().Apply(context.TODO(), mcnApplyConfig, metav1.ApplyOptions{FieldManager: "machine-config-operator", Force: true}) + if !needNewMCNode { + nodeRefApplyConfig := machineconfigurationv1.MCOObjectReference().WithName(newMCNode.Spec.Node.Name) + poolRefApplyConfig := machineconfigurationv1.MCOObjectReference().WithName(newMCNode.Spec.Pool.Name) + specconfigVersionApplyConfig := machineconfigurationv1.MachineConfigNodeSpecMachineConfigVersion().WithDesired(newMCNode.Spec.ConfigVersion.Desired) + specApplyConfig := machineconfigurationv1.MachineConfigNodeSpec().WithNode(nodeRefApplyConfig).WithPool(poolRefApplyConfig).WithConfigVersion(specconfigVersionApplyConfig) + mcnodeApplyConfig := machineconfigurationv1.MachineConfigNode(newMCNode.Name).WithSpec(specApplyConfig) + _, err := mcfgClient.MachineconfigurationV1().MachineConfigNodes().Apply(context.TODO(), mcnodeApplyConfig, metav1.ApplyOptions{FieldManager: "machine-config-operator", Force: true}) if err != nil { klog.Errorf("Error applying MCN Spec: %v", err) return err } - } else { // Create a new MCN if one does not yet exist - _, err := mcfgClient.MachineconfigurationV1().MachineConfigNodes().Create(context.TODO(), newMCN, metav1.CreateOptions{}) + } else { + _, err := mcfgClient.MachineconfigurationV1().MachineConfigNodes().Create(context.TODO(), newMCNode, metav1.CreateOptions{}) if err != nil { klog.Errorf("Error creating MCN: %v", err) return err } } - return nil }