From 3f6528747806d29c25d4eb32e09c59d6ecf6db7f Mon Sep 17 00:00:00 2001 From: Arvind Thirumurugan Date: Wed, 1 Nov 2023 14:11:50 -0700 Subject: [PATCH 1/2] flaky fleet guard rail E2Es --- test/e2e/fleet_guard_rail_test.go | 10 +++++++++- test/e2e/utils_test.go | 29 +++++++++++++++++++++-------- 2 files changed, 30 insertions(+), 9 deletions(-) diff --git a/test/e2e/fleet_guard_rail_test.go b/test/e2e/fleet_guard_rail_test.go index 96b6692c6..e3d63cf2d 100644 --- a/test/e2e/fleet_guard_rail_test.go +++ b/test/e2e/fleet_guard_rail_test.go @@ -70,6 +70,7 @@ var _ = Describe("fleet guard rail tests for deny MC CREATE operations", func() var _ = Describe("fleet guard rail tests for allow/deny MC UPDATE, DELETE operations", Serial, Ordered, func() { mcName := fmt.Sprintf(mcNameTemplate, GinkgoParallelProcess()) + imcNamespace := fmt.Sprintf(utils.NamespaceNameFormat, mcName) BeforeAll(func() { createMemberClusterResource(mcName, testUser) @@ -77,6 +78,7 @@ var _ = Describe("fleet guard rail tests for allow/deny MC UPDATE, DELETE operat AfterAll(func() { deleteMemberClusterResource(mcName) + checkMemberClusterNamespaceIsDeleted(imcNamespace) }) It("should deny UPDATE operation on member cluster CR for user not in MC identity", func() { @@ -180,6 +182,7 @@ var _ = Describe("fleet guard rail tests for deny IMC CREATE operations", func() }, } Expect(hubClient.Delete(ctx, &ns)).Should(Succeed()) + checkMemberClusterNamespaceIsDeleted(imcNamespace) }) It("should deny CREATE operation on internal member cluster CR for user not in MC identity in fleet member namespace", func() { @@ -213,6 +216,7 @@ var _ = Describe("fleet guard rail tests for IMC UPDATE operation, in fleet-memb AfterAll(func() { deleteMemberClusterResource(mcName) + checkMemberClusterNamespaceIsDeleted(imcNamespace) }) It("should deny UPDATE operation on internal member cluster CR for user not in MC identity in fleet member namespace", func() { @@ -237,7 +241,7 @@ var _ = Describe("fleet guard rail tests for IMC UPDATE operation, in fleet-memb var imc clusterv1beta1.InternalMemberCluster Expect(hubClient.Get(ctx, types.NamespacedName{Name: mcName, Namespace: imcNamespace}, &imc)).Should(Succeed()) - By("expecting denial of operation UPDATE of Internal Member Cluster") + By("expecting denial of operation DELETE of Internal Member Cluster") err := impersonateHubClient.Delete(ctx, &imc) var statusErr *k8sErrors.StatusError Expect(errors.As(err, &statusErr)).To(BeTrue(), fmt.Sprintf("Delete internal member cluster call produced error %s. Error type wanted is %s.", reflect.TypeOf(err), reflect.TypeOf(&k8sErrors.StatusError{}))) @@ -282,6 +286,7 @@ var _ = Describe("fleet guard rail tests for IMC UPDATE operation, in fleet-memb AfterAll(func() { deleteMemberClusterResource(mcName) + checkMemberClusterNamespaceIsDeleted(imcNamespace) }) It("should allow UPDATE operation on internal member cluster CR status for user in MC identity", func() { @@ -346,6 +351,7 @@ var _ = Describe("fleet guard rail tests for deny Work CREATE operations", func( }, } Expect(hubClient.Delete(ctx, &ns)).Should(Succeed()) + checkMemberClusterNamespaceIsDeleted(imcNamespace) }) It("should deny CREATE operation on internal member cluster CR for user not in MC identity in fleet member namespace", func() { @@ -408,6 +414,7 @@ var _ = Describe("fleet guard rail for UPDATE work operations, in fleet prefixed AfterAll(func() { deleteWorkResource(workName, imcNamespace) deleteMemberClusterResource(mcName) + checkMemberClusterNamespaceIsDeleted(imcNamespace) }) It("should deny UPDATE operation on work CR status for user not in MC identity", func() { @@ -462,6 +469,7 @@ var _ = Describe("fleet guard rail for UPDATE work operations, in fleet prefixed AfterAll(func() { deleteWorkResource(workName, imcNamespace) deleteMemberClusterResource(mcName) + checkMemberClusterNamespaceIsDeleted(imcNamespace) }) It("should allow UPDATE operation on work CR for user in MC identity", func() { diff --git a/test/e2e/utils_test.go b/test/e2e/utils_test.go index 74a8f8efc..0d1783467 100644 --- a/test/e2e/utils_test.go +++ b/test/e2e/utils_test.go @@ -8,12 +8,19 @@ package e2e import ( "encoding/json" "fmt" + corev1 "k8s.io/api/core/v1" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" "time" "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + clusterv1beta1 "go.goms.io/fleet/apis/cluster/v1beta1" + placementv1beta1 "go.goms.io/fleet/apis/placement/v1beta1" + imcv1beta1 "go.goms.io/fleet/pkg/controllers/internalmembercluster/v1beta1" + "go.goms.io/fleet/pkg/utils" + "go.goms.io/fleet/test/e2e/framework" appsv1 "k8s.io/api/apps/v1" rbacv1 "k8s.io/api/rbac/v1" "k8s.io/apimachinery/pkg/api/errors" @@ -22,13 +29,6 @@ import ( "k8s.io/apimachinery/pkg/types" utilrand "k8s.io/apimachinery/pkg/util/rand" "sigs.k8s.io/controller-runtime/pkg/client" - "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" - - clusterv1beta1 "go.goms.io/fleet/apis/cluster/v1beta1" - placementv1beta1 "go.goms.io/fleet/apis/placement/v1beta1" - imcv1beta1 "go.goms.io/fleet/pkg/controllers/internalmembercluster/v1beta1" - "go.goms.io/fleet/pkg/utils" - "go.goms.io/fleet/test/e2e/framework" ) // setAllMemberClustersToJoin creates a MemberCluster object for each member cluster. @@ -280,7 +280,10 @@ func deleteMemberClusterResource(name string) { } g.Expect(err).Should(Succeed(), "Failed to get MC %s", name) controllerutil.RemoveFinalizer(&mc, placementv1beta1.MemberClusterFinalizer) - g.Expect(hubClient.Update(ctx, &mc)).Should(Succeed()) + err = hubClient.Update(ctx, &mc) + if errors.IsConflict(err) { + return err + } g.Expect(hubClient.Delete(ctx, &mc)).Should(Succeed()) return nil }, eventuallyDuration, eventuallyInterval).Should(Succeed()) @@ -306,6 +309,16 @@ func checkInternalMemberClusterExists(name, namespace string) { }, eventuallyDuration, eventuallyInterval).Should(Succeed()) } +func checkMemberClusterNamespaceIsDeleted(name string) { + Eventually(func(g Gomega) error { + var ns corev1.Namespace + if err := hubClient.Get(ctx, types.NamespacedName{Name: name}, &ns); !errors.IsNotFound(err) { + return fmt.Errorf("member cluster namespace %s still exists or an unexpected error occurred: %w", name, err) + } + return nil + }, eventuallyDuration, eventuallyInterval).Should(Succeed()) +} + func createWorkResource(name, namespace string) { testDeployment := appsv1.Deployment{ TypeMeta: metav1.TypeMeta{ From 0cad02c0439a3615512a0f20094612fe03be6ea3 Mon Sep 17 00:00:00 2001 From: Arvind Thirumurugan Date: Wed, 1 Nov 2023 14:14:16 -0700 Subject: [PATCH 2/2] fix import order --- test/e2e/utils_test.go | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/test/e2e/utils_test.go b/test/e2e/utils_test.go index 0d1783467..98ea64da3 100644 --- a/test/e2e/utils_test.go +++ b/test/e2e/utils_test.go @@ -8,20 +8,14 @@ package e2e import ( "encoding/json" "fmt" - corev1 "k8s.io/api/core/v1" - "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" "time" "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" - clusterv1beta1 "go.goms.io/fleet/apis/cluster/v1beta1" - placementv1beta1 "go.goms.io/fleet/apis/placement/v1beta1" - imcv1beta1 "go.goms.io/fleet/pkg/controllers/internalmembercluster/v1beta1" - "go.goms.io/fleet/pkg/utils" - "go.goms.io/fleet/test/e2e/framework" appsv1 "k8s.io/api/apps/v1" + corev1 "k8s.io/api/core/v1" rbacv1 "k8s.io/api/rbac/v1" "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -29,6 +23,13 @@ import ( "k8s.io/apimachinery/pkg/types" utilrand "k8s.io/apimachinery/pkg/util/rand" "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" + + clusterv1beta1 "go.goms.io/fleet/apis/cluster/v1beta1" + placementv1beta1 "go.goms.io/fleet/apis/placement/v1beta1" + imcv1beta1 "go.goms.io/fleet/pkg/controllers/internalmembercluster/v1beta1" + "go.goms.io/fleet/pkg/utils" + "go.goms.io/fleet/test/e2e/framework" ) // setAllMemberClustersToJoin creates a MemberCluster object for each member cluster.