Bug 1993922: fixes 1 to 1 kubelet config mapping

pkg/controller/kubelet-config/helpers.go

@@ -12,6 +12,7 @@ import (
 	"github.com/vincent-petithory/dataurl"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apimachinery/pkg/runtime/serializer"
@@ -145,22 +146,36 @@ func findKubeletConfig(mc *mcfgv1.MachineConfig) (*ign3types.File, error) {
 // nolint: dupl
 func getManagedKubeletConfigKey(pool *mcfgv1.MachineConfigPool, client mcfgclientset.Interface, cfg *mcfgv1.KubeletConfig) (string, error) {
 	// Get all the kubelet config CRs
-	kcList, err := client.MachineconfigurationV1().KubeletConfigs().List(context.TODO(), metav1.ListOptions{})
+	kcListAll, err := client.MachineconfigurationV1().KubeletConfigs().List(context.TODO(), metav1.ListOptions{})
 	if err != nil {
 		return "", fmt.Errorf("error listing kubelet configs: %v", err)
 	}
+
 	// If there is no kubelet config in the list, return the default MC name with no suffix
-	if kcList == nil || len(kcList.Items) == 0 {
+	if kcListAll == nil || len(kcListAll.Items) == 0 {
 		return ctrlcommon.GetManagedKey(pool, client, "99", "kubelet", getManagedKubeletConfigKeyDeprecated(pool))
 	}
-	for _, kc := range kcList.Items {
+
+	var kcList []mcfgv1.KubeletConfig
+	for _, kc := range kcListAll.Items {
+		selector, err := metav1.LabelSelectorAsSelector(kc.Spec.MachineConfigPoolSelector)
+		if err != nil {
+			return "", fmt.Errorf("invalid label selector: %v", err)
+		}
+		if selector.Empty() || !selector.Matches(labels.Set(pool.Labels)) {
+			continue
+		}
+		kcList = append(kcList, kc)
+	}
+
+	for _, kc := range kcList {
 		if kc.Name != cfg.Name {
 			continue
 		}
 		val, ok := kc.GetAnnotations()[ctrlcommon.MCNameSuffixAnnotationKey]
 		// If we find a matching kubelet config and it is the only one in the list, then return the default MC name with no suffix
-		// add check len(kcList.Items) < 2, mc name should not suffixed if cfg is the first kubelet config to be updated/created
-		if !ok && len(kcList.Items) < 2 {
+		// add check len(kcList) < 2, mc name should not suffixed if cfg is the first kubelet config to be updated/created
+		if !ok && len(kcList) < 2 {
 			return ctrlcommon.GetManagedKey(pool, client, "99", "kubelet", getManagedKubeletConfigKeyDeprecated(pool))
 		}
 		// Otherwise if an MC name suffix exists, append it to the default MC name and return that as this kubelet config exists and
@@ -173,7 +188,7 @@ func getManagedKubeletConfigKey(pool *mcfgv1.MachineConfigPool, client mcfgclien
 	// If we are here, this means that a new kubelet config was created, so we have to calculate the suffix value for its MC name
 	suffixNum := 0
 	// Go through the list of kubelet config objects created and get the max suffix value currently created
-	for _, item := range kcList.Items {
+	for _, item := range kcList {
 		val, ok := item.GetAnnotations()[ctrlcommon.MCNameSuffixAnnotationKey]
 		if ok {
 			// Convert the suffix value to int so we can look through the list and grab the max suffix created so far

pkg/controller/kubelet-config/kubelet_config_controller_test.go

@@ -56,7 +56,8 @@ type fixture struct {
 	featLister      []*osev1.FeatureGate
 	apiserverLister []*osev1.APIServer
 
-	actions []core.Action
+	actions               []core.Action
+	skipActionsValidation bool
 
 	objects    []runtime.Object
 	oseobjects []runtime.Object
@@ -78,7 +79,23 @@ func newFixture(t *testing.T) *fixture {
 }
 
 func (f *fixture) validateActions() {
+	if f.skipActionsValidation {
+		f.t.Log("Skipping actions validation")
+		return
+	}
 	actions := filterInformerActions(f.client.Actions())
+	if len(f.actions) != len(actions) {
+		f.t.Log("Expected Actions:")
+		for i := range f.actions {
+			f.t.Logf("\t%v %#v", f.actions[i].GetVerb(), f.actions[i])
+		}
+		f.t.Log("Seen Actions:")
+		for i := range actions {
+			f.t.Logf("\t%v %#v", actions[i].GetVerb(), actions[i])
+		}
+		f.t.Errorf("number of seen actions do not match expected actions count")
+		return
+	}
 	for i, action := range actions {
 		glog.Infof("  Action: %v", action)
 
@@ -88,7 +105,7 @@ func (f *fixture) validateActions() {
 		}
 
 		expectedAction := f.actions[i]
-		checkAction(expectedAction, action, f.t)
+		checkAction(expectedAction, action, f.t, i)
 	}
 
 	if len(f.actions) > len(actions) {
@@ -287,9 +304,14 @@ func filterOSEActions(actions []core.Action) []core.Action {
 
 // checkAction verifies that expected and actual actions are equal and both have
 // same attached resources
-func checkAction(expected, actual core.Action, t *testing.T) {
+func checkAction(expected, actual core.Action, t *testing.T, index int) {
 	if !(expected.Matches(actual.GetVerb(), actual.GetResource().Resource) && actual.GetSubresource() == expected.GetSubresource()) {
-		t.Errorf("Expected\n\t%#v\ngot\n\t%#v", expected, actual)
+		if actual.GetVerb() == "patch" {
+			actual := actual.(core.PatchAction)
+			t.Errorf("Expected(index=%v)\n\t%#v\ngot\n\t%#v\npatch\t%#v", index, expected, actual, string(actual.GetPatch()))
+		} else {
+			t.Errorf("Expected(index=%v)\n\t%#v\ngot\n\t%#v", index, expected, actual)
+		}
 		return
 	}
 
@@ -350,6 +372,15 @@ func (f *fixture) expectUpdateKubeletConfig(config *mcfgv1.KubeletConfig) {
 	f.actions = append(f.actions, core.NewRootUpdateSubresourceAction(schema.GroupVersionResource{Version: "v1", Group: "machineconfiguration.openshift.io", Resource: "kubeletconfigs"}, "status", config))
 }
 
+func (f *fixture) expectUpdateKubeletConfigRoot(config *mcfgv1.KubeletConfig) {
+	f.actions = append(f.actions, core.NewRootUpdateAction(schema.GroupVersionResource{Version: "v1", Group: "machineconfiguration.openshift.io", Resource: "kubeletconfigs"}, config))
+}
+
+func (f *fixture) resetActions() {
+	f.actions = []core.Action{}
+	f.client.ClearActions()
+}
+
 func TestKubeletConfigCreate(t *testing.T) {
 	for _, platform := range []osev1.PlatformType{osev1.AWSPlatformType, osev1.NonePlatformType, "unrecognized"} {
 		t.Run(string(platform), func(t *testing.T) {
@@ -383,6 +414,54 @@ func TestKubeletConfigCreate(t *testing.T) {
 	}
 }
 
+func TestKubeletConfigMultiCreate(t *testing.T) {
+	for _, platform := range []osev1.PlatformType{osev1.AWSPlatformType, osev1.NonePlatformType, "unrecognized"} {
+		t.Run(string(platform), func(t *testing.T) {
+			f := newFixture(t)
+			f.newController()
+
+			cc := newControllerConfig(ctrlcommon.ControllerConfigName, platform)
+			f.ccLister = append(f.ccLister, cc)
+
+			kcCount := 30
+			for i := 0; i < kcCount; i++ {
+				f.resetActions()
+
+				poolName := fmt.Sprintf("subpool%v", i)
+				poolLabelName := fmt.Sprintf("pools.operator.machineconfiguration.openshift.io/%s", poolName)
+				labelSelector := metav1.AddLabelToSelector(&metav1.LabelSelector{}, poolLabelName, "")
+
+				mcp := helpers.NewMachineConfigPool(poolName, nil, labelSelector, "v0")
+				mcp.ObjectMeta.Labels[poolLabelName] = ""
+
+				kc := newKubeletConfig(poolName, &kubeletconfigv1beta1.KubeletConfiguration{MaxPods: 100}, labelSelector)
+
+				f.mcpLister = append(f.mcpLister, mcp)
+				f.mckLister = append(f.mckLister, kc)
+				f.objects = append(f.objects, kc)
+
+				mcs := helpers.NewMachineConfig(generateManagedKey(kc, 1), labelSelector.MatchLabels, "dummy://", []ign3types.File{{}})
+				mcsDeprecated := mcs.DeepCopy()
+				mcsDeprecated.Name = getManagedKubeletConfigKeyDeprecated(mcp)
+
+				expectedPatch := fmt.Sprintf("{\"metadata\":{\"finalizers\":[\"99-%v-generated-kubelet-1\"]}}", poolName)
+
+				f.expectGetMachineConfigAction(mcs)
+				f.expectGetMachineConfigAction(mcsDeprecated)
+				f.expectGetMachineConfigAction(mcs)
+				f.expectCreateMachineConfigAction(mcs)
+				f.expectPatchKubeletConfig(kc, []byte(expectedPatch))
+				f.expectUpdateKubeletConfig(kc)
+				f.run(poolName)
+			}
+		})
+	}
+}
+
+func generateManagedKey(kcfg *mcfgv1.KubeletConfig, generation uint64) string {
+	return fmt.Sprintf("99-%s-generated-kubelet-%v", kcfg.Name, generation)
+}
+
 func TestKubeletConfigAutoSizingReserved(t *testing.T) {
 	for _, platform := range []osev1.PlatformType{osev1.AWSPlatformType, osev1.NonePlatformType, "unrecognized"} {
 		t.Run(string(platform), func(t *testing.T) {