Merges gitrepo and gitopts controllers

Moves all the functionality implemented in the `gitrepo` controller to the `gitopts` controller. Signed-off-by: Xavi Garcia <[email protected]>
rancher · Jun 19, 2024 · 5061a33 · 5061a33
1 parent 9f6d9b3
commit 5061a33
Show file tree

Hide file tree

Showing 23 changed files with 695 additions and 822 deletions.
diff --git a/charts/fleet/templates/deployment_gitjob.yaml b/charts/fleet/templates/deployment_gitjob.yaml
@@ -1,8 +1,13 @@
-{{- if .Values.gitops.enabled }}
+{{ $shards := list "" }}
+{{ if .Values.shards }}
+{{ $shards = concat $shards .Values.shards | uniq }}
+{{ end }}
+{{ range $shards }}
+{{- if $.Values.gitops.enabled }}
 apiVersion: apps/v1
 kind: Deployment
 metadata:
-  name: gitjob
+  name: "gitjob{{if . }}-shard-{{ . }}{{end}}"
 spec:
   selector:
     matchLabels:
@@ -11,33 +16,48 @@ spec:
     metadata:
       labels:
         app: "gitjob"
+        fleet.cattle.io/shard-id: "{{ . }}"
+        {{- if empty . }}
+        fleet.cattle.io/shard-default: "true"
+        {{- end }}
     spec:
       serviceAccountName: gitjob
       containers:
-        - image: "{{ template "system_default_registry" . }}{{ .Values.image.repository }}:{{ .Values.image.tag }}"
+        - image: "{{ template "system_default_registry" $ }}{{ $.Values.image.repository }}:{{ $.Values.image.tag }}"
           name: gitjob
           args:
           - fleetcontroller
           - gitjob
           - --gitjob-image
-          - "{{ template "system_default_registry" . }}{{ .Values.image.repository }}:{{ .Values.image.tag }}"
-          {{- if .Values.debug }}
+          - "{{ template "system_default_registry" $ }}{{ $.Values.image.repository }}:{{ $.Values.image.tag }}"
+          {{- if $.Values.debug }}
           - --debug
           {{- end }}
+          {{- if . }}
+          - --shard-id
+          - {{ quote . }}
+          {{- end }}
+          {{- if not $.Values.metrics.enabled }}
+          - --disable-metrics
+          {{- end }}
           env:
             - name: NAMESPACE
               valueFrom:
                 fieldRef:
                   fieldPath: metadata.namespace
-          {{- if .Values.proxy }}
+          {{- if $.Values.proxy }}
             - name: HTTP_PROXY
-              value: {{ .Values.proxy }}
+              value: {{ $.Values.proxy }}
             - name: HTTPS_PROXY
-              value: {{ .Values.proxy }}
+              value: {{ $.Values.proxy }}
             - name: NO_PROXY
-              value: {{ .Values.noProxy }}
+              value: {{ $.Values.noProxy }}
+          {{- end }}
+          {{- if $.Values.controller.reconciler.workers.gitrepo }}
+            - name: GITREPO_RECONCILER_WORKERS
+              value: {{ quote $.Values.controller.reconciler.workers.gitrepo }}
           {{- end }}
-          {{- if .Values.debug }}
+          {{- if $.Values.debug }}
             - name: CATTLE_DEV_MODE
               value: "true"
           {{- else }}
@@ -53,21 +73,23 @@ spec:
 {{ toYaml $.Values.extraEnv | indent 12}}
 {{- end }}
       nodeSelector: {{ include "linux-node-selector" . | nindent 8 }}
-{{- if .Values.nodeSelector }}
-{{ toYaml .Values.nodeSelector | indent 8 }}
+{{- if $.Values.nodeSelector }}
+{{ toYaml $.Values.nodeSelector | indent 8 }}
 {{- end }}
       tolerations: {{ include "linux-node-tolerations" . | nindent 8 }}
-{{- if .Values.tolerations }}
-{{ toYaml .Values.tolerations | indent 8 }}
+{{- if $.Values.tolerations }}
+{{ toYaml $.Values.tolerations | indent 8 }}
 {{- end }}
-      {{- if .Values.priorityClassName }}
-      priorityClassName: "{{.Values.priorityClassName}}"
+      {{- if $.Values.priorityClassName }}
+      priorityClassName: "{{$.Values.priorityClassName}}"
       {{- end }}
-{{- end }}
 
-{{- if not .Values.debug }}
+{{- if not $.Values.debug }}
       securityContext:
         runAsNonRoot: true
         runAsUser: 1000
         runAsGroup: 1000
 {{- end }}
+{{- end }}
+---
+{{- end }}
diff --git a/charts/fleet/templates/rbac_gitjob.yaml b/charts/fleet/templates/rbac_gitjob.yaml
@@ -45,6 +45,18 @@ rules:
       - list
       - get
       - watch
+  - apiGroups:
+      - "fleet.cattle.io"
+    resources:
+      - "bundles"
+      - "bundledeployments"
+      - "imagescans"
+    verbs:
+      - list
+      - delete
+      - get
+      - watch
+      - update
   - apiGroups:
       - ""
     resources:
@@ -57,6 +69,12 @@ rules:
       - serviceaccounts
     verbs:
       - "create"
+  - apiGroups:
+      - ""
+    resources:
+      - namespaces
+    verbs:
+      - "delete"
   - apiGroups:
       - rbac.authorization.k8s.io
     resources:

diff --git a/e2e/single-cluster/delete_namespaces_test.go b/e2e/single-cluster/delete_namespaces_test.go
@@ -72,6 +72,7 @@ var _ = Describe("delete namespaces", func() {
 	When("delete namespaces is true", func() {
 		BeforeEach(func() {
 			deleteNamespace = true
+			targetNamespace = "my-custom-namespace"
 		})
 
 		It("targetNamespace is deleted after deleting gitRepo", func() {

diff --git a/e2e/single-cluster/finalizers_test.go b/e2e/single-cluster/finalizers_test.go
@@ -43,6 +43,15 @@ var _ = Describe("Deleting a resource with finalizers", func() {
 		)
 		Expect(err).ToNot(HaveOccurred())
 
+		_, err = k.Namespace("cattle-fleet-system").Run(
+			"scale",
+			"deployment",
+			"gitjob",
+			"--replicas=1",
+			"--timeout=5s",
+		)
+		Expect(err).ToNot(HaveOccurred())
+
 		_, _ = k.Delete("gitrepo", gitrepoName)
 		_, _ = k.Delete("bundle", fmt.Sprintf("%s-%s", gitrepoName, path))
 	})
@@ -66,11 +75,11 @@ var _ = Describe("Deleting a resource with finalizers", func() {
 				return out
 			}).Should(ContainSubstring(gitrepoName))
 
-			By("scaling down the Fleet controller to 0 replicas")
+			By("scaling down the gitjob controller to 0 replicas")
 			_, err := k.Namespace("cattle-fleet-system").Run(
 				"scale",
 				"deployment",
-				"fleet-controller",
+				"gitjob",
 				"--replicas=0",
 				"--timeout=5s",
 			)
@@ -129,14 +138,14 @@ var _ = Describe("Deleting a resource with finalizers", func() {
 			_, err = k.Namespace("cattle-fleet-system").Run(
 				"scale",
 				"deployment",
-				"fleet-controller",
+				"gitjob",
 				"--replicas=1",
 				"--timeout=5s",
 			)
 			Expect(err).ToNot(HaveOccurred())
 
-			_, err = k.Delete("gitrepo", gitrepoName)
-			Expect(err).NotTo(HaveOccurred())
+			// As soon as the controller is back, it deletes the gitrepo
+			// as its delete timestamp was already set
 
 			// These resources should be deleted when the GitRepo is deleted.
 			By("checking that the auxiliary resources don't exist anymore")
@@ -243,7 +252,7 @@ var _ = Describe("Deleting a resource with finalizers", func() {
 			_, err := k.Namespace("cattle-fleet-system").Run(
 				"scale",
 				"deployment",
-				"fleet-controller",
+				"gitjob",
 				"--replicas=0",
 				"--timeout=5s",
 			)

diff --git a/e2e/single-cluster/imagescan_test.go b/e2e/single-cluster/imagescan_test.go
@@ -154,11 +154,9 @@ var _ = Describe("Image Scan dynamic tests pushing to ttl.sh", Label("infra-setu
 			}).Should(
 				MatchRegexp(fmt.Sprintf(`image: %s # {"\$imagescan": "test-scan"}`, imageTag)))
 
-			By("pushing a new tag to the registry and checking the fleet controller does not crash")
-			// store number of fleet controller restarts to compare later
-			index, ok := getFleetControllerContainerIndexInPod(k, "fleet-controller")
-			Expect(ok).To(BeTrue())
-			fleetControllerInitialRestarts := getFleetControllerRestarts(k, index)
+			By("pushing a new tag to the registry and checking the gijob controller does not crash")
+			// store number of gitopts controller restarts to compare later
+			gitjobControllerInitialRestarts := getGitjobControllerRestarts(k)
 			newTag := "0.0.0-50"
 			previousImageTag := imageTag
 			imageTag = tagAndPushImage("k8s.gcr.io/pause", image, newTag)
@@ -167,11 +165,9 @@ var _ = Describe("Image Scan dynamic tests pushing to ttl.sh", Label("infra-setu
 			// we check for 10 seconds so we're sure that the image has been scanned and the controller didn't crash
 			// Checks for number of restarts and also to the status.ready property to be more robust
 			Consistently(func() bool {
-				indexNow, ok := getFleetControllerContainerIndexInPod(k, "fleet-controller")
-				Expect(ok).To(BeTrue())
-				restarts := getFleetControllerRestarts(k, indexNow)
-				ready := getFleetControllerReady(k, indexNow)
-				return (restarts == fleetControllerInitialRestarts) && ready
+				restarts := getGitjobControllerRestarts(k)
+				ready := getGitjobControllerReady(k)
+				return (restarts == gitjobControllerInitialRestarts) && ready
 			}, 10*time.Second, 1*time.Second).Should(BeTrue())
 
 			By("checking the bundle has the original image tag")
@@ -188,45 +184,28 @@ var _ = Describe("Image Scan dynamic tests pushing to ttl.sh", Label("infra-setu
 	})
 })
 
-func getFleetControllerRestarts(k kubectl.Command, index int) int {
-	out, err := k.Namespace("cattle-fleet-system").Get("pods", "-l", "app=fleet-controller", "-l", "fleet.cattle.io/shard-id=",
+func getGitjobControllerRestarts(k kubectl.Command) int {
+	out, err := k.Namespace("cattle-fleet-system").Get("pods", "-l", "app=gitjob,fleet.cattle.io/shard-id=",
 		"--no-headers",
-		"-o", fmt.Sprintf("custom-columns=RESTARTS:.status.containerStatuses[%d].restartCount", index))
+		"-o", "custom-columns=RESTARTS:.status.containerStatuses[0].restartCount")
 	Expect(err).NotTo(HaveOccurred())
 	out = strings.TrimSuffix(out, "\n")
 	n, err := strconv.Atoi(out)
 	Expect(err).NotTo(HaveOccurred())
 	return n
 }
 
-func getFleetControllerReady(k kubectl.Command, index int) bool {
-	out, err := k.Namespace("cattle-fleet-system").Get("pods", "-l", "app=fleet-controller", "-l", "fleet.cattle.io/shard-id=",
+func getGitjobControllerReady(k kubectl.Command) bool {
+	out, err := k.Namespace("cattle-fleet-system").Get("pods", "-l", "app=gitjob,fleet.cattle.io/shard-id=",
 		"--no-headers",
-		"-o", fmt.Sprintf("custom-columns=RESTARTS:.status.containerStatuses[%d].ready", index))
+		"-o", "custom-columns=RESTARTS:.status.containerStatuses[0].ready")
 	Expect(err).NotTo(HaveOccurred())
 	out = strings.TrimSuffix(out, "\n")
 	boolValue, err := strconv.ParseBool(out)
 	Expect(err).NotTo(HaveOccurred())
 	return boolValue
 }
 
-func getFleetControllerContainerIndexInPod(k kubectl.Command, container string) (int, bool) {
-	// the fleet controller pod runs 3 containers.
-	// we need to know the index of the fleet-controller container inside the pod.
-	// get all the container names, and return the index of the given container name
-	out, err := k.Namespace("cattle-fleet-system").Get("pods", "-l", "app=fleet-controller", "-l", "fleet.cattle.io/shard-id=",
-		"--no-headers", "-o", "custom-columns=RESTARTS:.status.containerStatuses[*].name")
-	Expect(err).NotTo(HaveOccurred())
-	out = strings.TrimSuffix(out, "\n")
-	containers := strings.Split(out, ",")
-	for i, n := range containers {
-		if container == n {
-			return i, true
-		}
-	}
-	return -1, false
-}
-
 func setupRepo(k kubectl.Command, tmpdir, clonedir, repoDir string) *git.Repository {
 	// Create git secret
 	out, err := k.Create(

diff --git a/e2e/single-cluster/sharding_test.go b/e2e/single-cluster/sharding_test.go
@@ -22,11 +22,11 @@ var _ = Describe("Filtering events by shard", Label("sharding"), Ordered, func()
 	)
 
 	BeforeAll(func() {
-		// No sharded controller should have reconciled any GitRepo until this point.
+		// No sharded gitjob controller should have reconciled any GitRepo until this point.
 		for _, shard := range shards {
 			logs, err := k.Namespace("cattle-fleet-system").Logs(
 				"-l",
-				"app=fleet-controller",
+				"app=gitjob",
 				"-l",
 				fmt.Sprintf("fleet.cattle.io/shard-id=%s", shard),
 				"--tail=-1",
@@ -69,7 +69,7 @@ var _ = Describe("Filtering events by shard", Label("sharding"), Ordered, func()
 				Expect(err).ToNot(HaveOccurred())
 			})
 
-			It(fmt.Sprintf("deploys the gitrepo via the controller labeled with shard ID %s", shard), func() {
+			It(fmt.Sprintf("deploys the gitrepo via the gitjob labeled with shard ID %s", shard), func() {
 				By("checking the configmap exists")
 				Eventually(func() string {
 					out, _ := k.Namespace(targetNamespace).Get("configmaps")
@@ -80,7 +80,7 @@ var _ = Describe("Filtering events by shard", Label("sharding"), Ordered, func()
 					Eventually(func(g Gomega) {
 						logs, err := k.Namespace("cattle-fleet-system").Logs(
 							"-l",
-							"app=fleet-controller",
+							"app=gitjob",
 							"-l",
 							fmt.Sprintf("fleet.cattle.io/shard-id=%s", s),
 							"--tail=100",
@@ -94,7 +94,7 @@ var _ = Describe("Filtering events by shard", Label("sharding"), Ordered, func()
 						if s == shard {
 							g.Expect(hasReconciledGitRepo).To(BeTrueBecause(
 								"GitRepo %q labeled with shard %q should have been"+
-									" deployed by controller for shard %q in namespace %q",
+									" deployed by gitjob for shard %q in namespace %q",
 								gitrepoName,
 								shard,
 								shard,
@@ -103,7 +103,7 @@ var _ = Describe("Filtering events by shard", Label("sharding"), Ordered, func()
 						} else {
 							g.Expect(hasReconciledGitRepo).To(BeFalseBecause(
 								"GitRepo %q labeled with shard %q should not have been"+
-									" deployed by controller for shard %q",
+									" deployed by gitjob for shard %q",
 								gitrepoName,
 								shard,
 								s,
@@ -152,7 +152,7 @@ var _ = Describe("Filtering events by shard", Label("sharding"), Ordered, func()
 			for _, s := range shards {
 				logs, err := k.Namespace("cattle-fleet-system").Logs(
 					"-l",
-					"app=fleet-controller",
+					"app=gitjob",
 					"-l",
 					fmt.Sprintf("fleet.cattle.io/shard-id=%s", s),
 					"--tail=100",
@@ -169,7 +169,7 @@ var _ = Describe("Filtering events by shard", Label("sharding"), Ordered, func()
 				Expect(err).ToNot(HaveOccurred())
 				Expect(hasReconciledGitRepos).To(BeFalseBecause(
 					"GitRepo labeled with shard %q should not have been deployed by"+
-						" controller for shard %q",
+						" gitjob for shard %q",
 					"unknown",
 					s,
 				))