Modify and improve unit test on deployment

Signed-off-by: Vu Dinh <[email protected]>
operator-framework · Apr 13, 2019 · a52214c · a52214c
1 parent cd6bde3
commit a52214c
Show file tree

Hide file tree

Showing 5 changed files with 101 additions and 27 deletions.
diff --git a/pkg/controller/install/deployment.go b/pkg/controller/install/deployment.go
@@ -139,9 +139,15 @@ func (i *StrategyDeploymentInstaller) checkForDeployments(deploymentSpecs []Stra
 		depNames = append(depNames, dep.Name)
 	}
 
-	existingDeployments, err := i.strategyClient.FindAnyDeploymentsMatchingNames(depNames)
+	// Check the owner is a CSV
+	csv, ok := i.owner.(*v1alpha1.ClusterServiceVersion)
+	if !ok {
+		return StrategyError{Reason: StrategyErrReasonComponentMissing, Message: fmt.Sprintf("owner %s is not a CSV", i.owner.GetName())}
+	}
+
+	existingDeployments, err := i.strategyClient.FindAnyDeploymentsMatchingLabels(ownerutil.CSVOwnerSelector(csv))
 	if err != nil {
-		return StrategyError{Reason: StrategyErrReasonComponentMissing, Message: fmt.Sprintf("error querying for %s: %s", depNames, err)}
+		return StrategyError{Reason: StrategyErrReasonComponentMissing, Message: fmt.Sprintf("error querying existing deployments for CSV %s: %s", csv.GetName(), err)}
 	}
 
 	// compare deployments to see if any need to be created/updated

diff --git a/pkg/controller/install/deployment_test.go b/pkg/controller/install/deployment_test.go
@@ -286,6 +286,8 @@ func TestInstallStrategyDeploymentCheckInstallErrors(t *testing.T) {
 		},
 	}
 
+	mockOwnerLabel := ownerutil.CSVOwnerSelector(&mockOwner)
+
 	tests := []struct {
 		createDeploymentErr error
 		description         string
@@ -304,13 +306,13 @@ func TestInstallStrategyDeploymentCheckInstallErrors(t *testing.T) {
 
 			dep := testDeployment("olm-dep-1", namespace, &mockOwner)
 			dep.Spec.Template.SetAnnotations(map[string]string{"test": "annotation"})
-			fakeClient.FindAnyDeploymentsMatchingNamesReturns(
+			fakeClient.FindAnyDeploymentsMatchingLabelsReturns(
 				[]*appsv1.Deployment{
 					&dep,
 				}, nil,
 			)
 			defer func() {
-				require.Equal(t, []string{dep.Name}, fakeClient.FindAnyDeploymentsMatchingNamesArgsForCall(0))
+				require.Equal(t, mockOwnerLabel, fakeClient.FindAnyDeploymentsMatchingLabelsArgsForCall(0))
 			}()
 
 			installed, err := installer.CheckInstalled(strategy)

diff --git a/pkg/controller/operators/catalog/operator_test.go b/pkg/controller/operators/catalog/operator_test.go
@@ -204,7 +204,6 @@ func TestExecutePlan(t *testing.T) {
 				}
 
 				require.NoError(t, err, "couldn't fetch %s %v", namespace, obj)
-				fmt.Printf("fetched: %v", fetched)
 				require.EqualValues(t, obj, fetched)
 			}
 		})

diff --git a/pkg/controller/operators/olm/operator_test.go b/pkg/controller/operators/olm/operator_test.go
@@ -1081,7 +1081,10 @@ func TestTransitionCSV(t *testing.T) {
 					crd("c1", "v1", "g1"),
 				},
 				objs: []runtime.Object{
-					deployment("csv1-dep1", namespace, "sa", defaultTemplateAnnotations),
+					withLabels(
+						deployment("csv1-dep1", namespace, "sa", defaultTemplateAnnotations),
+						ownerLabelFromCSV("csv1", namespace),
+					),
 				},
 			},
 			expected: expected{
@@ -1122,9 +1125,12 @@ func TestTransitionCSV(t *testing.T) {
 				clientObjs: []runtime.Object{addAnnotation(defaultOperatorGroup, v1.OperatorGroupProvidedAPIsAnnotationKey, "a1Kind.v1.a1")},
 				apis:       []runtime.Object{apiService("a1", "v1", "v1-a1", namespace, "", validCAPEM, apiregistrationv1.ConditionTrue, ownerLabelFromCSV("csv1", namespace))},
 				objs: []runtime.Object{
-					deployment("a1", namespace, "sa", addAnnotations(defaultTemplateAnnotations, map[string]string{
-						OLMCAHashAnnotationKey: validCAHash,
-					})),
+					withLabels(
+						deployment("a1", namespace, "sa", addAnnotations(defaultTemplateAnnotations, map[string]string{
+							OLMCAHashAnnotationKey: validCAHash,
+						})),
+						ownerLabelFromCSV("csv1", namespace),
+					),
 					withAnnotations(keyPairToTLSSecret("v1.a1-cert", namespace, signedServingPair(time.Now().Add(24*time.Hour), validCA, []string{"v1-a1.ns", "v1-a1.ns.svc"})), map[string]string{
 						OLMCAHashAnnotationKey: validCAHash,
 					}),
@@ -2195,7 +2201,10 @@ func TestTransitionCSV(t *testing.T) {
 					crd("c1", "v1", "g1"),
 				},
 				objs: []runtime.Object{
-					deployment("csv1-dep1", namespace, "sa", defaultTemplateAnnotations),
+					withLabels(
+						deployment("csv1-dep1", namespace, "sa", defaultTemplateAnnotations),
+						ownerLabelFromCSV("csv1", namespace),
+					),
 					deployment("extra-dep", namespace, "sa", nil),
 				},
 			},
@@ -2335,8 +2344,14 @@ func TestTransitionCSV(t *testing.T) {
 					crd("c1", "v1", "g1"),
 				},
 				objs: []runtime.Object{
-					deployment("csv1-dep1", namespace, "sa", defaultTemplateAnnotations),
-					deployment("csv2-dep1", namespace, "sa", defaultTemplateAnnotations),
+					withLabels(
+						deployment("csv1-dep1", namespace, "sa", defaultTemplateAnnotations),
+						ownerLabelFromCSV("csv1", namespace),
+					),
+					withLabels(
+						deployment("csv2-dep1", namespace, "sa", defaultTemplateAnnotations),
+						ownerLabelFromCSV("csv2", namespace),
+					),
 				},
 			},
 			expected: expected{
@@ -2374,8 +2389,14 @@ func TestTransitionCSV(t *testing.T) {
 					crd("c1", "v1", "g1"),
 				},
 				objs: []runtime.Object{
-					deployment("csv1-dep1", namespace, "sa", defaultTemplateAnnotations),
-					deployment("csv2-dep1", namespace, "sa", defaultTemplateAnnotations),
+					withLabels(
+						deployment("csv1-dep1", namespace, "sa", defaultTemplateAnnotations),
+						ownerLabelFromCSV("csv1", namespace),
+					),
+					withLabels(
+						deployment("csv2-dep1", namespace, "sa", defaultTemplateAnnotations),
+						ownerLabelFromCSV("csv2", namespace),
+					),
 				},
 			},
 			expected: expected{
@@ -2429,9 +2450,18 @@ func TestTransitionCSV(t *testing.T) {
 					crd("c1", "v1", "g1"),
 				},
 				objs: []runtime.Object{
-					deployment("csv1-dep1", namespace, "sa", defaultTemplateAnnotations),
-					deployment("csv2-dep1", namespace, "sa", defaultTemplateAnnotations),
-					deployment("csv3-dep1", namespace, "sa", defaultTemplateAnnotations),
+					withLabels(
+						deployment("csv1-dep1", namespace, "sa", defaultTemplateAnnotations),
+						ownerLabelFromCSV("csv1", namespace),
+					),
+					withLabels(
+						deployment("csv2-dep1", namespace, "sa", defaultTemplateAnnotations),
+						ownerLabelFromCSV("csv2", namespace),
+					),
+					withLabels(
+						deployment("csv3-dep1", namespace, "sa", defaultTemplateAnnotations),
+						ownerLabelFromCSV("csv3", namespace),
+					),
 				},
 			},
 			expected: expected{
@@ -2479,9 +2509,18 @@ func TestTransitionCSV(t *testing.T) {
 					crd("c1", "v1", "g1"),
 				},
 				objs: []runtime.Object{
-					deployment("csv1-dep1", namespace, "sa", defaultTemplateAnnotations),
-					deployment("csv2-dep1", namespace, "sa", defaultTemplateAnnotations),
-					deployment("csv3-dep1", namespace, "sa", defaultTemplateAnnotations),
+					withLabels(
+						deployment("csv1-dep1", namespace, "sa", defaultTemplateAnnotations),
+						ownerLabelFromCSV("csv1", namespace),
+					),
+					withLabels(
+						deployment("csv2-dep1", namespace, "sa", defaultTemplateAnnotations),
+						ownerLabelFromCSV("csv2", namespace),
+					),
+					withLabels(
+						deployment("csv3-dep1", namespace, "sa", defaultTemplateAnnotations),
+						ownerLabelFromCSV("csv3", namespace),
+					),
 				},
 			},
 			expected: expected{
@@ -2520,12 +2559,19 @@ func TestTransitionCSV(t *testing.T) {
 					crd("c1", "v1", "g1"),
 				},
 				objs: []runtime.Object{
-					deployment("csv2-dep1", namespace, "sa", defaultTemplateAnnotations),
-					deployment("csv3-dep1", namespace, "sa", defaultTemplateAnnotations),
+					withLabels(
+						deployment("csv2-dep1", namespace, "sa", defaultTemplateAnnotations),
+						ownerLabelFromCSV("csv2", namespace),
+					),
+					withLabels(
+						deployment("csv3-dep1", namespace, "sa", defaultTemplateAnnotations),
+						ownerLabelFromCSV("csv3", namespace),
+					),
 				},
 			},
 			expected: expected{
 				csvStates: map[string]csvState{
+
 					"csv1": {exists: false, phase: v1alpha1.CSVPhaseNone},
 					"csv2": {exists: true, phase: v1alpha1.CSVPhaseDeleting},
 					"csv3": {exists: true, phase: v1alpha1.CSVPhaseSucceeded},
@@ -2560,8 +2606,14 @@ func TestTransitionCSV(t *testing.T) {
 					crd("c1", "v1", "g1"),
 				},
 				objs: []runtime.Object{
-					deployment("csv2-dep1", namespace, "sa", defaultTemplateAnnotations),
-					deployment("csv3-dep1", namespace, "sa", defaultTemplateAnnotations),
+					withLabels(
+						deployment("csv2-dep1", namespace, "sa", defaultTemplateAnnotations),
+						ownerLabelFromCSV("csv2", namespace),
+					),
+					withLabels(
+						deployment("csv3-dep1", namespace, "sa", defaultTemplateAnnotations),
+						ownerLabelFromCSV("csv3", namespace),
+					),
 				},
 			},
 			expected: expected{

diff --git a/test/e2e/csv_e2e_test.go b/test/e2e/csv_e2e_test.go
@@ -2590,6 +2590,10 @@ func TestUpdateCSVModifyDeploymentName(t *testing.T) {
 				Name: genName("dep-"),
 				Spec: newNginxDeployment(genName("nginx-")),
 			},
+			{
+				Name: "dep2-test",
+				Spec: newNginxDeployment("nginx2"),
+			},
 		},
 	}
 	strategyRaw, err := json.Marshal(strategy)
@@ -2648,17 +2652,24 @@ func TestUpdateCSVModifyDeploymentName(t *testing.T) {
 	_, err = fetchCSV(t, crc, csv.Name, testNamespace, csvSucceededChecker)
 	require.NoError(t, err)
 
-	// Should have created deployment
+	// Should have created deployments
 	dep, err := c.GetDeployment(testNamespace, strategy.DeploymentSpecs[0].Name)
 	require.NoError(t, err)
 	require.NotNil(t, dep)
+	dep2, err := c.GetDeployment(testNamespace, strategy.DeploymentSpecs[1].Name)
+	require.NoError(t, err)
+	require.NotNil(t, dep2)
 
 	// Create "updated" CSV
 	strategyNew := install.StrategyDetailsDeployment{
 		DeploymentSpecs: []install.StrategyDeploymentSpec{
 			{
-				Name: genName("dep2-"),
-				Spec: newNginxDeployment(genName("nginx-")),
+				Name: genName("dep3-"),
+				Spec: newNginxDeployment(genName("nginx3-")),
+			},
+			{
+				Name: "dep2-test",
+				Spec: newNginxDeployment("nginx2"),
 			},
 		},
 	}
@@ -2684,6 +2695,10 @@ func TestUpdateCSVModifyDeploymentName(t *testing.T) {
 	depNew, err := c.GetDeployment(testNamespace, strategyNew.DeploymentSpecs[0].Name)
 	require.NoError(t, err)
 	require.NotNil(t, depNew)
+	// Make sure the unchanged deployment still exists
+	depNew2, err := c.GetDeployment(testNamespace, strategyNew.DeploymentSpecs[1].Name)
+	require.NoError(t, err)
+	require.NotNil(t, depNew2)
 	err = waitForDeploymentToDelete(t, c, strategy.DeploymentSpecs[0].Name)
 	require.NoError(t, err)
 }
-Original file line number
+Diff line change
@@ Expand Up / @@ -204,7 +204,6 @@ func TestExecutePlan(t *testing.T) { @@
     				}
     				require.NoError(t, err, "couldn't fetch %s %v", namespace, obj)
-    				fmt.Printf("fetched: %v", fetched)
     				require.EqualValues(t, obj, fetched)
     			}
     		})
@@ Expand Down @@