[release-4.9] Bug 2032985: fixes 1 to 1 containerruntime config mapping

pkg/controller/container-runtime-config/container_runtime_config_controller_test.go

@@ -68,7 +68,8 @@ type fixture struct {
 	cvLister   []*apicfgv1.ClusterVersion
 	icspLister []*apioperatorsv1alpha1.ImageContentSourcePolicy
 
-	actions []core.Action
+	actions               []core.Action
+	skipActionsValidation bool
 
 	objects         []runtime.Object
 	imgObjects      []runtime.Object
@@ -83,6 +84,10 @@ 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())
 	for i, action := range actions {
 		glog.Infof("Action: %v", action)
@@ -93,14 +98,19 @@ 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) {
 		f.t.Errorf("%d additional expected actions:%+v", len(f.actions)-len(actions), f.actions[len(actions):])
 	}
 }
 
+func (f *fixture) resetActions() {
+	f.actions = []core.Action{}
+	f.client.ClearActions()
+}
+
 func newControllerConfig(name string, platform apicfgv1.PlatformType) *mcfgv1.ControllerConfig {
 	cc := &mcfgv1.ControllerConfig{
 		TypeMeta:   metav1.TypeMeta{APIVersion: mcfgv1.SchemeGroupVersion.String()},
@@ -275,9 +285,15 @@ func filterInformerActions(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
 	}
 
@@ -1012,3 +1028,51 @@ func TestCleanUpDuplicatedMC(t *testing.T) {
 	_, ok = actual[machineConfigUpgrade.Name]
 	require.True(t, ok, "expect generated-containerruntime-1 in the list, but got false")
 }
+
+func generateManagedKey(ctrcfg *mcfgv1.ContainerRuntimeConfig, generation uint64) string {
+	return fmt.Sprintf("99-%s-generated-containerruntime-%v", ctrcfg.Name, generation)
+}
+
+func TestCtrruntimeConfigMultiCreate(t *testing.T) {
+	for _, platform := range []apicfgv1.PlatformType{apicfgv1.AWSPlatformType, apicfgv1.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)
+
+			ctrcfgCount := 30
+			for i := 0; i < ctrcfgCount; 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] = ""
+
+				ccr := newContainerRuntimeConfig(poolName, &mcfgv1.ContainerRuntimeConfiguration{LogLevel: "debug"}, labelSelector)
+
+				f.mcpLister = append(f.mcpLister, mcp)
+				f.mccrLister = append(f.mccrLister, ccr)
+				f.objects = append(f.objects, ccr)
+
+				mcs := helpers.NewMachineConfig(generateManagedKey(ccr, 1), labelSelector.MatchLabels, "dummy://", []ign3types.File{{}})
+				mcsDeprecated := mcs.DeepCopy()
+				mcsDeprecated.Name = getManagedKeyCtrCfgDeprecated(mcp)
+
+				expectedPatch := fmt.Sprintf("{\"metadata\":{\"finalizers\":[\"99-%v-generated-containerruntime-1\"]}}", poolName)
+
+				f.expectGetMachineConfigAction(mcs)
+				f.expectGetMachineConfigAction(mcsDeprecated)
+				f.expectGetMachineConfigAction(mcs)
+				f.expectCreateMachineConfigAction(mcs)
+				f.expectPatchContainerRuntimeConfig(ccr, []byte(expectedPatch))
+				f.expectUpdateContainerRuntimeConfig(ccr)
+				f.run(poolName)
+			}
+		})
+	}
+}

pkg/controller/container-runtime-config/helpers.go

@@ -23,6 +23,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"
 
 	mcfgv1 "github.com/openshift/machine-config-operator/pkg/apis/machineconfiguration.openshift.io/v1"
 	ctrlcommon "github.com/openshift/machine-config-operator/pkg/controller/common"
@@ -184,33 +185,47 @@ func getManagedKeyCtrCfgDeprecated(pool *mcfgv1.MachineConfigPool) string {
 // nolint: dupl
 func getManagedKeyCtrCfg(pool *mcfgv1.MachineConfigPool, client mcfgclientset.Interface, cfg *mcfgv1.ContainerRuntimeConfig) (string, error) {
 	// Get all the ctrcfg CRs
-	ctrcfgList, err := client.MachineconfigurationV1().ContainerRuntimeConfigs().List(context.TODO(), metav1.ListOptions{})
+	ctrcfgListAll, err := client.MachineconfigurationV1().ContainerRuntimeConfigs().List(context.TODO(), metav1.ListOptions{})
 	if err != nil {
 		return "", fmt.Errorf("error listing container runtime configs: %v", err)
 	}
 	// If there is no ctrcfg in the list, return the default MC name with no suffix
-	if ctrcfgList == nil || len(ctrcfgList.Items) == 0 {
+	if ctrcfgListAll == nil || len(ctrcfgListAll.Items) == 0 {
 		return ctrlcommon.GetManagedKey(pool, client, "99", "containerruntime", getManagedKeyCtrCfgDeprecated(pool))
 	}
-	for _, ctrcfg := range ctrcfgList.Items {
-		if ctrcfg.Name == cfg.Name {
-			val, ok := ctrcfg.GetAnnotations()[ctrlcommon.MCNameSuffixAnnotationKey]
-			// If we find a matching ctrcfg and it is the only one in the list, then return the default MC name with no suffix
-			if !ok && len(ctrcfgList.Items) < 2 {
-				return ctrlcommon.GetManagedKey(pool, client, "99", "containerruntime", getManagedKeyCtrCfgDeprecated(pool))
-			}
-			// Otherwise if an MC name suffix exists, append it to the default MC name and return that as this ctrcfg exists and
-			// we are probably doing an update action on it
-			if val != "" {
-				return fmt.Sprintf("99-%s-generated-containerruntime-%s", pool.Name, val), nil
-			}
+
+	var ctrcfgList []mcfgv1.ContainerRuntimeConfig
+	for _, ctrcfg := range ctrcfgListAll.Items {
+		selector, err := metav1.LabelSelectorAsSelector(ctrcfg.Spec.MachineConfigPoolSelector)
+		if err != nil {
+			return "", fmt.Errorf("invalid label selector: %v", err)
+		}
+		if selector.Empty() || !selector.Matches(labels.Set(pool.Labels)) {
+			continue
+		}
+		ctrcfgList = append(ctrcfgList, ctrcfg)
+	}
+
+	for _, ctrcfg := range ctrcfgList {
+		if ctrcfg.Name != cfg.Name {
+			continue
+		}
+		val, ok := ctrcfg.GetAnnotations()[ctrlcommon.MCNameSuffixAnnotationKey]
+		// If we find a matching ctrcfg and it is the only one in the list, then return the default MC name with no suffix
+		if !ok && len(ctrcfgList) < 2 {
+			return ctrlcommon.GetManagedKey(pool, client, "99", "containerruntime", getManagedKeyCtrCfgDeprecated(pool))
+		}
+		// Otherwise if an MC name suffix exists, append it to the default MC name and return that as this ctrcfg exists and
+		// we are probably doing an update action on it
+		if val != "" {
+			return fmt.Sprintf("99-%s-generated-containerruntime-%s", pool.Name, val), nil
 		}
 	}
 
 	// If we are here, this means that a new ctrcfg was created, so we have to calculate the suffix value for its MC name
 	suffixNum := 0
 	// Go through the list of ctrcfg objects created and get the max suffix value currently created
-	for _, item := range ctrcfgList.Items {
+	for _, item := range ctrcfgList {
 		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