Skip to content

Testing pr #5240

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from
Draft

Testing pr #5240

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
a9ad0c1
OCPBUGS-57979: Update Kubelet Config MachineConfig upon FeatureGate u…
MaysaMacedo Aug 7, 2025
cd87e1e
Improve logic and fix unit tests
MaysaMacedo Aug 8, 2025
f941395
Include unit tests
MaysaMacedo Aug 10, 2025
caeb4b4
adding log
MaysaMacedo Aug 11, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 50 additions & 10 deletions pkg/controller/kubelet-config/kubelet_config_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -532,9 +532,9 @@ func (ctrl *Controller) addAnnotation(cfg *mcfgv1.KubeletConfig, annotationKey,
//nolint:gocyclo
func (ctrl *Controller) syncKubeletConfig(key string) error {
startTime := time.Now()
klog.V(4).Infof("Started syncing kubeletconfig %q (%v)", key, startTime)
klog.Infof("Started syncing kubeletconfig %q (%v)", key, startTime)
defer func() {
klog.V(4).Infof("Finished syncing kubeletconfig %q (%v)", key, time.Since(startTime))
klog.Infof("Finished syncing kubeletconfig %q (%v)", key, time.Since(startTime))
}()

// Wait to apply a kubelet config if the controller config is not completed
Expand Down Expand Up @@ -609,14 +609,7 @@ func (ctrl *Controller) syncKubeletConfig(key string) error {
return ctrl.syncStatusOnly(cfg, err, "could not find MachineConfig: %v", managedKey)
}
isNotFound := macherrors.IsNotFound(err)
// If we have seen this generation and the sync didn't fail, then skip
if !isNotFound && cfg.Status.ObservedGeneration >= cfg.Generation && cfg.Status.Conditions[len(cfg.Status.Conditions)-1].Type == mcfgv1.KubeletConfigSuccess {
// But we still need to compare the generated controller version because during an upgrade we need a new one
mcCtrlVersion := mc.Annotations[ctrlcommon.GeneratedByControllerVersionAnnotationKey]
if mcCtrlVersion == version.Hash {
return nil
}
}

// Generate the original KubeletConfig
cc, err := ctrl.ccLister.Get(ctrlcommon.ControllerConfigName)
if err != nil {
Expand All @@ -632,6 +625,26 @@ func (ctrl *Controller) syncKubeletConfig(key string) error {
updateOriginalKubeConfigwithNodeConfig(nodeConfig, originalKubeConfig)
}

if !isNotFound {
// When a Machine Config already exists, let's check if it needs an update on the feature Gates
match, err := machineConfigFeatureGatesMatchesOriginalFeatureGates(mc, originalKubeConfig)
if err != nil {
return ctrl.syncStatusOnly(cfg, err, "could not check if original and machine config feature gates match: %v", err)
}
configSuccess := false
if len(cfg.Status.Conditions) > 0 {
configSuccess = cfg.Status.Conditions[len(cfg.Status.Conditions)-1].Type == mcfgv1.KubeletConfigSuccess
}
if match && cfg.Status.ObservedGeneration >= cfg.Generation && configSuccess {
// But we still need to compare the generated controller version because during an upgrade we need a new one
mcCtrlVersion := mc.Annotations[ctrlcommon.GeneratedByControllerVersionAnnotationKey]
if mcCtrlVersion == version.Hash {
klog.Infof("Skipping update")
return nil
}
}
}

// If the provided kubeletconfig has a TLS profile, override the one generated from templates.
if cfg.Spec.TLSSecurityProfile != nil {
klog.Infof("Using tlsSecurityProfile provided by KubeletConfig %s", cfg.Name)
Expand Down Expand Up @@ -700,8 +713,10 @@ func (ctrl *Controller) syncKubeletConfig(key string) error {
if err := retry.RetryOnConflict(updateBackoff, func() error {
var err error
if isNotFound {
klog.Infof("Creating Machine Config %s", mc.Name)
_, err = ctrl.client.MachineconfigurationV1().MachineConfigs().Create(context.TODO(), mc, metav1.CreateOptions{})
} else {
klog.Infof("Updating Machine Config %s", mc.Name)
_, err = ctrl.client.MachineconfigurationV1().MachineConfigs().Update(context.TODO(), mc, metav1.UpdateOptions{})
}
return err
Expand Down Expand Up @@ -749,6 +764,31 @@ func (ctrl *Controller) cleanUpDuplicatedMC(prefix string) error {
return nil
}

// machineConfigFeatureGatesMatchesOriginalFeatureGates checks if the Kubelet Config defined at the Machine Config matches the latest
// feature gate defined.
func machineConfigFeatureGatesMatchesOriginalFeatureGates(mc *mcfgv1.MachineConfig, originalKubeConfig *kubeletconfigv1beta1.KubeletConfiguration) (bool, error) {
if len(mc.Spec.Config.Raw) == 0 {
if len(originalKubeConfig.FeatureGates) > 0 {
return false, nil
}
return true, nil
}
file, err := findKubeletConfig(mc)
if err != nil {
return false, nil
}
// Extract and decode the encoded data
decodedData, err := ctrlcommon.DecodeIgnitionFileContents(file.Contents.Source, file.Contents.Compression)
if err != nil {
return false, fmt.Errorf("error decoding %s: %v", file.Path, err)
}
mckubeConfig, err := DecodeKubeletConfig(decodedData)
if err != nil {
return false, fmt.Errorf("could not deserialize the Kubelet source: %w", err)
}
return reflect.DeepEqual(mckubeConfig.FeatureGates, originalKubeConfig.FeatureGates), nil
}

func (ctrl *Controller) popFinalizerFromKubeletConfig(kc *mcfgv1.KubeletConfig) error {
return retry.RetryOnConflict(updateBackoff, func() error {
newcfg, err := ctrl.mckLister.Get(kc.Name)
Expand Down
169 changes: 167 additions & 2 deletions pkg/controller/kubelet-config/kubelet_config_controller_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -597,7 +597,7 @@ func TestKubeletConfigUpdates(t *testing.T) {
mcp2 := helpers.NewMachineConfigPool("worker", nil, helpers.WorkerSelector, "v0")
kc1 := newKubeletConfig("smaller-max-pods", &kubeletconfigv1beta1.KubeletConfiguration{MaxPods: 100}, metav1.AddLabelToSelector(&metav1.LabelSelector{}, "pools.operator.machineconfiguration.openshift.io/master", ""))
kubeletConfigKey, _ := getManagedKubeletConfigKey(mcp, f.client, kc1)
mcs := helpers.NewMachineConfig(kubeletConfigKey, map[string]string{"node-role/master": ""}, "dummy://", []ign3types.File{{}})
mcs := helpers.NewMachineConfig(kubeletConfigKey, map[string]string{"node-role/master": ""}, "dummy://", []ign3types.File{})
mcsDeprecated := mcs.DeepCopy()
mcsDeprecated.Name = getManagedKubeletConfigKeyDeprecated(mcp)

Expand Down Expand Up @@ -672,6 +672,171 @@ func TestKubeletConfigUpdates(t *testing.T) {
}
}

func TestMachineConfigUpdateUponFeatureGateUpdate(t *testing.T) {
for _, platform := range []osev1.PlatformType{osev1.AWSPlatformType, osev1.NonePlatformType, "unrecognized"} {
t.Run(string(platform), func(t *testing.T) {
f := newFixture(t)
fgHandler := ctrlcommon.NewFeatureGatesHardcodedHandler([]osev1.FeatureGateName{"Example"}, nil)
f.newController(fgHandler)

cc := newControllerConfig(ctrlcommon.ControllerConfigName, platform)
mcp := helpers.NewMachineConfigPool("master", nil, helpers.MasterSelector, "v0")
mcp2 := helpers.NewMachineConfigPool("worker", nil, helpers.WorkerSelector, "v0")
kc1 := newKubeletConfig("smaller-max-pods", &kubeletconfigv1beta1.KubeletConfiguration{MaxPods: 100}, metav1.AddLabelToSelector(&metav1.LabelSelector{}, "pools.operator.machineconfiguration.openshift.io/master", ""))
kubeletConfigKey, _ := getManagedKubeletConfigKey(mcp, f.client, kc1)
originalKubeletConfiguration := kubeletconfigv1beta1.KubeletConfiguration{MaxPods: 100, FeatureGates: map[string]bool{"Example": true}}
cfgIgn, err := kubeletConfigToIgnFile(&originalKubeletConfiguration)
if err != nil {
t.Errorf("kubeletConfigToIgnFile returned: %v", err)
}
mcs := helpers.NewMachineConfig(kubeletConfigKey, map[string]string{"node-role/master": ""}, "dummy://", []ign3types.File{*cfgIgn})
mcs.SetAnnotations(map[string]string{
ctrlcommon.GeneratedByControllerVersionAnnotationKey: version.Hash,
})
mcsDeprecated := mcs.DeepCopy()
mcsDeprecated.Name = getManagedKubeletConfigKeyDeprecated(mcp)

f.ccLister = append(f.ccLister, cc)
f.mcpLister = append(f.mcpLister, mcp)
f.mcpLister = append(f.mcpLister, mcp2)
f.mckLister = append(f.mckLister, kc1)
f.objects = append(f.objects, kc1)

f.expectGetMachineConfigAction(mcs)
f.expectGetMachineConfigAction(mcsDeprecated)
f.expectGetMachineConfigAction(mcs)
f.expectUpdateKubeletConfigRoot(kc1)
f.expectCreateMachineConfigAction(mcs)
f.expectPatchKubeletConfig(kc1, kcfgPatchBytes)
f.expectUpdateKubeletConfig(kc1)

c := f.newController(fgHandler)
stopCh := make(chan struct{})

err = c.syncHandler(getKey(kc1, t))
if err != nil {
t.Errorf("syncHandler returned: %v", err)
}

f.validateActions()
close(stopCh)

// Perform Update
f = newFixture(t)

f.ccLister = append(f.ccLister, cc)
f.mcpLister = append(f.mcpLister, mcp)
f.mcpLister = append(f.mcpLister, mcp2)
f.mckLister = append(f.mckLister, kc1)
f.objects = append(f.objects, mcs, kc1) // MachineConfig exists

// no feature gates update
fgHandler = ctrlcommon.NewFeatureGatesHardcodedHandler([]osev1.FeatureGateName{"Example", "Example2"}, nil)
c = f.newController(fgHandler)
stopCh = make(chan struct{})

// Apply update
err = c.syncHandler(getKey(kc1, t))
if err != nil {
t.Errorf("syncHandler returned: %v", err)
}

f.expectGetMachineConfigAction(mcs)
f.expectGetMachineConfigAction(mcs)
f.expectUpdateMachineConfigAction(mcs)
f.expectPatchKubeletConfig(kc1, kcfgPatchBytes)
f.expectUpdateKubeletConfig(kc1)

f.validateActions()

close(stopCh)

})
}
}

func TestMachineConfigSkipUpdate(t *testing.T) {
for _, platform := range []osev1.PlatformType{osev1.AWSPlatformType, osev1.NonePlatformType, "unrecognized"} {
t.Run(string(platform), func(t *testing.T) {
f := newFixture(t)
fgHandler := ctrlcommon.NewFeatureGatesHardcodedHandler([]osev1.FeatureGateName{"Example"}, nil)
f.newController(fgHandler)

cc := newControllerConfig(ctrlcommon.ControllerConfigName, platform)
mcp := helpers.NewMachineConfigPool("master", nil, helpers.MasterSelector, "v0")
mcp2 := helpers.NewMachineConfigPool("worker", nil, helpers.WorkerSelector, "v0")
kc1 := newKubeletConfig("smaller-max-pods", &kubeletconfigv1beta1.KubeletConfiguration{MaxPods: 100}, metav1.AddLabelToSelector(&metav1.LabelSelector{}, "pools.operator.machineconfiguration.openshift.io/master", ""))
kubeletConfigKey, _ := getManagedKubeletConfigKey(mcp, f.client, kc1)
originalKubeletConfiguration := kubeletconfigv1beta1.KubeletConfiguration{MaxPods: 100, FeatureGates: map[string]bool{"Example": true}}
cfgIgn, err := kubeletConfigToIgnFile(&originalKubeletConfiguration)
if err != nil {
t.Errorf("kubeletConfigToIgnFile returned: %v", err)
}
mcs := helpers.NewMachineConfig(kubeletConfigKey, map[string]string{"node-role/master": ""}, "dummy://", []ign3types.File{*cfgIgn})
mcs.SetAnnotations(map[string]string{
ctrlcommon.GeneratedByControllerVersionAnnotationKey: version.Hash,
})
mcsDeprecated := mcs.DeepCopy()
mcsDeprecated.Name = getManagedKubeletConfigKeyDeprecated(mcp)

f.ccLister = append(f.ccLister, cc)
f.mcpLister = append(f.mcpLister, mcp)
f.mcpLister = append(f.mcpLister, mcp2)
f.mckLister = append(f.mckLister, kc1)
f.objects = append(f.objects, kc1)

f.expectGetMachineConfigAction(mcs)
f.expectGetMachineConfigAction(mcsDeprecated)
f.expectGetMachineConfigAction(mcs)
f.expectUpdateKubeletConfigRoot(kc1)
f.expectCreateMachineConfigAction(mcs)
f.expectPatchKubeletConfig(kc1, kcfgPatchBytes)
f.expectUpdateKubeletConfig(kc1)

c := f.newController(fgHandler)
stopCh := make(chan struct{})

err = c.syncHandler(getKey(kc1, t))
if err != nil {
t.Errorf("syncHandler returned: %v", err)
}

f.validateActions()
close(stopCh)

// Perform Update
f = newFixture(t)

f.ccLister = append(f.ccLister, cc)
f.mcpLister = append(f.mcpLister, mcp)
f.mcpLister = append(f.mcpLister, mcp2)
f.mckLister = append(f.mckLister, kc1)
f.objects = append(f.objects, mcs, kc1) // MachineConfig exists

// no feature gates update
fgHandler = ctrlcommon.NewFeatureGatesHardcodedHandler([]osev1.FeatureGateName{"Example"}, nil)
c = f.newController(fgHandler)
stopCh = make(chan struct{})

klog.Info("Applying no update")

// Apply update
err = c.syncHandler(getKey(kc1, t))
if err != nil {
t.Errorf("syncHandler returned: %v", err)
}

f.expectGetMachineConfigAction(mcs)
f.expectGetMachineConfigAction(mcs)

f.validateActions()

close(stopCh)

})
}
}

func TestKubeletConfigDenylistedOptions(t *testing.T) {
failureTests := []struct {
name string
Expand Down Expand Up @@ -969,7 +1134,7 @@ func TestAddAnnotationExistingKubeletConfig(t *testing.T) {
kc1 := newKubeletConfig("smaller-max-pods-1", &kubeletconfigv1beta1.KubeletConfiguration{MaxPods: 200}, metav1.AddLabelToSelector(&metav1.LabelSelector{}, "pools.operator.machineconfiguration.openshift.io/master", ""))
kc1.SetAnnotations(map[string]string{ctrlcommon.MCNameSuffixAnnotationKey: "1"})
kc1.Finalizers = []string{kc1MCKey}
kcMC := helpers.NewMachineConfig(kcMCKey, map[string]string{"node-role/master": ""}, "dummy://", []ign3types.File{{}})
kcMC := helpers.NewMachineConfig(kcMCKey, map[string]string{"node-role/master": ""}, "dummy://", []ign3types.File{})

f.ccLister = append(f.ccLister, cc)
f.mcpLister = append(f.mcpLister, mcp)
Expand Down