From 118ad16c22336a0d80e6641fe9e8d9a07198d956 Mon Sep 17 00:00:00 2001 From: Danil-Grigorev Date: Tue, 29 Jun 2021 15:42:39 +0200 Subject: [PATCH 1/8] Add feature gate exclusion list to kubelet config --- pkg/controller/kubelet-config/kubelet_config_controller.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/controller/kubelet-config/kubelet_config_controller.go b/pkg/controller/kubelet-config/kubelet_config_controller.go index 814f09d1b0..8cd30d185b 100644 --- a/pkg/controller/kubelet-config/kubelet_config_controller.go +++ b/pkg/controller/kubelet-config/kubelet_config_controller.go @@ -478,7 +478,7 @@ 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) + featureGates, err := generateFeatureMap(features, openshiftOnlyFeatureGates...) if err != nil { err := fmt.Errorf("could not generate FeatureMap: %v", err) glog.V(2).Infof("%v", err) From 64cdcdfad4145cadf8c76b7442dd896e5ea1ce53 Mon Sep 17 00:00:00 2001 From: Danil-Grigorev Date: Thu, 1 Jul 2021 13:42:13 +0200 Subject: [PATCH 2/8] Add featureGate evaluation in manifests folder to bootstrap implementaion --- pkg/controller/bootstrap/bootstrap.go | 8 +++++++- pkg/controller/bootstrap/bootstrap_test.go | 13 +++++++++++++ pkg/controller/template/template_controller.go | 4 ++-- 3 files changed, 22 insertions(+), 3 deletions(-) diff --git a/pkg/controller/bootstrap/bootstrap.go b/pkg/controller/bootstrap/bootstrap.go index c18f391a8e..12dd895c50 100644 --- a/pkg/controller/bootstrap/bootstrap.go +++ b/pkg/controller/bootstrap/bootstrap.go @@ -20,6 +20,7 @@ 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" "github.com/openshift/machine-config-operator/pkg/controller/render" "github.com/openshift/machine-config-operator/pkg/controller/template" @@ -70,6 +71,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 +114,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 +127,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 } diff --git a/pkg/controller/bootstrap/bootstrap_test.go b/pkg/controller/bootstrap/bootstrap_test.go index 4d5fe08743..469a0d070b 100644 --- a/pkg/controller/bootstrap/bootstrap_test.go +++ b/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: ` diff --git a/pkg/controller/template/template_controller.go b/pkg/controller/template/template_controller.go index 909224bd03..96fa61e296 100644 --- a/pkg/controller/template/template_controller.go +++ b/pkg/controller/template/template_controller.go @@ -523,6 +523,6 @@ func getMachineConfigsForControllerConfig(templatesDir string, config *mcfgv1.Co } // RunBootstrap runs the tempate controller in boostrap mode. -func RunBootstrap(templatesDir string, config *mcfgv1.ControllerConfig, pullSecretRaw []byte) ([]*mcfgv1.MachineConfig, error) { - return getMachineConfigsForControllerConfig(templatesDir, config, pullSecretRaw, nil) +func RunBootstrap(templatesDir string, config *mcfgv1.ControllerConfig, pullSecretRaw []byte, featureGate *configv1.FeatureGate) ([]*mcfgv1.MachineConfig, error) { + return getMachineConfigsForControllerConfig(templatesDir, config, pullSecretRaw, featureGate) } From 0e5157327f32eee1450aeb7859ce224c3c718e6e Mon Sep 17 00:00:00 2001 From: Joel Speed Date: Thu, 8 Jul 2021 11:27:50 +0100 Subject: [PATCH 3/8] Extract KubeConfig FeatureGate ignition to a standalone function --- .../kubelet_config_controller.go | 14 +-- .../kubelet-config/kubelet_config_features.go | 85 ++++++++++++------- .../kubelet_config_features_test.go | 2 +- 3 files changed, 61 insertions(+), 40 deletions(-) diff --git a/pkg/controller/kubelet-config/kubelet_config_controller.go b/pkg/controller/kubelet-config/kubelet_config_controller.go index 8cd30d185b..0282bdd6e3 100644 --- a/pkg/controller/kubelet-config/kubelet_config_controller.go +++ b/pkg/controller/kubelet-config/kubelet_config_controller.go @@ -330,14 +330,10 @@ 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) - if err != nil { - return nil, fmt.Errorf("could not get ControllerConfig %v", err) - } +func generateOriginalKubeletConfig(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) } @@ -512,7 +508,11 @@ func (ctrl *Controller) syncKubeletConfig(key string) error { userDefinedSystemReserved := make(map[string]string, 2) // Generate the original KubeletConfig - originalKubeletIgn, err := ctrl.generateOriginalKubeletConfig(role, features) + cc, err := ctrl.ccLister.Get(ctrlcommon.ControllerConfigName) + if err != nil { + return fmt.Errorf("could not get ControllerConfig %v", err) + } + originalKubeletIgn, err := generateOriginalKubeletConfig(cc, ctrl.templatesDir, role, features) if err != nil { return ctrl.syncStatusOnly(cfg, err, "could not generate the original Kubelet config: %v", err) } diff --git a/pkg/controller/kubelet-config/kubelet_config_features.go b/pkg/controller/kubelet-config/kubelet_config_features.go index 965a79f8e5..ab4fdfe09e 100644 --- a/pkg/controller/kubelet-config/kubelet_config_features.go +++ b/pkg/controller/kubelet-config/kubelet_config_features.go @@ -20,6 +20,7 @@ import ( "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" ) @@ -74,6 +75,11 @@ func (ctrl *Controller) syncFeatureHandler(key string) error { return err } + cc, err := ctrl.ccLister.Get(ctrlcommon.ControllerConfigName) + if err != nil { + return fmt.Errorf("could not get ControllerConfig %v", err) + } + // Find all MachineConfigPools mcpPools, err := ctrl.mcpLister.List(labels.Everything()) if err != nil { @@ -100,43 +106,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, *featureGates) 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 +209,46 @@ func generateFeatureMap(features *osev1.FeatureGate, exclusions ...string) (*map } return &rv, nil } + +func generateKubeConfigIgnFromFeatures(cc *mcfgv1.ControllerConfig, templatesDir, role string, featureGates map[string]bool) ([]byte, error) { + // Generate the original KubeletConfig + originalKubeletIgn, err := generateOriginalKubeletConfig(cc, templatesDir, role, nil) + if err != nil { + return nil, err + } + if originalKubeletIgn.Contents.Source == nil { + return nil, fmt.Errorf("could not find original Kubelet config to decode") + } + dataURL, err := dataurl.DecodeString(*originalKubeletIgn.Contents.Source) + if err != nil { + return nil, err + } + originalKubeConfig, err := decodeKubeletConfig(dataURL.Data) + if err != nil { + return nil, err + } + // Check to see if FeatureGates are equal + if reflect.DeepEqual(originalKubeConfig.FeatureGates, featureGates) { + // When there is no difference, this isn't an error, but no machine config should be created + return nil, nil + } + + // Merge in Feature Gates + err = mergo.Merge(&originalKubeConfig.FeatureGates, featureGates, mergo.WithOverride) + if err != nil { + return nil, err + } + // Encode the new config into raw JSON + cfgJSON, err := EncodeKubeletConfig(originalKubeConfig, kubeletconfigv1beta1.SchemeGroupVersion) + if err != nil { + return nil, err + } + tempIgnConfig := ctrlcommon.NewIgnConfig() + cfgIgn := createNewKubeletIgnition(cfgJSON) + tempIgnConfig.Storage.Files = append(tempIgnConfig.Storage.Files, *cfgIgn) + rawCfgIgn, err := json.Marshal(tempIgnConfig) + if err != nil { + return nil, err + } + return rawCfgIgn, nil +} diff --git a/pkg/controller/kubelet-config/kubelet_config_features_test.go b/pkg/controller/kubelet-config/kubelet_config_features_test.go index 6625468d13..847983961c 100644 --- a/pkg/controller/kubelet-config/kubelet_config_features_test.go +++ b/pkg/controller/kubelet-config/kubelet_config_features_test.go @@ -23,7 +23,7 @@ func TestFeatureGateDrift(t *testing.T) { f.ccLister = append(f.ccLister, cc) ctrl := f.newController() - kubeletConfig, err := ctrl.generateOriginalKubeletConfig("master", nil) + kubeletConfig, err := generateOriginalKubeletConfig(cc, ctrl.templatesDir, "master", nil) if err != nil { t.Errorf("could not generate kubelet config from templates %v", err) } From 5f9562976b32507658724becf5e61f97a834ddfa Mon Sep 17 00:00:00 2001 From: Joel Speed Date: Thu, 8 Jul 2021 11:35:57 +0100 Subject: [PATCH 4/8] Add Kubelet Feature Gate bootstrap --- pkg/controller/bootstrap/bootstrap.go | 9 ++++ .../kubelet-config/kubelet_config_features.go | 41 +++++++++++++++++++ 2 files changed, 50 insertions(+) diff --git a/pkg/controller/bootstrap/bootstrap.go b/pkg/controller/bootstrap/bootstrap.go index 12dd895c50..6933200f9f 100644 --- a/pkg/controller/bootstrap/bootstrap.go +++ b/pkg/controller/bootstrap/bootstrap.go @@ -22,6 +22,7 @@ import ( 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" ) @@ -139,6 +140,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 diff --git a/pkg/controller/kubelet-config/kubelet_config_features.go b/pkg/controller/kubelet-config/kubelet_config_features.go index ab4fdfe09e..26c9464e93 100644 --- a/pkg/controller/kubelet-config/kubelet_config_features.go +++ b/pkg/controller/kubelet-config/kubelet_config_features.go @@ -252,3 +252,44 @@ func generateKubeConfigIgnFromFeatures(cc *mcfgv1.ControllerConfig, templatesDir } return rawCfgIgn, nil } + +func RunFeatureGateBootstrap(templateDir string, features *osev1.FeatureGate, controllerConfig *mcfgv1.ControllerConfig, mcpPools []*mcfgv1.MachineConfigPool) ([]*mcfgv1.MachineConfig, error) { + machineConfigs := []*mcfgv1.MachineConfig{} + + featureGates, err := generateFeatureMap(features, openshiftOnlyFeatureGates...) + if err != nil { + return nil, err + } + + for _, pool := range mcpPools { + role := pool.Name + rawCfgIgn, err := generateKubeConfigIgnFromFeatures(controllerConfig, templateDir, role, *featureGates) + 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 +} From 36b4168b0028c2617c0b7853e51e8fb5f202914a Mon Sep 17 00:00:00 2001 From: Joel Speed Date: Thu, 8 Jul 2021 12:47:45 +0100 Subject: [PATCH 5/8] Add tests for KubeletConfig Feature bootstrap --- .../kubelet_config_features_test.go | 77 +++++++++++++++++++ 1 file changed, 77 insertions(+) diff --git a/pkg/controller/kubelet-config/kubelet_config_features_test.go b/pkg/controller/kubelet-config/kubelet_config_features_test.go index 847983961c..b2bd07f0aa 100644 --- a/pkg/controller/kubelet-config/kubelet_config_features_test.go +++ b/pkg/controller/kubelet-config/kubelet_config_features_test.go @@ -11,8 +11,10 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" 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/test/helpers" + "github.com/stretchr/testify/require" ) func TestFeatureGateDrift(t *testing.T) { @@ -131,3 +133,78 @@ func TestFeaturesCustomNoUpgrade(t *testing.T) { }) } } + +func TestBootstrapFeaturesDefault(t *testing.T) { + for _, platform := range []configv1.PlatformType{configv1.AWSPlatformType, configv1.NonePlatformType, "unrecognized"} { + t.Run(string(platform), func(t *testing.T) { + + cc := newControllerConfig(ctrlcommon.ControllerConfigName, platform) + mcp := helpers.NewMachineConfigPool("master", nil, helpers.MasterSelector, "v0") + mcp2 := helpers.NewMachineConfigPool("worker", nil, helpers.WorkerSelector, "v0") + mcps := []*mcfgv1.MachineConfigPool{mcp, mcp2} + + features := createNewDefaultFeatureGate() + + mcs, err := RunFeatureGateBootstrap("../../../templates", features, cc, mcps) + if err != nil { + t.Errorf("could not run feature gate bootstrap: %v", err) + } + if len(mcs) > 0 { + t.Errorf("expected no machine config generated with the default feature gate, got %d configs", len(mcs)) + } + }) + } +} + +func TestBootstrapFeaturesCustomNoUpgrade(t *testing.T) { + for _, platform := range []configv1.PlatformType{configv1.AWSPlatformType, configv1.NonePlatformType, "unrecognized"} { + t.Run(string(platform), func(t *testing.T) { + + cc := newControllerConfig(ctrlcommon.ControllerConfigName, platform) + mcp := helpers.NewMachineConfigPool("master", nil, helpers.MasterSelector, "v0") + mcp2 := helpers.NewMachineConfigPool("worker", nil, helpers.WorkerSelector, "v0") + mcps := []*mcfgv1.MachineConfigPool{mcp, mcp2} + + features := &osev1.FeatureGate{ + ObjectMeta: metav1.ObjectMeta{ + Name: ctrlcommon.ClusterFeatureInstanceName, + }, + Spec: osev1.FeatureGateSpec{ + FeatureGateSelection: osev1.FeatureGateSelection{ + FeatureSet: osev1.CustomNoUpgrade, + CustomNoUpgrade: &osev1.CustomFeatureGates{ + Enabled: []string{"CSIMigration"}, + }, + }, + }, + } + + mcs, err := RunFeatureGateBootstrap("../../../templates", features, cc, mcps) + if err != nil { + t.Errorf("could not run feature gate bootstrap: %v", err) + } + if len(mcs) != 2 { + t.Errorf("expected 2 machine configs generated with the custom feature gate, got %d configs", len(mcs)) + } + + for _, mc := range mcs { + ignCfg, err := ctrlcommon.ParseAndConvertConfig(mc.Spec.Config.Raw) + regfile := ignCfg.Storage.Files[0] + conf, err := dataurl.DecodeString(*regfile.Contents.Source) + require.NoError(t, err) + + originalKubeConfig, _ := decodeKubeletConfig(conf.Data) + defaultFeatureGates, err := generateFeatureMap(createNewDefaultFeatureGate()) + if err != nil { + t.Errorf("could not generate defaultFeatureGates: %v", err) + } + if reflect.DeepEqual(originalKubeConfig.FeatureGates, *defaultFeatureGates) { + t.Errorf("template FeatureGates should not match default openshift/api FeatureGates: (default=%v)", defaultFeatureGates) + } + if !originalKubeConfig.FeatureGates["CSIMigration"] { + t.Errorf("template FeatureGates should contain CSIMigration: %v", originalKubeConfig.FeatureGates) + } + } + }) + } +} From 0c555186774b89964c3a6156095107a35c9d5359 Mon Sep 17 00:00:00 2001 From: Joel Speed Date: Thu, 8 Jul 2021 18:02:51 +0100 Subject: [PATCH 6/8] Prevent go cyclo lint error on bootstrap Run --- pkg/controller/bootstrap/bootstrap.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/controller/bootstrap/bootstrap.go b/pkg/controller/bootstrap/bootstrap.go index 6933200f9f..5f0a0a5b1f 100644 --- a/pkg/controller/bootstrap/bootstrap.go +++ b/pkg/controller/bootstrap/bootstrap.go @@ -48,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 { From 1b4429b675618d4fdfacb8c9aa19ebd77cde53c8 Mon Sep 17 00:00:00 2001 From: Joel Speed Date: Mon, 19 Jul 2021 11:38:54 +0100 Subject: [PATCH 7/8] Extract feature gate merge into kubelet config extraction --- .../kubelet_config_controller.go | 66 ++++++++++++------- .../kubelet-config/kubelet_config_features.go | 39 +++-------- .../kubelet_config_features_test.go | 2 +- 3 files changed, 50 insertions(+), 57 deletions(-) diff --git a/pkg/controller/kubelet-config/kubelet_config_controller.go b/pkg/controller/kubelet-config/kubelet_config_controller.go index 0282bdd6e3..2bdba55949 100644 --- a/pkg/controller/kubelet-config/kubelet_config_controller.go +++ b/pkg/controller/kubelet-config/kubelet_config_controller.go @@ -330,7 +330,40 @@ func (ctrl *Controller) handleFeatureErr(err error, key interface{}) { ctrl.featureQueue.AddAfter(key, 1*time.Minute) } -func generateOriginalKubeletConfig(cc *mcfgv1.ControllerConfig, templatesDir, role string, featureGate *configv1.FeatureGate) (*ign3types.File, error) { +// 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 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, templatesDir) @@ -474,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, openshiftOnlyFeatureGates...) - 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 +539,10 @@ func (ctrl *Controller) syncKubeletConfig(key string) error { if err != nil { return fmt.Errorf("could not get ControllerConfig %v", err) } - originalKubeletIgn, err := generateOriginalKubeletConfig(cc, ctrl.templatesDir, 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) - if err != nil { - return ctrl.syncStatusOnly(cfg, err, "could not decode the original Kubelet source string: %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,16 +578,15 @@ 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 { diff --git a/pkg/controller/kubelet-config/kubelet_config_features.go b/pkg/controller/kubelet-config/kubelet_config_features.go index 26c9464e93..2d7c9ebd89 100644 --- a/pkg/controller/kubelet-config/kubelet_config_features.go +++ b/pkg/controller/kubelet-config/kubelet_config_features.go @@ -8,10 +8,8 @@ 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" @@ -70,10 +68,6 @@ func (ctrl *Controller) syncFeatureHandler(key string) error { } else if err != nil { return err } - featureGates, err := generateFeatureMap(features, openshiftOnlyFeatureGates...) - if err != nil { - return err - } cc, err := ctrl.ccLister.Get(ctrlcommon.ControllerConfigName) if err != nil { @@ -107,7 +101,7 @@ func (ctrl *Controller) syncFeatureHandler(key string) error { } } - rawCfgIgn, err := generateKubeConfigIgnFromFeatures(cc, ctrl.templatesDir, role, *featureGates) + rawCfgIgn, err := generateKubeConfigIgnFromFeatures(cc, ctrl.templatesDir, role, features) if err != nil { return err } @@ -210,34 +204,22 @@ func generateFeatureMap(features *osev1.FeatureGate, exclusions ...string) (*map return &rv, nil } -func generateKubeConfigIgnFromFeatures(cc *mcfgv1.ControllerConfig, templatesDir, role string, featureGates map[string]bool) ([]byte, error) { - // Generate the original KubeletConfig - originalKubeletIgn, err := generateOriginalKubeletConfig(cc, templatesDir, role, nil) - if err != nil { - return nil, err - } - if originalKubeletIgn.Contents.Source == nil { - return nil, fmt.Errorf("could not find original Kubelet config to decode") - } - dataURL, err := dataurl.DecodeString(*originalKubeletIgn.Contents.Source) +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 } - originalKubeConfig, err := decodeKubeletConfig(dataURL.Data) + defaultFeatures, err := generateFeatureMap(createNewDefaultFeatureGate(), openshiftOnlyFeatureGates...) if err != nil { return nil, err } - // Check to see if FeatureGates are equal - if reflect.DeepEqual(originalKubeConfig.FeatureGates, featureGates) { + + // 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 } - // Merge in Feature Gates - err = mergo.Merge(&originalKubeConfig.FeatureGates, featureGates, mergo.WithOverride) - if err != nil { - return nil, err - } // Encode the new config into raw JSON cfgJSON, err := EncodeKubeletConfig(originalKubeConfig, kubeletconfigv1beta1.SchemeGroupVersion) if err != nil { @@ -256,14 +238,9 @@ func generateKubeConfigIgnFromFeatures(cc *mcfgv1.ControllerConfig, templatesDir func RunFeatureGateBootstrap(templateDir string, features *osev1.FeatureGate, controllerConfig *mcfgv1.ControllerConfig, mcpPools []*mcfgv1.MachineConfigPool) ([]*mcfgv1.MachineConfig, error) { machineConfigs := []*mcfgv1.MachineConfig{} - featureGates, err := generateFeatureMap(features, openshiftOnlyFeatureGates...) - if err != nil { - return nil, err - } - for _, pool := range mcpPools { role := pool.Name - rawCfgIgn, err := generateKubeConfigIgnFromFeatures(controllerConfig, templateDir, role, *featureGates) + rawCfgIgn, err := generateKubeConfigIgnFromFeatures(controllerConfig, templateDir, role, features) if err != nil { return nil, err } diff --git a/pkg/controller/kubelet-config/kubelet_config_features_test.go b/pkg/controller/kubelet-config/kubelet_config_features_test.go index b2bd07f0aa..ecc7cbbf17 100644 --- a/pkg/controller/kubelet-config/kubelet_config_features_test.go +++ b/pkg/controller/kubelet-config/kubelet_config_features_test.go @@ -25,7 +25,7 @@ func TestFeatureGateDrift(t *testing.T) { f.ccLister = append(f.ccLister, cc) ctrl := f.newController() - kubeletConfig, err := generateOriginalKubeletConfig(cc, ctrl.templatesDir, "master", nil) + kubeletConfig, err := generateOriginalKubeletConfigIgn(cc, ctrl.templatesDir, "master", nil) if err != nil { t.Errorf("could not generate kubelet config from templates %v", err) } From 46b6acece9c68b9941610b7b31284a8d892ad44f Mon Sep 17 00:00:00 2001 From: Joel Speed Date: Mon, 19 Jul 2021 12:01:04 +0100 Subject: [PATCH 8/8] Deduplicate kubelet configuration to ignition conversion --- pkg/controller/kubelet-config/helpers.go | 10 ++++++++++ .../kubelet_config_controller.go | 19 ++++++------------- .../kubelet-config/kubelet_config_features.go | 5 ++--- 3 files changed, 18 insertions(+), 16 deletions(-) diff --git a/pkg/controller/kubelet-config/helpers.go b/pkg/controller/kubelet-config/helpers.go index cddfb91765..7a1a10af53 100644 --- a/pkg/controller/kubelet-config/helpers.go +++ b/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 +} diff --git a/pkg/controller/kubelet-config/kubelet_config_controller.go b/pkg/controller/kubelet-config/kubelet_config_controller.go index 2bdba55949..e69a18cecc 100644 --- a/pkg/controller/kubelet-config/kubelet_config_controller.go +++ b/pkg/controller/kubelet-config/kubelet_config_controller.go @@ -587,19 +587,12 @@ func (ctrl *Controller) syncKubeletConfig(key string) error { if err != nil { return ctrl.syncStatusOnly(cfg, err, "could not merge original config and new config: %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 { diff --git a/pkg/controller/kubelet-config/kubelet_config_features.go b/pkg/controller/kubelet-config/kubelet_config_features.go index 2d7c9ebd89..9375328251 100644 --- a/pkg/controller/kubelet-config/kubelet_config_features.go +++ b/pkg/controller/kubelet-config/kubelet_config_features.go @@ -16,7 +16,6 @@ import ( 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" @@ -221,12 +220,12 @@ func generateKubeConfigIgnFromFeatures(cc *mcfgv1.ControllerConfig, templatesDir } // Encode the new config into raw JSON - cfgJSON, err := EncodeKubeletConfig(originalKubeConfig, kubeletconfigv1beta1.SchemeGroupVersion) + cfgIgn, err := kubeletConfigToIgnFile(originalKubeConfig) if err != nil { return nil, err } + tempIgnConfig := ctrlcommon.NewIgnConfig() - cfgIgn := createNewKubeletIgnition(cfgJSON) tempIgnConfig.Storage.Files = append(tempIgnConfig.Storage.Files, *cfgIgn) rawCfgIgn, err := json.Marshal(tempIgnConfig) if err != nil {