Merge pull request #2054 from dmvolod/issue-1522

Set labels for the operator Deployment created via the ClusterServiceVersion

Author: openshift-merge-robot

doc/design/building-your-csv.md

@@ -314,13 +314,18 @@ Ensure that the `serviceAccountName` used in the `deployment` spec matches one o
 
 Multiple Roles should be described to reduce the scope of any actions needed containers that the Operator may run on the cluster. For example, if you have a component that generates a TLS Secret upon start up, a Role that allows `create` but not `list` on Secrets is more secure than using a single all-powerful Service Account.
 
+It is also possible to set custom labels for Deployment in addition to the system labels set by the OLM. This labels should be present in the `labels` fields of the `deployments` section.  
+
 Here’s a full example:
 
 ```yaml
   install:
     spec:
       deployments:
         - name: example-operator
+          labels:
+            application: example-operator
+            technology: general
           spec:
             replicas: 1
             selector:

pkg/controller/install/deployment.go

@@ -8,7 +8,9 @@ import (
 	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
 	k8serrors "k8s.io/apimachinery/pkg/api/errors"
+	k8slabels "k8s.io/apimachinery/pkg/labels"
 	"k8s.io/apimachinery/pkg/util/rand"
+	"k8s.io/utils/pointer"
 
 	"github.com/operator-framework/api/pkg/operators/v1alpha1"
 	"github.com/operator-framework/operator-lifecycle-manager/pkg/api/wrappers"
@@ -87,7 +89,7 @@ func NewStrategyDeploymentInstaller(strategyClient wrappers.InstallStrategyDeplo
 
 func (i *StrategyDeploymentInstaller) installDeployments(deps []v1alpha1.StrategyDeploymentSpec) error {
 	for _, d := range deps {
-		deployment, _, err := i.deploymentForSpec(d.Name, d.Spec)
+		deployment, _, err := i.deploymentForSpec(d.Name, d.Spec, d.Label)
 		if err != nil {
 			return err
 		}
@@ -96,14 +98,14 @@ func (i *StrategyDeploymentInstaller) installDeployments(deps []v1alpha1.Strateg
 			return err
 		}
 
-		if err := i.createOrUpdateCertResourcesForDeployment(deployment.GetName()); err != nil {
+		if err := i.createOrUpdateCertResourcesForDeployment(); err != nil {
 			return err
 		}
 	}
 	return nil
 }
 
-func (i *StrategyDeploymentInstaller) createOrUpdateCertResourcesForDeployment(deploymentName string) error {
+func (i *StrategyDeploymentInstaller) createOrUpdateCertResourcesForDeployment() error {
 	for _, desc := range i.getCertResources() {
 		switch d := desc.(type) {
 		case *apiServiceDescriptionsWithCAPEM:
@@ -123,13 +125,13 @@ func (i *StrategyDeploymentInstaller) createOrUpdateCertResourcesForDeployment(d
 				return err
 			}
 		default:
-			return fmt.Errorf("Unsupported CA Resource")
+			return fmt.Errorf("unsupported CA Resource")
 		}
 	}
 	return nil
 }
 
-func (i *StrategyDeploymentInstaller) deploymentForSpec(name string, spec appsv1.DeploymentSpec) (deployment *appsv1.Deployment, hash string, err error) {
+func (i *StrategyDeploymentInstaller) deploymentForSpec(name string, spec appsv1.DeploymentSpec, specLabels k8slabels.Set) (deployment *appsv1.Deployment, hash string, err error) {
 	dep := &appsv1.Deployment{Spec: spec}
 	dep.SetName(name)
 	dep.SetNamespace(i.owner.GetNamespace())
@@ -144,6 +146,9 @@ func (i *StrategyDeploymentInstaller) deploymentForSpec(name string, spec appsv1
 	}
 	dep.Spec.Template.SetAnnotations(annotations)
 
+	// Set custom labels before CSV owner labels
+	dep.SetLabels(specLabels)
+
 	ownerutil.AddNonBlockingOwner(dep, i.owner)
 	ownerutil.AddOwnerLabelsForKind(dep, i.owner, v1alpha1.ClusterServiceVersionKind)
 
@@ -153,17 +158,19 @@ func (i *StrategyDeploymentInstaller) deploymentForSpec(name string, spec appsv1
 	}
 
 	podSpec := &dep.Spec.Template.Spec
-	inject.InjectEnvIntoDeployment(podSpec, []corev1.EnvVar{{
+	if injectErr := inject.InjectEnvIntoDeployment(podSpec, []corev1.EnvVar{{
 		Name:  "OPERATOR_CONDITION_NAME",
 		Value: i.owner.GetName(),
-	}})
+	}}); injectErr != nil {
+		err = injectErr
+		return
+	}
 
 	// OLM does not support Rollbacks.
 	// By default, each deployment created by OLM could spawn up to 10 replicaSets.
 	// By setting the deployments revisionHistoryLimit to 1, OLM will only create up
 	// to 2 ReplicaSets per deployment it manages, saving memory.
-	revisionHistoryLimit := int32(1)
-	dep.Spec.RevisionHistoryLimit = &revisionHistoryLimit
+	dep.Spec.RevisionHistoryLimit = pointer.Int32Ptr(1)
 
 	hash = HashDeploymentSpec(dep.Spec)
 	dep.Labels[DeploymentSpecHashLabelKey] = hash
@@ -286,7 +293,7 @@ func (i *StrategyDeploymentInstaller) checkForDeployments(deploymentSpecs []v1al
 			return StrategyError{Reason: StrategyErrDeploymentUpdated, Message: fmt.Sprintf("deployment doesn't have a spec hash, update it")}
 		}
 
-		_, calculatedDeploymentHash, err := i.deploymentForSpec(spec.Name, spec.Spec)
+		_, calculatedDeploymentHash, err := i.deploymentForSpec(spec.Name, spec.Spec, labels)
 		if err != nil {
 			return StrategyError{Reason: StrategyErrDeploymentUpdated, Message: fmt.Sprintf("couldn't calculate deployment spec hash: %v", err)}
 		}

pkg/controller/install/deployment_test.go

@@ -4,16 +4,17 @@ import (
 	"fmt"
 	"testing"
 
-	"github.com/operator-framework/operator-lifecycle-manager/pkg/lib/kubernetes/pkg/util/labels"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
 	rbacv1 "k8s.io/api/rbac/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	k8slabels "k8s.io/apimachinery/pkg/labels"
 
 	"github.com/operator-framework/api/pkg/operators/v1alpha1"
 	clientfakes "github.com/operator-framework/operator-lifecycle-manager/pkg/api/wrappers/wrappersfakes"
+	"github.com/operator-framework/operator-lifecycle-manager/pkg/lib/kubernetes/pkg/util/labels"
 	"github.com/operator-framework/operator-lifecycle-manager/pkg/lib/ownerutil"
 )
 
@@ -142,6 +143,11 @@ func TestInstallStrategyDeploymentInstallDeployments(t *testing.T) {
 						Name: "test-deployment-3",
 						Spec: appsv1.DeploymentSpec{},
 					},
+					{
+						Name:  "test-deployment-4",
+						Spec:  appsv1.DeploymentSpec{},
+						Label: k8slabels.Set{"custom-label": "custom-label-value"},
+					},
 				},
 			},
 			setup: setup{
@@ -228,6 +234,29 @@ func TestInstallStrategyDeploymentInstallDeployments(t *testing.T) {
 					},
 					returnError: nil,
 				},
+				{
+					expectedDeployment: appsv1.Deployment{
+						ObjectMeta: metav1.ObjectMeta{
+							Name:            "test-deployment-4",
+							Namespace:       mockOwner.GetNamespace(),
+							OwnerReferences: mockOwnerRefs,
+							Labels: map[string]string{
+								"olm.owner":           mockOwner.GetName(),
+								"olm.owner.namespace": mockOwner.GetNamespace(),
+								"custom-label":        "custom-label-value",
+							},
+						},
+						Spec: appsv1.DeploymentSpec{
+							RevisionHistoryLimit: &expectedRevisionHistoryLimit,
+							Template: corev1.PodTemplateSpec{
+								ObjectMeta: metav1.ObjectMeta{
+									Annotations: map[string]string{},
+								},
+							},
+						},
+					},
+					returnError: nil,
+				},
 			},
 			output: nil,
 		},
@@ -243,6 +272,10 @@ func TestInstallStrategyDeploymentInstallDeployments(t *testing.T) {
 					dep := fakeClient.CreateOrUpdateDeploymentArgsForCall(i)
 					expectedDeployment.Spec.Template.Annotations = map[string]string{}
 					require.Equal(t, expectedDeployment.OwnerReferences, dep.OwnerReferences)
+					for labelKey, labelValue := range expectedDeployment.Labels {
+						require.Contains(t, dep.GetLabels(), labelKey)
+						require.Equal(t, dep.Labels[labelKey], labelValue)
+					}
 					require.Equal(t, expectedDeployment.Spec.RevisionHistoryLimit, dep.Spec.RevisionHistoryLimit)
 				}(i, m.expectedDeployment)
 			}

test/e2e/csv_e2e_test.go

@@ -17,6 +17,7 @@ import (
 	"k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1"
 	k8serrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	k8slabels "k8s.io/apimachinery/pkg/labels"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/util/intstr"
 	"k8s.io/apimachinery/pkg/util/wait"
@@ -1798,6 +1799,78 @@ var _ = Describe("ClusterServiceVersion", func() {
 		Expect(annotations["foo2"]).Should(Equal("bar2"))
 		Expect(annotations["foo3"]).Should(Equal("bar3"))
 	})
+	It("Set labels for the Deployment created via the ClusterServiceVersion", func() {
+		// Create a StrategyDetailsDeployment that defines labels for Deployment inside
+		nginxName := genName("nginx-")
+		strategy := v1alpha1.StrategyDetailsDeployment{
+			DeploymentSpecs: []v1alpha1.StrategyDeploymentSpec{
+				{
+					Name: genName("dep-"),
+					Spec: newNginxDeployment(nginxName),
+					Label: k8slabels.Set{
+						"application":      "nginx",
+						"application.type": "proxy",
+					},
+				},
+			},
+		}
+
+		csv := v1alpha1.ClusterServiceVersion{
+			TypeMeta: metav1.TypeMeta{
+				Kind:       v1alpha1.ClusterServiceVersionKind,
+				APIVersion: v1alpha1.ClusterServiceVersionAPIVersion,
+			},
+			ObjectMeta: metav1.ObjectMeta{
+				Name: genName("csv"),
+			},
+			Spec: v1alpha1.ClusterServiceVersionSpec{
+				MinKubeVersion: "0.0.0",
+				InstallModes: []v1alpha1.InstallMode{
+					{
+						Type:      v1alpha1.InstallModeTypeOwnNamespace,
+						Supported: true,
+					},
+					{
+						Type:      v1alpha1.InstallModeTypeSingleNamespace,
+						Supported: true,
+					},
+					{
+						Type:      v1alpha1.InstallModeTypeMultiNamespace,
+						Supported: true,
+					},
+					{
+						Type:      v1alpha1.InstallModeTypeAllNamespaces,
+						Supported: true,
+					},
+				},
+				InstallStrategy: v1alpha1.NamedInstallStrategy{
+					StrategyName: v1alpha1.InstallStrategyNameDeployment,
+					StrategySpec: strategy,
+				},
+			},
+		}
+
+		// Create the CSV and make sure to clean it up
+		cleanupCSV, err := createCSV(c, crc, csv, testNamespace, false, false)
+		Expect(err).ShouldNot(HaveOccurred())
+		defer cleanupCSV()
+
+		// Wait for current CSV to succeed
+		_, err = fetchCSV(crc, csv.Name, testNamespace, csvSucceededChecker)
+		Expect(err).ShouldNot(HaveOccurred())
+
+		// Should have created deployment
+		dep, err := c.GetDeployment(testNamespace, strategy.DeploymentSpecs[0].Name)
+		Expect(err).ShouldNot(HaveOccurred())
+		Expect(dep).ShouldNot(BeNil())
+
+		// Make sure that the deployment labels are correct
+		labels := dep.GetLabels()
+		Expect(labels["olm.owner"]).Should(Equal(csv.GetName()))
+		Expect(labels["olm.owner.namespace"]).Should(Equal(testNamespace))
+		Expect(labels["application"]).Should(Equal("nginx"))
+		Expect(labels["application.type"]).Should(Equal("proxy"))
+	})
 	It("update same deployment name", func() {
 
 		// Create dependency first (CRD)