Bug: Fix SubscriptionConfig NodeSelector field

Problem: OLM's Subscription CRD allows cluster admins to set the
nodeSelector on Operators they deploy. Currently this API is exposed
but performs no action.

Solution: Wire the NodeSelector specified in the SubscriptionConfig into
the pods deployed when installing the operator.

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

@@ -1142,6 +1142,7 @@ var _ = Describe("Subscription", func() {
 				Operator: corev1.TolerationOpEqual,
 			},
 		}
+
 		podResources := corev1.ResourceRequirements{
 			Limits: corev1.ResourceList{
 				corev1.ResourceCPU: resource.MustParse("100m"),
@@ -1152,12 +1153,17 @@ var _ = Describe("Subscription", func() {
 			},
 		}
 
+		podNodeSelector := map[string]string{
+			"foo": "bar",
+		}
+
 		podConfig := v1alpha1.SubscriptionConfig{
 			Env:          podEnv,
 			Volumes:      podVolumes,
 			VolumeMounts: podVolumeMounts,
 			Tolerations:  podTolerations,
 			Resources:    podResources,
+			NodeSelector: podNodeSelector,
 		}
 
 		permissions := deploymentPermissions()
@@ -1184,7 +1190,7 @@ var _ = Describe("Subscription", func() {
 		expected := podEnv
 		expected = append(expected, proxyEnv...)
 
-		checkDeploymentWithPodConfiguration(GinkgoT(), kubeClient, csv, podConfig.Env, podConfig.Volumes, podConfig.VolumeMounts, podConfig.Tolerations, podConfig.Resources)
+		checkDeploymentWithPodConfiguration(GinkgoT(), kubeClient, csv, podConfig.Env, podConfig.Volumes, podConfig.VolumeMounts, podConfig.Tolerations, podConfig.Resources, podNodeSelector)
 	})
 
 	It("creation with dependencies", func() {
@@ -2338,7 +2344,7 @@ func createSubscriptionForCatalogWithSpec(t GinkgoTInterface, crc versioned.Inte
 	return buildSubscriptionCleanupFunc(crc, subscription)
 }
 
-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) {
+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, nodeSelector map[string]string) {
 	resolver := install.StrategyResolver{}
 
 	strategy, err := resolver.UnmarshalStrategy(csv.Spec.InstallStrategy)
@@ -2446,6 +2452,8 @@ func checkDeploymentWithPodConfiguration(t GinkgoTInterface, client operatorclie
 			require.Equalf(t, *existing, toleration, "Toleration=%v does not match expected Toleration=%v", existing, toleration)
 		}
 
+		require.Equal(t, nodeSelector, deploymentSpec.Spec.Template.Spec.NodeSelector)
+
 		for i := range deployment.Spec.Template.Spec.Containers {
 			check(&deployment.Spec.Template.Spec.Containers[i])
 		}