kubelet config: write back minimum kubelet version status

When a cluster admin rolls out a minimum kubelet verison, the cluster-config-operator (eventually) will render
a new set of featuregates based on the new minimum version (enabling ones that are safe given the new minimum).

However, on a rollback, we need to make sure the MCS won't return a rendered config using the old features, in the very
odd case a cluster admin creates a newly old node and somehow overrides the osImage (unlikely). Thus, we use the similar condition
as MCS to serve the config--once one node has updated, we treat the MCP as using the new config. Then, we write the status to the
node config object so the node authorization plugin can allow older nodes that are now deemed safe.

Signed-off-by: Peter Hunt <[email protected]>

Author: haircommander

pkg/controller/kubelet-config/kubelet_config_bootstrap.go

@@ -27,7 +27,8 @@ func RunKubeletBootstrap(templateDir string, kubeletConfigs []*mcfgv1.KubeletCon
 		nodeConfig = createNewDefaultNodeconfig()
 	}
 
-	featureGates, err := generateFeatureMap(featureGateAccess, openshiftOnlyFeatureGates...)
+	// TODO FIXME: ignoring min kubelet version writeback for now
+	featureGates, _, err := generateFeatureMap(featureGateAccess, openshiftOnlyFeatureGates...)
 	if err != nil {
 		return nil, fmt.Errorf("could not generate features map: %w", err)
 	}

pkg/controller/kubelet-config/kubelet_config_controller.go

@@ -42,6 +42,7 @@ import (
 	mcfglistersv1 "github.com/openshift/client-go/machineconfiguration/listers/machineconfiguration/v1"
 	"github.com/openshift/library-go/pkg/operator/configobserver/featuregates"
 	"github.com/openshift/machine-config-operator/pkg/apihelpers"
+	"github.com/openshift/machine-config-operator/pkg/constants"
 	ctrlcommon "github.com/openshift/machine-config-operator/pkg/controller/common"
 	mtmpl "github.com/openshift/machine-config-operator/pkg/controller/template"
 	"github.com/openshift/machine-config-operator/pkg/version"
@@ -528,7 +529,7 @@ func (ctrl *Controller) addAnnotation(cfg *mcfgv1.KubeletConfig, annotationKey,
 // This function is not meant to be invoked concurrently with the same key.
 //
 //nolint:gocyclo
-func (ctrl *Controller) syncKubeletConfig(key string) error {
+func (ctrl *Controller) syncKubeletConfig(key string) (retErr error) {
 	startTime := time.Now()
 	klog.V(4).Infof("Started syncing kubeletconfig %q (%v)", key, startTime)
 	defer func() {
@@ -595,6 +596,13 @@ func (ctrl *Controller) syncKubeletConfig(key string) error {
 		return ctrl.syncStatusOnly(cfg, err, "could not get the TLSSecurityProfile from %v: %v", ctrlcommon.APIServerInstanceName, err)
 	}
 
+	updatedPools := []string{}
+
+	featureGates, renderedVersions, err := generateFeatureMap(ctrl.featureGateAccess, openshiftOnlyFeatureGates...)
+	if err != nil {
+		return fmt.Errorf("could not generate features map: %w", err)
+	}
+
 	for _, pool := range mcpPools {
 		if pool.Spec.Configuration.Name == "" {
 			updateDelay := 5 * time.Second
@@ -629,11 +637,6 @@ func (ctrl *Controller) syncKubeletConfig(key string) error {
 			return fmt.Errorf("could not get ControllerConfig %w", err)
 		}
 
-		featureGates, err := generateFeatureMap(ctrl.featureGateAccess, openshiftOnlyFeatureGates...)
-		if err != nil {
-			return fmt.Errorf("could not generate features map: %w", err)
-		}
-
 		originalKubeConfig, err := generateOriginalKubeletConfigWithFeatureGates(cc, ctrl.templatesDir, role, featureGates, apiServer)
 		if err != nil {
 			return ctrl.syncStatusOnly(cfg, err, "could not get original kubelet config: %v", err)
@@ -725,13 +728,65 @@ func (ctrl *Controller) syncKubeletConfig(key string) error {
 		}
 		klog.Infof("Applied KubeletConfig %v on MachineConfigPool %v", key, pool.Name)
 		ctrlcommon.UpdateStateMetric(ctrlcommon.MCCSubControllerState, "machine-config-controller-kubelet-config", "Sync Kubelet Config", pool.Name)
+		updatedPools = append(updatedPools, pool.Name)
 	}
+	go ctrl.writebackMinimumKubeletVersionIfAppropriate(updatedPools, renderedVersions, nodeConfig, func() ([]*mcfgv1.MachineConfigPool, error) {
+		return ctrl.getPoolsForKubeletConfig(cfg)
+	})
 	if err := ctrl.cleanUpDuplicatedMC(managedKubeletConfigKeyPrefix); err != nil {
 		return err
 	}
 	return ctrl.syncStatusOnly(cfg, nil)
 }
 
+func (ctrl *Controller) writebackMinimumKubeletVersionIfAppropriate(updatedPools []string, renderedVersions []configv1.RequiredMinimumComponentVersion, node *osev1.Node, poolGetter func() ([]*mcfgv1.MachineConfigPool, error)) {
+	renderedKubeletVersion := ""
+	for _, cv := range renderedVersions {
+		if cv.Component == configv1.RequiredMinimumComponentKubelet {
+			renderedKubeletVersion = cv.Version
+		}
+	}
+	if node.Spec.MinimumKubeletVersion == node.Status.MinimumKubeletVersion ||
+		node.Status.MinimumKubeletVersion == renderedKubeletVersion ||
+		node.Spec.MinimumKubeletVersion != renderedKubeletVersion {
+		klog.InfoS("Skipping writeback to nodes.config.Status.MinimumKubeletVersion because situation not correct",
+			"nodes.config.Spec.MinimumKubeletVerison", node.Spec.MinimumKubeletVersion,
+			"nodes.config.Status.MinimumKubeletVerison", node.Status.MinimumKubeletVersion,
+			"renderedKubeletVersion", renderedKubeletVersion)
+		return
+	}
+
+	poolsUpdated := map[string]bool{}
+	for _, poolName := range updatedPools {
+		poolsUpdated[poolName] = false
+	}
+	// This featuregate rollout was done as a result of a new minimum kubelet version rolling out, which means we need to wait for at least one
+	// node in each MCP to finish updating before we set the spec.
+	if err := wait.ExponentialBackoff(constants.NodeUpdateBackoff, func() (bool, error) {
+		// no do-while :(
+		mcps, err := poolGetter()
+		if err != nil {
+			return true, err
+		}
+		allUpdated := true
+		for _, mcp := range mcps {
+			if mcp.Status.UpdatedMachineCount > 0 {
+				poolsUpdated[mcp.Name] = true
+			} else if _, updated := poolsUpdated[mcp.Name]; !updated {
+				allUpdated = false
+			}
+		}
+		return allUpdated, nil
+	}); err != nil {
+		klog.Errorf("Failed to update rendered kubelet version: %v", err)
+	}
+
+	node.Status.MinimumKubeletVersion = renderedKubeletVersion
+	if _, err := ctrl.configClient.ConfigV1().Nodes().Update(context.TODO(), node, metav1.UpdateOptions{}); err != nil {
+		klog.Errorf("Failed to update node object for rendered kubelet version: %v", err)
+	}
+}
+
 // cleanUpDuplicatedMC removes the MC of non-updated GeneratedByControllerVersionKey if its name contains 'generated-kubelet'.
 // BZ 1955517: upgrade when there are more than one configs, the duplicated and upgraded MC will be generated (func getManagedKubeletConfigKey())
 // MC with old GeneratedByControllerVersionKey fails the upgrade.

pkg/controller/kubelet-config/kubelet_config_features.go

@@ -17,6 +17,7 @@ import (
 	"k8s.io/client-go/util/retry"
 	"k8s.io/klog/v2"
 
+	configv1 "github.com/openshift/api/config/v1"
 	mcfgv1 "github.com/openshift/api/machineconfiguration/v1"
 	ctrlcommon "github.com/openshift/machine-config-operator/pkg/controller/common"
 	"github.com/openshift/machine-config-operator/pkg/version"
@@ -73,14 +74,20 @@ func (ctrl *Controller) syncFeatureHandler(key string) error {
 		return fmt.Errorf("could not get the TLSSecurityProfile from %v: %v", ctrlcommon.APIServerInstanceName, err)
 	}
 
+	featureGates, renderedVersions, err := generateFeatureMap(ctrl.featureGateAccess, openshiftOnlyFeatureGates...)
+	if err != nil {
+		return fmt.Errorf("could not generate features map: %w", err)
+	}
+	// Fetch the Node Config object
+	nodeConfig, err := ctrl.nodeConfigLister.Get(ctrlcommon.ClusterNodeInstanceName)
+	if errors.IsNotFound(err) {
+		nodeConfig = createNewDefaultNodeconfig()
+	}
+
+	updatedPools := []string{}
+
 	for _, pool := range mcpPools {
-		var nodeConfig *osev1.Node
 		role := pool.Name
-		// Fetch the Node Config object
-		nodeConfig, err = ctrl.nodeConfigLister.Get(ctrlcommon.ClusterNodeInstanceName)
-		if errors.IsNotFound(err) {
-			nodeConfig = createNewDefaultNodeconfig()
-		}
 		// Get MachineConfig
 		managedKey, err := getManagedFeaturesKey(pool, ctrl.client)
 		if err != nil {
@@ -99,11 +106,6 @@ func (ctrl *Controller) syncFeatureHandler(key string) error {
 			}
 		}
 
-		featureGates, err := generateFeatureMap(ctrl.featureGateAccess, openshiftOnlyFeatureGates...)
-		if err != nil {
-			return fmt.Errorf("could not generate features map: %w", err)
-		}
-
 		rawCfgIgn, err := generateKubeConfigIgnFromFeatures(cc, ctrl.templatesDir, role, featureGates, nodeConfig, apiServer)
 		if err != nil {
 			return err
@@ -130,9 +132,12 @@ func (ctrl *Controller) syncFeatureHandler(key string) error {
 		}
 		klog.Infof("Applied FeatureSet %v on MachineConfigPool %v", key, pool.Name)
 		ctrlcommon.UpdateStateMetric(ctrlcommon.MCCSubControllerState, "machine-config-controller-kubelet-config", "Sync FeatureSet", pool.Name)
+		updatedPools = append(updatedPools, pool.Name)
 	}
+	go ctrl.writebackMinimumKubeletVersionIfAppropriate(updatedPools, renderedVersions, nodeConfig, func() ([]*mcfgv1.MachineConfigPool, error) {
+		return ctrl.mcpLister.List(labels.Everything())
+	})
 	return ctrl.cleanUpDuplicatedMC(managedFeaturesKeyPrefix)
-
 }
 
 func (ctrl *Controller) enqueueFeature(feat *osev1.FeatureGate) {
@@ -179,16 +184,16 @@ func (ctrl *Controller) deleteFeature(obj interface{}) {
 // generateFeatureMap returns a map of enabled/disabled feature gate selection with exclusion list
 //
 //nolint:gocritic
-func generateFeatureMap(featuregateAccess featuregates.FeatureGateAccess, exclusions ...osev1.FeatureGateName) (map[string]bool, error) {
+func generateFeatureMap(featuregateAccess featuregates.FeatureGateAccess, exclusions ...osev1.FeatureGateName) (map[string]bool, []configv1.RequiredMinimumComponentVersion, error) {
 	rv := make(map[string]bool)
 
 	if !featuregateAccess.AreInitialFeatureGatesObserved() {
-		return nil, fmt.Errorf("initial feature gates are not observed")
+		return nil, nil, fmt.Errorf("initial feature gates are not observed")
 	}
 
 	features, err := featuregateAccess.CurrentFeatureGates()
 	if err != nil {
-		return nil, fmt.Errorf("could not get current feature gates: %w", err)
+		return nil, nil, fmt.Errorf("could not get current feature gates: %w", err)
 	}
 
 	for _, feat := range features.KnownFeatures() {
@@ -203,7 +208,7 @@ func generateFeatureMap(featuregateAccess featuregates.FeatureGateAccess, exclus
 	for _, excluded := range exclusions {
 		delete(rv, string(excluded))
 	}
-	return rv, nil
+	return rv, features.RenderedMinimumComponentVersions(), nil
 }
 
 func generateKubeConfigIgnFromFeatures(cc *mcfgv1.ControllerConfig, templatesDir, role string, featureGates map[string]bool, nodeConfig *osev1.Node, apiServer *osev1.APIServer) ([]byte, error) {
@@ -233,7 +238,8 @@ func generateKubeConfigIgnFromFeatures(cc *mcfgv1.ControllerConfig, templatesDir
 func RunFeatureGateBootstrap(templateDir string, featureGateAccess featuregates.FeatureGateAccess, nodeConfig *osev1.Node, controllerConfig *mcfgv1.ControllerConfig, mcpPools []*mcfgv1.MachineConfigPool, apiServer *osev1.APIServer) ([]*mcfgv1.MachineConfig, error) {
 	machineConfigs := []*mcfgv1.MachineConfig{}
 
-	featureGates, err := generateFeatureMap(featureGateAccess, openshiftOnlyFeatureGates...)
+	// TODO FIXME: do we need rendered versions here?
+	featureGates, _, err := generateFeatureMap(featureGateAccess, openshiftOnlyFeatureGates...)
 	if err != nil {
 		return nil, fmt.Errorf("could not generate features map: %w", err)
 	}

pkg/controller/kubelet-config/kubelet_config_features_test.go

@@ -44,7 +44,7 @@ func TestFeatureGateDrift(t *testing.T) {
 			fgAccess := featuregates.NewHardcodedFeatureGateAccess(features.Spec.FeatureGateSelection.CustomNoUpgrade.Enabled, features.Spec.FeatureGateSelection.CustomNoUpgrade.Disabled)
 			ctrl := f.newController(fgAccess)
 
-			featureGates, err := generateFeatureMap(ctrl.featureGateAccess, openshiftOnlyFeatureGates...)
+			featureGates, _, err := generateFeatureMap(ctrl.featureGateAccess, openshiftOnlyFeatureGates...)
 			require.NoError(t, err)
 
 			// Generate kubelet config with feature gates applied
@@ -222,7 +222,7 @@ func TestBootstrapFeaturesCustomNoUpgrade(t *testing.T) {
 				}
 
 				fgAccess := featuregates.NewHardcodedFeatureGateAccess(features.Spec.FeatureGateSelection.CustomNoUpgrade.Enabled, features.Spec.FeatureGateSelection.CustomNoUpgrade.Disabled)
-				defaultFeatureGates, err := generateFeatureMap(fgAccess)
+				defaultFeatureGates, _, err := generateFeatureMap(fgAccess)
 				if err != nil {
 					t.Errorf("could not generate defaultFeatureGates: %v", err)
 				}

pkg/controller/kubelet-config/kubelet_config_nodes.go

@@ -101,7 +101,8 @@ func (ctrl *Controller) syncNodeConfigHandler(key string) error {
 		return fmt.Errorf("could not get the TLSSecurityProfile from %v: %v", ctrlcommon.APIServerInstanceName, err)
 	}
 
-	featureGates, err := generateFeatureMap(ctrl.featureGateAccess, openshiftOnlyFeatureGates...)
+	// renderedVersions will be handled in syncFeatureHandler below
+	featureGates, _, err := generateFeatureMap(ctrl.featureGateAccess, openshiftOnlyFeatureGates...)
 	if err != nil {
 		return fmt.Errorf("could not generate features map: %w", err)
 	}
@@ -309,7 +310,8 @@ func RunNodeConfigBootstrap(templateDir string, featureGateAccess featuregates.F
 
 	configs := []*mcfgv1.MachineConfig{}
 
-	featureGates, err := generateFeatureMap(featureGateAccess, openshiftOnlyFeatureGates...)
+	// Ignoring rendered versions here.
+	featureGates, _, err := generateFeatureMap(featureGateAccess, openshiftOnlyFeatureGates...)
 	if err != nil {
 		return nil, fmt.Errorf("could not generate features map: %w", err)
 	}