diff --git a/charts/fleet/templates/deployment.yaml b/charts/fleet/templates/deployment.yaml index f6990f45ce..2b6b0f0160 100644 --- 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 index bac5f624b5..519b37f89b 100644 --- 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 index 0b1bad4fd2..35d2072af2 100644 --- 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 index a694f56fe8..98a8fe7746 100644 --- 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/e2e/assets/metrics/fleetcontroller_service.yaml b/e2e/assets/metrics/service.yaml similarity index 87% rename from e2e/assets/metrics/fleetcontroller_service.yaml rename to e2e/assets/metrics/service.yaml index 00a13f4d3a..40008b47ac 100644 --- a/e2e/assets/metrics/fleetcontroller_service.yaml +++ b/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 index 5c76a3443d..b11e0360da 100644 --- 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 index 8b67d7880d..bb44860da0 100644 --- a/e2e/metrics/suite_test.go +++ b/e2e/metrics/suite_test.go @@ -23,9 +23,10 @@ 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 { @@ -33,21 +34,25 @@ type ServiceData struct { 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 index b8a2b8e114..f3ad74e7ed 100644 --- 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 index 0e2b7dcdec..b666303f4d 100644 --- 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", ) diff --git a/e2e/single-cluster/imagescan_test.go b/e2e/single-cluster/imagescan_test.go index aa37695d85..596b7e084e 100644 --- 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,10 +184,10 @@ 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) @@ -199,10 +195,10 @@ func getFleetControllerRestarts(k kubectl.Command, index int) int { 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) @@ -210,23 +206,6 @@ func getFleetControllerReady(k kubectl.Command, index int) bool { 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 index 700fa080d8..d609cc5fb0 100644 --- 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, @@ -153,7 +153,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", @@ -170,7 +170,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, )) diff --git a/e2e/single-cluster/status_test.go b/e2e/single-cluster/status_test.go index a8c9a58169..48e68ee9ec 100644 --- a/e2e/single-cluster/status_test.go +++ b/e2e/single-cluster/status_test.go @@ -58,27 +58,32 @@ var _ = Describe("Checks status updates happen for a simple deployment", Ordered }) It("correctly sets the status values for GitRepos", func() { - out, err := k.Get("gitrepo", "my-gitrepo", "-n", "fleet-local", "-o", "jsonpath='{.status.summary}'") - Expect(err).ToNot(HaveOccurred(), out) + Eventually(func(g Gomega) { + out, err := k.Get("gitrepo", "my-gitrepo", "-n", "fleet-local", "-o", "jsonpath='{.status.summary}'") + g.Expect(err).ToNot(HaveOccurred(), out) - Expect(out).Should(ContainSubstring("\"desiredReady\":1")) - Expect(out).Should(ContainSubstring("\"ready\":1")) + g.Expect(out).Should(ContainSubstring("\"desiredReady\":1")) + g.Expect(out).Should(ContainSubstring("\"ready\":1")) - out, err = k.Get("gitrepo", "my-gitrepo", "-n", "fleet-local", "-o", "jsonpath='{.status.display}'") - Expect(err).ToNot(HaveOccurred(), out) - Expect(out).Should(ContainSubstring("\"readyBundleDeployments\":\"1/1\"")) + out, err = k.Get("gitrepo", "my-gitrepo", "-n", "fleet-local", "-o", "jsonpath='{.status.display}'") + g.Expect(err).ToNot(HaveOccurred(), out) + g.Expect(out).Should(ContainSubstring("\"readyBundleDeployments\":\"1/1\"")) + }).Should(Succeed()) }) It("correctly sets the status values for bundle", func() { - out, err := k.Get("bundle", "my-gitrepo-helm-verify", "-n", "fleet-local", "-o", "jsonpath='{.status.summary}'") - Expect(err).ToNot(HaveOccurred(), out) + Eventually(func(g Gomega) { + out, err := k.Get("bundle", "my-gitrepo-helm-verify", "-n", "fleet-local", "-o", "jsonpath='{.status.summary}'") + g.Expect(err).ToNot(HaveOccurred(), out) - Expect(out).Should(ContainSubstring("\"desiredReady\":1")) - Expect(out).Should(ContainSubstring("\"ready\":1")) + g.Expect(out).Should(ContainSubstring("\"desiredReady\":1")) + g.Expect(out).Should(ContainSubstring("\"ready\":1")) + + out, err = k.Get("bundle", "my-gitrepo-helm-verify", "-n", "fleet-local", "-o", "jsonpath='{.status.display}'") + g.Expect(err).ToNot(HaveOccurred(), out) + g.Expect(out).Should(ContainSubstring("\"readyClusters\":\"1/1\"")) + }).Should(Succeed()) - out, err = k.Get("bundle", "my-gitrepo-helm-verify", "-n", "fleet-local", "-o", "jsonpath='{.status.display}'") - Expect(err).ToNot(HaveOccurred(), out) - Expect(out).Should(ContainSubstring("\"readyClusters\":\"1/1\"")) }) }) @@ -88,11 +93,13 @@ var _ = Describe("Checks status updates happen for a simple deployment", Ordered }) It("correctly updates the status fields for GitRepos", func() { - out, err := k.Delete("bundle", "my-gitrepo-helm-verify", "-n", "fleet-local") - Expect(err).ToNot(HaveOccurred(), out) + Eventually(func(g Gomega) { + out, err := k.Delete("bundle", "my-gitrepo-helm-verify", "-n", "fleet-local") + g.Expect(err).ToNot(HaveOccurred(), out) + }).Should((Succeed())) Eventually(func() error { - out, err = k.Get("gitrepo", "my-gitrepo", "-n", "fleet-local", "-o", "jsonpath='{.status.summary}'") + out, err := k.Get("gitrepo", "my-gitrepo", "-n", "fleet-local", "-o", "jsonpath='{.status.summary}'") if err != nil { return err } diff --git a/integrationtests/controller/cluster/suite_test.go b/integrationtests/controller/cluster/suite_test.go index c75936affa..a502b52974 100644 --- a/integrationtests/controller/cluster/suite_test.go +++ b/integrationtests/controller/cluster/suite_test.go @@ -83,14 +83,6 @@ var _ = BeforeSuite(func() { sched := quartz.NewStdScheduler() Expect(sched).ToNot(BeNil()) - err = (&reconciler.GitRepoReconciler{ - Client: mgr.GetClient(), - Scheme: mgr.GetScheme(), - - Scheduler: sched, - }).SetupWithManager(mgr) - Expect(err).ToNot(HaveOccurred(), "failed to set up manager") - go func() { defer GinkgoRecover() err = mgr.Start(ctx) diff --git a/integrationtests/controller/gitrepo/gitrepo_test.go b/integrationtests/controller/gitrepo/gitrepo_test.go deleted file mode 100644 index 361be4a850..0000000000 --- a/integrationtests/controller/gitrepo/gitrepo_test.go +++ /dev/null @@ -1,77 +0,0 @@ -package gitrepo - -import ( - "encoding/hex" - "fmt" - "math/rand" - - . "github.com/onsi/ginkgo/v2" - . "github.com/onsi/gomega" - - "github.com/rancher/fleet/integrationtests/utils" - "github.com/rancher/fleet/pkg/apis/fleet.cattle.io/v1alpha1" - - corev1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/types" -) - -var _ = Describe("GitRepo", func() { - var ( - gitrepo *v1alpha1.GitRepo - gitrepoName string - ) - - BeforeEach(func() { - var err error - namespace, err = utils.NewNamespaceName() - Expect(err).ToNot(HaveOccurred()) - ns := &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: namespace}} - Expect(k8sClient.Create(ctx, ns)).ToNot(HaveOccurred()) - - p := make([]byte, 12) - s := rand.New(rand.NewSource(GinkgoRandomSeed())) // nolint:gosec // non-crypto usage - if _, err := s.Read(p); err != nil { - panic(err) - } - gitrepoName = fmt.Sprintf("test-gitrepo-%.12s", hex.EncodeToString(p)) - - gitrepo = &v1alpha1.GitRepo{ - ObjectMeta: metav1.ObjectMeta{ - Name: gitrepoName, - Namespace: namespace, - }, - Spec: v1alpha1.GitRepoSpec{ - Repo: "https://github.com/rancher/fleet-test-data/not-found", - }, - } - - DeferCleanup(func() { - Expect(k8sClient.Delete(ctx, gitrepo)).ToNot(HaveOccurred()) - }) - }) - - When("creating a gitrepo", func() { - JustBeforeEach(func() { - err := k8sClient.Create(ctx, gitrepo) - Expect(err).NotTo(HaveOccurred()) - }) - - It("updates the gitrepo status", func() { - org := gitrepo.ResourceVersion - Eventually(func() bool { - _ = k8sClient.Get(ctx, types.NamespacedName{Name: gitrepoName, Namespace: namespace}, gitrepo) - return gitrepo.ResourceVersion > org && - gitrepo.Status.Display.ReadyBundleDeployments == "0/0" && - gitrepo.Status.Display.State == "GitUpdating" && - !gitrepo.Status.Display.Error && - len(gitrepo.Status.Conditions) == 2 && - gitrepo.Status.Conditions[0].Type == "Ready" && - string(gitrepo.Status.Conditions[0].Status) == "True" && - gitrepo.Status.Conditions[1].Type == "Accepted" && - string(gitrepo.Status.Conditions[1].Status) == "True" && - gitrepo.Status.DeepCopy().ObservedGeneration == int64(1) - }).Should(BeTrue()) - }) - }) -}) diff --git a/integrationtests/controller/gitrepo/status_test.go b/integrationtests/controller/gitrepo/status_test.go deleted file mode 100644 index 48b14bd440..0000000000 --- a/integrationtests/controller/gitrepo/status_test.go +++ /dev/null @@ -1,109 +0,0 @@ -package gitrepo - -import ( - . "github.com/onsi/ginkgo/v2" - . "github.com/onsi/gomega" - - "github.com/rancher/fleet/integrationtests/utils" - "github.com/rancher/fleet/pkg/apis/fleet.cattle.io/v1alpha1" - - corev1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/types" -) - -var _ = Describe("GitRepo Status Fields", func() { - - var ( - gitrepo *v1alpha1.GitRepo - bd *v1alpha1.BundleDeployment - ) - - BeforeEach(func() { - var err error - namespace, err = utils.NewNamespaceName() - Expect(err).ToNot(HaveOccurred()) - - ns := &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: namespace}} - Expect(k8sClient.Create(ctx, ns)).ToNot(HaveOccurred()) - - DeferCleanup(func() { - Expect(k8sClient.Delete(ctx, ns)).ToNot(HaveOccurred()) - }) - }) - - When("Bundle changes", func() { - BeforeEach(func() { - cluster, err := utils.CreateCluster(ctx, k8sClient, "cluster", namespace, nil, namespace) - Expect(err).NotTo(HaveOccurred()) - Expect(cluster).To(Not(BeNil())) - targets := []v1alpha1.BundleTarget{ - { - BundleDeploymentOptions: v1alpha1.BundleDeploymentOptions{ - TargetNamespace: "targetNs", - }, - Name: "cluster", - ClusterName: "cluster", - }, - } - bundle, err := utils.CreateBundle(ctx, k8sClient, "name", namespace, targets, targets) - Expect(err).NotTo(HaveOccurred()) - Expect(bundle).To(Not(BeNil())) - - gitrepo = &v1alpha1.GitRepo{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-gitrepo", - Namespace: namespace, - }, - Spec: v1alpha1.GitRepoSpec{ - Repo: "https://github.com/rancher/fleet-test-data/not-found", - }, - } - err = k8sClient.Create(ctx, gitrepo) - Expect(err).NotTo(HaveOccurred()) - - bd = &v1alpha1.BundleDeployment{} - Eventually(func() bool { - err = k8sClient.Get(ctx, types.NamespacedName{Namespace: namespace, Name: "name"}, bd) - return err == nil - }).Should(BeTrue()) - }) - - It("updates the status fields", func() { - bundle := &v1alpha1.Bundle{} - Eventually(func() error { - err := k8sClient.Get(ctx, types.NamespacedName{Namespace: namespace, Name: "name"}, bundle) - Expect(err).ToNot(HaveOccurred()) - bundle.Labels["fleet.cattle.io/repo-name"] = gitrepo.Name - return k8sClient.Update(ctx, bundle) - }).ShouldNot(HaveOccurred()) - Expect(bundle.Status.Summary.Ready).ToNot(Equal(1)) - - err := k8sClient.Get(ctx, types.NamespacedName{Namespace: namespace, Name: gitrepo.Name}, gitrepo) - Expect(err).ToNot(HaveOccurred()) - Expect(gitrepo.Status.Summary.Ready).To(Equal(0)) - - bd := &v1alpha1.BundleDeployment{} - Eventually(func() error { - err := k8sClient.Get(ctx, types.NamespacedName{Namespace: namespace, Name: "name"}, bd) - if err != nil { - return err - } - bd.Status.Display.State = "Ready" - bd.Status.AppliedDeploymentID = bd.Spec.DeploymentID - bd.Status.Ready = true - bd.Status.NonModified = true - return k8sClient.Status().Update(ctx, bd) - }).ShouldNot(HaveOccurred()) - - Eventually(func() bool { - err := k8sClient.Get(ctx, types.NamespacedName{Namespace: namespace, Name: "name"}, bundle) - Expect(err).NotTo(HaveOccurred()) - return bundle.Status.Summary.Ready == 1 - }).Should(BeTrue()) - err = k8sClient.Get(ctx, types.NamespacedName{Namespace: namespace, Name: gitrepo.Name}, gitrepo) - Expect(err).ToNot(HaveOccurred()) - Expect(gitrepo.Status.Summary.Ready).To(Equal(1)) - }) - }) -}) diff --git a/integrationtests/controller/gitrepo/suite_test.go b/integrationtests/controller/gitrepo/suite_test.go deleted file mode 100644 index a203de6240..0000000000 --- a/integrationtests/controller/gitrepo/suite_test.go +++ /dev/null @@ -1,96 +0,0 @@ -package gitrepo - -import ( - "context" - "testing" - - . "github.com/onsi/ginkgo/v2" - . "github.com/onsi/gomega" - "github.com/reugn/go-quartz/quartz" - - "github.com/rancher/fleet/integrationtests/utils" - "github.com/rancher/fleet/internal/cmd/controller/reconciler" - "github.com/rancher/fleet/internal/cmd/controller/target" - "github.com/rancher/fleet/internal/config" - "github.com/rancher/fleet/internal/manifest" - - "k8s.io/client-go/rest" - ctrl "sigs.k8s.io/controller-runtime" - "sigs.k8s.io/controller-runtime/pkg/client" - "sigs.k8s.io/controller-runtime/pkg/envtest" - "sigs.k8s.io/controller-runtime/pkg/log/zap" -) - -var ( - cancel context.CancelFunc - cfg *rest.Config - ctx context.Context - testenv *envtest.Environment - k8sClient client.Client - - namespace string -) - -func TestFleet(t *testing.T) { - RegisterFailHandler(Fail) - RunSpecs(t, "Fleet GitRepo Suite") -} - -var _ = BeforeSuite(func() { - ctx, cancel = context.WithCancel(context.TODO()) - testenv = utils.NewEnvTest() - - var err error - cfg, err = testenv.Start() - Expect(err).NotTo(HaveOccurred()) - - k8sClient, err = utils.NewClient(cfg) - Expect(err).NotTo(HaveOccurred()) - - ctrl.SetLogger(zap.New(zap.UseFlagOptions(&zap.Options{Development: true}))) - - mgr, err := utils.NewManager(cfg) - Expect(err).ToNot(HaveOccurred()) - - // Set up the gitrepo reconciler - config.Set(config.DefaultConfig()) - - sched := quartz.NewStdScheduler() - Expect(sched).ToNot(BeNil()) - - err = (&reconciler.GitRepoReconciler{ - Client: mgr.GetClient(), - Scheme: mgr.GetScheme(), - - Scheduler: sched, - }).SetupWithManager(mgr) - Expect(err).ToNot(HaveOccurred(), "failed to set up manager") - - store := manifest.NewStore(mgr.GetClient()) - builder := target.New(mgr.GetClient()) - - err = (&reconciler.BundleReconciler{ - Client: mgr.GetClient(), - Scheme: mgr.GetScheme(), - Builder: builder, - Store: store, - Query: builder, - }).SetupWithManager(mgr) - Expect(err).ToNot(HaveOccurred(), "failed to set up manager") - - sched.Start(ctx) - DeferCleanup(func() { - sched.Stop() - }) - - go func() { - defer GinkgoRecover() - err = mgr.Start(ctx) - Expect(err).ToNot(HaveOccurred(), "failed to run manager") - }() -}) - -var _ = AfterSuite(func() { - cancel() - Expect(testenv.Stop()).ToNot(HaveOccurred()) -}) diff --git a/integrationtests/gitjob/controller/controller_test.go b/integrationtests/gitjob/controller/controller_test.go index 9c0c46ce6f..94d7961891 100644 --- a/integrationtests/gitjob/controller/controller_test.go +++ b/integrationtests/gitjob/controller/controller_test.go @@ -1,6 +1,9 @@ package controller import ( + "encoding/hex" + "fmt" + "math/rand" "strings" "time" @@ -8,6 +11,7 @@ import ( . "github.com/onsi/gomega" "sigs.k8s.io/controller-runtime/pkg/client" + "github.com/rancher/fleet/integrationtests/utils" v1alpha1 "github.com/rancher/fleet/pkg/apis/fleet.cattle.io/v1alpha1" "github.com/rancher/wrangler/v2/pkg/name" @@ -41,10 +45,24 @@ var _ = Describe("GitJob controller", func() { JustBeforeEach(func() { gitRepo = createGitRepo(gitRepoName) Expect(k8sClient.Create(ctx, &gitRepo)).ToNot(HaveOccurred()) + Eventually(func() string { + var gitRepoFromCluster v1alpha1.GitRepo + err := k8sClient.Get(ctx, types.NamespacedName{Name: gitRepo.Name, Namespace: gitRepo.Namespace}, &gitRepoFromCluster) + if err != nil { + // maybe the resource gitrepo is not created yet + return "" + } + return gitRepoFromCluster.Status.Display.ReadyBundleDeployments + }).Should(ContainSubstring("0/0")) Expect(simulateGitPollerUpdatingCommitInStatus(gitRepo, commit)).ToNot(HaveOccurred()) By("Creating a job") Eventually(func() error { + var gitRepoFromCluster v1alpha1.GitRepo + err := k8sClient.Get(ctx, types.NamespacedName{Name: gitRepo.Name, Namespace: gitRepo.Namespace}, &gitRepoFromCluster) + if err != nil { + return err + } jobName = name.SafeConcatName(gitRepoName, name.Hex(repo+commit, 5)) return k8sClient.Get(ctx, types.NamespacedName{Name: jobName, Namespace: gitRepoNamespace}, &job) }).Should(Not(HaveOccurred())) @@ -129,7 +147,6 @@ var _ = Describe("GitJob controller", func() { Eventually(func() bool { Expect(k8sClient.Get(ctx, types.NamespacedName{Name: gitRepoName, Namespace: gitRepoNamespace}, &gitRepo)).ToNot(HaveOccurred()) // XXX: do we need an additional `LastExecutedCommit` or similar status field? - //return gitRepo.Status.Commit != commit && gitRepo.Status.GitJobStatus == "Failed" return gitRepo.Status.GitJobStatus == "Failed" }).Should(BeTrue()) @@ -202,8 +219,8 @@ var _ = Describe("GitJob controller", func() { gitRepoName = "force-deletion" }) AfterEach(func() { - err := k8sClient.Delete(ctx, &gitRepo) - Expect(err).ToNot(HaveOccurred()) + // delete the gitrepo and wait until it is deleted + waitDeleteGitrepo(gitRepo) }) It("Verifies that the Job is recreated", func() { @@ -332,8 +349,8 @@ var _ = Describe("GitJob controller", func() { }) AfterEach(func() { - err := k8sClient.Delete(ctx, &gitRepo) - Expect(err).ToNot(HaveOccurred()) + // delete the gitrepo and wait until it is deleted + waitDeleteGitrepo(gitRepo) // reset the logs buffer so we don't read logs from previous tests logsBuffer.Reset() }) @@ -419,6 +436,165 @@ var _ = Describe("GitJob controller", func() { }) }) +var _ = Describe("GitRepo", func() { + var ( + gitrepo *v1alpha1.GitRepo + gitrepoName string + ) + + BeforeEach(func() { + var err error + namespace, err = utils.NewNamespaceName() + Expect(err).ToNot(HaveOccurred()) + ns := &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: namespace}} + Expect(k8sClient.Create(ctx, ns)).ToNot(HaveOccurred()) + + p := make([]byte, 12) + s := rand.New(rand.NewSource(GinkgoRandomSeed())) // nolint:gosec // non-crypto usage + if _, err := s.Read(p); err != nil { + panic(err) + } + gitrepoName = fmt.Sprintf("test-gitrepo-%.12s", hex.EncodeToString(p)) + + gitrepo = &v1alpha1.GitRepo{ + ObjectMeta: metav1.ObjectMeta{ + Name: gitrepoName, + Namespace: namespace, + }, + Spec: v1alpha1.GitRepoSpec{ + Repo: "https://github.com/rancher/fleet-test-data/not-found", + }, + } + + DeferCleanup(func() { + Expect(k8sClient.Delete(ctx, gitrepo)).ToNot(HaveOccurred()) + }) + }) + + When("creating a gitrepo", func() { + JustBeforeEach(func() { + err := k8sClient.Create(ctx, gitrepo) + Expect(err).NotTo(HaveOccurred()) + }) + + It("updates the gitrepo status", func() { + org := gitrepo.ResourceVersion + Eventually(func() bool { + _ = k8sClient.Get(ctx, types.NamespacedName{Name: gitrepoName, Namespace: namespace}, gitrepo) + return gitrepo.ResourceVersion > org && + gitrepo.Status.Display.ReadyBundleDeployments == "0/0" && + !gitrepo.Status.Display.Error && + len(gitrepo.Status.Conditions) == 4 && + gitrepo.Status.Conditions[0].Type == "Reconciling" && + string(gitrepo.Status.Conditions[0].Status) == "False" && + gitrepo.Status.Conditions[1].Type == "Stalled" && + string(gitrepo.Status.Conditions[1].Status) == "False" && + gitrepo.Status.Conditions[2].Type == "Ready" && + string(gitrepo.Status.Conditions[2].Status) == "True" && + gitrepo.Status.Conditions[3].Type == "Accepted" && + string(gitrepo.Status.Conditions[3].Status) == "True" && + gitrepo.Status.DeepCopy().ObservedGeneration == int64(1) + }).Should(BeTrue()) + }) + }) +}) + +var _ = Describe("GitRepo Status Fields", func() { + + var ( + gitrepo *v1alpha1.GitRepo + bd *v1alpha1.BundleDeployment + ) + + BeforeEach(func() { + var err error + namespace, err = utils.NewNamespaceName() + Expect(err).ToNot(HaveOccurred()) + + ns := &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: namespace}} + Expect(k8sClient.Create(ctx, ns)).ToNot(HaveOccurred()) + + DeferCleanup(func() { + Expect(k8sClient.Delete(ctx, ns)).ToNot(HaveOccurred()) + }) + }) + + When("Bundle changes", func() { + BeforeEach(func() { + cluster, err := utils.CreateCluster(ctx, k8sClient, "cluster", namespace, nil, namespace) + Expect(err).NotTo(HaveOccurred()) + Expect(cluster).To(Not(BeNil())) + targets := []v1alpha1.BundleTarget{ + { + BundleDeploymentOptions: v1alpha1.BundleDeploymentOptions{ + TargetNamespace: "targetNs", + }, + Name: "cluster", + ClusterName: "cluster", + }, + } + bundle, err := utils.CreateBundle(ctx, k8sClient, "name", namespace, targets, targets) + Expect(err).NotTo(HaveOccurred()) + Expect(bundle).To(Not(BeNil())) + + gitrepo = &v1alpha1.GitRepo{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-gitrepo", + Namespace: namespace, + }, + Spec: v1alpha1.GitRepoSpec{ + Repo: "https://github.com/rancher/fleet-test-data/not-found", + }, + } + err = k8sClient.Create(ctx, gitrepo) + Expect(err).NotTo(HaveOccurred()) + + bd = &v1alpha1.BundleDeployment{} + Eventually(func() bool { + err = k8sClient.Get(ctx, types.NamespacedName{Namespace: namespace, Name: "name"}, bd) + return err == nil + }).Should(BeTrue()) + }) + + It("updates the status fields", func() { + bundle := &v1alpha1.Bundle{} + Eventually(func() error { + err := k8sClient.Get(ctx, types.NamespacedName{Namespace: namespace, Name: "name"}, bundle) + Expect(err).ToNot(HaveOccurred()) + bundle.Labels["fleet.cattle.io/repo-name"] = gitrepo.Name + return k8sClient.Update(ctx, bundle) + }).ShouldNot(HaveOccurred()) + Expect(bundle.Status.Summary.Ready).ToNot(Equal(1)) + + err := k8sClient.Get(ctx, types.NamespacedName{Namespace: namespace, Name: gitrepo.Name}, gitrepo) + Expect(err).ToNot(HaveOccurred()) + Expect(gitrepo.Status.Summary.Ready).To(Equal(0)) + + bd := &v1alpha1.BundleDeployment{} + Eventually(func() error { + err := k8sClient.Get(ctx, types.NamespacedName{Namespace: namespace, Name: "name"}, bd) + if err != nil { + return err + } + bd.Status.Display.State = "Ready" + bd.Status.AppliedDeploymentID = bd.Spec.DeploymentID + bd.Status.Ready = true + bd.Status.NonModified = true + return k8sClient.Status().Update(ctx, bd) + }).ShouldNot(HaveOccurred()) + + Eventually(func() bool { + err := k8sClient.Get(ctx, types.NamespacedName{Namespace: namespace, Name: "name"}, bundle) + Expect(err).NotTo(HaveOccurred()) + return bundle.Status.Summary.Ready == 1 + }).Should(BeTrue()) + err = k8sClient.Get(ctx, types.NamespacedName{Namespace: namespace, Name: gitrepo.Name}, gitrepo) + Expect(err).ToNot(HaveOccurred()) + Expect(gitrepo.Status.Summary.Ready).To(Equal(1)) + }) + }) +}) + func simulateIncreaseForceSyncGeneration(gitRepo v1alpha1.GitRepo) error { return retry.RetryOnConflict(retry.DefaultRetry, func() error { var gitRepoFromCluster v1alpha1.GitRepo @@ -482,3 +658,13 @@ func createGitRepoWithDisablePolling(gitRepoName string) v1alpha1.GitRepo { }, } } + +func waitDeleteGitrepo(gitRepo v1alpha1.GitRepo) { + err := k8sClient.Delete(ctx, &gitRepo) + Expect(err).ToNot(HaveOccurred()) + Eventually(func() bool { + var gitRepoFromCluster v1alpha1.GitRepo + err := k8sClient.Get(ctx, types.NamespacedName{Name: gitRepo.Name, Namespace: gitRepo.Namespace}, &gitRepoFromCluster) + return errors.IsNotFound(err) + }).Should(BeTrue()) +} diff --git a/integrationtests/gitjob/controller/suite_test.go b/integrationtests/gitjob/controller/suite_test.go index 582d0a591f..328a48b72b 100644 --- a/integrationtests/gitjob/controller/suite_test.go +++ b/integrationtests/gitjob/controller/suite_test.go @@ -9,10 +9,14 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + "github.com/reugn/go-quartz/quartz" "go.uber.org/mock/gomock" "github.com/rancher/fleet/internal/cmd/controller/gitops/reconciler" + ctrlreconciler "github.com/rancher/fleet/internal/cmd/controller/reconciler" + "github.com/rancher/fleet/internal/cmd/controller/target" + "github.com/rancher/fleet/internal/manifest" "github.com/rancher/fleet/internal/mocks" v1alpha1 "github.com/rancher/fleet/pkg/apis/fleet.cattle.io/v1alpha1" @@ -35,6 +39,7 @@ var ( cancel context.CancelFunc k8sClient client.Client logsBuffer bytes.Buffer + namespace string ) func TestGitJobController(t *testing.T) { @@ -78,14 +83,29 @@ var _ = BeforeSuite(func() { gitPollerMock.EXPECT().AddOrModifyGitRepoPollJob(gomock.Any(), gomock.Any()).AnyTimes() gitPollerMock.EXPECT().CleanUpGitRepoPollJobs(gomock.Any()).AnyTimes() + sched := quartz.NewStdScheduler() + Expect(sched).ToNot(BeNil()) + err = (&reconciler.GitJobReconciler{ Client: mgr.GetClient(), Scheme: mgr.GetScheme(), Image: "image", GitPoller: gitPollerMock, + Scheduler: sched, }).SetupWithManager(mgr) Expect(err).ToNot(HaveOccurred()) + store := manifest.NewStore(mgr.GetClient()) + builder := target.New(mgr.GetClient()) + err = (&ctrlreconciler.BundleReconciler{ + Client: mgr.GetClient(), + Scheme: mgr.GetScheme(), + Builder: builder, + Store: store, + Query: builder, + }).SetupWithManager(mgr) + Expect(err).ToNot(HaveOccurred(), "failed to set up manager") + go func() { defer GinkgoRecover() defer ctlr.Finish() diff --git a/internal/cmd/controller/finalize/finalize.go b/internal/cmd/controller/finalize/finalize.go new file mode 100644 index 0000000000..9af3a24879 --- /dev/null +++ b/internal/cmd/controller/finalize/finalize.go @@ -0,0 +1,161 @@ +package finalize + +import ( + "context" + "slices" + "strings" + + "github.com/rancher/fleet/pkg/apis/fleet.cattle.io/v1alpha1" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + "k8s.io/client-go/util/retry" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" +) + +const ( + GitRepoFinalizer = "fleet.cattle.io/gitrepo-finalizer" + BundleFinalizer = "fleet.cattle.io/bundle-finalizer" + BundleDeploymentFinalizer = "fleet.cattle.io/bundle-deployment-finalizer" +) + +// PurgeBundles deletes all bundles related to the given GitRepo namespaced name +// It deletes resources in cascade. Deleting Bundles, its BundleDeployments, and +// the related namespace if Bundle.Spec.DeleteNamespace is set to true. +func PurgeBundles(ctx context.Context, c client.Client, gitrepo types.NamespacedName) error { + bundles := &v1alpha1.BundleList{} + err := c.List(ctx, bundles, client.MatchingLabels{v1alpha1.RepoLabel: gitrepo.Name}, client.InNamespace(gitrepo.Namespace)) + if err != nil { + return err + } + + // At this point, access to the GitRepo is unavailable as it has been deleted and cannot be found within the cluster. + // Nevertheless, `deleteNamespace` can be found within all bundles generated from that GitRepo. Checking any bundle to get this value would be enough. + namespace := "" + deleteNamespace := false + sampleBundle := v1alpha1.Bundle{} + if len(bundles.Items) > 0 { + sampleBundle = bundles.Items[0] + deleteNamespace = sampleBundle.Spec.DeleteNamespace + namespace = sampleBundle.Spec.TargetNamespace + + if sampleBundle.Spec.KeepResources { + deleteNamespace = false + } + } + + if err = PurgeNamespace(ctx, c, deleteNamespace, namespace); err != nil { + return err + } + + for _, bundle := range bundles.Items { + err := c.Delete(ctx, &bundle) + if client.IgnoreNotFound(err) != nil { + return err + } + + nn := types.NamespacedName{Namespace: bundle.Namespace, Name: bundle.Name} + if err = PurgeBundleDeployments(ctx, c, nn); err != nil { + return client.IgnoreNotFound(err) + } + } + + return nil +} + +// PurgeBundleDeployments deletes all BundleDeployments related with the given Bundle namespaced name. +func PurgeBundleDeployments(ctx context.Context, c client.Client, bundle types.NamespacedName) error { + list := &v1alpha1.BundleDeploymentList{} + err := c.List( + ctx, + list, + client.MatchingLabels{ + v1alpha1.BundleLabel: bundle.Name, + v1alpha1.BundleNamespaceLabel: bundle.Namespace, + }, + ) + if err != nil { + return err + } + for _, bd := range list.Items { + if controllerutil.ContainsFinalizer(&bd, BundleDeploymentFinalizer) { // nolint: gosec // does not store pointer + nn := types.NamespacedName{Namespace: bd.Namespace, Name: bd.Name} + err = retry.RetryOnConflict(retry.DefaultRetry, func() error { + t := &v1alpha1.BundleDeployment{} + if err := c.Get(ctx, nn, t); err != nil { + return err + } + + controllerutil.RemoveFinalizer(t, BundleDeploymentFinalizer) + + return c.Update(ctx, t) + }) + if err != nil { + return err + } + } + + err := c.Delete(ctx, &bd) + if err != nil { + return err + } + } + + return nil +} + +// PurgeImageScans deletes all ImageScan resources related with the given GitRepo namespaces name. +func PurgeImageScans(ctx context.Context, c client.Client, gitrepo types.NamespacedName) error { + images := &v1alpha1.ImageScanList{} + err := c.List(ctx, images, client.InNamespace(gitrepo.Namespace)) + if err != nil { + return err + } + + for _, image := range images.Items { + if image.Spec.GitRepoName == gitrepo.Name { + err := c.Delete(ctx, &image) + if err != nil { + return err + } + } + + } + return nil +} + +// PurgeNamespace deletes the given namespace if deleteNamespace is set to true. +// It ignores the following namespaces, that are considered as default by fleet or kubernetes: +// fleet-local, cattle-fleet-system, fleet-default, cattle-fleet-clusters-system, default +func PurgeNamespace(ctx context.Context, c client.Client, deleteNamespace bool, ns string) error { + if !deleteNamespace { + return nil + } + + if ns == "" { + return nil + } + + // Ignore default namespaces + defaultNamespaces := []string{"fleet-local", "cattle-fleet-system", "fleet-default", "cattle-fleet-clusters-system", "default"} + if slices.Contains(defaultNamespaces, ns) { + return nil + } + + // Ignore system namespaces + if _, isKubeNamespace := strings.CutPrefix(ns, "kube-"); isKubeNamespace { + return nil + } + + namespace := &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: ns, + }, + } + if err := c.Delete(ctx, namespace); err != nil { + return err + } + + return nil +} diff --git a/internal/cmd/controller/gitops/operator.go b/internal/cmd/controller/gitops/operator.go index b423c9e527..d2baee6206 100644 --- a/internal/cmd/controller/gitops/operator.go +++ b/internal/cmd/controller/gitops/operator.go @@ -5,14 +5,17 @@ import ( "fmt" "net/http" "os" + "strconv" "time" command "github.com/rancher/fleet/internal/cmd" "github.com/rancher/fleet/internal/cmd/controller/gitops/reconciler" + "github.com/rancher/fleet/internal/metrics" fleet "github.com/rancher/fleet/pkg/apis/fleet.cattle.io/v1alpha1" "github.com/rancher/fleet/pkg/git/poll" "github.com/rancher/fleet/pkg/version" "github.com/rancher/fleet/pkg/webhook" + "github.com/reugn/go-quartz/quartz" "github.com/spf13/cobra" "k8s.io/apimachinery/pkg/runtime" @@ -48,6 +51,7 @@ type GitOperator struct { EnableLeaderElection bool `name:"leader-elect" default:"true" usage:"Enable leader election for controller manager. Enabling this will ensure there is only one active controller manager."` Image string `name:"gitjob-image" default:"rancher/fleet:dev" usage:"The gitjob image that will be used in the generated job."` Listen string `default:":8080" usage:"The port the webhook listens."` + ShardID string `usage:"only manage resources labeled with a specific shard ID" name:"shard-id"` } func App(zo *zap.Options) *cobra.Command { @@ -82,32 +86,37 @@ func (g *GitOperator) Run(cmd *cobra.Command, args []string) error { namespace := g.Namespace - if envMetricsAddr := os.Getenv("GITOPS_METRICS_BIND_ADDRESS"); envMetricsAddr != "" { - g.MetricsAddr = envMetricsAddr - } - - var metricServerOptions metricsserver.Options - if g.DisableMetrics { - metricServerOptions = metricsserver.Options{BindAddress: "0"} - } else { - metricServerOptions = metricsserver.Options{BindAddress: g.MetricsAddr} - } - mgr, err := ctrl.NewManager(ctrl.GetConfigOrDie(), ctrl.Options{ Scheme: scheme, - Metrics: metricServerOptions, + Metrics: g.setupMetrics(), LeaderElection: g.EnableLeaderElection, LeaderElectionID: "gitjob-leader", LeaderElectionNamespace: namespace, }) + if err != nil { return err } + + sched := quartz.NewStdScheduler() + + var workers int + if d := os.Getenv("GITREPO_RECONCILER_WORKERS"); d != "" { + w, err := strconv.Atoi(d) + if err != nil { + setupLog.Error(err, "failed to parse GITREPO_RECONCILER_WORKERS", "value", d) + } + workers = w + } + reconciler := &reconciler.GitJobReconciler{ Client: mgr.GetClient(), Scheme: mgr.GetScheme(), Image: g.Image, GitPoller: poll.NewHandler(ctx, mgr.GetClient()), + Scheduler: sched, + Workers: workers, + ShardID: g.ShardID, } group := errgroup.Group{} @@ -126,6 +135,22 @@ func (g *GitOperator) Run(cmd *cobra.Command, args []string) error { return group.Wait() } +func (g *GitOperator) setupMetrics() metricsserver.Options { + if g.DisableMetrics { + return metricsserver.Options{BindAddress: "0"} + } + + metricsAddr := g.MetricsAddr + if d := os.Getenv("GITOPS_METRICS_BIND_ADDRESS"); d != "" { + metricsAddr = d + } + + metricServerOpts := metricsserver.Options{BindAddress: metricsAddr} + metrics.RegisterGitOptsMetrics() // enable gitops related metrics + + return metricServerOpts +} + func startWebhook(ctx context.Context, namespace string, addr string, client client.Client, cacheClient cache.Cache) error { setupLog.Info("Setting up webhook listener") handler, err := webhook.HandleHooks(ctx, namespace, client, cacheClient) diff --git a/internal/cmd/controller/gitops/reconciler/gitjob_controller.go b/internal/cmd/controller/gitops/reconciler/gitjob_controller.go index c0a5e9f8ac..8ddfa7ee97 100644 --- a/internal/cmd/controller/gitops/reconciler/gitjob_controller.go +++ b/internal/cmd/controller/gitops/reconciler/gitjob_controller.go @@ -4,33 +4,37 @@ import ( "context" "fmt" "os" - "sort" + "reflect" "strconv" - "strings" "time" + "github.com/go-logr/logr" + "github.com/rancher/fleet/internal/cmd/controller/finalize" "github.com/rancher/fleet/internal/cmd/controller/grutil" + "github.com/rancher/fleet/internal/cmd/controller/imagescan" + "github.com/rancher/fleet/internal/metrics" v1alpha1 "github.com/rancher/fleet/pkg/apis/fleet.cattle.io/v1alpha1" "github.com/rancher/fleet/pkg/git" + "github.com/rancher/fleet/pkg/sharding" + "github.com/reugn/go-quartz/quartz" - "github.com/rancher/wrangler/v2/pkg/condition" - "github.com/rancher/wrangler/v2/pkg/kstatus" "github.com/rancher/wrangler/v2/pkg/name" batchv1 "k8s.io/api/batch/v1" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/util/retry" - "sigs.k8s.io/cli-utils/pkg/kstatus/status" ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/builder" "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/controller" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" "sigs.k8s.io/controller-runtime/pkg/event" + "sigs.k8s.io/controller-runtime/pkg/handler" "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/predicate" ) @@ -57,13 +61,45 @@ type GitJobReconciler struct { Scheme *runtime.Scheme Image string GitPoller GitPoller + Scheduler quartz.Scheduler + Workers int + ShardID string } func (r *GitJobReconciler) SetupWithManager(mgr ctrl.Manager) error { return ctrl.NewControllerManagedBy(mgr). - For(&v1alpha1.GitRepo{}). + For(&v1alpha1.GitRepo{}, + builder.WithPredicates( + // do not trigger for GitRepo status changes (except for commit changes) + predicate.Or( + predicate.GenerationChangedPredicate{}, + predicate.AnnotationChangedPredicate{}, + predicate.LabelChangedPredicate{}, + commitChangedPredicate(), + ), + ), + ). Owns(&batchv1.Job{}). - WithEventFilter(generationOrCommitChangedPredicate()). + Watches( + // Fan out from bundle to gitrepo + &v1alpha1.Bundle{}, + handler.EnqueueRequestsFromMapFunc(func(ctx context.Context, a client.Object) []ctrl.Request { + repo := a.GetLabels()[v1alpha1.RepoLabel] + if repo != "" { + return []ctrl.Request{{ + NamespacedName: types.NamespacedName{ + Namespace: a.GetNamespace(), + Name: repo, + }, + }} + } + + return []ctrl.Request{} + }), + builder.WithPredicates(bundleStatusChangedPredicate()), + ). + WithEventFilter(sharding.FilterByShardID(r.ShardID)). + WithOptions(controller.Options{MaxConcurrentReconciles: r.Workers}). Complete(r) } @@ -88,15 +124,37 @@ func (r *GitJobReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr return ctrl.Result{}, nil } + // Restrictions / Overrides, gitrepo reconciler is responsible for setting error in status + oldStatus := gitrepo.Status.DeepCopy() + gitrepo, err := grutil.AuthorizeAndAssignDefaults(ctx, r.Client, gitrepo) + if err != nil { + return ctrl.Result{}, grutil.UpdateErrorStatus(ctx, r.Client, req.NamespacedName, *oldStatus, err) + } + + if !gitrepo.DeletionTimestamp.IsZero() { + if controllerutil.ContainsFinalizer(gitrepo, finalize.GitRepoFinalizer) { + if err := r.cleanupGitRepo(ctx, logger, gitrepo); err != nil { + return ctrl.Result{}, err + } + } + + return ctrl.Result{}, nil + } + + if !controllerutil.ContainsFinalizer(gitrepo, finalize.GitRepoFinalizer) { + err := r.addGitRepoFinalizer(ctx, req.NamespacedName) + if client.IgnoreNotFound(err) != nil { + return ctrl.Result{}, err + } + } + logger = logger.WithValues("generation", gitrepo.Generation, "commit", gitrepo.Status.Commit) ctx = log.IntoContext(ctx, logger) logger.V(1).Info("Reconciling GitRepo") - // Restrictions / Overrides, gitrepo reconciler is responsible for setting error in status - gitrepo, err := grutil.AuthorizeAndAssignDefaults(ctx, r.Client, gitrepo) - if err != nil { - return ctrl.Result{}, err + if gitrepo.Spec.Repo == "" { + return ctrl.Result{}, nil } r.GitPoller.AddOrModifyGitRepoPollJob(ctx, *gitrepo) @@ -110,6 +168,7 @@ func (r *GitJobReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr return ctrl.Result{}, fmt.Errorf("error retrieving git job: %w", err) } + // Gitjob handling if errors.IsNotFound(err) { if gitrepo.Spec.DisablePolling { if err := r.updateCommit(ctx, gitrepo); err != nil { @@ -141,12 +200,40 @@ func (r *GitJobReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr } } - if err = r.updateStatus(ctx, gitrepo, &job); err != nil { - if errors.IsConflict(err) { - logger.V(1).Info("conflict updating status, retrying", "message", err) - return ctrl.Result{Requeue: true}, nil // just retry, but don't show an error - } - return ctrl.Result{}, fmt.Errorf("error updating git job status: %w", err) + gitrepo.Status.ObservedGeneration = gitrepo.Generation + + // Refresh the status + if err = grutil.SetStatusFromGitjob(ctx, r.Client, gitrepo, &job); err != nil { + return ctrl.Result{}, grutil.UpdateErrorStatus(ctx, r.Client, req.NamespacedName, gitrepo.Status, err) + } + + err = grutil.SetStatusFromBundleDeployments(ctx, r.Client, gitrepo) + if err != nil { + return ctrl.Result{}, grutil.UpdateErrorStatus(ctx, r.Client, req.NamespacedName, gitrepo.Status, err) + } + + err = grutil.SetStatusFromBundles(ctx, r.Client, gitrepo) + if err != nil { + return ctrl.Result{}, grutil.UpdateErrorStatus(ctx, r.Client, req.NamespacedName, gitrepo.Status, err) + } + + if err = grutil.UpdateDisplayState(gitrepo); err != nil { + return ctrl.Result{}, grutil.UpdateErrorStatus(ctx, r.Client, req.NamespacedName, gitrepo.Status, err) + } + + grutil.SetStatusFromResourceKey(ctx, r.Client, gitrepo) + + gitrepo.Status.Display.ReadyBundleDeployments = fmt.Sprintf("%d/%d", + gitrepo.Status.Summary.Ready, + gitrepo.Status.Summary.DesiredReady) + + grutil.SetCondition(&gitrepo.Status, nil) + + err = grutil.UpdateStatus(ctx, r.Client, req.NamespacedName, gitrepo.Status) + if err != nil { + logger.V(1).Error(err, "Reconcile failed final update to git repo status", "status", gitrepo.Status) + + return ctrl.Result{}, err } return ctrl.Result{}, nil @@ -164,7 +251,61 @@ func (r *GitJobReconciler) updateCommit(ctx context.Context, gitRepo *v1alpha1.G }) } -func generationOrCommitChangedPredicate() predicate.Predicate { +func (r *GitJobReconciler) cleanupGitRepo(ctx context.Context, logger logr.Logger, gitrepo *v1alpha1.GitRepo) error { + // Clean up + logger.V(1).Info("Gitrepo deleted, deleting bundle, image scans") + + metrics.GitRepoCollector.Delete(gitrepo.Name, gitrepo.Namespace) + + nsName := types.NamespacedName{Name: gitrepo.Name, Namespace: gitrepo.Namespace} + if err := finalize.PurgeBundles(ctx, r.Client, nsName); err != nil { + return err + } + + // remove the job scheduled by imagescan, if any + _ = r.Scheduler.DeleteJob(imagescan.GitCommitKey(gitrepo.Namespace, gitrepo.Name)) + + if err := finalize.PurgeImageScans(ctx, r.Client, nsName); err != nil { + return err + } + + err := retry.RetryOnConflict(retry.DefaultRetry, func() error { + if err := r.Get(ctx, nsName, gitrepo); err != nil { + return err + } + + controllerutil.RemoveFinalizer(gitrepo, finalize.GitRepoFinalizer) + + return r.Update(ctx, gitrepo) + }) + + if client.IgnoreNotFound(err) != nil { + return err + } + + return nil +} + +func (r *GitJobReconciler) addGitRepoFinalizer(ctx context.Context, nsName types.NamespacedName) error { + err := retry.RetryOnConflict(retry.DefaultRetry, func() error { + gitrepo := &v1alpha1.GitRepo{} + if err := r.Get(ctx, nsName, gitrepo); err != nil { + return err + } + + controllerutil.AddFinalizer(gitrepo, finalize.GitRepoFinalizer) + + return r.Update(ctx, gitrepo) + }) + + if err != nil { + return client.IgnoreNotFound(err) + } + + return nil +} + +func commitChangedPredicate() predicate.Predicate { return predicate.Funcs{ UpdateFunc: func(e event.UpdateEvent) bool { oldGitJob, ok := e.ObjectOld.(*v1alpha1.GitRepo) @@ -176,7 +317,7 @@ func generationOrCommitChangedPredicate() predicate.Predicate { return true } - return oldGitJob.Generation != newGitJob.Generation || oldGitJob.Status.Commit != newGitJob.Status.Commit + return oldGitJob.Status.Commit != newGitJob.Status.Commit }, } } @@ -252,72 +393,6 @@ func (r *GitJobReconciler) createJob(ctx context.Context, gitRepo *v1alpha1.GitR return r.Create(ctx, job) } -func (r *GitJobReconciler) updateStatus(ctx context.Context, gitRepo *v1alpha1.GitRepo, job *batchv1.Job) error { - obj, err := runtime.DefaultUnstructuredConverter.ToUnstructured(job) - if err != nil { - return err - } - uJob := &unstructured.Unstructured{Object: obj} - - result, err := status.Compute(uJob) - if err != nil { - return err - } - - terminationMessage := "" - if result.Status == status.FailedStatus { - selector := labels.SelectorFromSet(labels.Set{"job-name": job.Name}) - podList := &corev1.PodList{} - err := r.List(ctx, podList, &client.ListOptions{LabelSelector: selector}) - if err != nil { - return err - } - - sort.Slice(podList.Items, func(i, j int) bool { - return podList.Items[i].CreationTimestamp.Before(&podList.Items[j].CreationTimestamp) - }) - - terminationMessage = result.Message - if len(podList.Items) > 0 { - for _, podStatus := range podList.Items[len(podList.Items)-1].Status.ContainerStatuses { - if podStatus.Name != "step-git-source" && podStatus.State.Terminated != nil { - terminationMessage += podStatus.State.Terminated.Message - } - } - } - } - - return retry.RetryOnConflict(retry.DefaultRetry, func() error { - currentGitRepo := &v1alpha1.GitRepo{} - err := r.Get(ctx, client.ObjectKeyFromObject(gitRepo), currentGitRepo) - if err != nil { - return err - } - - currentGitRepo.Status.GitJobStatus = result.Status.String() - currentGitRepo.Status.ObservedGeneration = gitRepo.Generation - currentGitRepo.Status.UpdateGeneration = gitRepo.Status.UpdateGeneration - - for _, con := range result.Conditions { - condition.Cond(con.Type.String()).SetStatus(currentGitRepo, string(con.Status)) - condition.Cond(con.Type.String()).SetMessageIfBlank(currentGitRepo, con.Message) - condition.Cond(con.Type.String()).Reason(currentGitRepo, con.Reason) - } - - switch result.Status { - case status.FailedStatus: - kstatus.SetError(currentGitRepo, terminationMessage) - case status.CurrentStatus: - if strings.Contains(result.Message, "Job Completed") { - currentGitRepo.Status.Commit = job.Annotations["commit"] - } - kstatus.SetActive(currentGitRepo) - } - - return r.Status().Update(ctx, currentGitRepo) - }) -} - func (r *GitJobReconciler) deleteJobIfNeeded(ctx context.Context, gitRepo *v1alpha1.GitRepo, job *batchv1.Job) error { logger := log.FromContext(ctx) // if force delete is set, delete the job to make sure a new job is created @@ -331,6 +406,7 @@ func (r *GitJobReconciler) deleteJobIfNeeded(ctx context.Context, gitRepo *v1alp // k8s Jobs are immutable. Recreate the job if the GitRepo Spec has changed. if gitRepo.Generation != gitRepo.Status.ObservedGeneration { + gitRepo.Status.ObservedGeneration = gitRepo.Generation logger.Info("job deletion triggered because of generation change") if err := r.Delete(ctx, job, client.PropagationPolicy(metav1.DeletePropagationBackground)); err != nil && !errors.IsNotFound(err) { return err @@ -365,6 +441,15 @@ func (r *GitJobReconciler) newGitJob(ctx context.Context, obj *v1alpha1.GitRepo) }, Spec: *jobSpec, } + // if the repo references a shard, add the same label to the job + // this avoids a call to Reconcile for controllers that do not match + // the shard-id + label, hasLabel := obj.GetLabels()[sharding.ShardingRefLabel] + if hasLabel { + job.Labels = labels.Merge(job.Labels, map[string]string{ + sharding.ShardingRefLabel: label, + }) + } initContainer, err := r.newGitCloner(ctx, obj) if err != nil { @@ -810,3 +895,27 @@ func proxyEnvVars() []corev1.EnvVar { return envVars } + +// bundleStatusChangedPredicate returns true if the bundle +// status has changed, or the bundle was created +func bundleStatusChangedPredicate() predicate.Funcs { + return predicate.Funcs{ + CreateFunc: func(e event.CreateEvent) bool { + return true + }, + UpdateFunc: func(e event.UpdateEvent) bool { + n, isBundle := e.ObjectNew.(*v1alpha1.Bundle) + if !isBundle { + return false + } + o := e.ObjectOld.(*v1alpha1.Bundle) + if n == nil || o == nil { + return false + } + return !reflect.DeepEqual(n.Status, o.Status) + }, + DeleteFunc: func(e event.DeleteEvent) bool { + return true + }, + } +} diff --git a/internal/cmd/controller/gitops/reconciler/gitjob_test.go b/internal/cmd/controller/gitops/reconciler/gitjob_test.go index a48ca3ee0c..3423929b82 100644 --- a/internal/cmd/controller/gitops/reconciler/gitjob_test.go +++ b/internal/cmd/controller/gitops/reconciler/gitjob_test.go @@ -42,7 +42,16 @@ func TestReconcile_AddOrModifyGitRepoPollJobIsCalled_WhenGitRepoIsCreatedOrModif client := mocks.NewMockClient(mockCtrl) statusClient := mocks.NewMockSubResourceWriter(mockCtrl) statusClient.EXPECT().Update(gomock.Any(), gomock.Any()) + client.EXPECT().Update(gomock.Any(), gomock.Any()).Times(1).Return(nil) client.EXPECT().List(gomock.Any(), gomock.Any(), gomock.Any()).AnyTimes().Return(nil) + client.EXPECT().Get(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Times(1).DoAndReturn( + func(ctx context.Context, req types.NamespacedName, gitrepo *fleetv1.GitRepo, opts ...interface{}) error { + gitrepo.Name = gitRepo.Name + gitrepo.Namespace = gitRepo.Namespace + gitrepo.Spec.Repo = "repo" + return nil + }, + ) client.EXPECT().Get(gomock.Any(), gomock.Any(), gomock.Any()).AnyTimes().Return(nil) client.EXPECT().Status().Return(statusClient) poller := mocks.NewMockGitPoller(mockCtrl) @@ -111,6 +120,18 @@ func TestReconcile_Error_WhenGitrepoRestrictionsAreNotMet(t *testing.T) { }, ) mockClient.EXPECT().Get(gomock.Any(), gomock.Any(), gomock.Any()).AnyTimes().Return(nil) + statusClient := mocks.NewMockSubResourceWriter(mockCtrl) + mockClient.EXPECT().Status().Times(1).Return(statusClient) + statusClient.EXPECT().Update(gomock.Any(), gomock.Any(), gomock.Any()).Do( + func(ctx context.Context, repo *fleetv1.GitRepo, opts ...interface{}) { + if len(repo.Status.Conditions) == 0 { + t.Errorf("expecting to have Conditions, got none") + } + if repo.Status.Conditions[0].Message != "empty targetNamespace denied, because allowedTargetNamespaces restriction is present" { + t.Errorf("Expecting condition message [empty targetNamespace denied, because allowedTargetNamespaces restriction is present], got [%s]", repo.Status.Conditions[0].Message) + } + }, + ) poller := mocks.NewMockGitPoller(mockCtrl) r := GitJobReconciler{ @@ -123,7 +144,7 @@ func TestReconcile_Error_WhenGitrepoRestrictionsAreNotMet(t *testing.T) { ctx := context.TODO() _, err := r.Reconcile(ctx, ctrl.Request{NamespacedName: namespacedName}) if err == nil { - t.Errorf("expecting error, got nil") + t.Errorf("expecting an error, got nil") } if err.Error() != "empty targetNamespace denied, because allowedTargetNamespaces restriction is present" { t.Errorf("unexpected error %v", err) diff --git a/internal/cmd/controller/grutil/status.go b/internal/cmd/controller/grutil/status.go index 17820abc4b..a5f579dbda 100644 --- a/internal/cmd/controller/grutil/status.go +++ b/internal/cmd/controller/grutil/status.go @@ -4,18 +4,26 @@ import ( "context" "fmt" "sort" + "strings" "time" "github.com/rancher/fleet/internal/cmd/controller/summary" "github.com/rancher/fleet/internal/metrics" fleet "github.com/rancher/fleet/pkg/apis/fleet.cattle.io/v1alpha1" "github.com/rancher/wrangler/v2/pkg/condition" + "github.com/rancher/wrangler/v2/pkg/kstatus" + batchv1 "k8s.io/api/batch/v1" + corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/equality" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/labels" + "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/util/retry" fleetutil "github.com/rancher/fleet/internal/cmd/controller/errorutil" errutil "k8s.io/apimachinery/pkg/util/errors" + "sigs.k8s.io/cli-utils/pkg/kstatus/status" "sigs.k8s.io/controller-runtime/pkg/client" ) @@ -99,6 +107,63 @@ func SetStatusFromBundleDeployments(ctx context.Context, c client.Client, gitrep return nil } +// SetStatusFromGitjob sets the status fields relative to the given job in the gitRepo +func SetStatusFromGitjob(ctx context.Context, c client.Client, gitRepo *fleet.GitRepo, job *batchv1.Job) error { + obj, err := runtime.DefaultUnstructuredConverter.ToUnstructured(job) + if err != nil { + return err + } + uJob := &unstructured.Unstructured{Object: obj} + + result, err := status.Compute(uJob) + if err != nil { + return err + } + + terminationMessage := "" + if result.Status == status.FailedStatus { + selector := labels.SelectorFromSet(labels.Set{"job-name": job.Name}) + podList := &corev1.PodList{} + err := c.List(ctx, podList, &client.ListOptions{LabelSelector: selector}) + if err != nil { + return err + } + + sort.Slice(podList.Items, func(i, j int) bool { + return podList.Items[i].CreationTimestamp.Before(&podList.Items[j].CreationTimestamp) + }) + + terminationMessage = result.Message + if len(podList.Items) > 0 { + for _, podStatus := range podList.Items[len(podList.Items)-1].Status.ContainerStatuses { + if podStatus.Name != "step-git-source" && podStatus.State.Terminated != nil { + terminationMessage += podStatus.State.Terminated.Message + } + } + } + } + + gitRepo.Status.GitJobStatus = result.Status.String() + + for _, con := range result.Conditions { + condition.Cond(con.Type.String()).SetStatus(gitRepo, string(con.Status)) + condition.Cond(con.Type.String()).SetMessageIfBlank(gitRepo, con.Message) + condition.Cond(con.Type.String()).Reason(gitRepo, con.Reason) + } + + switch result.Status { + case status.FailedStatus: + kstatus.SetError(gitRepo, terminationMessage) + case status.CurrentStatus: + if strings.Contains(result.Message, "Job Completed") { + gitRepo.Status.Commit = job.Annotations["commit"] + } + kstatus.SetActive(gitRepo) + } + + return nil +} + func UpdateDisplayState(gitrepo *fleet.GitRepo) error { if gitrepo.Status.GitJobStatus != "Current" { gitrepo.Status.Display.State = "GitUpdating" @@ -137,7 +202,15 @@ func UpdateStatus(ctx context.Context, c client.Client, req types.NamespacedName if err != nil { return err } + commit := t.Status.Commit t.Status = status + if commit != "" && status.Commit == "" { + // we could incur in a race condition between the poller job + // setting the Commit and the first time the reconciler runs. + // The poller could be faster than the reconciler setting the + // Commit and we could reset back to "" in here + t.Status.Commit = commit + } err = c.Status().Update(ctx, t) if err != nil { diff --git a/internal/cmd/controller/operator.go b/internal/cmd/controller/operator.go index 906be6dac0..19a22af105 100644 --- a/internal/cmd/controller/operator.go +++ b/internal/cmd/controller/operator.go @@ -38,12 +38,10 @@ func start( leaderOpts LeaderElectionOptions, workersOpts ControllerReconcilerWorkers, bindAddresses BindAddresses, - disableGitops bool, disableMetrics bool, shardID string, ) error { setupLog.Info("listening for changes on local cluster", - "disableGitops", disableGitops, "disableMetrics", disableMetrics, ) @@ -121,21 +119,6 @@ func start( sched := quartz.NewStdScheduler() - if !disableGitops { - if err = (&reconciler.GitRepoReconciler{ - Client: mgr.GetClient(), - Scheme: mgr.GetScheme(), - - Scheduler: sched, - ShardID: shardID, - - Workers: workersOpts.GitRepo, - }).SetupWithManager(mgr); err != nil { - setupLog.Error(err, "unable to create controller", "controller", "GitRepo") - return err - } - } - // controllers that update status.display if err = (&reconciler.ClusterGroupReconciler{ Client: mgr.GetClient(), diff --git a/internal/cmd/controller/reconciler/bundle_controller.go b/internal/cmd/controller/reconciler/bundle_controller.go index 2305626b04..c9bd83764c 100644 --- a/internal/cmd/controller/reconciler/bundle_controller.go +++ b/internal/cmd/controller/reconciler/bundle_controller.go @@ -5,6 +5,7 @@ package reconciler import ( "context" + "github.com/rancher/fleet/internal/cmd/controller/finalize" "github.com/rancher/fleet/internal/cmd/controller/summary" "github.com/rancher/fleet/internal/cmd/controller/target" "github.com/rancher/fleet/internal/manifest" @@ -89,7 +90,7 @@ func (r *BundleReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr metrics.BundleCollector.Delete(req.Name, req.Namespace) logger.V(1).Info("Bundle not found, purging bundle deployments") - if err := purgeBundleDeployments(ctx, r.Client, req.NamespacedName); err != nil { + if err := finalize.PurgeBundleDeployments(ctx, r.Client, req.NamespacedName); err != nil { // A bundle deployment may have been purged by the GitRepo reconciler, hence we ignore // not-found errors here. return ctrl.Result{}, client.IgnoreNotFound(err) diff --git a/internal/cmd/controller/reconciler/gitrepo_controller.go b/internal/cmd/controller/reconciler/gitrepo_controller.go deleted file mode 100644 index 20dde5ac92..0000000000 --- a/internal/cmd/controller/reconciler/gitrepo_controller.go +++ /dev/null @@ -1,368 +0,0 @@ -// Copyright (c) 2021-2023 SUSE LLC - -package reconciler - -import ( - "context" - "fmt" - "reflect" - "slices" - "strings" - - "github.com/rancher/fleet/internal/cmd/controller/grutil" - "github.com/rancher/fleet/internal/cmd/controller/imagescan" - "github.com/rancher/fleet/internal/metrics" - fleet "github.com/rancher/fleet/pkg/apis/fleet.cattle.io/v1alpha1" - "github.com/rancher/fleet/pkg/sharding" - "github.com/reugn/go-quartz/quartz" - - "github.com/rancher/wrangler/v2/pkg/genericcondition" - - corev1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/runtime" - "k8s.io/apimachinery/pkg/types" - "k8s.io/client-go/util/retry" - ctrl "sigs.k8s.io/controller-runtime" - "sigs.k8s.io/controller-runtime/pkg/builder" - "sigs.k8s.io/controller-runtime/pkg/client" - "sigs.k8s.io/controller-runtime/pkg/controller" - "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" - "sigs.k8s.io/controller-runtime/pkg/event" - "sigs.k8s.io/controller-runtime/pkg/handler" - "sigs.k8s.io/controller-runtime/pkg/log" - "sigs.k8s.io/controller-runtime/pkg/predicate" -) - -const gitRepoFinalizer = "fleet.cattle.io/gitrepo-finalizer" - -// GitRepoReconciler reconciles a GitRepo object -type GitRepoReconciler struct { - client.Client - Scheme *runtime.Scheme - - Scheduler quartz.Scheduler - ShardID string - - Workers int -} - -//+kubebuilder:rbac:groups=fleet.cattle.io,resources=gitrepos,verbs=get;list;watch;create;update;patch;delete -//+kubebuilder:rbac:groups=fleet.cattle.io,resources=gitrepos/status,verbs=get;update;patch -//+kubebuilder:rbac:groups=fleet.cattle.io,resources=gitrepos/finalizers,verbs=update - -// Reconcile creates resources for a GitRepo -// nolint:gocyclo // creates multiple owned resources -func (r *GitRepoReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { - logger := log.FromContext(ctx).WithName("gitrepo") - - gitrepo := &fleet.GitRepo{} - if err := r.Get(ctx, req.NamespacedName, gitrepo); err != nil { - return ctrl.Result{}, client.IgnoreNotFound(err) - } - - if !gitrepo.DeletionTimestamp.IsZero() { - if controllerutil.ContainsFinalizer(gitrepo, gitRepoFinalizer) { - // Clean up - logger.V(1).Info("Gitrepo deleted, deleting bundle, image scans") - - metrics.GitRepoCollector.Delete(req.NamespacedName.Name, req.NamespacedName.Namespace) - - if err := purgeBundles(ctx, r.Client, req.NamespacedName); err != nil { - return ctrl.Result{}, err - } - - // remove the job scheduled by imagescan, if any - _ = r.Scheduler.DeleteJob(imagescan.GitCommitKey(req.Namespace, req.Name)) - - if err := purgeImageScans(ctx, r.Client, req.NamespacedName); err != nil { - return ctrl.Result{}, err - } - - err := retry.RetryOnConflict(retry.DefaultRetry, func() error { - if err := r.Get(ctx, req.NamespacedName, gitrepo); err != nil { - return err - } - - controllerutil.RemoveFinalizer(gitrepo, gitRepoFinalizer) - - return r.Update(ctx, gitrepo) - }) - - if client.IgnoreNotFound(err) != nil { - return ctrl.Result{}, err - } - } - - return ctrl.Result{}, nil - } - - if !controllerutil.ContainsFinalizer(gitrepo, gitRepoFinalizer) { - err := retry.RetryOnConflict(retry.DefaultRetry, func() error { - if err := r.Get(ctx, req.NamespacedName, gitrepo); err != nil { - return err - } - - controllerutil.AddFinalizer(gitrepo, gitRepoFinalizer) - - return r.Update(ctx, gitrepo) - }) - - if err != nil { - return ctrl.Result{}, client.IgnoreNotFound(err) - } - } - - logger = logger.WithValues("commit", gitrepo.Status.Commit) - logger.V(1).Info("Reconciling GitRepo", "lastAccepted", acceptedLastUpdate(gitrepo.Status.Conditions)) - - gitrepo.Status.ObservedGeneration = gitrepo.Generation - - if gitrepo.Spec.Repo == "" { - return ctrl.Result{}, nil - } - - // Restrictions / Overrides, copy status because AuthorizeAndAssignDefaults may return nil - oldStatus := gitrepo.Status.DeepCopy() - gitrepo, err := grutil.AuthorizeAndAssignDefaults(ctx, r.Client, gitrepo) - if err != nil { - return ctrl.Result{}, grutil.UpdateErrorStatus(ctx, r.Client, req.NamespacedName, *oldStatus, err) - } - - // Refresh the status - err = grutil.SetStatusFromBundleDeployments(ctx, r.Client, gitrepo) - if err != nil { - return ctrl.Result{}, grutil.UpdateErrorStatus(ctx, r.Client, req.NamespacedName, gitrepo.Status, err) - } - - err = grutil.SetStatusFromBundles(ctx, r.Client, gitrepo) - if err != nil { - return ctrl.Result{}, grutil.UpdateErrorStatus(ctx, r.Client, req.NamespacedName, gitrepo.Status, err) - } - - // Ideally, this should be done in the git job reconciler, but setting the status from bundle deployments - // updates the display state too. - if err = grutil.UpdateDisplayState(gitrepo); err != nil { - return ctrl.Result{}, grutil.UpdateErrorStatus(ctx, r.Client, req.NamespacedName, gitrepo.Status, err) - } - - grutil.SetStatusFromResourceKey(ctx, r.Client, gitrepo) - - gitrepo.Status.Display.ReadyBundleDeployments = fmt.Sprintf("%d/%d", - gitrepo.Status.Summary.Ready, - gitrepo.Status.Summary.DesiredReady) - - grutil.SetCondition(&gitrepo.Status, nil) - - err = grutil.UpdateStatus(ctx, r.Client, req.NamespacedName, gitrepo.Status) - if err != nil { - logger.V(1).Error(err, "Reconcile failed final update to git repo status", "status", gitrepo.Status) - - return ctrl.Result{}, err - } - - return ctrl.Result{}, nil -} - -// SetupWithManager sets up the controller with the Manager. -func (r *GitRepoReconciler) SetupWithManager(mgr ctrl.Manager) error { - // Note: Maybe use mgr.GetFieldIndexer().IndexField for better performance? - return ctrl.NewControllerManagedBy(mgr). - For(&fleet.GitRepo{}, - builder.WithPredicates( - // do not trigger for GitRepo status changes - predicate.Or( - predicate.GenerationChangedPredicate{}, - predicate.AnnotationChangedPredicate{}, - predicate.LabelChangedPredicate{}, - ), - ), - ). - Watches( - // Fan out from bundle to gitrepo - &fleet.Bundle{}, - handler.EnqueueRequestsFromMapFunc(func(ctx context.Context, a client.Object) []ctrl.Request { - repo := a.GetLabels()[fleet.RepoLabel] - if repo != "" { - return []ctrl.Request{{ - NamespacedName: types.NamespacedName{ - Namespace: a.GetNamespace(), - Name: repo, - }, - }} - } - - return []ctrl.Request{} - }), - builder.WithPredicates(bundleStatusChangedPredicate()), - ). - WithEventFilter(sharding.FilterByShardID(r.ShardID)). - WithOptions(controller.Options{MaxConcurrentReconciles: r.Workers}). - Complete(r) -} - -// bundleStatusChangedPredicate returns true if the bundle -// status has changed, or the bundle was created -func bundleStatusChangedPredicate() predicate.Funcs { - return predicate.Funcs{ - CreateFunc: func(e event.CreateEvent) bool { - return true - }, - UpdateFunc: func(e event.UpdateEvent) bool { - n, isBundle := e.ObjectNew.(*fleet.Bundle) - if !isBundle { - return false - } - o := e.ObjectOld.(*fleet.Bundle) - if n == nil || o == nil { - return false - } - return !reflect.DeepEqual(n.Status, o.Status) - }, - DeleteFunc: func(e event.DeleteEvent) bool { - return true - }, - } -} - -func purgeBundles(ctx context.Context, c client.Client, gitrepo types.NamespacedName) error { - bundles := &fleet.BundleList{} - err := c.List(ctx, bundles, client.MatchingLabels{fleet.RepoLabel: gitrepo.Name}, client.InNamespace(gitrepo.Namespace)) - if err != nil { - return err - } - - // At this point, access to the GitRepo is unavailable as it has been deleted and cannot be found within the cluster. - // Nevertheless, `deleteNamespace` can be found within all bundles generated from that GitRepo. Checking any bundle to get this value would be enough. - namespace := "" - deleteNamespace := false - sampleBundle := fleet.Bundle{} - if len(bundles.Items) > 0 { - sampleBundle = bundles.Items[0] - deleteNamespace = sampleBundle.Spec.DeleteNamespace - namespace = sampleBundle.Spec.TargetNamespace - - if sampleBundle.Spec.KeepResources { - deleteNamespace = false - } - } - - if err = purgeNamespace(ctx, c, deleteNamespace, namespace); err != nil { - return err - } - - for _, bundle := range bundles.Items { - err := c.Delete(ctx, &bundle) - if client.IgnoreNotFound(err) != nil { - return err - } - - nn := types.NamespacedName{Namespace: bundle.Namespace, Name: bundle.Name} - if err = purgeBundleDeployments(ctx, c, nn); err != nil { - return client.IgnoreNotFound(err) - } - } - - return nil -} - -func purgeBundleDeployments(ctx context.Context, c client.Client, bundle types.NamespacedName) error { - list := &fleet.BundleDeploymentList{} - err := c.List( - ctx, - list, - client.MatchingLabels{ - fleet.BundleLabel: bundle.Name, - fleet.BundleNamespaceLabel: bundle.Namespace, - }, - ) - if err != nil { - return err - } - for _, bd := range list.Items { - if controllerutil.ContainsFinalizer(&bd, bundleDeploymentFinalizer) { // nolint: gosec // does not store pointer - nn := types.NamespacedName{Namespace: bd.Namespace, Name: bd.Name} - err = retry.RetryOnConflict(retry.DefaultRetry, func() error { - t := &fleet.BundleDeployment{} - if err := c.Get(ctx, nn, t); err != nil { - return err - } - - controllerutil.RemoveFinalizer(t, bundleDeploymentFinalizer) - - return c.Update(ctx, t) - }) - if err != nil { - return err - } - } - - err := c.Delete(ctx, &bd) - if err != nil { - return err - } - } - - return nil -} - -func purgeImageScans(ctx context.Context, c client.Client, gitrepo types.NamespacedName) error { - images := &fleet.ImageScanList{} - err := c.List(ctx, images, client.InNamespace(gitrepo.Namespace)) - if err != nil { - return err - } - - for _, image := range images.Items { - if image.Spec.GitRepoName == gitrepo.Name { - err := c.Delete(ctx, &image) - if err != nil { - return err - } - } - - } - return nil -} - -func purgeNamespace(ctx context.Context, c client.Client, deleteNamespace bool, ns string) error { - if !deleteNamespace { - return nil - } - - if ns == "" { - return nil - } - - // Ignore default namespaces - defaultNamespaces := []string{"fleet-local", "cattle-fleet-system", "fleet-default", "cattle-fleet-clusters-system", "default"} - if slices.Contains(defaultNamespaces, ns) { - return nil - } - - // Ignore system namespaces - if _, isKubeNamespace := strings.CutPrefix(ns, "kube-"); isKubeNamespace { - return nil - } - - namespace := &corev1.Namespace{ - ObjectMeta: metav1.ObjectMeta{ - Name: ns, - }, - } - if err := c.Delete(ctx, namespace); err != nil { - return err - } - - return nil -} - -func acceptedLastUpdate(conds []genericcondition.GenericCondition) string { - for _, cond := range conds { - if cond.Type == "Accepted" { - return cond.LastUpdateTime - } - } - - return "" -} diff --git a/internal/cmd/controller/root.go b/internal/cmd/controller/root.go index ea04c6be95..df94deb8f7 100644 --- a/internal/cmd/controller/root.go +++ b/internal/cmd/controller/root.go @@ -34,7 +34,6 @@ type FleetManager struct { command.DebugConfig Kubeconfig string `usage:"Kubeconfig file"` Namespace string `usage:"namespace to watch" default:"cattle-fleet-system" env:"NAMESPACE"` - DisableGitops bool `usage:"disable gitops components" name:"disable-gitops"` DisableMetrics bool `usage:"disable metrics" name:"disable-metrics"` ShardID string `usage:"only manage resources labeled with a specific shard ID" name:"shard-id"` } @@ -136,13 +135,6 @@ func (f *FleetManager) Run(cmd *cobra.Command, args []string) error { bindAddresses.HealthProbe = d } - if d := os.Getenv("GITREPO_RECONCILER_WORKERS"); d != "" { - w, err := strconv.Atoi(d) - if err != nil { - setupLog.Error(err, "failed to parse GITREPO_RECONCILER_WORKERS", "value", d) - } - workersOpts.GitRepo = w - } if d := os.Getenv("BUNDLE_RECONCILER_WORKERS"); d != "" { w, err := strconv.Atoi(d) if err != nil { @@ -169,7 +161,6 @@ func (f *FleetManager) Run(cmd *cobra.Command, args []string) error { leaderOpts, workersOpts, bindAddresses, - f.DisableGitops, f.DisableMetrics, f.ShardID, ); err != nil { diff --git a/internal/metrics/metrics.go b/internal/metrics/metrics.go index 45d379acea..8424a11a6e 100644 --- a/internal/metrics/metrics.go +++ b/internal/metrics/metrics.go @@ -40,6 +40,12 @@ func RegisterMetrics() { BundleDeploymentCollector.Register() } +func RegisterGitOptsMetrics() { + enabled = true + + GitRepoCollector.Register() +} + // CollectorCollection implements the generic methods `Delete` and `Register` // for a collection of Prometheus collectors. It is used to manage the lifecycle // of a collection of Prometheus collectors.