Backport fix for infinite reconcile loop (#205)

* Add observedGeneration to status to avoid infinite reconcile loop Indirectly fixes #185 * Tests must now refresh to get the correct observedGeneration value
knative · Jul 15, 2020 · e73aa5e · e73aa5e
1 parent 97240b1
commit e73aa5e
Show file tree

Hide file tree

Showing 3 changed files with 31 additions and 21 deletions.
diff --git a/config/300-eventing.yaml b/config/300-eventing.yaml
@@ -135,6 +135,9 @@ spec:
           type: object
         status:
           properties:
+            observedGeneration:
+              description: The generation last processed by the controller
+              type: integer
             conditions:
               description: The latest available observations of a resource's current
                 state.

diff --git a/config/300-serving.yaml b/config/300-serving.yaml
@@ -172,6 +172,9 @@ spec:
         status:
           description: Status defines the observed state of KnativeServing
           properties:
+            observedGeneration:
+              description: The generation last processed by the controller
+              type: integer
             conditions:
               description: The latest available observations of a resource's current
                 state.

diff --git a/test/resources/verify.go b/test/resources/verify.go
@@ -55,23 +55,22 @@ func KSOperatorCRVerifyConfiguration(t *testing.T, clients *test.Clients, names
 	ks.Spec = getTestKSOperatorCRSpec()
 
 	// verify the default config map
-	ks = verifyDefaultConfig(t, ks, defaultsConfigMapName, clients, names)
+	verifyDefaultConfig(t, ks, defaultsConfigMapName, clients, names)
 
 	// verify the logging config map
-	verifyLoggingConfig(t, ks, loggingConfigMapName, clients, names)
+	verifyLoggingConfig(t, loggingConfigMapName, clients, names)
 
 	// Delete a single key/value pair
-	ks = verifySingleKeyDeletion(t, ks, LoggingConfigKey, loggingConfigMapName, clients, names)
+	verifySingleKeyDeletion(t, LoggingConfigKey, loggingConfigMapName, clients, names)
 
 	// Use an empty map as the value
-	ks = verifyEmptyKey(t, ks, DefaultsConfigKey, defaultsConfigMapName, clients, names)
+	verifyEmptyKey(t, DefaultsConfigKey, defaultsConfigMapName, clients, names)
 
 	// Now remove the config from the spec and update
-	verifyEmptySpec(t, ks, loggingConfigMapName, clients, names)
+	verifyEmptySpec(t, loggingConfigMapName, clients, names)
 }
 
-func verifyDefaultConfig(t *testing.T, ks *v1alpha1.KnativeServing, defaultsConfigMapName string, clients *test.Clients,
-	names test.ResourceNames) *v1alpha1.KnativeServing {
+func verifyDefaultConfig(t *testing.T, ks *v1alpha1.KnativeServing, defaultsConfigMapName string, clients *test.Clients, names test.ResourceNames) {
 	ks, err := clients.KnativeServing().Update(ks)
 	if err != nil {
 		t.Fatalf("KnativeServing %q failed to update: %v", names.KnativeServing, err)
@@ -83,11 +82,9 @@ func verifyDefaultConfig(t *testing.T, ks *v1alpha1.KnativeServing, defaultsConf
 	if err != nil {
 		t.Fatalf("The operator failed to update %s configmap", defaultsConfigMapName)
 	}
-	return ks
 }
 
-func verifyLoggingConfig(t *testing.T, ks *v1alpha1.KnativeServing, loggingConfigMapName string, clients *test.Clients,
-	names test.ResourceNames) {
+func verifyLoggingConfig(t *testing.T, loggingConfigMapName string, clients *test.Clients, names test.ResourceNames) {
 	err := WaitForConfigMap(loggingConfigMapName, clients.KubeClient.Kube, func(m map[string]string) bool {
 		return m["loglevel.controller"] == "debug" && m["loglevel.autoscaler"] == "debug"
 	})
@@ -96,10 +93,13 @@ func verifyLoggingConfig(t *testing.T, ks *v1alpha1.KnativeServing, loggingConfi
 	}
 }
 
-func verifySingleKeyDeletion(t *testing.T, ks *v1alpha1.KnativeServing, loggingConfigKey string,
-	loggingConfigMapName string, clients *test.Clients, names test.ResourceNames) *v1alpha1.KnativeServing {
+func verifySingleKeyDeletion(t *testing.T, loggingConfigKey string, loggingConfigMapName string, clients *test.Clients, names test.ResourceNames) {
+	ks, err := clients.KnativeServing().Get(names.KnativeServing, metav1.GetOptions{})
+	if err != nil || ks.Spec.Config[loggingConfigKey]["loglevel.autoscaler"] == "" {
+		t.Fatalf("Existing KS operator CR lacks proper key: %v", ks.Spec.Config)
+	}
 	delete(ks.Spec.Config[loggingConfigKey], "loglevel.autoscaler")
-	ks, err := clients.KnativeServing().Update(ks)
+	ks, err = clients.KnativeServing().Update(ks)
 	if err != nil {
 		t.Fatalf("KnativeServing %q failed to update: %v", names.KnativeServing, err)
 	}
@@ -112,13 +112,15 @@ func verifySingleKeyDeletion(t *testing.T, ks *v1alpha1.KnativeServing, loggingC
 	if err != nil {
 		t.Fatalf("The operator failed to update %s configmap", loggingConfigMapName)
 	}
-	return ks
 }
 
-func verifyEmptyKey(t *testing.T, ks *v1alpha1.KnativeServing, defaultsConfigKey string,
-	defaultsConfigMapName string, clients *test.Clients, names test.ResourceNames) *v1alpha1.KnativeServing {
+func verifyEmptyKey(t *testing.T, defaultsConfigKey string, defaultsConfigMapName string, clients *test.Clients, names test.ResourceNames) {
+	ks, err := clients.KnativeServing().Get(names.KnativeServing, metav1.GetOptions{})
+	if err != nil {
+		t.Fatalf("Existing KS operator CR gone: %s", names.KnativeServing)
+	}
 	ks.Spec.Config[defaultsConfigKey] = map[string]string{}
-	ks, err := clients.KnativeServing().Update(ks)
+	ks, err = clients.KnativeServing().Update(ks)
 	if err != nil {
 		t.Fatalf("KnativeServing %q failed to update: %v", names.KnativeServing, err)
 	}
@@ -130,16 +132,18 @@ func verifyEmptyKey(t *testing.T, ks *v1alpha1.KnativeServing, defaultsConfigKey
 	if err != nil {
 		t.Fatalf("The operator failed to update %s configmap", defaultsConfigMapName)
 	}
-	return ks
 }
 
-func verifyEmptySpec(t *testing.T, ks *v1alpha1.KnativeServing, loggingConfigMapName string, clients *test.Clients,
-	names test.ResourceNames) {
+func verifyEmptySpec(t *testing.T, loggingConfigMapName string, clients *test.Clients, names test.ResourceNames) {
+	ks, err := clients.KnativeServing().Get(names.KnativeServing, metav1.GetOptions{})
+	if err != nil {
+		t.Fatalf("Existing KS operator CR gone: %s", names.KnativeServing)
+	}
 	ks.Spec = v1alpha1.KnativeServingSpec{}
 	if _, err := clients.KnativeServing().Update(ks); err != nil {
 		t.Fatalf("KnativeServing %q failed to update: %v", names.KnativeServing, err)
 	}
-	err := WaitForConfigMap(loggingConfigMapName, clients.KubeClient.Kube, func(m map[string]string) bool {
+	err = WaitForConfigMap(loggingConfigMapName, clients.KubeClient.Kube, func(m map[string]string) bool {
 		_, exists := m["loglevel.controller"]
 		return !exists
 	})