From b6d538dca857cf8dfc72934abdcb9dc79b5b536f Mon Sep 17 00:00:00 2001 From: Yoga Yu Date: Sat, 7 Jun 2025 01:40:47 +0800 Subject: [PATCH 1/6] fix: ignore filter when in-ns --- internal/pkg/cmd/reloader.go | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/internal/pkg/cmd/reloader.go b/internal/pkg/cmd/reloader.go index f0aac83..86f8379 100644 --- a/internal/pkg/cmd/reloader.go +++ b/internal/pkg/cmd/reloader.go @@ -128,9 +128,11 @@ func startReloader(cmd *cobra.Command, args []string) { } logrus.Info("Starting Reloader") + isGlobal := false currentNamespace := os.Getenv("KUBERNETES_NAMESPACE") if len(currentNamespace) == 0 { currentNamespace = v1.NamespaceAll + isGlobal = true logrus.Warnf("KUBERNETES_NAMESPACE is unset, will detect changes in all namespaces.") } @@ -150,7 +152,7 @@ func startReloader(cmd *cobra.Command, args []string) { logrus.Fatal(err) } - namespaceLabelSelector, err := getNamespaceLabelSelector(cmd) + namespaceLabelSelector, err := getNamespaceLabelSelector(cmd, isGlobal) if err != nil { logrus.Fatal(err) } @@ -215,7 +217,7 @@ func getIgnoredNamespacesList(cmd *cobra.Command) (util.List, error) { return getStringSliceFromFlags(cmd, "namespaces-to-ignore") } -func getNamespaceLabelSelector(cmd *cobra.Command) (string, error) { +func getNamespaceLabelSelector(cmd *cobra.Command, isGlobal bool) (string, error) { slice, err := getStringSliceFromFlags(cmd, "namespace-selector") if err != nil { logrus.Fatal(err) @@ -246,6 +248,11 @@ func getNamespaceLabelSelector(cmd *cobra.Command) (string, error) { logrus.Fatal(err) } + if !isGlobal && len(namespaceLabelSelector) > 0 { + logrus.Warnf("KUBERNETES_NAMESPACE is set but also namespace-selector is set, will ignore the filter and detect changes in the specific namespace.") + return "", nil + } + return namespaceLabelSelector, nil } From 8c2f2e574cd930fddd0ba9c5a8c4ab271b3c7fa0 Mon Sep 17 00:00:00 2001 From: Yoga Yu Date: Sun, 8 Jun 2025 21:48:44 +0800 Subject: [PATCH 2/6] feat: ignore namespaceSelector when not necessory --- .../kubernetes/chart/reloader/templates/_helpers.tpl | 9 +++++++++ .../kubernetes/chart/reloader/templates/clusterrole.yaml | 2 +- .../kubernetes/chart/reloader/templates/deployment.yaml | 6 +++--- 3 files changed, 13 insertions(+), 4 deletions(-) diff --git a/deployments/kubernetes/chart/reloader/templates/_helpers.tpl b/deployments/kubernetes/chart/reloader/templates/_helpers.tpl index 1c27621..4e39b75 100644 --- a/deployments/kubernetes/chart/reloader/templates/_helpers.tpl +++ b/deployments/kubernetes/chart/reloader/templates/_helpers.tpl @@ -70,3 +70,12 @@ Create the annotations to support helm3 meta.helm.sh/release-namespace: {{ .Release.Namespace | quote }} meta.helm.sh/release-name: {{ .Release.Name | quote }} {{- end -}} + +{{/* +Create the namespace selector if it does not watch globally +*/}} +{{- define "reloader-namespaceSelector" -}} +{{- if and .Values.reloader.watchGlobally .Values.reloader.namespaceSelector -}} + {{ .Values.reloader.namespaceSelector }} +{{- end -}} +{{- end -}} diff --git a/deployments/kubernetes/chart/reloader/templates/clusterrole.yaml b/deployments/kubernetes/chart/reloader/templates/clusterrole.yaml index f62c406..9cd3ec5 100644 --- a/deployments/kubernetes/chart/reloader/templates/clusterrole.yaml +++ b/deployments/kubernetes/chart/reloader/templates/clusterrole.yaml @@ -31,7 +31,7 @@ rules: - list - get - watch -{{- if .Values.reloader.namespaceSelector }} +{{- if (include "reloader-namespaceSelector" .) }} - apiGroups: - "" resources: diff --git a/deployments/kubernetes/chart/reloader/templates/deployment.yaml b/deployments/kubernetes/chart/reloader/templates/deployment.yaml index 851230e..23d6447 100644 --- a/deployments/kubernetes/chart/reloader/templates/deployment.yaml +++ b/deployments/kubernetes/chart/reloader/templates/deployment.yaml @@ -198,7 +198,7 @@ spec: {{- . | toYaml | nindent 10 }} {{- end }} {{- end }} - {{- if or (.Values.reloader.logFormat) (.Values.reloader.logLevel) (.Values.reloader.ignoreSecrets) (.Values.reloader.ignoreNamespaces) (.Values.reloader.namespaceSelector) (.Values.reloader.resourceLabelSelector) (.Values.reloader.ignoreConfigMaps) (.Values.reloader.custom_annotations) (eq .Values.reloader.isArgoRollouts true) (eq .Values.reloader.reloadOnCreate true) (eq .Values.reloader.reloadOnDelete true) (ne .Values.reloader.reloadStrategy "default") (.Values.reloader.enableHA) (.Values.reloader.autoReloadAll)}} + {{- if or (.Values.reloader.logFormat) (.Values.reloader.logLevel) (.Values.reloader.ignoreSecrets) (.Values.reloader.ignoreNamespaces) (include "reloader-namespaceSelector" .) (.Values.reloader.resourceLabelSelector) (.Values.reloader.ignoreConfigMaps) (.Values.reloader.custom_annotations) (eq .Values.reloader.isArgoRollouts true) (eq .Values.reloader.reloadOnCreate true) (eq .Values.reloader.reloadOnDelete true) (ne .Values.reloader.reloadStrategy "default") (.Values.reloader.enableHA) (.Values.reloader.autoReloadAll)}} args: {{- if .Values.reloader.logFormat }} - "--log-format={{ .Values.reloader.logFormat }}" @@ -215,8 +215,8 @@ spec: {{- if .Values.reloader.ignoreNamespaces }} - "--namespaces-to-ignore={{ .Values.reloader.ignoreNamespaces }}" {{- end }} - {{- if .Values.reloader.namespaceSelector }} - - "--namespace-selector={{ .Values.reloader.namespaceSelector }}" + {{- if (include "reloader-namespaceSelector" .) }} + - "--namespace-selector=\"{{ include "reloader-namespaceSelector" . }}\"" {{- end }} {{- if .Values.reloader.resourceLabelSelector }} - "--resource-label-selector={{ .Values.reloader.resourceLabelSelector }}" From 023425d4e17287954547a753b9b70c2a602dbb54 Mon Sep 17 00:00:00 2001 From: Yoga Yu Date: Sun, 8 Jun 2025 21:54:13 +0800 Subject: [PATCH 3/6] chore: update doc --- deployments/kubernetes/chart/reloader/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/deployments/kubernetes/chart/reloader/README.md b/deployments/kubernetes/chart/reloader/README.md index d85e270..95fdc31 100644 --- a/deployments/kubernetes/chart/reloader/README.md +++ b/deployments/kubernetes/chart/reloader/README.md @@ -52,7 +52,7 @@ helm uninstall {{RELEASE_NAME}} -n {{NAMESPACE}} | `reloader.syncAfterRestart` | Enable sync after Reloader restarts for **Add** events, works only when reloadOnCreate is `true`. Valid value are either `true` or `false` | boolean | `false` | | `reloader.reloadStrategy` | Strategy to trigger resource restart, set to either `default`, `env-vars` or `annotations` | enumeration | `default` | | `reloader.ignoreNamespaces` | List of comma separated namespaces to ignore, if multiple are provided, they are combined with the AND operator | string | `""` | -| `reloader.namespaceSelector` | List of comma separated k8s label selectors for namespaces selection. See [LIST and WATCH filtering](https://kubernetes.io/docs/concepts/overview/working-with-objects/labels/#list-and-watch-filtering) for more details on label-selector | string | `""` | +| `reloader.namespaceSelector` | List of comma separated k8s label selectors for namespaces selection. The parameter only used when `reloader.watchGlobally` is `true`. See [LIST and WATCH filtering](https://kubernetes.io/docs/concepts/overview/working-with-objects/labels/#list-and-watch-filtering) for more details on label-selector | string | `""` | | `reloader.resourceLabelSelector` | List of comma separated label selectors, if multiple are provided they are combined with the AND operator | string | `""` | | `reloader.logFormat` | Set type of log format. Value could be either `json` or `""` | string | `""` | | `reloader.watchGlobally` | Allow Reloader to watch in all namespaces (`true`) or just in a single namespace (`false`) | boolean | `true` | From ad0407517d86e55d6520ce010f5f57b1cde2d7f9 Mon Sep 17 00:00:00 2001 From: Christian Ciach Date: Fri, 14 Mar 2025 11:35:40 +0100 Subject: [PATCH 4/6] Add OwnerReference to generated Jobs --- internal/pkg/callbacks/rolling_upgrade.go | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/internal/pkg/callbacks/rolling_upgrade.go b/internal/pkg/callbacks/rolling_upgrade.go index 81c0b65..332a8b0 100644 --- a/internal/pkg/callbacks/rolling_upgrade.go +++ b/internal/pkg/callbacks/rolling_upgrade.go @@ -444,10 +444,15 @@ func PatchDeployment(clients kube.Clients, namespace string, resource runtime.Ob func CreateJobFromCronjob(clients kube.Clients, namespace string, resource runtime.Object) error { cronJob := resource.(*batchv1.CronJob) job := &batchv1.Job{ - ObjectMeta: cronJob.Spec.JobTemplate.ObjectMeta, - Spec: cronJob.Spec.JobTemplate.Spec, + ObjectMeta: meta_v1.ObjectMeta{ + GenerateName: cronJob.Name + "-", + Namespace: cronJob.Namespace, + Annotations: cronJob.Spec.JobTemplate.Annotations, + Labels: cronJob.Spec.JobTemplate.Labels, + OwnerReferences: []meta_v1.OwnerReference{*meta_v1.NewControllerRef(cronJob, batchv1.SchemeGroupVersion.WithKind("CronJob"))}, + }, + Spec: cronJob.Spec.JobTemplate.Spec, } - job.GenerateName = cronJob.Name + "-" _, err := clients.KubernetesClient.BatchV1().Jobs(namespace).Create(context.TODO(), job, meta_v1.CreateOptions{FieldManager: "Reloader"}) return err } From 81315adc9bd8a7a4dfc40f8e844906c4b29b3c99 Mon Sep 17 00:00:00 2001 From: Christian Ciach Date: Tue, 18 Mar 2025 14:18:13 +0100 Subject: [PATCH 5/6] Add conventional annotation to triggered jobs --- internal/pkg/callbacks/rolling_upgrade.go | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/internal/pkg/callbacks/rolling_upgrade.go b/internal/pkg/callbacks/rolling_upgrade.go index 332a8b0..c7b2e5c 100644 --- a/internal/pkg/callbacks/rolling_upgrade.go +++ b/internal/pkg/callbacks/rolling_upgrade.go @@ -16,6 +16,8 @@ import ( "k8s.io/apimachinery/pkg/runtime" patchtypes "k8s.io/apimachinery/pkg/types" + "maps" + argorolloutv1alpha1 "github.com/argoproj/argo-rollouts/pkg/apis/rollouts/v1alpha1" ) @@ -443,11 +445,16 @@ func PatchDeployment(clients kube.Clients, namespace string, resource runtime.Ob // CreateJobFromCronjob performs rolling upgrade on cronjob func CreateJobFromCronjob(clients kube.Clients, namespace string, resource runtime.Object) error { cronJob := resource.(*batchv1.CronJob) + + annotations := make(map[string]string) + annotations["cronjob.kubernetes.io/instantiate"] = "manual" + maps.Copy(annotations, cronJob.Spec.JobTemplate.Annotations) + job := &batchv1.Job{ ObjectMeta: meta_v1.ObjectMeta{ GenerateName: cronJob.Name + "-", Namespace: cronJob.Namespace, - Annotations: cronJob.Spec.JobTemplate.Annotations, + Annotations: annotations, Labels: cronJob.Spec.JobTemplate.Labels, OwnerReferences: []meta_v1.OwnerReference{*meta_v1.NewControllerRef(cronJob, batchv1.SchemeGroupVersion.WithKind("CronJob"))}, }, From 99c45b3ca35834b73a9b0fad9a47bf6baa24abbe Mon Sep 17 00:00:00 2001 From: Christian Ciach Date: Wed, 9 Jul 2025 12:28:48 +0200 Subject: [PATCH 6/6] Add test for CronJob owner references --- .../pkg/callbacks/rolling_upgrade_test.go | 28 +++++++++++++++++-- 1 file changed, 25 insertions(+), 3 deletions(-) diff --git a/internal/pkg/callbacks/rolling_upgrade_test.go b/internal/pkg/callbacks/rolling_upgrade_test.go index 31d4411..b9f48f3 100644 --- a/internal/pkg/callbacks/rolling_upgrade_test.go +++ b/internal/pkg/callbacks/rolling_upgrade_test.go @@ -415,13 +415,26 @@ func TestPatchResources(t *testing.T) { func TestCreateJobFromCronjob(t *testing.T) { fixtures := newTestFixtures() - cronJob, err := createTestCronJobWithAnnotations(clients, fixtures.namespace, "1") + runtimeObj, err := createTestCronJobWithAnnotations(clients, fixtures.namespace, "1") assert.NoError(t, err) - err = callbacks.CreateJobFromCronjob(clients, fixtures.namespace, cronJob.(*batchv1.CronJob)) + cronJob := runtimeObj.(*batchv1.CronJob) + err = callbacks.CreateJobFromCronjob(clients, fixtures.namespace, cronJob) assert.NoError(t, err) - err = deleteTestCronJob(clients, fixtures.namespace, "test-cronjob") + jobList, err := clients.KubernetesClient.BatchV1().Jobs(fixtures.namespace).List(context.TODO(), metav1.ListOptions{}) + assert.NoError(t, err) + + ownerFound := false + for _, job := range jobList.Items { + if isControllerOwner("CronJob", cronJob.Name, job.OwnerReferences) { + ownerFound = true + break + } + } + assert.Truef(t, ownerFound, "Missing CronJob owner reference") + + err = deleteTestCronJob(clients, fixtures.namespace, cronJob.Name) assert.NoError(t, err) } @@ -749,3 +762,12 @@ func createTestJobWithAnnotations(clients kube.Clients, namespace, version strin func deleteTestJob(clients kube.Clients, namespace, name string) error { return clients.KubernetesClient.BatchV1().Jobs(namespace).Delete(context.TODO(), name, metav1.DeleteOptions{}) } + +func isControllerOwner(kind, name string, ownerRefs []metav1.OwnerReference) bool { + for _, ownerRef := range ownerRefs { + if *ownerRef.Controller && ownerRef.Kind == kind && ownerRef.Name == name { + return true + } + } + return false +}