Merges gitrepo and gitops controllers (#2537)

* 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]> * Remove the disableGitOpts from fleet-controller It also renames the `finalizerutils` to just `finalizer` and documents its functions Signed-off-by: Xavi Garcia <[email protected]> * Moves setting UpdateGeneration to the code block where it is checked Changes documentatin as suggested in code review. Signed-off-by: Xavi Garcia <[email protected]> --------- Signed-off-by: Xavi Garcia <[email protected]>
rancher · Jun 25, 2024 · f9450f6 · f9450f6
1 parent 1774fda
commit f9450f6
Show file tree

Hide file tree

Showing 28 changed files with 885 additions and 890 deletions.
diff --git a/charts/fleet/templates/deployment.yaml b/charts/fleet/templates/deployment.yaml
@@ -89,9 +89,6 @@ spec:
         {{- end }}
         command:
         - fleetcontroller
-        {{- if not $.Values.gitops.enabled }}
-        - --disable-gitops
-        {{- end }}
         {{- if . }}
         - --shard-id
         - {{ quote . }}

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,53 @@ 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
+          {{- if $.Values.metrics.enabled }}
+          ports:
+          - containerPort: 8081
+            name: metrics
+          {{- end }}
           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 +78,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,13 @@ rules:
       - serviceaccounts
     verbs:
       - "create"
+  - apiGroups:
+      - ""
+    resources:
+      - namespaces
+    verbs:
+      - "create"
+      - "delete"
   - apiGroups:
       - rbac.authorization.k8s.io
     resources:

diff --git a/charts/fleet/templates/service_gitjob.yaml b/charts/fleet/templates/service_gitjob.yaml
@@ -11,4 +11,34 @@ spec:
       targetPort: 8080
   selector:
     app: "gitjob"
+---
+{{- if .Values.metrics.enabled }}
+{{ $shards := list "" }}
+{{ if .Values.shards }}
+{{ $shards = concat $shards .Values.shards | uniq }}
+{{ end }}
+{{ range $shards }}
+apiVersion: v1
+kind: Service
+metadata:
+  name: "monitoring-gitjob{{if . }}-shard-{{ . }}{{end}}"
+  labels:
+    app: gitjob
+spec:
+  type: ClusterIP
+  ports:
+  - port: 8081
+    targetPort: 8081
+    protocol: TCP
+    name: metrics
+  selector:
+    app: gitjob
+    {{- if empty . }}
+    fleet.cattle.io/shard-default: "true"
+    {{- else }}
+    fleet.cattle.io/shard-id: "{{ . }}"
+    {{- end }}
+---
+{{- end }}
+{{- end }}
 {{- end }}
diff --git a/...sets/metrics/fleetcontroller_service.yaml → e2e/assets/metrics/service.yaml b/...sets/metrics/fleetcontroller_service.yaml → e2e/assets/metrics/service.yaml
@@ -3,11 +3,11 @@ kind: Service
 metadata:
   name: {{ .Name }}
   labels:
-    app: fleet-controller
+    app: {{ .App }}
     env: test
 spec:
   selector:
-    app: fleet-controller
+    app: {{ .App }}
     {{- if .IsDefaultShard }}
     fleet.cattle.io/shard-default: "{{ .IsDefaultShard }}"
     {{ else }}

diff --git a/e2e/metrics/gitrepo_test.go b/e2e/metrics/gitrepo_test.go
@@ -66,10 +66,10 @@ var _ = Describe("GitRepo Metrics", Label("gitrepo"), func() {
 
 		It("should have exactly one metric of each type for the gitrepo", func() {
 			Eventually(func() error {
-				metrics, err := et.Get()
+				metrics, err := etGitjob.Get()
 				Expect(err).ToNot(HaveOccurred())
 				for _, metricName := range gitrepoMetricNames {
-					metric, err := et.FindOneMetric(
+					metric, err := etGitjob.FindOneMetric(
 						metrics,
 						metricName,
 						map[string]string{
@@ -104,10 +104,10 @@ var _ = Describe("GitRepo Metrics", Label("gitrepo"), func() {
 				var metric *metrics.Metric
 				// Expect still no metrics to be duplicated.
 				Eventually(func() error {
-					metrics, err := et.Get()
+					metrics, err := etGitjob.Get()
 					Expect(err).ToNot(HaveOccurred())
 					for _, metricName := range gitrepoMetricNames {
-						metric, err = et.FindOneMetric(
+						metric, err = etGitjob.FindOneMetric(
 							metrics,
 							metricName,
 							map[string]string{
@@ -131,10 +131,10 @@ var _ = Describe("GitRepo Metrics", Label("gitrepo"), func() {
 				Expect(err).ToNot(HaveOccurred(), out)
 
 				Eventually(func() error {
-					metrics, err := et.Get()
+					metrics, err := etGitjob.Get()
 					Expect(err).ToNot(HaveOccurred())
 					for _, metricName := range gitrepoMetricNames {
-						_, err := et.FindOneMetric(
+						_, err := etGitjob.FindOneMetric(
 							metrics,
 							metricName,
 							map[string]string{

diff --git a/e2e/metrics/suite_test.go b/e2e/metrics/suite_test.go
@@ -23,31 +23,36 @@ func TestE2E(t *testing.T) {
 var (
 	env *testenv.Env
 	// k is the kubectl command for the cluster registration namespace
-	k     kubectl.Command
-	et    metrics.ExporterTest
-	shard string
+	k        kubectl.Command
+	et       metrics.ExporterTest
+	etGitjob metrics.ExporterTest
+	shard    string
 )
 
 type ServiceData struct {
 	Name           string
 	Port           int64
 	IsDefaultShard bool
 	Shard          string
+	App            string
 }
 
-// setupLoadBalancer creates a load balancer service for the fleet controller.
+// setupLoadBalancer creates a load balancer service for the given app controller.
 // If shard is empty, it creates a service for the default (unsharded)
 // controller.
-func setupLoadBalancer(shard string) (metricsURL string) {
+// Valid app values are: fleet-controller, gitjob
+func setupLoadBalancer(shard string, app string) (metricsURL string) {
+	Expect(app).To(Or(Equal("fleet-controller"), Equal("gitjob")))
 	rs := rand.NewSource(time.Now().UnixNano())
 	port := rs.Int63()%1000 + 30000
-	loadBalancerName := testenv.AddRandomSuffix("fleetcontroller", rs)
+	loadBalancerName := testenv.AddRandomSuffix(app, rs)
 
 	ks := k.Namespace("cattle-fleet-system")
 	err := testenv.ApplyTemplate(
 		ks,
-		testenv.AssetPath("metrics/fleetcontroller_service.yaml"),
+		testenv.AssetPath("metrics/service.yaml"),
 		ServiceData{
+			App:            app,
 			Name:           loadBalancerName,
 			Port:           port,
 			IsDefaultShard: shard == "",
@@ -89,9 +94,12 @@ var _ = BeforeSuite(func() {
 	if os.Getenv("METRICS_URL") != "" {
 		metricsURL = os.Getenv("METRICS_URL")
 	} else {
-		metricsURL = setupLoadBalancer(shard)
+		metricsURL = setupLoadBalancer(shard, "fleet-controller")
 	}
 	et = metrics.NewExporterTest(metricsURL)
 
+	gitjobMetricsURL := setupLoadBalancer(shard, "gitjob")
+	etGitjob = metrics.NewExporterTest(gitjobMetricsURL)
+
 	env = testenv.New()
 })
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))
 		_, _ = k.Delete("ns", targetNamespace, "--wait=false")
@@ -67,11 +76,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",
 			)
@@ -130,14 +139,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")
@@ -244,7 +253,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",
 			)