From ca4740dc34caab7339adafb40b4f17e0abd73f58 Mon Sep 17 00:00:00 2001 From: yiraeChristineKim Date: Tue, 7 May 2024 12:50:56 -0400 Subject: [PATCH 1/2] Add hosted mode tests for OperatorPolicy The "hosting cluster" should be used for the Policy, OperatorPolicy, and compliance events Ref: https://issues.redhat.com/browse/ACM-11255 Signed-off-by: yiraeChristineKim (cherry picked from commit a2ebc6ba7c4b8e5d42dde5fc233bdf61065a4606) --- .vscode/launch.json | 19 +- Makefile | 27 +- controllers/operatorpolicy_controller.go | 23 +- main.go | 10 +- .../e2e/case21_alternative_kubeconfig_test.go | 18 -- test/e2e/case38_install_operator_test.go | 243 ++++++++---------- test/e2e/e2e_suite_test.go | 41 +++ test/utils/utils.go | 4 + 8 files changed, 206 insertions(+), 179 deletions(-) diff --git a/.vscode/launch.json b/.vscode/launch.json index 5380ddcd..1a3455da 100644 --- a/.vscode/launch.json +++ b/.vscode/launch.json @@ -29,6 +29,23 @@ "KUBECONFIG": "${workspaceFolder}/kubeconfig_managed_e2e", } }, + // Set FDescribe or FIt on the test to debug. Then set the desired breakpoint. + { + "name": "Launch Hosted Test Function", + "type": "go", + "request": "launch", + "mode": "auto", + "program": "${workspaceFolder}/test/e2e/e2e_suite_test.go", + "args": [ + "-ginkgo.debug", + "-ginkgo.v", + "-is_hosted=true" + ], + "env": { + "KUBECONFIG": "${workspaceFolder}/kubeconfig_managed_e2e", + "TARGET_KUBECONFIG_PATH": "${workspaceFolder}/kubeconfig_managed2_e2e", + } + }, // Set the correct path to the governance-policy-framework repo directory in the env section. { "name": "Launch Package (Framework E2E) (instructions in launch.json)", @@ -45,4 +62,4 @@ } } ] -} +} \ No newline at end of file diff --git a/Makefile b/Makefile index 00fdc66e..6f46ee9b 100644 --- a/Makefile +++ b/Makefile @@ -106,7 +106,7 @@ create-ns: # Run against the current locally configured Kubernetes cluster .PHONY: run run: - WATCH_NAMESPACE=$(WATCH_NAMESPACE) go run ./main.go controller --leader-elect=false + WATCH_NAMESPACE=$(WATCH_NAMESPACE) go run ./main.go controller --leader-elect=false --enable-operator-policy=true ############################################################ # clean section @@ -193,7 +193,7 @@ kind-create-cluster: .PHONY: kind-additional-cluster kind-additional-cluster: MANAGED_CLUSTER_SUFFIX = 2 kind-additional-cluster: CLUSTER_NAME = $(MANAGED_CLUSTER_NAME) -kind-additional-cluster: kind-create-cluster kind-controller-kubeconfig +kind-additional-cluster: kind-create-cluster kind-controller-kubeconfig install-crds-hosted .PHONY: kind-delete-cluster kind-delete-cluster: @@ -204,11 +204,10 @@ kind-delete-cluster: kind-tests: kind-delete-cluster kind-bootstrap-cluster-dev kind-deploy-controller-dev e2e-test OLM_VERSION := v0.26.0 +OLM_INSTALLER = $(LOCAL_BIN)/install.sh .PHONY: install-crds -install-crds: - @echo installing olm - curl -L https://github.com/operator-framework/operator-lifecycle-manager/releases/download/$(OLM_VERSION)/install.sh -o $(LOCAL_BIN)/install.sh +install-crds: $(OLM_INSTALLER) chmod +x $(LOCAL_BIN)/install.sh $(LOCAL_BIN)/install.sh $(OLM_VERSION) @echo installing crds @@ -221,6 +220,15 @@ install-crds: kubectl apply -f deploy/crds/policy.open-cluster-management.io_configurationpolicies.yaml kubectl apply -f deploy/crds/policy.open-cluster-management.io_operatorpolicies.yaml +$(OLM_INSTALLER): + @echo getting OLM installer + curl -L https://github.com/operator-framework/operator-lifecycle-manager/releases/download/$(OLM_VERSION)/install.sh -o $(LOCAL_BIN)/install.sh + +install-crds-hosted: export KUBECONFIG=./kubeconfig_managed$(MANAGED_CLUSTER_SUFFIX)_e2e +install-crds-hosted: $(OLM_INSTALLER) + chmod +x $(LOCAL_BIN)/install.sh + $(LOCAL_BIN)/install.sh $(OLM_VERSION) + .PHONY: install-resources install-resources: # creating namespaces @@ -230,17 +238,22 @@ install-resources: kubectl apply -k deploy/rbac kubectl apply -f deploy/manager/service-account.yaml -n $(KIND_NAMESPACE) +IS_HOSTED ?= false +E2E_PROCS = 20 + .PHONY: e2e-test e2e-test: e2e-dependencies - $(GINKGO) -v --procs=20 $(E2E_TEST_ARGS) test/e2e + $(GINKGO) -v --procs=$(E2E_PROCS) $(E2E_TEST_ARGS) test/e2e -- -is_hosted=$(IS_HOSTED) .PHONY: e2e-test-coverage e2e-test-coverage: E2E_TEST_ARGS = --json-report=report_e2e.json --label-filter='!hosted-mode && !running-in-cluster' --output-dir=. e2e-test-coverage: e2e-run-instrumented e2e-test e2e-stop-instrumented .PHONY: e2e-test-hosted-mode-coverage -e2e-test-hosted-mode-coverage: E2E_TEST_ARGS = --json-report=report_e2e_hosted_mode.json --label-filter="hosted-mode && !running-in-cluster" --output-dir=. +e2e-test-hosted-mode-coverage: E2E_TEST_ARGS = --json-report=report_e2e_hosted_mode.json --label-filter="hosted-mode || supports-hosted && !running-in-cluster" --output-dir=. e2e-test-hosted-mode-coverage: COVERAGE_E2E_OUT = coverage_e2e_hosted_mode.out +e2e-test-hosted-mode-coverage: IS_HOSTED=true +e2e-test-hosted-mode-coverage: E2E_PROCS=2 e2e-test-hosted-mode-coverage: export TARGET_KUBECONFIG_PATH = $(PWD)/kubeconfig_managed2 e2e-test-hosted-mode-coverage: e2e-run-instrumented e2e-test e2e-stop-instrumented diff --git a/controllers/operatorpolicy_controller.go b/controllers/operatorpolicy_controller.go index 9ed5a7ed..9e668fe2 100644 --- a/controllers/operatorpolicy_controller.go +++ b/controllers/operatorpolicy_controller.go @@ -103,6 +103,7 @@ type OperatorPolicyReconciler struct { DynamicWatcher depclient.DynamicWatcher InstanceName string DefaultNamespace string + TargetClient client.Client } // SetupWithManager sets up the controller with the Manager and will reconcile when the dynamic watcher @@ -750,7 +751,7 @@ func (r *OperatorPolicyReconciler) musthaveOpGroup( desiredOpGroup.ResourceVersion = opGroup.GetResourceVersion() - err = r.Update(ctx, &opGroup) + err = r.TargetClient.Update(ctx, &opGroup) if err != nil { return false, nil, changed, fmt.Errorf("error updating the OperatorGroup: %w", err) } @@ -771,7 +772,7 @@ func (r *OperatorPolicyReconciler) musthaveOpGroup( func (r *OperatorPolicyReconciler) createWithNamespace( ctx context.Context, policy *policyv1beta1.OperatorPolicy, object client.Object, ) error { - err := r.Create(ctx, object) + err := r.TargetClient.Create(ctx, object) if err == nil { return nil } @@ -789,13 +790,13 @@ func (r *OperatorPolicyReconciler) createWithNamespace( }, } - err = r.Create(ctx, &ns) + err = r.TargetClient.Create(ctx, &ns) if err != nil && !k8serrors.IsAlreadyExists(err) { return err } // Try creating the object again now that the namespace was created. - return r.Create(ctx, object) + return r.TargetClient.Create(ctx, object) } // isNamespaceNotFound detects if the input error from r.Create failed due to the specified namespace not existing. @@ -904,7 +905,7 @@ func (r *OperatorPolicyReconciler) mustnothaveOpGroup( earlyConds = append(earlyConds, calculateComplianceCondition(policy)) } - err := r.Delete(ctx, desiredOpGroup) + err := r.TargetClient.Delete(ctx, desiredOpGroup) if err != nil { return earlyConds, changed, fmt.Errorf("error deleting the OperatorGroup: %w", err) } @@ -1046,7 +1047,7 @@ func (r *OperatorPolicyReconciler) musthaveSubscription( earlyConds = append(earlyConds, calculateComplianceCondition(policy)) } - err = r.Update(ctx, foundSub) + err = r.TargetClient.Update(ctx, foundSub) if err != nil { return mergedSub, nil, changed, fmt.Errorf("error updating the Subscription: %w", err) } @@ -1100,7 +1101,7 @@ func (r *OperatorPolicyReconciler) mustnothaveSubscription( earlyConds = append(earlyConds, calculateComplianceCondition(policy)) } - err := r.Delete(ctx, foundUnstructSub) + err := r.TargetClient.Delete(ctx, foundUnstructSub) if err != nil { return foundSub, earlyConds, changed, fmt.Errorf("error deleting the Subscription: %w", err) } @@ -1364,7 +1365,7 @@ func (r *OperatorPolicyReconciler) musthaveInstallPlan( return false, fmt.Errorf("error approving InstallPlan: %w", err) } - if err := r.Update(ctx, &approvableInstallPlans[0]); err != nil { + if err := r.TargetClient.Update(ctx, &approvableInstallPlans[0]); err != nil { return false, fmt.Errorf("error updating approved InstallPlan: %w", err) } @@ -1502,7 +1503,7 @@ func (r *OperatorPolicyReconciler) mustnothaveCSV( continue } - err := r.Delete(ctx, &csvList[i]) + err := r.TargetClient.Delete(ctx, &csvList[i]) if err != nil { changed := updateStatus(policy, foundNotWantedCond("ClusterServiceVersion", csvNames...), relatedCSVs...) @@ -1672,7 +1673,7 @@ func (r *OperatorPolicyReconciler) handleCRDs( continue } - err := r.Delete(ctx, &crdList[i]) + err := r.TargetClient.Delete(ctx, &crdList[i]) if err != nil { changed := updateStatus(policy, foundNotWantedCond("CustomResourceDefinition"), relatedCRDs...) @@ -1835,7 +1836,7 @@ func (r *OperatorPolicyReconciler) mergeObjects( } if updateNeeded { - err := r.Update(ctx, existing, client.DryRunAll) + err := r.TargetClient.Update(ctx, existing, client.DryRunAll) if err != nil { if k8serrors.IsForbidden(err) { // This indicates the update would make a change, but the change is not allowed, diff --git a/main.go b/main.go index a17ad797..6a9e31df 100644 --- a/main.go +++ b/main.go @@ -322,6 +322,7 @@ func main() { var targetK8sClient kubernetes.Interface var targetK8sDynamicClient dynamic.Interface var targetK8sConfig *rest.Config + var targetClient client.Client var nsSelMgr manager.Manager // A separate controller-manager is needed in hosted mode if opts.targetKubeConfig == "" { @@ -329,6 +330,7 @@ func main() { targetK8sClient = kubernetes.NewForConfigOrDie(targetK8sConfig) targetK8sDynamicClient = dynamic.NewForConfigOrDie(targetK8sConfig) nsSelMgr = mgr + targetClient = mgr.GetClient() } else { // "Hosted mode" var err error @@ -343,6 +345,11 @@ func main() { targetK8sClient = kubernetes.NewForConfigOrDie(targetK8sConfig) targetK8sDynamicClient = dynamic.NewForConfigOrDie(targetK8sConfig) + targetClient, err = client.New(targetK8sConfig, client.Options{Scheme: scheme}) + if err != nil { + log.Error(err, "Failed to load the target kubeconfig", "path", opts.targetKubeConfig) + os.Exit(1) + } // The managed cluster's API server is potentially not the same as the hosting cluster and it could be // offline already as part of the uninstall process. In this case, the manager's instantiation will fail. @@ -428,7 +435,7 @@ func main() { if opts.enableOperatorPolicy { depReconciler, depEvents := depclient.NewControllerRuntimeSource() - watcher, err := depclient.New(cfg, depReconciler, + watcher, err := depclient.New(targetK8sConfig, depReconciler, &depclient.Options{ DisableInitialReconcile: true, EnableCache: true, @@ -455,6 +462,7 @@ func main() { DynamicWatcher: watcher, InstanceName: instanceName, DefaultNamespace: opts.operatorPolDefaultNS, + TargetClient: targetClient, } if err = OpReconciler.SetupWithManager(mgr, depEvents); err != nil { diff --git a/test/e2e/case21_alternative_kubeconfig_test.go b/test/e2e/case21_alternative_kubeconfig_test.go index 6155274b..454314d7 100644 --- a/test/e2e/case21_alternative_kubeconfig_test.go +++ b/test/e2e/case21_alternative_kubeconfig_test.go @@ -5,22 +5,18 @@ package e2e import ( "context" "fmt" - "os" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/client-go/kubernetes" - "k8s.io/client-go/tools/clientcmd" "open-cluster-management.io/config-policy-controller/test/utils" ) var _ = Describe("Test an alternative kubeconfig for policy evaluation", Ordered, Label("hosted-mode"), func() { const ( - envName = "TARGET_KUBECONFIG_PATH" namespaceName = "e2e-test-ns" policyName = "create-ns" policyYAML = "../resources/case21_alternative_kubeconfig/policy.yaml" @@ -28,20 +24,6 @@ var _ = Describe("Test an alternative kubeconfig for policy evaluation", Ordered parentPolicyYAML = "../resources/case21_alternative_kubeconfig/parent-policy.yaml" ) - var targetK8sClient *kubernetes.Clientset - - BeforeAll(func() { - By("Checking that the " + envName + " environment variable is valid") - altKubeconfigPath := os.Getenv(envName) - Expect(altKubeconfigPath).ToNot(Equal("")) - - targetK8sConfig, err := clientcmd.BuildConfigFromFlags("", altKubeconfigPath) - Expect(err).ToNot(HaveOccurred()) - - targetK8sClient, err = kubernetes.NewForConfig(targetK8sConfig) - Expect(err).ToNot(HaveOccurred()) - }) - AfterAll(func() { deleteConfigPolicies([]string{policyName}) diff --git a/test/e2e/case38_install_operator_test.go b/test/e2e/case38_install_operator_test.go index b8ea0dfe..02743aed 100644 --- a/test/e2e/case38_install_operator_test.go +++ b/test/e2e/case38_install_operator_test.go @@ -23,12 +23,12 @@ import ( "open-cluster-management.io/config-policy-controller/test/utils" ) -var _ = Describe("Testing OperatorPolicy", Ordered, func() { +var _ = Describe("Testing OperatorPolicy", Ordered, Label("supports-hosted"), func() { const ( opPolTestNS = "operator-policy-testns" parentPolicyYAML = "../resources/case38_operator_install/parent-policy.yaml" parentPolicyName = "parent-policy" - eventuallyTimeout = 10 + eventuallyTimeout = 60 consistentlyDuration = 5 olmWaitTimeout = 60 ) @@ -170,6 +170,19 @@ var _ = Describe("Testing OperatorPolicy", Ordered, func() { ConsistentlyWithOffset(1, checkFunc, consistentlyDuration, 1).Should(Succeed()) } + preFunc := func() { + if IsHosted { + KubectlTarget("create", "ns", opPolTestNS) + DeferCleanup(func() { + KubectlTarget("delete", "ns", opPolTestNS) + }) + } + utils.Kubectl("create", "ns", opPolTestNS) + DeferCleanup(func() { + utils.Kubectl("delete", "ns", opPolTestNS) + }) + } + Describe("Testing an all default operator policy", Ordered, func() { const ( opPolYAML = "../resources/case38_operator_install/operator-policy-all-defaults.yaml" @@ -177,10 +190,7 @@ var _ = Describe("Testing OperatorPolicy", Ordered, func() { subName = "project-quay" ) BeforeAll(func() { - utils.Kubectl("create", "ns", opPolTestNS) - DeferCleanup(func() { - utils.Kubectl("delete", "ns", opPolTestNS) - }) + preFunc() createObjWithParent(parentPolicyYAML, parentPolicyName, opPolYAML, opPolTestNS, gvrPolicy, gvrOperatorPolicy) @@ -209,7 +219,7 @@ var _ = Describe("Testing OperatorPolicy", Ordered, func() { ) By("Verifying the subscription has the correct defaults") - sub, err := clientManagedDynamic.Resource(gvrSubscription).Namespace(opPolTestNS). + sub, err := targetK8sDynamic.Resource(gvrSubscription).Namespace(opPolTestNS). Get(ctx, subName, metav1.GetOptions{}) Expect(err).ToNot(HaveOccurred()) @@ -232,11 +242,7 @@ var _ = Describe("Testing OperatorPolicy", Ordered, func() { subName = "project-quay" ) BeforeAll(func() { - utils.Kubectl("create", "ns", opPolTestNS) - DeferCleanup(func() { - utils.Kubectl("delete", "ns", opPolTestNS) - }) - + preFunc() createObjWithParent(parentPolicyYAML, parentPolicyName, opPolYAML, opPolTestNS, gvrPolicy, gvrOperatorPolicy) }) @@ -270,10 +276,7 @@ var _ = Describe("Testing OperatorPolicy", Ordered, func() { extraOpGroupName = "extra-operator-group" ) BeforeAll(func() { - utils.Kubectl("create", "ns", opPolTestNS) - DeferCleanup(func() { - utils.Kubectl("delete", "ns", opPolTestNS) - }) + preFunc() createObjWithParent(parentPolicyYAML, parentPolicyName, opPolYAML, opPolTestNS, gvrPolicy, gvrOperatorPolicy) @@ -328,7 +331,7 @@ var _ = Describe("Testing OperatorPolicy", Ordered, func() { ) }) It("Should become NonCompliant when an extra OperatorGroup is added", func() { - utils.Kubectl("apply", "-f", extraOpGroupYAML, "-n", opPolTestNS) + KubectlTarget("apply", "-f", extraOpGroupYAML, "-n", opPolTestNS) check( opPolName, true, @@ -363,8 +366,8 @@ var _ = Describe("Testing OperatorPolicy", Ordered, func() { It("Should warn about the OperatorGroup when it doesn't match the default", func() { utils.Kubectl("patch", "operatorpolicy", opPolName, "-n", opPolTestNS, "--type=json", "-p", `[{"op": "replace", "path": "/spec/remediationAction", "value": "inform"}]`) - utils.Kubectl("delete", "operatorgroup", "-n", opPolTestNS, "--all") - utils.Kubectl("apply", "-f", extraOpGroupYAML, "-n", opPolTestNS) + KubectlTarget("delete", "operatorgroup", "-n", opPolTestNS, "--all") + KubectlTarget("apply", "-f", extraOpGroupYAML, "-n", opPolTestNS) check( opPolName, false, @@ -398,6 +401,9 @@ var _ = Describe("Testing OperatorPolicy", Ordered, func() { utils.Kubectl( "delete", "-f", parentPolicyYAML, "-n", testNamespace, "--ignore-not-found", "--cascade=foreground", ) + if IsHosted { + KubectlTarget("delete", "ns", opPolTestNS, "--ignore-not-found") + } utils.Kubectl("delete", "ns", opPolTestNS, "--ignore-not-found") }) @@ -426,12 +432,9 @@ var _ = Describe("Testing OperatorPolicy", Ordered, func() { ) BeforeAll(func() { - utils.Kubectl("create", "ns", opPolTestNS) - DeferCleanup(func() { - utils.Kubectl("delete", "ns", opPolTestNS) - }) + preFunc() - utils.Kubectl("apply", "-f", incorrectOpGroupYAML, "-n", opPolTestNS) + KubectlTarget("apply", "-f", incorrectOpGroupYAML, "-n", opPolTestNS) createObjWithParent(parentPolicyYAML, parentPolicyName, opPolYAML, opPolTestNS, gvrPolicy, gvrOperatorPolicy) @@ -474,8 +477,8 @@ var _ = Describe("Testing OperatorPolicy", Ordered, func() { ) }) It("Should match when the OperatorGroup is manually corrected", func() { - utils.Kubectl("delete", "operatorgroup", incorrectOpGroupName, "-n", opPolTestNS) - utils.Kubectl("apply", "-f", scopedOpGroupYAML, "-n", opPolTestNS) + KubectlTarget("delete", "operatorgroup", incorrectOpGroupName, "-n", opPolTestNS) + KubectlTarget("apply", "-f", scopedOpGroupYAML, "-n", opPolTestNS) check( opPolName, false, @@ -501,7 +504,7 @@ var _ = Describe("Testing OperatorPolicy", Ordered, func() { ) }) It("Should report a mismatch when the OperatorGroup is manually edited", func() { - utils.Kubectl("patch", "operatorgroup", scopedOpGroupName, "-n", opPolTestNS, "--type=json", "-p", + KubectlTarget("patch", "operatorgroup", scopedOpGroupName, "-n", opPolTestNS, "--type=json", "-p", `[{"op": "replace", "path": "/spec/targetNamespaces", "value": []}]`) check( opPolName, @@ -555,7 +558,7 @@ var _ = Describe("Testing OperatorPolicy", Ordered, func() { ) }) It("Should become NonCompliant when an extra OperatorGroup is added", func() { - utils.Kubectl("apply", "-f", extraOpGroupYAML, "-n", opPolTestNS) + KubectlTarget("apply", "-f", extraOpGroupYAML, "-n", opPolTestNS) check( opPolName, true, @@ -598,12 +601,9 @@ var _ = Describe("Testing OperatorPolicy", Ordered, func() { ) BeforeAll(func() { - utils.Kubectl("create", "ns", opPolTestNS) - DeferCleanup(func() { - utils.Kubectl("delete", "ns", opPolTestNS) - }) + preFunc() - utils.Kubectl("apply", "-f", extraOpGroupYAML, "-n", opPolTestNS) + KubectlTarget("apply", "-f", extraOpGroupYAML, "-n", opPolTestNS) createObjWithParent(parentPolicyYAML, parentPolicyName, opPolYAML, opPolTestNS, gvrPolicy, gvrOperatorPolicy) @@ -697,7 +697,7 @@ var _ = Describe("Testing OperatorPolicy", Ordered, func() { }) It("Should create the Subscription after the additional OperatorGroup is removed", func() { - utils.Kubectl("delete", "operatorgroup", extraOpGroupName, "-n", opPolTestNS) + KubectlTarget("delete", "operatorgroup", extraOpGroupName, "-n", opPolTestNS) check( opPolName, false, @@ -760,12 +760,9 @@ var _ = Describe("Testing OperatorPolicy", Ordered, func() { ) BeforeAll(func() { - utils.Kubectl("create", "ns", opPolTestNS) - DeferCleanup(func() { - utils.Kubectl("delete", "ns", opPolTestNS) - }) + preFunc() - utils.Kubectl("apply", "-f", subYAML, "-n", opPolTestNS) + KubectlTarget("apply", "-f", subYAML, "-n", opPolTestNS) createObjWithParent(parentPolicyYAML, parentPolicyName, opPolYAML, opPolTestNS, gvrPolicy, gvrOperatorPolicy) @@ -831,10 +828,7 @@ var _ = Describe("Testing OperatorPolicy", Ordered, func() { opPolNoExistName = "oppol-no-exist-enforce" ) BeforeAll(func() { - utils.Kubectl("create", "ns", opPolTestNS) - DeferCleanup(func() { - utils.Kubectl("delete", "ns", opPolTestNS) - }) + preFunc() createObjWithParent(parentPolicyYAML, parentPolicyName, opPolYAML, opPolTestNS, gvrPolicy, gvrOperatorPolicy) @@ -845,7 +839,7 @@ var _ = Describe("Testing OperatorPolicy", Ordered, func() { It("Should generate conditions and relatedobjects of CSV", func(ctx SpecContext) { Eventually(func(ctx SpecContext) string { - csv, _ := clientManagedDynamic.Resource(gvrClusterServiceVersion).Namespace(opPolTestNS). + csv, _ := targetK8sDynamic.Resource(gvrClusterServiceVersion).Namespace(opPolTestNS). Get(ctx, "quay-operator.v3.10.0", metav1.GetOptions{}) if csv == nil { @@ -958,10 +952,7 @@ var _ = Describe("Testing OperatorPolicy", Ordered, func() { opPolName = "oppol-no-allnamespaces" ) BeforeAll(func() { - utils.Kubectl("create", "ns", opPolTestNS) - DeferCleanup(func() { - utils.Kubectl("delete", "ns", opPolTestNS) - }) + preFunc() createObjWithParent(parentPolicyYAML, parentPolicyName, opPolYAML, opPolTestNS, gvrPolicy, gvrOperatorPolicy) @@ -969,7 +960,7 @@ var _ = Describe("Testing OperatorPolicy", Ordered, func() { It("Should generate conditions and relatedobjects of CSV", func(ctx SpecContext) { Eventually(func(ctx SpecContext) []unstructured.Unstructured { - csvList, _ := clientManagedDynamic.Resource(gvrClusterServiceVersion).Namespace(opPolTestNS). + csvList, _ := targetK8sDynamic.Resource(gvrClusterServiceVersion).Namespace(opPolTestNS). List(ctx, metav1.ListOptions{}) return csvList.Items @@ -1038,11 +1029,10 @@ var _ = Describe("Testing OperatorPolicy", Ordered, func() { BeforeAll(func() { By("Applying creating a ns and the test policy") - utils.Kubectl("create", "ns", opPolTestNS) + preFunc() DeferCleanup(func() { - utils.Kubectl("patch", "catalogsource", catSrcName, "-n", "olm", "--type=json", "-p", + KubectlTarget("patch", "catalogsource", catSrcName, "-n", "olm", "--type=json", "-p", `[{"op": "replace", "path": "/spec/image", "value": "quay.io/operatorhubio/catalog:latest"}]`) - utils.Kubectl("delete", "ns", opPolTestNS) }) createObjWithParent(parentPolicyYAML, parentPolicyName, @@ -1141,7 +1131,7 @@ var _ = Describe("Testing OperatorPolicy", Ordered, func() { `[{"op": "replace", "path": "/spec/subscription/source", "value": "operatorhubio-catalog"}]`) By("Patching the CatalogSource to reference a broken image link") - utils.Kubectl("patch", "catalogsource", catSrcName, "-n", "olm", "--type=json", "-p", + KubectlTarget("patch", "catalogsource", catSrcName, "-n", "olm", "--type=json", "-p", `[{"op": "replace", "path": "/spec/image", "value": "quay.io/operatorhubio/fakecatalog:latest"}]`) By("Checking the conditions and relatedObj in the policy") @@ -1183,10 +1173,7 @@ var _ = Describe("Testing OperatorPolicy", Ordered, func() { ) BeforeAll(func() { - utils.Kubectl("create", "ns", opPolTestNS) - DeferCleanup(func() { - utils.Kubectl("delete", "ns", opPolTestNS) - }) + preFunc() createObjWithParent(parentPolicyYAML, parentPolicyName, opPolYAML, opPolTestNS, gvrPolicy, gvrOperatorPolicy) @@ -1194,7 +1181,7 @@ var _ = Describe("Testing OperatorPolicy", Ordered, func() { It("Should initially report the ConstraintsNotSatisfiable Subscription", func(ctx SpecContext) { Eventually(func(ctx SpecContext) interface{} { - sub, _ := clientManagedDynamic.Resource(gvrSubscription).Namespace(opPolTestNS). + sub, _ := targetK8sDynamic.Resource(gvrSubscription).Namespace(opPolTestNS). Get(ctx, subName, metav1.GetOptions{}) if sub == nil { @@ -1273,10 +1260,10 @@ var _ = Describe("Testing OperatorPolicy", Ordered, func() { utils.Kubectl("patch", "operatorpolicy", opPolName, "-n", opPolTestNS, "--type=json", "-p", `[{"op": "replace", "path": "/spec/subscription/startingCSV", "value": "`+goodVersion+`"},`+ `{"op": "replace", "path": "/spec/remediationAction", "value": "inform"}]`) - utils.Kubectl("patch", "subscription.operator", subName, "-n", opPolTestNS, "--type=json", "-p", + KubectlTarget("patch", "subscription.operator", subName, "-n", opPolTestNS, "--type=json", "-p", `[{"op": "replace", "path": "/spec/startingCSV", "value": "`+goodVersion+`"}]`) Eventually(func(ctx SpecContext) int { - ipList, _ := clientManagedDynamic.Resource(gvrInstallPlan).Namespace(opPolTestNS). + ipList, _ := targetK8sDynamic.Resource(gvrInstallPlan).Namespace(opPolTestNS). List(ctx, metav1.ListOptions{}) return len(ipList.Items) @@ -1305,7 +1292,7 @@ var _ = Describe("Testing OperatorPolicy", Ordered, func() { ) }) It("Should do the upgrade when enforced, and stop at the next version", func(ctx SpecContext) { - ipList, err := clientManagedDynamic.Resource(gvrInstallPlan).Namespace(opPolTestNS). + ipList, err := targetK8sDynamic.Resource(gvrInstallPlan).Namespace(opPolTestNS). List(ctx, metav1.ListOptions{}) Expect(err).NotTo(HaveOccurred()) Expect(ipList.Items).To(HaveLen(1)) @@ -1316,7 +1303,7 @@ var _ = Describe("Testing OperatorPolicy", Ordered, func() { `[{"op": "replace", "path": "/spec/remediationAction", "value": "enforce"}]`) Eventually(func(ctx SpecContext) int { - ipList, err = clientManagedDynamic.Resource(gvrInstallPlan).Namespace(opPolTestNS). + ipList, err = targetK8sDynamic.Resource(gvrInstallPlan).Namespace(opPolTestNS). List(ctx, metav1.ListOptions{}) return len(ipList.Items) @@ -1328,7 +1315,7 @@ var _ = Describe("Testing OperatorPolicy", Ordered, func() { } Eventually(func(ctx SpecContext) string { - ip, _ := clientManagedDynamic.Resource(gvrInstallPlan).Namespace(opPolTestNS). + ip, _ := targetK8sDynamic.Resource(gvrInstallPlan).Namespace(opPolTestNS). Get(ctx, firstInstallPlanName, metav1.GetOptions{}) phase, _, _ := unstructured.NestedString(ip.Object, "status", "phase") @@ -1378,7 +1365,7 @@ var _ = Describe("Testing OperatorPolicy", Ordered, func() { `[{"op": "add", "path": "/spec/versions/-", "value": "strimzi-cluster-operator.v0.36.1"}]`) Eventually(func(ctx SpecContext) string { - ip, _ := clientManagedDynamic.Resource(gvrInstallPlan).Namespace(opPolTestNS). + ip, _ := targetK8sDynamic.Resource(gvrInstallPlan).Namespace(opPolTestNS). Get(ctx, secondInstallPlanName, metav1.GetOptions{}) phase, _, _ := unstructured.NestedString(ip.Object, "status", "phase") @@ -1425,12 +1412,8 @@ var _ = Describe("Testing OperatorPolicy", Ordered, func() { opPolName = "oppol-no-group" ) BeforeAll(func() { - utils.Kubectl("delete", "crd", "--selector=olm.managed=true") - utils.Kubectl("create", "ns", opPolTestNS) - DeferCleanup(func() { - utils.Kubectl("delete", "ns", opPolTestNS) - }) - + preFunc() + KubectlTarget("delete", "crd", "--selector=olm.managed=true") createObjWithParent(parentPolicyYAML, parentPolicyName, opPolYAML, opPolTestNS, gvrPolicy, gvrOperatorPolicy) }) @@ -1465,7 +1448,7 @@ var _ = Describe("Testing OperatorPolicy", Ordered, func() { `[{"op": "replace", "path": "/spec/remediationAction", "value": "enforce"}]`) By("Waiting for a CRD to appear, which should indicate the operator is installing") Eventually(func(ctx SpecContext) *unstructured.Unstructured { - crd, _ := clientManagedDynamic.Resource(gvrCRD).Get(ctx, + crd, _ := targetK8sDynamic.Resource(gvrCRD).Get(ctx, "quayregistries.quay.redhat.com", metav1.GetOptions{}) return crd @@ -1473,7 +1456,7 @@ var _ = Describe("Testing OperatorPolicy", Ordered, func() { By("Waiting for the Deployment to be available, indicating the installation is complete") Eventually(func(g Gomega) { - dep, err := clientManagedDynamic.Resource(gvrDeployment).Namespace(opPolTestNS).Get( + dep, err := targetK8sDynamic.Resource(gvrDeployment).Namespace(opPolTestNS).Get( ctx, "quay-operator-tng", metav1.GetOptions{}) g.Expect(err).NotTo(HaveOccurred()) g.Expect(dep).NotTo(BeNil()) @@ -1519,10 +1502,9 @@ var _ = Describe("Testing OperatorPolicy", Ordered, func() { ) BeforeAll(func() { - utils.Kubectl("create", "ns", opPolTestNS) + preFunc() DeferCleanup(func() { - utils.Kubectl("delete", "ns", opPolTestNS) - utils.Kubectl("delete", "ns", "nonexist-testns") + KubectlTarget("delete", "ns", "nonexist-testns") }) createObjWithParent(parentPolicyYAML, parentPolicyName, @@ -1621,7 +1603,7 @@ var _ = Describe("Testing OperatorPolicy", Ordered, func() { ) }) It("Should update the status after the namespace is created", func() { - utils.Kubectl("create", "namespace", "nonexist-testns") + KubectlTarget("create", "namespace", "nonexist-testns") check( opPolName, true, @@ -1658,11 +1640,8 @@ var _ = Describe("Testing OperatorPolicy", Ordered, func() { ) BeforeAll(func() { - utils.Kubectl("create", "ns", opPolTestNS) - utils.Kubectl("delete", "crd", "--selector=olm.managed=true") - DeferCleanup(func() { - utils.Kubectl("delete", "ns", opPolTestNS) - }) + preFunc() + KubectlTarget("delete", "crd", "--selector=olm.managed=true") createObjWithParent(parentPolicyYAML, parentPolicyName, opPolYAML, opPolTestNS, gvrPolicy, gvrOperatorPolicy) @@ -1819,7 +1798,7 @@ var _ = Describe("Testing OperatorPolicy", Ordered, func() { By("Waiting for a CRD to appear, which should indicate the operator is installing") Eventually(func(ctx SpecContext) *unstructured.Unstructured { - crd, _ := clientManagedDynamic.Resource(gvrCRD).Get(ctx, + crd, _ := targetK8sDynamic.Resource(gvrCRD).Get(ctx, "quayregistries.quay.redhat.com", metav1.GetOptions{}) return crd @@ -2200,11 +2179,11 @@ var _ = Describe("Testing OperatorPolicy", Ordered, func() { ) By("Checking that certain (named) resources are still there") - utils.GetWithTimeout(clientManagedDynamic, gvrClusterServiceVersion, "quay-operator.v3.10.0", + utils.GetWithTimeout(targetK8sDynamic, gvrClusterServiceVersion, "quay-operator.v3.10.0", opPolTestNS, true, eventuallyTimeout) - utils.GetWithTimeout(clientManagedDynamic, gvrSubscription, subName, + utils.GetWithTimeout(targetK8sDynamic, gvrSubscription, subName, opPolTestNS, true, eventuallyTimeout) - utils.GetWithTimeout(clientManagedDynamic, gvrCRD, "quayregistries.quay.redhat.com", + utils.GetWithTimeout(targetK8sDynamic, gvrCRD, "quayregistries.quay.redhat.com", "", true, eventuallyTimeout) }) It("Should report a special status when the resources are stuck", func(ctx SpecContext) { @@ -2249,11 +2228,11 @@ var _ = Describe("Testing OperatorPolicy", Ordered, func() { continue } - utils.Kubectl("patch", objKind, objName, "-n", opPolTestNS, "--type=json", "-p", + KubectlTarget("patch", objKind, objName, "-n", opPolTestNS, "--type=json", "-p", `[{"op": "add", "path": "/metadata/finalizers", "value": ["donutdelete"]}]`) DeferCleanup(func() { By("removing the finalizer from " + objKind + " " + objName) - utils.Kubectl("patch", objKind, objName, "-n", opPolTestNS, "--type=json", "-p", + KubectlTarget("patch", objKind, objName, "-n", opPolTestNS, "--type=json", "-p", `[{"op": "remove", "path": "/metadata/finalizers"}]`) }) } @@ -2397,11 +2376,11 @@ var _ = Describe("Testing OperatorPolicy", Ordered, func() { }) It("Should report things as gone after the finalizers are removed", func() { By("Checking that certain (named) resources are not there, indicating the removal was completed") - utils.GetWithTimeout(clientManagedDynamic, gvrClusterServiceVersion, "quay-operator.v3.10.0", + utils.GetWithTimeout(targetK8sDynamic, gvrClusterServiceVersion, "quay-operator.v3.10.0", opPolTestNS, false, eventuallyTimeout) - utils.GetWithTimeout(clientManagedDynamic, gvrSubscription, subName, + utils.GetWithTimeout(targetK8sDynamic, gvrSubscription, subName, opPolTestNS, false, eventuallyTimeout) - utils.GetWithTimeout(clientManagedDynamic, gvrCRD, "quayregistries.quay.redhat.com", + utils.GetWithTimeout(targetK8sDynamic, gvrCRD, "quayregistries.quay.redhat.com", "", false, eventuallyTimeout) By("Checking the OperatorPolicy status") @@ -2578,24 +2557,21 @@ var _ = Describe("Testing OperatorPolicy", Ordered, func() { ) BeforeAll(func(ctx SpecContext) { - utils.Kubectl("create", "ns", opPolTestNS) - utils.Kubectl("delete", "crd", "--selector=olm.managed=true") - DeferCleanup(func() { - utils.Kubectl("delete", "ns", opPolTestNS) - }) + preFunc() + KubectlTarget("delete", "crd", "--selector=olm.managed=true") createObjWithParent(parentPolicyYAML, parentPolicyName, opPolYAML, opPolTestNS, gvrPolicy, gvrOperatorPolicy) }) AfterAll(func(ctx SpecContext) { - crd, err := clientManagedDynamic.Resource(gvrCRD).Get( + crd, err := targetK8sDynamic.Resource(gvrCRD).Get( ctx, "quayregistries.quay.redhat.com", metav1.GetOptions{}) if k8serrors.IsNotFound(err) { return } Expect(crd).NotTo(BeNil()) - utils.Kubectl("patch", "crd", "quayregistries.quay.redhat.com", "--type=json", "-p", + KubectlTarget("patch", "crd", "quayregistries.quay.redhat.com", "--type=json", "-p", `[{"op": "remove", "path": "/metadata/finalizers"}]`) }) It("Initially behaves correctly as musthave", func(ctx SpecContext) { @@ -2606,7 +2582,7 @@ var _ = Describe("Testing OperatorPolicy", Ordered, func() { By("Waiting for a CRD to appear, which should indicate the operator is installing") Eventually(func(ctx SpecContext) *unstructured.Unstructured { - crd, _ := clientManagedDynamic.Resource(gvrCRD).Get(ctx, + crd, _ := targetK8sDynamic.Resource(gvrCRD).Get(ctx, "quayregistries.quay.redhat.com", metav1.GetOptions{}) return crd @@ -2638,7 +2614,7 @@ var _ = Describe("Testing OperatorPolicy", Ordered, func() { ) By("Adding a finalizer to the CRD") - utils.Kubectl("patch", "crd", "quayregistries.quay.redhat.com", "--type=json", "-p", + KubectlTarget("patch", "crd", "quayregistries.quay.redhat.com", "--type=json", "-p", `[{"op": "add", "path": "/metadata/finalizers", "value": ["donutdelete"]}]`) // cleanup for this is handled in an AfterAll }) @@ -2671,7 +2647,7 @@ var _ = Describe("Testing OperatorPolicy", Ordered, func() { ) }) It("Should become compliant after the finalizer is removed", func(ctx SpecContext) { - utils.Kubectl("patch", "crd", "quayregistries.quay.redhat.com", "--type=json", "-p", + KubectlTarget("patch", "crd", "quayregistries.quay.redhat.com", "--type=json", "-p", `[{"op": "remove", "path": "/metadata/finalizers"}]`) check( @@ -2708,10 +2684,7 @@ var _ = Describe("Testing OperatorPolicy", Ordered, func() { ) BeforeEach(func() { - utils.Kubectl("create", "ns", opPolTestNS) - DeferCleanup(func() { - utils.Kubectl("delete", "ns", opPolTestNS) - }) + preFunc() createObjWithParent(parentPolicyYAML, parentPolicyName, opPolYAML, opPolTestNS, gvrPolicy, gvrOperatorPolicy) @@ -2719,7 +2692,7 @@ var _ = Describe("Testing OperatorPolicy", Ordered, func() { It("should not report an operator group that does not match the spec", func() { // create the extra operator group - utils.Kubectl("apply", "-f", "../resources/case38_operator_install/incorrect-operator-group.yaml", + KubectlTarget("apply", "-f", "../resources/case38_operator_install/incorrect-operator-group.yaml", "-n", opPolTestNS) // change the operator policy to mustnothave utils.Kubectl("patch", "operatorpolicy", opPolName, "-n", opPolTestNS, "--type=json", "-p", @@ -2758,10 +2731,7 @@ var _ = Describe("Testing OperatorPolicy", Ordered, func() { ) BeforeEach(func() { - utils.Kubectl("create", "ns", opPolTestNS) - DeferCleanup(func() { - utils.Kubectl("delete", "ns", opPolTestNS) - }) + preFunc() createObjWithParent(parentPolicyYAML, parentPolicyName, opPolYAML, opPolTestNS, gvrPolicy, gvrOperatorPolicy) @@ -2797,11 +2767,8 @@ var _ = Describe("Testing OperatorPolicy", Ordered, func() { ) BeforeEach(func() { - utils.Kubectl("create", "ns", opPolTestNS) - utils.Kubectl("delete", "crd", "--selector=olm.managed=true") - DeferCleanup(func() { - utils.Kubectl("delete", "ns", opPolTestNS) - }) + preFunc() + KubectlTarget("delete", "crd", "--selector=olm.managed=true") createObjWithParent(parentPolicyYAML, parentPolicyName, opPolYAML, opPolTestNS, gvrPolicy, gvrOperatorPolicy) @@ -2816,7 +2783,7 @@ var _ = Describe("Testing OperatorPolicy", Ordered, func() { By("Waiting for a CRD to appear, which should indicate the operator is installing.") Eventually(func(ctx SpecContext) *unstructured.Unstructured { - crd, _ := clientManagedDynamic.Resource(gvrCRD).Get(ctx, + crd, _ := targetK8sDynamic.Resource(gvrCRD).Get(ctx, "quayregistries.quay.redhat.com", metav1.GetOptions{}) return crd @@ -2827,7 +2794,7 @@ var _ = Describe("Testing OperatorPolicy", Ordered, func() { By("Verifying that an operator group exists") Eventually(func(g Gomega) []unstructured.Unstructured { - list, err := clientManagedDynamic.Resource(gvrOperatorGroup).Namespace(opPolTestNS). + list, err := targetK8sDynamic.Resource(gvrOperatorGroup).Namespace(opPolTestNS). List(ctx, metav1.ListOptions{}) g.Expect(err).NotTo(HaveOccurred()) @@ -2840,7 +2807,7 @@ var _ = Describe("Testing OperatorPolicy", Ordered, func() { By("Verifying that the operator group was removed") Eventually(func(g Gomega) []unstructured.Unstructured { - list, err := clientManagedDynamic.Resource(gvrOperatorGroup).Namespace(opPolTestNS). + list, err := targetK8sDynamic.Resource(gvrOperatorGroup).Namespace(opPolTestNS). List(ctx, metav1.ListOptions{}) g.Expect(err).NotTo(HaveOccurred()) @@ -2859,7 +2826,7 @@ var _ = Describe("Testing OperatorPolicy", Ordered, func() { By("Waiting for a CRD to appear, which should indicate the operator is installing.") Eventually(func(ctx SpecContext) *unstructured.Unstructured { - crd, _ := clientManagedDynamic.Resource(gvrCRD).Get(ctx, + crd, _ := targetK8sDynamic.Resource(gvrCRD).Get(ctx, "quayregistries.quay.redhat.com", metav1.GetOptions{}) return crd @@ -2870,7 +2837,7 @@ var _ = Describe("Testing OperatorPolicy", Ordered, func() { By("Verifying that an operator group exists") Eventually(func(g Gomega) []unstructured.Unstructured { - list, err := clientManagedDynamic.Resource(gvrOperatorGroup).Namespace(opPolTestNS). + list, err := targetK8sDynamic.Resource(gvrOperatorGroup).Namespace(opPolTestNS). List(ctx, metav1.ListOptions{}) g.Expect(err).NotTo(HaveOccurred()) @@ -2883,7 +2850,7 @@ var _ = Describe("Testing OperatorPolicy", Ordered, func() { By("Verifying that the operator group was removed") Eventually(func(g Gomega) []unstructured.Unstructured { - list, err := clientManagedDynamic.Resource(gvrOperatorGroup).Namespace(opPolTestNS). + list, err := targetK8sDynamic.Resource(gvrOperatorGroup).Namespace(opPolTestNS). List(ctx, metav1.ListOptions{}) g.Expect(err).NotTo(HaveOccurred()) @@ -2902,7 +2869,7 @@ var _ = Describe("Testing OperatorPolicy", Ordered, func() { By("Waiting for a CRD to appear, which should indicate the operator is installing.") Eventually(func(ctx SpecContext) *unstructured.Unstructured { - crd, _ := clientManagedDynamic.Resource(gvrCRD).Get(ctx, + crd, _ := targetK8sDynamic.Resource(gvrCRD).Get(ctx, "quayregistries.quay.redhat.com", metav1.GetOptions{}) return crd @@ -2913,7 +2880,7 @@ var _ = Describe("Testing OperatorPolicy", Ordered, func() { By("Verifying that an operator group exists") Eventually(func(g Gomega) []unstructured.Unstructured { - list, err := clientManagedDynamic.Resource(gvrOperatorGroup).Namespace(opPolTestNS). + list, err := targetK8sDynamic.Resource(gvrOperatorGroup).Namespace(opPolTestNS). List(ctx, metav1.ListOptions{}) g.Expect(err).NotTo(HaveOccurred()) @@ -2921,14 +2888,14 @@ var _ = Describe("Testing OperatorPolicy", Ordered, func() { }, eventuallyTimeout, 3, ctx).ShouldNot(BeEmpty()) By("Creating and setting an owner for the operator group") - utils.Kubectl("create", "configmap", "ownercm", "-n", opPolTestNS, "--from-literal=foo=bar") + KubectlTarget("create", "configmap", "ownercm", "-n", opPolTestNS, "--from-literal=foo=bar") - ownerCM := utils.GetWithTimeout(clientManagedDynamic, gvrConfigMap, "ownercm", + ownerCM := utils.GetWithTimeout(targetK8sDynamic, gvrConfigMap, "ownercm", opPolTestNS, true, eventuallyTimeout) ownerUID := string(ownerCM.GetUID()) Expect(ownerUID).NotTo(BeEmpty()) - utils.Kubectl("patch", "operatorgroup", "scoped-operator-group", "-n", opPolTestNS, "--type=json", "-p", + KubectlTarget("patch", "operatorgroup", "scoped-operator-group", "-n", opPolTestNS, "--type=json", "-p", `[{"op": "add", "path": "/metadata/ownerReferences", "value": [{"apiVersion": "v1", "kind": "ConfigMap", "name": "ownercm", "uid": "`+ownerUID+`"}]}]`) @@ -2938,7 +2905,7 @@ var _ = Describe("Testing OperatorPolicy", Ordered, func() { By("Verifying the operator group was not removed") Consistently(func(g Gomega) []unstructured.Unstructured { - list, err := clientManagedDynamic.Resource(gvrOperatorGroup).Namespace(opPolTestNS). + list, err := targetK8sDynamic.Resource(gvrOperatorGroup).Namespace(opPolTestNS). List(ctx, metav1.ListOptions{}) g.Expect(err).NotTo(HaveOccurred()) @@ -2955,7 +2922,7 @@ var _ = Describe("Testing OperatorPolicy", Ordered, func() { By("Waiting for a CRD to appear, which should indicate the operator is installing.") Eventually(func(ctx SpecContext) *unstructured.Unstructured { - crd, _ := clientManagedDynamic.Resource(gvrCRD).Get(ctx, + crd, _ := targetK8sDynamic.Resource(gvrCRD).Get(ctx, "quayregistries.quay.redhat.com", metav1.GetOptions{}) return crd @@ -2966,7 +2933,7 @@ var _ = Describe("Testing OperatorPolicy", Ordered, func() { By("Verifying that an operator group exists") Eventually(func(g Gomega) []unstructured.Unstructured { - list, err := clientManagedDynamic.Resource(gvrOperatorGroup).Namespace(opPolTestNS). + list, err := targetK8sDynamic.Resource(gvrOperatorGroup).Namespace(opPolTestNS). List(ctx, metav1.ListOptions{}) g.Expect(err).NotTo(HaveOccurred()) @@ -2990,7 +2957,7 @@ var _ = Describe("Testing OperatorPolicy", Ordered, func() { By("Verifying the operator group was not removed") Consistently(func(g Gomega) []unstructured.Unstructured { - list, err := clientManagedDynamic.Resource(gvrOperatorGroup).Namespace(opPolTestNS). + list, err := targetK8sDynamic.Resource(gvrOperatorGroup).Namespace(opPolTestNS). List(ctx, metav1.ListOptions{}) g.Expect(err).NotTo(HaveOccurred()) @@ -3005,10 +2972,7 @@ var _ = Describe("Testing OperatorPolicy", Ordered, func() { ) BeforeEach(func() { - utils.Kubectl("create", "ns", opPolTestNS) - DeferCleanup(func() { - utils.Kubectl("delete", "ns", opPolTestNS) - }) + preFunc() createObjWithParent(parentPolicyYAML, parentPolicyName, opPolYAML, opPolTestNS, gvrPolicy, gvrOperatorPolicy) @@ -3040,12 +3004,9 @@ var _ = Describe("Testing OperatorPolicy", Ordered, func() { ) BeforeAll(func() { - utils.Kubectl("create", "ns", opPolTestNS) - DeferCleanup(func() { - utils.Kubectl("delete", "ns", opPolTestNS) - }) + preFunc() - utils.Kubectl("apply", "-f", configmapYAML, "-n", opPolTestNS) + KubectlTarget("apply", "-f", configmapYAML, "-n", opPolTestNS) createObjWithParent(parentPolicyYAML, parentPolicyName, opPolYAML, opPolTestNS, gvrPolicy, gvrOperatorPolicy) @@ -3081,7 +3042,7 @@ var _ = Describe("Testing OperatorPolicy", Ordered, func() { ) By("Verifying the targetNamespaces in the OperatorGroup") - og, err := clientManagedDynamic.Resource(gvrOperatorGroup).Namespace(opPolTestNS). + og, err := targetK8sDynamic.Resource(gvrOperatorGroup).Namespace(opPolTestNS). Get(ctx, opGroupName, metav1.GetOptions{}) Expect(err).NotTo(HaveOccurred()) Expect(og).NotTo(BeNil()) @@ -3092,7 +3053,7 @@ var _ = Describe("Testing OperatorPolicy", Ordered, func() { Expect(targetNamespaces).To(HaveExactElements("foo", "bar", opPolTestNS)) By("Verifying the Subscription channel") - sub, err := clientManagedDynamic.Resource(gvrSubscription).Namespace(opPolTestNS). + sub, err := targetK8sDynamic.Resource(gvrSubscription).Namespace(opPolTestNS). Get(ctx, subName, metav1.GetOptions{}) Expect(err).NotTo(HaveOccurred()) Expect(sub).NotTo(BeNil()) @@ -3104,7 +3065,7 @@ var _ = Describe("Testing OperatorPolicy", Ordered, func() { }) It("Should update the subscription after the configmap is updated", func(ctx SpecContext) { - utils.Kubectl("patch", "configmap", "op-config", "-n", opPolTestNS, "--type=json", "-p", + KubectlTarget("patch", "configmap", "op-config", "-n", opPolTestNS, "--type=json", "-p", `[{"op": "replace", "path": "/data/channel", "value": "stable-3.10"}]`) check( diff --git a/test/e2e/e2e_suite_test.go b/test/e2e/e2e_suite_test.go index 62517c96..dd9312bc 100644 --- a/test/e2e/e2e_suite_test.go +++ b/test/e2e/e2e_suite_test.go @@ -54,8 +54,14 @@ var ( gvrInstallPlan schema.GroupVersionResource gvrClusterServiceVersion schema.GroupVersionResource defaultImageRegistry string + IsHosted bool + targetK8sClient kubernetes.Interface + targetK8sDynamic dynamic.Interface + KubectlTarget func(args ...string) ) +const targetEnvName = "TARGET_KUBECONFIG_PATH" + func TestE2e(t *testing.T) { RegisterFailHandler(Fail) RunSpecs(t, "Config policy controller e2e Suite") @@ -66,6 +72,11 @@ func init() { klog.InitFlags(nil) flag.StringVar(&kubeconfigManaged, "kubeconfig_managed", "../../kubeconfig_managed_e2e", "Location of the kubeconfig to use; defaults to current kubeconfig if set to an empty string") + + flag.BoolVar( + &IsHosted, "is_hosted", false, + "Whether is hosted mode or not", + ) } var _ = BeforeSuite(func() { @@ -156,6 +167,36 @@ var _ = BeforeSuite(func() { }, metav1.CreateOptions{})).NotTo(BeNil()) } Expect(namespaces.Get(context.TODO(), testNamespace, metav1.GetOptions{})).NotTo(BeNil()) + + if IsHosted { + By("Checking that the " + targetEnvName + " environment variable is valid") + altKubeconfigPath := os.Getenv(targetEnvName) + Expect(altKubeconfigPath).ToNot(Equal("")) + + targetK8sConfig, err := clientcmd.BuildConfigFromFlags("", altKubeconfigPath) + Expect(err).ToNot(HaveOccurred()) + + targetK8sClient, err = kubernetes.NewForConfig(targetK8sConfig) + Expect(err).ToNot(HaveOccurred()) + + targetK8sDynamic, err = dynamic.NewForConfig(targetK8sConfig) + Expect(err).ToNot(HaveOccurred()) + } else { + targetK8sClient = clientManaged + targetK8sDynamic = clientManagedDynamic + + } + + KubectlTarget = func(args ...string) { + kubeconfig := "../../kubeconfig_managed_e2e" + if IsHosted { + kubeconfig = "../../kubeconfig_managed2_e2e" + } + + args = append(args, "--kubeconfig="+kubeconfig) + + utils.Kubectl(args...) + } }) func NewKubeClient(url, kubeconfig, context string) kubernetes.Interface { diff --git a/test/utils/utils.go b/test/utils/utils.go index 7a626360..bfbf22fe 100644 --- a/test/utils/utils.go +++ b/test/utils/utils.go @@ -150,6 +150,10 @@ func GetMatchingEvents( // Kubectl executes kubectl commands func Kubectl(args ...string) { + if !strings.HasPrefix(args[len(args)-1], "--kubeconfig=") { + args = append(args, "--kubeconfig="+"../../kubeconfig_managed_e2e") + } + cmd := exec.Command("kubectl", args...) output, err := cmd.CombinedOutput() From f2904a6b43cdde48ec4c97740e0430f240e79174 Mon Sep 17 00:00:00 2001 From: Justin Kulikauskas Date: Thu, 2 May 2024 14:51:34 -0400 Subject: [PATCH 2/2] Reduce number of related InstallPlans Previously, the operator policy controller would list all InstallPlans in the subscription namespace and filter down to ones it considered relevant. Among other things, it used OwnerReferences to do this filtering, but those are inconsistently applied by OLM. Now, it only looks at InstallPlans labelled specifically for the subscription in the policy, which seems to be much more reliably set and updated by OLM. Generally, only one InstallPlan will have the label, which makes it more possible to unambiguously assign a compliance to it based on its phase (previously, it was unclear what to assign to "historic" InstallPlans). Much of the controller logic still handles the possibility of there being multiple relevant InstallPlans, for robustness. Refs: - https://issues.redhat.com/browse/ACM-11025 Signed-off-by: Justin Kulikauskas (cherry picked from commit c636d7d59e9a30b814cdbb4e54a30c7894493972) --- controllers/operatorpolicy_controller.go | 40 ++++----------------- controllers/operatorpolicy_status.go | 10 +++--- test/e2e/case38_install_operator_test.go | 44 ++---------------------- 3 files changed, 13 insertions(+), 81 deletions(-) diff --git a/controllers/operatorpolicy_controller.go b/controllers/operatorpolicy_controller.go index 9e668fe2..a678a8f9 100644 --- a/controllers/operatorpolicy_controller.go +++ b/controllers/operatorpolicy_controller.go @@ -1176,50 +1176,27 @@ func (r *OperatorPolicyReconciler) handleInstallPlan( } watcher := opPolIdentifier(policy.Namespace, policy.Name) + selector := subLabelSelector(sub) - foundInstallPlans, err := r.DynamicWatcher.List( - watcher, installPlanGVK, sub.Namespace, labels.Everything()) + installPlans, err := r.DynamicWatcher.List(watcher, installPlanGVK, sub.Namespace, selector) if err != nil { return false, fmt.Errorf("error listing InstallPlans: %w", err) } - ownedInstallPlans := make([]unstructured.Unstructured, 0, len(foundInstallPlans)) - selector := subLabelSelector(sub) - - for _, installPlan := range foundInstallPlans { - // sometimes the OwnerReferences aren't correct, but the label should be - if selector.Matches(labels.Set(installPlan.GetLabels())) { - ownedInstallPlans = append(ownedInstallPlans, installPlan) - - break - } - - for _, owner := range installPlan.GetOwnerReferences() { - match := owner.Name == sub.Name && - owner.Kind == subscriptionGVK.Kind && - owner.APIVersion == subscriptionGVK.GroupVersion().String() - if match { - ownedInstallPlans = append(ownedInstallPlans, installPlan) - - break - } - } - } - // InstallPlans are generally kept in order to provide a history of actions on the cluster, but // they can be deleted without impacting the installed operator. So, not finding any should not // be considered a reason for NonCompliance, regardless of musthave or mustnothave. - if len(ownedInstallPlans) == 0 { + if len(installPlans) == 0 { return updateStatus(policy, noInstallPlansCond, noInstallPlansObj(sub.Namespace)), nil } if policy.Spec.ComplianceType.IsMustHave() { - changed, err := r.musthaveInstallPlan(ctx, policy, sub, ownedInstallPlans) + changed, err := r.musthaveInstallPlan(ctx, policy, sub, installPlans) return changed, err } - return r.mustnothaveInstallPlan(policy, ownedInstallPlans) + return r.mustnothaveInstallPlan(policy, installPlans) } func (r *OperatorPolicyReconciler) musthaveInstallPlan( @@ -1233,7 +1210,6 @@ func (r *OperatorPolicyReconciler) musthaveInstallPlan( ipsRequiringApproval := make([]unstructured.Unstructured, 0) anyInstalling := false currentPlanFailed := false - selector := subLabelSelector(sub) // Construct the relevant relatedObjects, and collect any that might be considered for approval for i, installPlan := range ownedInstallPlans { @@ -1253,11 +1229,7 @@ func (r *OperatorPolicyReconciler) musthaveInstallPlan( // consider some special phases switch phase { case string(operatorv1alpha1.InstallPlanPhaseRequiresApproval): - // only consider InstallPlans with this label for approval - this label is supposed to - // indicate the "current" InstallPlan for this subscription. - if selector.Matches(labels.Set(installPlan.GetLabels())) { - ipsRequiringApproval = append(ipsRequiringApproval, installPlan) - } + ipsRequiringApproval = append(ipsRequiringApproval, installPlan) case string(operatorv1alpha1.InstallPlanPhaseInstalling): anyInstalling = true case string(operatorv1alpha1.InstallPlanFailed): diff --git a/controllers/operatorpolicy_status.go b/controllers/operatorpolicy_status.go index 053c7576..9ef2f0e5 100644 --- a/controllers/operatorpolicy_status.go +++ b/controllers/operatorpolicy_status.go @@ -1042,9 +1042,8 @@ func noInstallPlansObj(namespace string) policyv1.RelatedObject { } // existingInstallPlanObj returns a RelatedObject for the InstallPlan, with a reason -// like 'The InstallPlan is ____' based on the phase. Usually the object will not -// have a compliance associated with it, but if it requires approval or is actively -// installing, then it will be NonCompliant. +// like 'The InstallPlan is ____' based on the phase. When the InstallPlan is in phase +// 'Complete', the object will be Compliant, otherwise it will be NonCompliant. func existingInstallPlanObj(ip client.Object, phase string) policyv1.RelatedObject { relObj := policyv1.RelatedObject{ Object: policyv1.ObjectResourceFromObj(ip), @@ -1064,8 +1063,9 @@ func existingInstallPlanObj(ip client.Object, phase string) policyv1.RelatedObje // FUTURE: check policy.spec.statusConfig.upgradesAvailable to determine `compliant`. // For now, assume it is set to 'NonCompliant' relObj.Compliant = string(policyv1.NonCompliant) - case string(operatorv1alpha1.InstallPlanPhaseInstalling): - // if it's still installing, then it shouldn't be considered compliant yet. + case string(operatorv1alpha1.InstallPlanPhaseComplete): + relObj.Compliant = string(policyv1.Compliant) + default: relObj.Compliant = string(policyv1.NonCompliant) } diff --git a/test/e2e/case38_install_operator_test.go b/test/e2e/case38_install_operator_test.go index 02743aed..966a7f97 100644 --- a/test/e2e/case38_install_operator_test.go +++ b/test/e2e/case38_install_operator_test.go @@ -1329,16 +1329,6 @@ var _ = Describe("Testing OperatorPolicy", Ordered, Label("supports-hosted"), fu opPolName, false, []policyv1.RelatedObject{{ - Object: policyv1.ObjectResource{ - Kind: "InstallPlan", - APIVersion: "operators.coreos.com/v1alpha1", - Metadata: policyv1.ObjectMetadata{ - Namespace: opPolTestNS, - Name: firstInstallPlanName, - }, - }, - Reason: "The InstallPlan is Complete", - }, { Object: policyv1.ObjectResource{ Kind: "InstallPlan", APIVersion: "operators.coreos.com/v1alpha1", @@ -1376,16 +1366,6 @@ var _ = Describe("Testing OperatorPolicy", Ordered, Label("supports-hosted"), fu opPolName, false, []policyv1.RelatedObject{{ - Object: policyv1.ObjectResource{ - Kind: "InstallPlan", - APIVersion: "operators.coreos.com/v1alpha1", - Metadata: policyv1.ObjectMetadata{ - Namespace: opPolTestNS, - Name: firstInstallPlanName, - }, - }, - Reason: "The InstallPlan is Complete", - }, { Object: policyv1.ObjectResource{ Kind: "InstallPlan", APIVersion: "operators.coreos.com/v1alpha1", @@ -1394,7 +1374,8 @@ var _ = Describe("Testing OperatorPolicy", Ordered, Label("supports-hosted"), fu Name: secondInstallPlanName, }, }, - Reason: "The InstallPlan is Complete", + Compliant: "Compliant", + Reason: "The InstallPlan is Complete", }}, metav1.Condition{ Type: "InstallPlanCompliant", @@ -1868,16 +1849,6 @@ var _ = Describe("Testing OperatorPolicy", Ordered, Label("supports-hosted"), fu }, Compliant: "Compliant", Reason: "Resource found but will not be handled in mustnothave mode", - }, { - Object: policyv1.ObjectResource{ - Kind: "InstallPlan", - APIVersion: "operators.coreos.com/v1alpha1", - Metadata: policyv1.ObjectMetadata{ - Namespace: opPolTestNS, - }, - }, - Compliant: "Compliant", - Reason: "Resource found but will not be handled in mustnothave mode", }}, metav1.Condition{ Type: "InstallPlanCompliant", @@ -2304,17 +2275,6 @@ var _ = Describe("Testing OperatorPolicy", Ordered, Label("supports-hosted"), fu }, Compliant: "Compliant", Reason: "Resource found but will not be handled in mustnothave mode", - }, { - Object: policyv1.ObjectResource{ - Kind: "InstallPlan", - APIVersion: "operators.coreos.com/v1alpha1", - Metadata: policyv1.ObjectMetadata{ - Name: installPlanName, - Namespace: opPolTestNS, - }, - }, - Compliant: "Compliant", - Reason: "Resource found but will not be handled in mustnothave mode", }}, metav1.Condition{ Type: "InstallPlanCompliant",