Merge pull request openshift#2668 from JoelSpeed/bootstrap-featuregate-sync

[OCPCLOUD-1209] Run KubeletConfig FeatureGate sync during bootstrap

Author: openshift-merge-robot

pkg/controller/bootstrap/bootstrap.go

@@ -20,7 +20,9 @@ import (
 	apicfgv1 "github.com/openshift/api/config/v1"
 	apioperatorsv1alpha1 "github.com/openshift/api/operator/v1alpha1"
 	mcfgv1 "github.com/openshift/machine-config-operator/pkg/apis/machineconfiguration.openshift.io/v1"
+	ctrlcommon "github.com/openshift/machine-config-operator/pkg/controller/common"
 	containerruntimeconfig "github.com/openshift/machine-config-operator/pkg/controller/container-runtime-config"
+	kubeletconfig "github.com/openshift/machine-config-operator/pkg/controller/kubelet-config"
 	"github.com/openshift/machine-config-operator/pkg/controller/render"
 	"github.com/openshift/machine-config-operator/pkg/controller/template"
 )
@@ -46,6 +48,7 @@ func New(templatesDir, manifestDir, pullSecretFile string) *Bootstrap {
 
 // Run runs boostrap for Machine Config Controller
 // It writes all the assets to destDir
+// nolint:gocyclo
 func (b *Bootstrap) Run(destDir string) error {
 	infos, err := ioutil.ReadDir(b.manifestDir)
 	if err != nil {
@@ -70,6 +73,7 @@ func (b *Bootstrap) Run(destDir string) error {
 	decoder := codecFactory.UniversalDecoder(mcfgv1.GroupVersion, apioperatorsv1alpha1.GroupVersion, apicfgv1.GroupVersion)
 
 	var cconfig *mcfgv1.ControllerConfig
+	var featureGate *apicfgv1.FeatureGate
 	var pools []*mcfgv1.MachineConfigPool
 	var configs []*mcfgv1.MachineConfig
 	var icspRules []*apioperatorsv1alpha1.ImageContentSourcePolicy
@@ -112,6 +116,10 @@ func (b *Bootstrap) Run(destDir string) error {
 				icspRules = append(icspRules, obj)
 			case *apicfgv1.Image:
 				imgCfg = obj
+			case *apicfgv1.FeatureGate:
+				if obj.GetName() == ctrlcommon.ClusterFeatureInstanceName {
+					featureGate = obj
+				}
 			default:
 				glog.Infof("skipping %q [%d] manifest because of unhandled %T", file.Name(), idx+1, obji)
 			}
@@ -121,7 +129,7 @@ func (b *Bootstrap) Run(destDir string) error {
 	if cconfig == nil {
 		return fmt.Errorf("error: no controllerconfig found in dir: %q", destDir)
 	}
-	iconfigs, err := template.RunBootstrap(b.templatesDir, cconfig, psraw)
+	iconfigs, err := template.RunBootstrap(b.templatesDir, cconfig, psraw, featureGate)
 	if err != nil {
 		return err
 	}
@@ -133,6 +141,14 @@ func (b *Bootstrap) Run(destDir string) error {
 	}
 	configs = append(configs, rconfigs...)
 
+	if featureGate != nil {
+		kConfigs, err := kubeletconfig.RunFeatureGateBootstrap(b.templatesDir, featureGate, cconfig, pools)
+		if err != nil {
+			return err
+		}
+		configs = append(configs, kConfigs...)
+	}
+
 	fpools, gconfigs, err := render.RunBootstrap(pools, configs, cconfig)
 	if err != nil {
 		return err

pkg/controller/bootstrap/bootstrap_test.go

@@ -44,6 +44,19 @@ spec:
 		want: []manifest{{
 			Raw: []byte(`{"apiVersion":"extensions/v1beta1","kind":"Ingress","metadata":{"name":"test-ingress","namespace":"test-namespace"},"spec":{"rules":[{"http":{"paths":[{"backend":{"serviceName":"test","servicePort":80},"path":"/testpath"}]}}]}}`),
 		}},
+	}, {
+		name: "feature gate",
+		raw: `
+apiVersion: config.openshift.io/v1
+kind: FeatureGate
+metadata:
+  name: cluster
+spec:
+  featureSet: TechPreviewNoUpgrade
+`,
+		want: []manifest{{
+			Raw: []byte(`{"apiVersion":"config.openshift.io/v1","kind":"FeatureGate","metadata":{"name":"cluster"},"spec":{"featureSet":"TechPreviewNoUpgrade"}}`),
+		}},
 	}, {
 		name: "two-resources",
 		raw: `

pkg/controller/kubelet-config/helpers.go

@@ -311,3 +311,13 @@ func newKubeletconfigJSONEncoder(targetVersion schema.GroupVersion) (runtime.Enc
 	}
 	return codecs.EncoderForVersion(info.Serializer, targetVersion), nil
 }
+
+// kubeletConfigToIgnFile converts a KubeletConfiguration to an Ignition File
+func kubeletConfigToIgnFile(cfg *kubeletconfigv1beta1.KubeletConfiguration) (*ign3types.File, error) {
+	cfgJSON, err := EncodeKubeletConfig(cfg, kubeletconfigv1beta1.SchemeGroupVersion)
+	if err != nil {
+		return nil, fmt.Errorf("could not encode kubelet configuration: %v", err)
+	}
+	cfgIgn := createNewKubeletIgnition(cfgJSON)
+	return cfgIgn, nil
+}

pkg/controller/kubelet-config/kubelet_config_controller.go

@@ -330,14 +330,43 @@ func (ctrl *Controller) handleFeatureErr(err error, key interface{}) {
 	ctrl.featureQueue.AddAfter(key, 1*time.Minute)
 }
 
-func (ctrl *Controller) generateOriginalKubeletConfig(role string, featureGate *configv1.FeatureGate) (*ign3types.File, error) {
-	cc, err := ctrl.ccLister.Get(ctrlcommon.ControllerConfigName)
+// generateOriginalKubeletConfigWithFeatureGates generates a KubeletConfig and ensure the correct feature gates are set
+// based on the given FeatureGate.
+func generateOriginalKubeletConfigWithFeatureGates(cc *mcfgv1.ControllerConfig, templatesDir, role string, features *configv1.FeatureGate) (*kubeletconfigv1beta1.KubeletConfiguration, error) {
+	originalKubeletIgn, err := generateOriginalKubeletConfigIgn(cc, templatesDir, role, features)
 	if err != nil {
-		return nil, fmt.Errorf("could not get ControllerConfig %v", err)
+		return nil, fmt.Errorf("could not generate the original Kubelet config ignition: %v", err)
 	}
+	if originalKubeletIgn.Contents.Source == nil {
+		return nil, fmt.Errorf("the original Kubelet source string is empty: %v", err)
+	}
+	dataURL, err := dataurl.DecodeString(*originalKubeletIgn.Contents.Source)
+	if err != nil {
+		return nil, fmt.Errorf("could not decode the original Kubelet source string: %v", err)
+	}
+	originalKubeConfig, err := decodeKubeletConfig(dataURL.Data)
+	if err != nil {
+		return nil, fmt.Errorf("could not deserialize the Kubelet source: %v", err)
+	}
+
+	featureGates, err := generateFeatureMap(features, openshiftOnlyFeatureGates...)
+	if err != nil {
+		return nil, fmt.Errorf("could not generate features map: %v", err)
+	}
+
+	// Merge in Feature Gates.
+	// If they are the same, this will be a no-op
+	if err := mergo.Merge(&originalKubeConfig.FeatureGates, featureGates, mergo.WithOverride); err != nil {
+		return nil, fmt.Errorf("could not merge feature gates: %v", err)
+	}
+
+	return originalKubeConfig, nil
+}
+
+func generateOriginalKubeletConfigIgn(cc *mcfgv1.ControllerConfig, templatesDir, role string, featureGate *configv1.FeatureGate) (*ign3types.File, error) {
 	// Render the default templates
 	rc := &mtmpl.RenderConfig{ControllerConfigSpec: &cc.Spec, FeatureGate: featureGate}
-	generatedConfigs, err := mtmpl.GenerateMachineConfigsForRole(rc, role, ctrl.templatesDir)
+	generatedConfigs, err := mtmpl.GenerateMachineConfigsForRole(rc, role, templatesDir)
 	if err != nil {
 		return nil, fmt.Errorf("GenerateMachineConfigsforRole failed with error %s", err)
 	}
@@ -478,12 +507,6 @@ func (ctrl *Controller) syncKubeletConfig(key string) error {
 		err := fmt.Errorf("could not fetch FeatureGates: %v", err)
 		return ctrl.syncStatusOnly(cfg, err)
 	}
-	featureGates, err := generateFeatureMap(features)
-	if err != nil {
-		err := fmt.Errorf("could not generate FeatureMap: %v", err)
-		glog.V(2).Infof("%v", err)
-		return ctrl.syncStatusOnly(cfg, err)
-	}
 
 	for _, pool := range mcpPools {
 		if pool.Spec.Configuration.Name == "" {
@@ -512,20 +535,14 @@ func (ctrl *Controller) syncKubeletConfig(key string) error {
 		userDefinedSystemReserved := make(map[string]string, 2)
 
 		// Generate the original KubeletConfig
-		originalKubeletIgn, err := ctrl.generateOriginalKubeletConfig(role, features)
-		if err != nil {
-			return ctrl.syncStatusOnly(cfg, err, "could not generate the original Kubelet config: %v", err)
-		}
-		if originalKubeletIgn.Contents.Source == nil {
-			return ctrl.syncStatusOnly(cfg, err, "the original Kubelet source string is empty: %v", err)
-		}
-		dataURL, err := dataurl.DecodeString(*originalKubeletIgn.Contents.Source)
+		cc, err := ctrl.ccLister.Get(ctrlcommon.ControllerConfigName)
 		if err != nil {
-			return ctrl.syncStatusOnly(cfg, err, "could not decode the original Kubelet source string: %v", err)
+			return fmt.Errorf("could not get ControllerConfig %v", err)
 		}
-		originalKubeConfig, err := decodeKubeletConfig(dataURL.Data)
+
+		originalKubeConfig, err := generateOriginalKubeletConfigWithFeatureGates(cc, ctrl.templatesDir, role, features)
 		if err != nil {
-			return ctrl.syncStatusOnly(cfg, err, "could not deserialize the Kubelet source: %v", err)
+			return ctrl.syncStatusOnly(cfg, err, "could not get original kubelet config: %v", err)
 		}
 
 		// Get the default API Server Security Profile
@@ -561,29 +578,21 @@ func (ctrl *Controller) syncKubeletConfig(key string) error {
 				delete(specKubeletConfig.SystemReserved, "cpu")
 			}
 
+			// FeatureGates must be set from the FeatureGate.
+			// Remove them here to prevent the specKubeletConfig merge overwriting them.
+			specKubeletConfig.FeatureGates = nil
+
 			// Merge the Old and New
 			err = mergo.Merge(originalKubeConfig, specKubeletConfig, mergo.WithOverride)
 			if err != nil {
 				return ctrl.syncStatusOnly(cfg, err, "could not merge original config and new config: %v", err)
 			}
-			// Merge in Feature Gates
-			err = mergo.Merge(&originalKubeConfig.FeatureGates, featureGates, mergo.WithOverride)
-			if err != nil {
-				return ctrl.syncStatusOnly(cfg, err, "could not merge FeatureGates: %v", err)
-			}
-			// Encode the new config into raw JSON
-			cfgJSON, err := EncodeKubeletConfig(originalKubeConfig, kubeletconfigv1beta1.SchemeGroupVersion)
-			if err != nil {
-				return ctrl.syncStatusOnly(cfg, err, "could not encode JSON: %v", err)
-			}
-			kubeletIgnition = createNewKubeletIgnition(cfgJSON)
-		} else {
-			// Encode the new config into raw JSON
-			cfgJSON, err := EncodeKubeletConfig(originalKubeConfig, kubeletconfigv1beta1.SchemeGroupVersion)
-			if err != nil {
-				return ctrl.syncStatusOnly(cfg, err, "could not encode JSON: %v", err)
-			}
-			kubeletIgnition = createNewKubeletIgnition(cfgJSON)
+		}
+
+		// Encode the new config into an Ignition File
+		kubeletIgnition, err = kubeletConfigToIgnFile(originalKubeConfig)
+		if err != nil {
+			return ctrl.syncStatusOnly(cfg, err, "could not encode JSON: %v", err)
 		}
 
 		if isNotFound {

pkg/controller/kubelet-config/kubelet_config_features.go

@@ -8,18 +8,16 @@ import (
 
 	"github.com/clarketm/json"
 	"github.com/golang/glog"
-	"github.com/imdario/mergo"
 	osev1 "github.com/openshift/api/config/v1"
 	"github.com/openshift/library-go/pkg/cloudprovider"
-	"github.com/vincent-petithory/dataurl"
 	"k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/labels"
 	utilruntime "k8s.io/apimachinery/pkg/util/runtime"
 	"k8s.io/client-go/tools/cache"
 	"k8s.io/client-go/util/retry"
-	kubeletconfigv1beta1 "k8s.io/kubelet/config/v1beta1"
 
+	mcfgv1 "github.com/openshift/machine-config-operator/pkg/apis/machineconfiguration.openshift.io/v1"
 	ctrlcommon "github.com/openshift/machine-config-operator/pkg/controller/common"
 	"github.com/openshift/machine-config-operator/pkg/version"
 )
@@ -69,9 +67,10 @@ func (ctrl *Controller) syncFeatureHandler(key string) error {
 	} else if err != nil {
 		return err
 	}
-	featureGates, err := generateFeatureMap(features, openshiftOnlyFeatureGates...)
+
+	cc, err := ctrl.ccLister.Get(ctrlcommon.ControllerConfigName)
 	if err != nil {
-		return err
+		return fmt.Errorf("could not get ControllerConfig %v", err)
 	}
 
 	// Find all MachineConfigPools
@@ -100,43 +99,15 @@ func (ctrl *Controller) syncFeatureHandler(key string) error {
 				return err
 			}
 		}
-		// Generate the original KubeletConfig
-		originalKubeletIgn, err := ctrl.generateOriginalKubeletConfig(role, nil)
-		if err != nil {
-			return err
-		}
-		if originalKubeletIgn.Contents.Source == nil {
-			return fmt.Errorf("could not find original Kubelet config to decode")
-		}
-		dataURL, err := dataurl.DecodeString(*originalKubeletIgn.Contents.Source)
-		if err != nil {
-			return err
-		}
-		originalKubeConfig, err := decodeKubeletConfig(dataURL.Data)
+
+		rawCfgIgn, err := generateKubeConfigIgnFromFeatures(cc, ctrl.templatesDir, role, features)
 		if err != nil {
 			return err
 		}
-		// Check to see if FeatureGates are equal
-		if reflect.DeepEqual(originalKubeConfig.FeatureGates, *featureGates) {
+		if rawCfgIgn == nil {
 			continue
 		}
-		// Merge in Feature Gates
-		err = mergo.Merge(&originalKubeConfig.FeatureGates, featureGates, mergo.WithOverride)
-		if err != nil {
-			return err
-		}
-		// Encode the new config into raw JSON
-		cfgJSON, err := EncodeKubeletConfig(originalKubeConfig, kubeletconfigv1beta1.SchemeGroupVersion)
-		if err != nil {
-			return err
-		}
-		tempIgnConfig := ctrlcommon.NewIgnConfig()
-		cfgIgn := createNewKubeletIgnition(cfgJSON)
-		tempIgnConfig.Storage.Files = append(tempIgnConfig.Storage.Files, *cfgIgn)
-		rawCfgIgn, err := json.Marshal(tempIgnConfig)
-		if err != nil {
-			return err
-		}
+
 		mc.Spec.Config.Raw = rawCfgIgn
 		mc.ObjectMeta.Annotations = map[string]string{
 			ctrlcommon.GeneratedByControllerVersionAnnotationKey: version.Hash,
@@ -231,3 +202,70 @@ func generateFeatureMap(features *osev1.FeatureGate, exclusions ...string) (*map
 	}
 	return &rv, nil
 }
+
+func generateKubeConfigIgnFromFeatures(cc *mcfgv1.ControllerConfig, templatesDir, role string, features *osev1.FeatureGate) ([]byte, error) {
+	originalKubeConfig, err := generateOriginalKubeletConfigWithFeatureGates(cc, templatesDir, role, features)
+	if err != nil {
+		return nil, err
+	}
+	defaultFeatures, err := generateFeatureMap(createNewDefaultFeatureGate(), openshiftOnlyFeatureGates...)
+	if err != nil {
+		return nil, err
+	}
+
+	// Check to see if configured FeatureGates are equivalent to the Default FeatureSet.
+	if reflect.DeepEqual(originalKubeConfig.FeatureGates, *defaultFeatures) {
+		// When there is no difference, this isn't an error, but no machine config should be created
+		return nil, nil
+	}
+
+	// Encode the new config into raw JSON
+	cfgIgn, err := kubeletConfigToIgnFile(originalKubeConfig)
+	if err != nil {
+		return nil, err
+	}
+
+	tempIgnConfig := ctrlcommon.NewIgnConfig()
+	tempIgnConfig.Storage.Files = append(tempIgnConfig.Storage.Files, *cfgIgn)
+	rawCfgIgn, err := json.Marshal(tempIgnConfig)
+	if err != nil {
+		return nil, err
+	}
+	return rawCfgIgn, nil
+}
+
+func RunFeatureGateBootstrap(templateDir string, features *osev1.FeatureGate, controllerConfig *mcfgv1.ControllerConfig, mcpPools []*mcfgv1.MachineConfigPool) ([]*mcfgv1.MachineConfig, error) {
+	machineConfigs := []*mcfgv1.MachineConfig{}
+
+	for _, pool := range mcpPools {
+		role := pool.Name
+		rawCfgIgn, err := generateKubeConfigIgnFromFeatures(controllerConfig, templateDir, role, features)
+		if err != nil {
+			return nil, err
+		}
+		if rawCfgIgn == nil {
+			continue
+		}
+
+		// Get MachineConfig
+		managedKey, err := getManagedFeaturesKey(pool, nil)
+		if err != nil {
+			return nil, err
+		}
+
+		ignConfig := ctrlcommon.NewIgnConfig()
+		mc, err := ctrlcommon.MachineConfigFromIgnConfig(role, managedKey, ignConfig)
+		if err != nil {
+			return nil, err
+		}
+
+		mc.Spec.Config.Raw = rawCfgIgn
+		mc.ObjectMeta.Annotations = map[string]string{
+			ctrlcommon.GeneratedByControllerVersionAnnotationKey: version.Hash,
+		}
+
+		machineConfigs = append(machineConfigs, mc)
+	}
+
+	return machineConfigs, nil
+}