Stabilize the testcases (#2)

* Use patch instead of update when tests patch policies

This prevents race conditions of the different controllers updating
the policy when the test tries to update the remediation action.

Signed-off-by: mprahl <[email protected]>

* Print the events when case3 fails for easier to debugging

Signed-off-by: mprahl <[email protected]>

* Handle when the template sync encounters a conflict

When there is a conflict, the reconcile should be retried rather than
emit a policy violation. This will make testcase 3 more reliable.

Signed-off-by: mprahl <[email protected]>

* Make testcase 9 more reliable

This checks just the remediationAction rather than the whole spec in
certain sections since that is what is being verified has changed.

Signed-off-by: mprahl <[email protected]>

Signed-off-by: mprahl <[email protected]>

Author: mprahl

controllers/templatesync/template_sync.go

@@ -303,6 +303,12 @@ func (r *PolicyReconciler) Reconcile(ctx context.Context, request reconcile.Requ
 
 			_, err = res.Update(ctx, eObject, metav1.UpdateOptions{})
 			if err != nil {
+				// If the policy template retrieved from the cache has since changed, there will be a conflict error
+				// and the reconcile should be retried since this is recoverable.
+				if errors.IsConflict(err) {
+					return reconcile.Result{}, err
+				}
+
 				resultError = err
 				errMsg := fmt.Sprintf("Failed to update policy template %s: %s", tName, err)
 

test/e2e/case1_mutation_recovery_test.go

@@ -47,11 +47,7 @@ var _ = Describe("Test mutation recovery", func() {
 				managedPlc := utils.GetWithTimeout(
 					clientManagedDynamic, gvrPolicy, case1PolicyName, clusterNamespace, true, defaultTimeoutSeconds,
 				)
-				Expect(managedPlc.Object["spec"].(map[string]interface{})["remediationAction"]).To(Equal("inform"))
-				managedPlc.Object["spec"].(map[string]interface{})["remediationAction"] = "enforce"
-				_, err := clientManagedDynamic.Resource(gvrPolicy).Namespace(clusterNamespace).Update(
-					context.TODO(), managedPlc, metav1.UpdateOptions{},
-				)
+				_, err := patchRemediationAction(clientManagedDynamic, managedPlc, "enforce")
 
 				return err
 			},

test/e2e/case3_multiple_templates_test.go

@@ -47,6 +47,12 @@ var _ = Describe("Test status sync with multiple templates", func() {
 		Expect(hubPlc).NotTo(BeNil())
 	})
 	AfterEach(func() {
+		if CurrentSpecReport().Failed() {
+			_, err := utils.KubectlWithOutput("-n", clusterNamespace, "get", "events")
+
+			Expect(err).To(BeNil())
+		}
+
 		By("Deleting a policy on hub cluster in ns:" + clusterNamespaceOnHub)
 		_, err := kubectlHub("delete", "-f", case3PolicyYaml, "-n", clusterNamespaceOnHub)
 		Expect(err).To(BeNil())

test/e2e/case7_spec_sync_test.go

@@ -4,8 +4,6 @@
 package e2e
 
 import (
-	"context"
-
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -59,11 +57,7 @@ var _ = Describe("Test spec sync", func() {
 			defaultTimeoutSeconds)
 		Expect(hubPlc).NotTo(BeNil())
 		Expect(hubPlc.Object["spec"].(map[string]interface{})["remediationAction"]).To(Equal("inform"))
-		hubPlc.Object["spec"].(map[string]interface{})["remediationAction"] = "enforce"
-		hubPlc, err := clientHubDynamic.Resource(gvrPolicy).Namespace(clusterNamespaceOnHub).Update(
-			context.TODO(),
-			hubPlc,
-			metav1.UpdateOptions{})
+		hubPlc, err := patchRemediationAction(clientHubDynamic, hubPlc, "enforce")
 		Expect(err).To(BeNil())
 		Eventually(func() interface{} {
 			managedPlc := utils.GetWithTimeout(

test/e2e/case9_template_sync_test.go

@@ -4,7 +4,6 @@
 package e2e
 
 import (
-	"context"
 	"errors"
 	"os/exec"
 
@@ -55,36 +54,29 @@ var _ = Describe("Test template sync", func() {
 		plc := utils.GetWithTimeout(
 			clientHubDynamic, gvrPolicy, case9PolicyName, clusterNamespaceOnHub, true, defaultTimeoutSeconds,
 		)
-		plc.Object["spec"].(map[string]interface{})["remediationAction"] = "enforce"
-		plc, err := clientHubDynamic.Resource(gvrPolicy).Namespace(clusterNamespaceOnHub).Update(
-			context.TODO(), plc, metav1.UpdateOptions{},
-		)
+		plc, err := patchRemediationAction(clientHubDynamic, plc, "enforce")
 		Expect(err).To(BeNil())
 		Expect(plc.Object["spec"].(map[string]interface{})["remediationAction"]).To(Equal("enforce"))
 		By("Checking template policy remediationAction")
-		yamlStr := "../resources/case9_template_sync/case9-config-policy-enforce.yaml"
-		yamlTrustedPlc := utils.ParseYaml(yamlStr)
 		Eventually(func() interface{} {
 			trustedPlc := utils.GetWithTimeout(clientManagedDynamic, gvrConfigurationPolicy,
 				case9ConfigPolicyName, clusterNamespace, true, defaultTimeoutSeconds)
 
-			return trustedPlc.Object["spec"]
-		}, defaultTimeoutSeconds, 1).Should(utils.SemanticEqual(yamlTrustedPlc.Object["spec"]))
+			return trustedPlc.Object["spec"].(map[string]interface{})["remediationAction"]
+		}, defaultTimeoutSeconds, 1).Should(Equal("enforce"))
 	})
 	It("should still override remediationAction in spec when there is no remediationAction", func() {
 		By("Updating policy with no remediationAction")
 		_, err := kubectlHub("apply", "-f",
 			"../resources/case9_template_sync/case9-test-policy-no-remediation.yaml", "-n", clusterNamespaceOnHub)
 		Expect(err).Should(BeNil())
 		By("Checking template policy remediationAction")
-		yamlTrustedPlc := utils.ParseYaml(
-			"../resources/case9_template_sync/case9-config-policy-enforce.yaml")
 		Eventually(func() interface{} {
 			trustedPlc := utils.GetWithTimeout(clientManagedDynamic, gvrConfigurationPolicy,
 				case9ConfigPolicyName, clusterNamespace, true, defaultTimeoutSeconds)
 
-			return trustedPlc.Object["spec"]
-		}, defaultTimeoutSeconds, 1).Should(utils.SemanticEqual(yamlTrustedPlc.Object["spec"]))
+			return trustedPlc.Object["spec"].(map[string]interface{})["remediationAction"]
+		}, defaultTimeoutSeconds, 1).Should(Equal("enforce"))
 	})
 	It("should contains labels from parent policy", func() {
 		By("Checking labels of template policy")

test/e2e/e2e_suite_test.go

@@ -17,7 +17,9 @@ import (
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/runtime/schema"
+	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/client-go/dynamic"
 	"k8s.io/client-go/kubernetes"
 	"k8s.io/client-go/rest"
@@ -229,3 +231,15 @@ func kubectlManaged(args ...string) (string, error) {
 
 	return propagatorutils.KubectlWithOutput(args...)
 }
+
+func patchRemediationAction(
+	client dynamic.Interface, plc *unstructured.Unstructured, remediationAction string,
+) (
+	*unstructured.Unstructured, error,
+) {
+	patch := []byte(`[{"op": "replace", "path": "/spec/remediationAction", "value": "` + remediationAction + `"}]`)
+
+	return client.Resource(gvrPolicy).Namespace(plc.GetNamespace()).Patch(
+		context.TODO(), plc.GetName(), types.JSONPatchType, patch, metav1.PatchOptions{},
+	)
+}