feat: Add an annotation to allow for hub appsub bypassing Git clone operations to speed up appsub propagation to managed cluster (#431)

Signed-off-by: Mike Ng <ming@redhat.com>
(cherry picked from commit 1c7c4c9a00df998c16a6e9713077d6ae4e035982)

Author: mikeshng

build/e2e-kc.sh

@@ -39,6 +39,38 @@ else
     exit 1
 fi
 
+### 000-git-skip-clone
+echo "STARTING test 000-git-skip-clone"
+kubectl config use-context kind-hub
+kubectl apply -f test/e2e/cases/000-git-skip-clone/
+sleep 20
+if kubectl get subscriptions.apps.open-cluster-management.io git-hub-skip-clone | grep Propagated; then
+    echo "000-git-skip-clone: hub subscriptions.apps.open-cluster-management.io status is Propagated"
+else
+    echo "000-git-skip-clone FAILED: hub subscriptions.apps.open-cluster-management.io status is not Propagated"
+    exit 1
+fi
+
+kubectl config use-context kind-cluster1
+if kubectl get subscriptions.apps.open-cluster-management.io git-hub-skip-clone | grep Subscribed; then
+    echo "000-git-skip-clone: cluster1 subscriptions.apps.open-cluster-management.io status is Subscribed"
+else
+    echo "000-git-skip-clone FAILED: cluster1 subscriptions.apps.open-cluster-management.io status is not Subscribed"
+    exit 1
+fi
+
+if kubectl get clusterrole | grep psp | grep rook; then
+    echo "000-git-skip-clone: git manifest clusterrole is applied on the managed cluster"
+else
+    echo "000-git-skip-clone FAILED:  git manifest clusterrole is not applied on the managed cluster"
+    exit 1
+fi
+
+kubectl config use-context kind-hub
+kubectl delete -f test/e2e/cases/000-git-skip-clone/
+echo "PASSED test case 000-git-skip-clone"
+sleep 10
+
 ### 00-helm-semver
 echo "STARTING test case 00-helm-semver"
 kubectl config use-context kind-hub
@@ -49,6 +81,7 @@ else
     exit 1
 fi
 kubectl delete -f test/e2e/cases/00-helm-semver/semver_appsub.yaml
+echo "PASSED test case 00-helm-semver"
 sleep 10
 
 ### 01-placement
@@ -57,7 +90,7 @@ kubectl config use-context kind-hub
 kubectl label managedcluster cluster1 cluster.open-cluster-management.io/clusterset=app-demo --overwrite
 kubectl label managedcluster cluster1 purpose=test --overwrite
 kubectl apply -f test/e2e/cases/01-placement/
-sleep 30
+sleep 20
 
 if kubectl get subscriptions.apps.open-cluster-management.io ingress | grep Propagated; then
     echo "01-placement: hub subscriptions.apps.open-cluster-management.io status is Propagated"
@@ -90,7 +123,7 @@ fi
 
 kubectl config use-context kind-hub
 kubectl delete -f test/e2e/cases/01-placement/
-sleep 30
+sleep 20
 kubectl config use-context kind-cluster1
 if kubectl get pod | grep ingress; then
     echo "01-placement FAILED: appsub deployment pod is not deleted"
@@ -104,7 +137,7 @@ echo "PASSED test case 01-placement"
 echo "STARTING test 02-placementrule"
 kubectl config use-context kind-hub
 kubectl apply -f test/e2e/cases/02-placementrule/
-sleep 30
+sleep 20
 
 if kubectl get subscriptions.apps.open-cluster-management.io ingress | grep Propagated; then
     echo "02-placementrule: hub subscriptions.apps.open-cluster-management.io status is Propagated"
@@ -137,7 +170,7 @@ fi
 
 kubectl config use-context kind-hub
 kubectl delete -f test/e2e/cases/02-placementrule/
-sleep 30
+sleep 20
 kubectl config use-context kind-cluster1
 if kubectl get pod | grep ingress; then
     echo "02-placementrule FAILED: appsub deployment pod is not deleted"
@@ -152,7 +185,7 @@ echo "STARTING test 03-keep-namespace"
 kubectl config use-context kind-hub
 kubectl create ns test-case-03
 kubectl apply -f test/e2e/cases/03-keep-namespace/
-sleep 30
+sleep 20
 
 kubectl config use-context kind-cluster1
 if kubectl get ns test-case-03; then
@@ -164,7 +197,7 @@ fi
 
 kubectl config use-context kind-hub
 kubectl delete -f test/e2e/cases/03-keep-namespace/
-sleep 30
+sleep 20
 kubectl config use-context kind-cluster1
 if kubectl get ns test-case-03; then
     echo "03-keep-namespace: cluster1 namespace 03-keep-namespace is still present"
@@ -256,7 +289,7 @@ echo "PASSED test case 06-ansiblejob-post"
 echo "STARTING test case 07-helm-install-error"
 kubectl config use-context kind-hub
 kubectl apply -f test/e2e/cases/07-helm-install-error/
-sleep 30
+sleep 20
 kubectl config use-context kind-cluster1
 if kubectl get subscriptionstatus.apps.open-cluster-management.io ingress -o yaml | grep "phase: Failed"; then
     echo "07-helm-install-error: found failed phase in subscription status output"
@@ -273,7 +306,7 @@ echo "PASSED test case 07-helm-install-error"
 echo "STARTING test case 08-helm-upgrade-error"
 kubectl config use-context kind-hub
 kubectl apply -f test/e2e/cases/08-helm-upgrade-error/install
-sleep 30
+sleep 20
 kubectl config use-context kind-cluster1
 if kubectl get subscriptionstatus.apps.open-cluster-management.io ingress -o yaml | grep "phase: Deployed"; then
     echo "08-helm-upgrade-error: found deployed phase in subscription status output"
@@ -302,7 +335,7 @@ echo "PASSED test case 08-helm-upgrade-error"
 echo "STARTING test case 09-helm-missing-phase"
 kubectl config use-context kind-hub
 kubectl apply -f test/e2e/cases/09-helm-missing-phase/
-sleep 30
+sleep 20
 kubectl config use-context kind-cluster1
 if kubectl get subscriptionstatus.apps.open-cluster-management.io preinstall-hook -o yaml | grep "kind: Deployment"; then
     echo "09-helm-missing-phase: found deployment kind in subscription status output"
@@ -324,7 +357,7 @@ echo "PASSED test case 09-helm-missing-phase"
 echo "STARTING test 10-cluster-override-ns"
 kubectl config use-context kind-hub
 kubectl apply -f test/e2e/cases/10-cluster-override-ns/
-sleep 30
+sleep 20
 kubectl config use-context kind-cluster1
 if kubectl -n test-10 get pod | grep nginx-placement | grep Running; then
     echo "10-cluster-override-ns: appsub deployment pod status is Running"
@@ -334,7 +367,7 @@ else
 fi
 kubectl config use-context kind-hub
 kubectl delete -f test/e2e/cases/10-cluster-override-ns/
-sleep 30
+sleep 20
 kubectl config use-context kind-cluster1
 if kubectl -n test-10 get pod | grep nginx-placement; then
     echo "10-cluster-override-ns FAILED: appsub deployment pod is not deleted"
@@ -348,14 +381,14 @@ echo "PASSED test case 10-cluster-override-ns"
 echo "STARTING test 11-helm-hub-dryrun"
 kubectl config use-context kind-hub
 kubectl apply -f test/e2e/cases/11-helm-hub-dryrun/
-sleep 30
+sleep 20
 if kubectl get subscriptions.apps.open-cluster-management.io -n default ingress-appsub | grep Propagated; then
     echo "11-helm-hub-dryrun: ingress-appsub status is Propagated"
 else
     echo "11-helm-hub-dryruns FAILED:  ingress-appsub status is not Propagated"
     exit 1
 fi
-sleep 30
+sleep 20
 kubectl config use-context kind-cluster1
 
 RUN_CMD="kubectl get subscriptionstatus.apps.open-cluster-management.io -n default ingress-appsub"
@@ -382,7 +415,7 @@ echo "PASSED test case 11-helm-hub-dryrun"
 echo "STARTING test 12-helm-update"
 kubectl config use-context kind-hub
 kubectl apply -f test/e2e/cases/12-helm-update/install
-sleep 30
+sleep 20
 if kubectl get subscriptions.apps.open-cluster-management.io ingress | grep Propagated; then
     echo "12-helm-update: ingress status is Propagated"
 else
@@ -419,7 +452,7 @@ echo "PASSED test case 12-helm-update"
 echo "STARTING test 13-git-res-name"
 kubectl config use-context kind-hub
 kubectl apply -f test/e2e/cases/13-git-res-name/
-sleep 30
+sleep 20
 if kubectl get subscriptions.apps.open-cluster-management.io git-app-sub | grep Propagated; then
     echo "13-git-res-name: hub subscriptions.apps.open-cluster-management.io status is Propagated"
 else
@@ -433,7 +466,7 @@ echo "PASSED test case 13-git-res-name"
 echo "STARTING test 14-helm-appsubstatus"
 kubectl config use-context kind-hub
 kubectl apply -f test/e2e/cases/14-helm-appsubstatus/install
-sleep 30
+sleep 20
 if kubectl get subscriptionreport.apps.open-cluster-management.io ingress | grep ingress; then
     echo "14-helm-appsubstatus: ingress subscriptionreport is found"
 else
@@ -474,7 +507,7 @@ echo "PASSED test case 14-helm-appsubstatus"
 echo "STARTING test 15-git-helm"
 kubectl config use-context kind-hub
 kubectl apply -f test/e2e/cases/15-git-helm/install
-sleep 30
+sleep 20
 if kubectl get subscriptions.apps.open-cluster-management.io git-app-sub | grep Propagated; then
     echo "15-git-helm: hub subscriptions.apps.open-cluster-management.io status is Propagated"
 else
@@ -498,7 +531,7 @@ echo "PASSED test case 15-git-helm"
 echo "STARTING test 16-helm-recreate"
 kubectl config use-context kind-hub
 kubectl apply -f test/e2e/cases/16-helm-recreate
-sleep 30
+sleep 20
 if kubectl get subscriptions.apps.open-cluster-management.io ingress | grep Propagated; then
     echo "16-helm-recreate: ingress status is Propagated"
 else
@@ -605,7 +638,7 @@ echo "19-verify-git-pull-time-metric: patching successful subscription and expet
 kubectl config use-context kind-hub
 kubectl apply -f test/e2e/cases/19-verify-git-pull-time-metric/failed
 # deliver an appsub with invalid channel info, it is expected to get the failure once the appsub is deployed on the managed cluster
-sleep 30
+sleep 20
 
 echo "19-verify-git-pull-time-metric: fetching failed managed cluster metrics"
 kubectl config use-context kind-cluster1
@@ -644,7 +677,7 @@ kubectl apply -f test/e2e/cases/20-verify-propagation-time-metric/failed-no-plac
 kubectl apply -f test/e2e/cases/20-verify-propagation-time-metric/failed-placement-wrong
 kubectl apply -f test/e2e/cases/20-verify-propagation-time-metric/standalone
 kubectl apply -f test/e2e/cases/20-verify-propagation-time-metric/successful
-sleep 30
+sleep 20
 
 echo "20-verify-propagation-time-metric: fetching collected hub cluster metrics"
 collectedMcMetrics=`kubectl exec -n open-cluster-management deploy/multicluster-operators-subscription -- curl http://localhost:8381/metrics`
@@ -692,7 +725,7 @@ kubectl label managedcluster cluster1 cluster.open-cluster-management.io/cluster
 kubectl label managedcluster cluster1 purpose=test --overwrite
 
 kubectl apply -f test/e2e/cases/21-verify-local-deployment-time-metric
-sleep 30
+sleep 20
 
 kubectl config use-context kind-cluster1
 kubectl -n local-deployment-metric-test rollout status deployment/git-simple-subscription

pkg/apis/apps/v1/subscription_types.go

@@ -96,6 +96,8 @@ var (
 	AnnotationHostingDeployable = SchemeGroupVersion.Group + "/hosting-deployable"
 	// AnnotationCurrentNamespaceScoped specifies to deloy resources into subscription namespace
 	AnnotationCurrentNamespaceScoped = SchemeGroupVersion.Group + "/current-namespace-scoped"
+	// AnnotationSkipHubValidation indicates the hub subscription should skip the "dry-run" validations and proceed to propagation phase
+	AnnotationSkipHubValidation = SchemeGroupVersion.Group + "/skip-hub-validation"
 )
 
 const (

pkg/controller/mcmhub/hub.go

@@ -93,6 +93,17 @@ func (r *ReconcileSubscription) doMCMHubReconcile(sub *appv1.Subscription) error
 	// Add or sync application labels
 	r.AddAppLabels(sub)
 
+	// Skip fetching resources and creating appsub report if skip hub validation is true
+	if shouldSkipHubValidation(sub) {
+		clusters, err := r.getClustersByPlacement(sub)
+		if err != nil {
+			klog.Error("Error in getting clusters:", err)
+			return err
+		}
+
+		return r.PropagateAppSubManifestWork(sub, clusters)
+	}
+
 	var resources []*v1.ObjectReference
 
 	switch tp := strings.ToLower(string(primaryChannel.Spec.Type)); tp {

pkg/controller/mcmhub/hub_git.go

@@ -342,6 +342,11 @@ func (h *HubGitOps) RegisterBranch(subIns *subv1.Subscription) error {
 	h.logger.Info("entry register branch for appsub " + subIns.Namespace + "/" + subIns.Name)
 	defer h.logger.Info("exit register branch for appsub " + subIns.Namespace + "/" + subIns.Name)
 
+	if shouldSkipHubValidation(subIns) {
+		h.logger.Info("skipping register branch for appsub " + subIns.Namespace + "/" + subIns.Name)
+		return nil
+	}
+
 	subKey := types.NamespacedName{Name: subIns.GetName(), Namespace: subIns.GetNamespace()}
 
 	// This does not pick up new changes to channel configuration
@@ -781,3 +786,12 @@ func (h *HubGitOps) GetHooks(subIns *subv1.Subscription, hookPath string) ([]ans
 
 	return newAnsibleJobs, nil
 }
+
+func shouldSkipHubValidation(subIns *subv1.Subscription) bool {
+	annos := subIns.GetAnnotations()
+	if len(annos) > 0 && annos[subv1.AnnotationSkipHubValidation] == "true" {
+		return true
+	}
+
+	return false
+}

pkg/controller/mcmhub/hub_git_test.go

@@ -431,3 +431,50 @@ var _ = PDescribe("hub git ops", func() {
 		Eventually(detectTargetCommit(subKey, defaultCommit), specTimeOut, pullInterval).Should(Succeed())
 	})
 })
+
+var _ = Describe("shouldSkipHubValidation", func() {
+	var (
+		sub *subv1.Subscription
+	)
+	BeforeEach(func() {
+		sub = &subv1.Subscription{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      "test-sub",
+				Namespace: "default",
+			},
+		}
+	})
+	It("should return false when annotation is nil", func() {
+		Expect(shouldSkipHubValidation(sub)).To(BeFalse())
+	})
+	It("should return false when skip annotation is missing", func() {
+		sub.SetAnnotations(map[string]string{
+			"foobar": "true",
+		})
+		Expect(shouldSkipHubValidation(sub)).To(BeFalse())
+	})
+	It("should return true when skip annotation is true'", func() {
+		sub.SetAnnotations(map[string]string{
+			subv1.AnnotationSkipHubValidation: "true",
+		})
+		Expect(shouldSkipHubValidation(sub)).To(BeTrue())
+	})
+	It("should return false when skip annotation is empty", func() {
+		sub.SetAnnotations(map[string]string{
+			subv1.AnnotationSkipHubValidation: "",
+		})
+		Expect(shouldSkipHubValidation(sub)).To(BeFalse())
+	})
+	It("should return false when skip annotation is false", func() {
+		sub.SetAnnotations(map[string]string{
+			subv1.AnnotationSkipHubValidation: "false",
+		})
+		Expect(shouldSkipHubValidation(sub)).To(BeFalse())
+	})
+	It("should return false when skip annotation is non boolean string values", func() {
+		sub.SetAnnotations(map[string]string{
+			subv1.AnnotationSkipHubValidation: "foobar",
+		})
+		Expect(shouldSkipHubValidation(sub)).To(BeFalse())
+	})
+})

test/e2e/cases/000-git-skip-clone/channel.yaml

@@ -0,0 +1,10 @@
+apiVersion: apps.open-cluster-management.io/v1
+kind: Channel
+metadata:
+  name: gitops
+  namespace: default
+  annotations:
+    apps.open-cluster-management.io/reconcile-rate: high
+spec:
+  pathname: 'https://github.com/open-cluster-management-io/multicloud-operators-subscription.git'
+  type: Git

test/e2e/cases/000-git-skip-clone/placement.yaml

@@ -0,0 +1,7 @@
+apiVersion: apps.open-cluster-management.io/v1
+kind: PlacementRule
+metadata:
+  name: git-pr
+  namespace: default
+spec:
+  clusterReplicas: 2

test/e2e/cases/000-git-skip-clone/subscription.yaml

@@ -0,0 +1,15 @@
+apiVersion: apps.open-cluster-management.io/v1
+kind: Subscription
+metadata:
+  annotations:
+    apps.open-cluster-management.io/github-branch: main
+    apps.open-cluster-management.io/github-path: test/e2e/cases/13-git-res-name/resource
+    apps.open-cluster-management.io/skip-hub-validation: 'true'
+  name: git-hub-skip-clone
+  namespace: default
+spec:
+  channel: default/gitops
+  placement:
+    placementRef:
+      kind: PlacementRule
+      name: git-pr

test/e2e/cases/23-git-appsub-status-failed/02-channel.yaml

@@ -8,4 +8,4 @@ metadata:
     apps.open-cluster-management.io/reconcile-rate: high
 spec:
   type: git
-  pathname: https://github.com/philipwu08/testrepo.git
+  pathname: 'https://github.com/open-cluster-management-io/multicloud-operators-subscription.git'

test/e2e/cases/23-git-appsub-status-failed/03-subscription.yaml

@@ -4,7 +4,7 @@ metadata:
   name: guestbook-failed
   namespace: default
   annotations:
-    apps.open-cluster-management.io/github-path: guestbook-failed
+    apps.open-cluster-management.io/github-path: test/e2e/cases/23-git-appsub-status-failed/resource
     apps.open-cluster-management.io/github-branch: main    
 spec:
   channel: default/git