Bug 1860035: Fix SubscriptionConfig NodeSelector field

doc/design/subscription-config.md

@@ -16,7 +16,7 @@ The `env` field defines a list of [Environment Variables](https://kubernetes.io/
 
 Increase log verbosity on an Operator's container that utilizes the `ARGS` variable:
 
-```
+```yaml
 kind: Subscription
 metadata:
   name: prometheus
@@ -39,7 +39,7 @@ The `envFrom` field defines a [list of sources to populate Environment Variables
 
 Inject a license key residing in a Secret to unlock Operator features:
 
-```
+```yaml
 kind: Subscription
 metadata:
   name: my-operator
@@ -68,7 +68,7 @@ The `volumeMounts` field defines a list of [VolumeMounts](https://kubernetes.io/
 
 Mount a ConfigMap as a Volume that contains configuration information that can change default Operator behavior. Modifications to the content of the ConfigMap should appear within the container's VolumeMount.
 
-```
+```yaml
 kind: Subscription
 metadata:
   name: my-operator
@@ -95,7 +95,7 @@ The `tolerations` field defines a list of [Tolerations](https://kubernetes.io/do
 
 Inject toleration to tolerate all taints.
 
-```
+```yaml
 kind: Subscription
 metadata:
   name: my-operator
@@ -117,7 +117,7 @@ The `resources` field defines [Resource Constraints](https://kubernetes.io/docs/
 
 Inject a request of 0.25 cpu and 64 MiB of memory, and a limit of 0.5 cpu and 128MiB of memory in each container.
 
-```
+```yaml
 kind: Subscription
 metadata:
   name: my-operator
@@ -133,3 +133,23 @@ spec:
         memory: "128Mi"
         cpu: "500m"
 ```
+
+### NodeSelector
+
+The `nodeSelector` field defines a [NodeSelector](https://kubernetes.io/docs/concepts/scheduling-eviction/assign-pod-node/) for the Pod created by OLM.
+
+#### Example
+
+Inject `nodeSelector` key-values pairs.
+
+```yaml
+kind: Subscription
+metadata:
+  name: my-operator
+spec:
+  package: etcd
+  channel: alpha
+  config:
+    nodeSelector:
+      foo: bar
+```

pkg/controller/operators/olm/overrides/config.go

@@ -16,7 +16,7 @@ type operatorConfig struct {
 	logger *logrus.Logger
 }
 
-func (o *operatorConfig) GetConfigOverrides(ownerCSV ownerutil.Owner) (envVarOverrides []corev1.EnvVar, volumeOverrides []corev1.Volume, volumeMountOverrides []corev1.VolumeMount, tolerationOverrides []corev1.Toleration, resourcesOverride corev1.ResourceRequirements, err error) {
+func (o *operatorConfig) GetConfigOverrides(ownerCSV ownerutil.Owner) (envVarOverrides []corev1.EnvVar, volumeOverrides []corev1.Volume, volumeMountOverrides []corev1.VolumeMount, tolerationOverrides []corev1.Toleration, resourcesOverride corev1.ResourceRequirements, nodeSelectorOverride map[string]string, err error) {
 	list, listErr := o.lister.OperatorsV1alpha1().SubscriptionLister().Subscriptions(ownerCSV.GetNamespace()).List(labels.Everything())
 	if listErr != nil {
 		err = fmt.Errorf("failed to list subscription namespace=%s - %v", ownerCSV.GetNamespace(), listErr)
@@ -34,6 +34,7 @@ func (o *operatorConfig) GetConfigOverrides(ownerCSV ownerutil.Owner) (envVarOve
 	volumeMountOverrides = owner.Spec.Config.VolumeMounts
 	tolerationOverrides = owner.Spec.Config.Tolerations
 	resourcesOverride = owner.Spec.Config.Resources
+	nodeSelectorOverride = owner.Spec.Config.NodeSelector
 
 	return
 }

pkg/controller/operators/olm/overrides/initializer.go

@@ -44,7 +44,7 @@ func (d *DeploymentInitializer) initialize(ownerCSV ownerutil.Owner, deployment
 	var envVarOverrides, proxyEnvVar, merged []corev1.EnvVar
 	var err error
 
-	envVarOverrides, volumeOverrides, volumeMountOverrides, tolerationOverrides, resourcesOverride, err := d.config.GetConfigOverrides(ownerCSV)
+	envVarOverrides, volumeOverrides, volumeMountOverrides, tolerationOverrides, resourcesOverride, nodeSelectorOverride, err := d.config.GetConfigOverrides(ownerCSV)
 	if err != nil {
 		err = fmt.Errorf("failed to get subscription pod configuration - %v", err)
 		return err
@@ -87,6 +87,10 @@ func (d *DeploymentInitializer) initialize(ownerCSV ownerutil.Owner, deployment
 		return fmt.Errorf("failed to inject resources into deployment spec name=%s - %v", deployment.Name, err)
 	}
 
+	if err = InjectNodeSelectorIntoDeployment(podSpec, nodeSelectorOverride); err != nil {
+		return fmt.Errorf("failed to inject nodeSelector into deployment spec name=%s - %v", deployment.Name, err)
+	}
+
 	return nil
 }
 

pkg/controller/operators/olm/overrides/inject.go

@@ -206,3 +206,17 @@ func InjectResourcesIntoDeployment(podSpec *corev1.PodSpec, resources corev1.Res
 
 	return nil
 }
+
+// InjectNodeSelectorIntoDeployment injects the provided NodeSelector
+// into the container(s) of the given PodSpec.
+//
+// If any Container in PodSpec already defines a NodeSelector it will
+// be overwritten.
+func InjectNodeSelectorIntoDeployment(podSpec *corev1.PodSpec, nodeSelector map[string]string) error {
+	if podSpec == nil {
+		return errors.New("no pod spec provided")
+	}
+
+	podSpec.NodeSelector = nodeSelector
+	return nil
+}

pkg/controller/operators/olm/overrides/inject_test.go

@@ -57,6 +57,13 @@ var (
 			corev1.ResourceMemory: resource.MustParse("128Mi"),
 		},
 	}
+
+	defaultNodeSelector = map[string]string{
+		"all":    "your",
+		"base":   "are",
+		"belong": "to",
+		"us":     "",
+	}
 )
 
 func TestInjectVolumeMountIntoDeployment(t *testing.T) {
@@ -701,3 +708,54 @@ func TestInjectResourcesIntoDeployment(t *testing.T) {
 		})
 	}
 }
+
+func TestInjectNodeSelectorIntoDeployment(t *testing.T) {
+	tests := []struct {
+		name         string
+		podSpec      *corev1.PodSpec
+		nodeSelector map[string]string
+		expected     *corev1.PodSpec
+	}{
+		{
+			// Nil PodSpec is injected with a nodeSelector
+			// Expected: PodSpec is nil
+			name:         "WithNilPodSpec",
+			podSpec:      nil,
+			nodeSelector: map[string]string{"foo": "bar"},
+			expected:     nil,
+		},
+		{
+			// PodSpec with no NodeSelector is injected with a nodeSelector
+			// Expected: NodeSelector is empty
+			name:         "WithEmptyNodeSelector",
+			podSpec:      &corev1.PodSpec{},
+			nodeSelector: map[string]string{"foo": "bar"},
+			expected: &corev1.PodSpec{
+				NodeSelector: map[string]string{"foo": "bar"},
+			},
+		},
+		{
+			// PodSpec with an existing NodeSelector is injected with a nodeSelector
+			// Expected: Existing NodeSelector is overwritten
+			name: "WithExistingNodeSelector",
+			podSpec: &corev1.PodSpec{
+				NodeSelector: defaultNodeSelector,
+			},
+			nodeSelector: map[string]string{"foo": "bar"},
+			expected: &corev1.PodSpec{
+				NodeSelector: map[string]string{"foo": "bar"},
+			},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			overrides.InjectNodeSelectorIntoDeployment(tt.podSpec, tt.nodeSelector)
+
+			podSpecWant := tt.expected
+			podSpecGot := tt.podSpec
+
+			assert.Equal(t, podSpecWant, podSpecGot)
+		})
+	}
+}

test/e2e/subscription_e2e_test.go

@@ -40,6 +40,11 @@ func Step(level int, text string, callbacks ...func()) {
 	By(strings.Repeat(" ", level*2)+text, callbacks...)
 }
 
+const (
+	timeout  = time.Second * 20
+	interval = time.Millisecond * 100
+)
+
 var _ = By
 
 var _ = Describe("Subscription", func() {
@@ -1187,6 +1192,60 @@ var _ = Describe("Subscription", func() {
 		checkDeploymentWithPodConfiguration(GinkgoT(), kubeClient, csv, podConfig.Env, podConfig.Volumes, podConfig.VolumeMounts, podConfig.Tolerations, podConfig.Resources)
 	})
 
+	It("creation with nodeSelector config", func() {
+		kubeClient := newKubeClient()
+		crClient := newCRClient()
+
+		// Create a ConfigMap that is mounted to the operator via the subscription
+		testConfigMapName := genName("test-configmap-")
+		testConfigMap := &corev1.ConfigMap{
+			ObjectMeta: metav1.ObjectMeta{
+				Name: testConfigMapName,
+			},
+		}
+
+		_, err := kubeClient.KubernetesInterface().CoreV1().ConfigMaps(testNamespace).Create(context.Background(), testConfigMap, metav1.CreateOptions{})
+		require.NoError(GinkgoT(), err)
+		defer func() {
+			err := kubeClient.KubernetesInterface().CoreV1().ConfigMaps(testNamespace).Delete(context.Background(), testConfigMap.Name, metav1.DeleteOptions{})
+			require.NoError(GinkgoT(), err)
+		}()
+
+		// Configure the Subscription.
+		podNodeSelector := map[string]string{
+			"foo": "bar",
+		}
+
+		podConfig := v1alpha1.SubscriptionConfig{
+			NodeSelector: podNodeSelector,
+		}
+
+		permissions := deploymentPermissions()
+		catsrc, subSpec, catsrcCleanup := newCatalogSource(GinkgoT(), kubeClient, crClient, "podconfig", testNamespace, permissions)
+		defer catsrcCleanup()
+
+		// Ensure that the catalog source is resolved before we create a subscription.
+		_, err = fetchCatalogSourceOnStatus(crClient, catsrc.GetName(), testNamespace, catalogSourceRegistryPodSynced)
+		require.NoError(GinkgoT(), err)
+
+		subscriptionName := genName("podconfig-sub-")
+		subSpec.Config = podConfig
+		cleanupSubscription := createSubscriptionForCatalogWithSpec(GinkgoT(), crClient, testNamespace, subscriptionName, subSpec)
+		defer cleanupSubscription()
+
+		subscription, err := fetchSubscription(crClient, testNamespace, subscriptionName, subscriptionStateAtLatestChecker)
+		require.NoError(GinkgoT(), err)
+		require.NotNil(GinkgoT(), subscription)
+
+		csv, err := fetchCSV(crClient, subscription.Status.CurrentCSV, testNamespace, buildCSVConditionChecker(v1alpha1.CSVPhaseInstalling))
+		require.NoError(GinkgoT(), err)
+
+		Eventually(func() error {
+			return checkDeploymentHasPodConfigNodeSelector(GinkgoT(), kubeClient, csv, podNodeSelector)
+		}, timeout, interval).Should(Succeed())
+
+	})
+
 	It("creation with dependencies", func() {
 
 		kubeClient := newKubeClient()
@@ -2338,6 +2397,31 @@ func createSubscriptionForCatalogWithSpec(t GinkgoTInterface, crc versioned.Inte
 	return buildSubscriptionCleanupFunc(crc, subscription)
 }
 
+func checkDeploymentHasPodConfigNodeSelector(t GinkgoTInterface, client operatorclient.ClientInterface, csv *v1alpha1.ClusterServiceVersion, nodeSelector map[string]string) error {
+	resolver := install.StrategyResolver{}
+
+	strategy, err := resolver.UnmarshalStrategy(csv.Spec.InstallStrategy)
+	if err != nil {
+		return err
+	}
+
+	strategyDetailsDeployment, ok := strategy.(*v1alpha1.StrategyDetailsDeployment)
+	require.Truef(t, ok, "could not cast install strategy as type %T", strategyDetailsDeployment)
+
+	for _, deploymentSpec := range strategyDetailsDeployment.DeploymentSpecs {
+		deployment, err := client.KubernetesInterface().AppsV1().Deployments(csv.GetNamespace()).Get(context.Background(), deploymentSpec.Name, metav1.GetOptions{})
+		if err != nil {
+			return err
+		}
+
+		isEqual := reflect.DeepEqual(nodeSelector, deployment.Spec.Template.Spec.NodeSelector)
+		if !isEqual {
+			err = fmt.Errorf("actual nodeSelector=%v does not match expected nodeSelector=%v", deploymentSpec.Spec.Template.Spec.NodeSelector, nodeSelector)
+		}
+	}
+	return nil
+}
+
 func checkDeploymentWithPodConfiguration(t GinkgoTInterface, client operatorclient.ClientInterface, csv *v1alpha1.ClusterServiceVersion, envVar []corev1.EnvVar, volumes []corev1.Volume, volumeMounts []corev1.VolumeMount, tolerations []corev1.Toleration, resources corev1.ResourceRequirements) {
 	resolver := install.StrategyResolver{}