diff --git a/util/argo/resource_tracking.go b/util/argo/resource_tracking.go index f6b98683e9e91..476f739c13924 100644 --- a/util/argo/resource_tracking.go +++ b/util/argo/resource_tracking.go @@ -149,6 +149,7 @@ func (rt *resourceTracking) SetAppInstance(un *unstructured.Unstructured, key, v if err != nil { return fmt.Errorf("failed to set app instance label: %w", err) } + return nil case TrackingMethodAnnotation: return setAppInstanceAnnotation() case TrackingMethodAnnotationAndLabel: @@ -171,13 +172,14 @@ func (rt *resourceTracking) SetAppInstance(un *unstructured.Unstructured, key, v if err != nil { return fmt.Errorf("failed to set app instance label: %w", err) } + return nil default: err := argokube.SetAppInstanceLabel(un, key, val) if err != nil { return fmt.Errorf("failed to set app instance label: %w", err) } + return nil } - return nil } // BuildAppInstanceValue build resource tracking id in format ;/// diff --git a/util/kube/kube.go b/util/kube/kube.go index e32aabac00b63..5ea4394b726f0 100644 --- a/util/kube/kube.go +++ b/util/kube/kube.go @@ -4,7 +4,11 @@ import ( "fmt" "regexp" + "github.com/argoproj/gitops-engine/pkg/utils/kube" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime/schema" + + "github.com/argoproj/argo-cd/v2/common" ) var resourceNamePattern = regexp.MustCompile("^[a-z0-9]([-a-z0-9]*[a-z0-9])?$") @@ -15,6 +19,7 @@ func IsValidResourceName(name string) bool { } // SetAppInstanceLabel the recommended app.kubernetes.io/instance label against an unstructured object +// Uses the legacy labeling if environment variable is set func SetAppInstanceLabel(target *unstructured.Unstructured, key, val string) error { labels, _, err := nestedNullableStringMap(target.Object, "metadata", "labels") if err != nil { @@ -25,10 +30,75 @@ func SetAppInstanceLabel(target *unstructured.Unstructured, key, val string) err } labels[key] = val target.SetLabels(labels) + if key != common.LabelKeyLegacyApplicationName { + // we no longer label the pod template sub resources in v0.11 + return nil + } + + gvk := schema.FromAPIVersionAndKind(target.GetAPIVersion(), target.GetKind()) + // special case for deployment and job types: make sure that derived replicaset, and pod has + // the application label + switch gvk.Group { + case "apps", "extensions": + switch gvk.Kind { + case kube.DeploymentKind, kube.ReplicaSetKind, kube.StatefulSetKind, kube.DaemonSetKind: + templateLabels, ok, err := unstructured.NestedMap(target.UnstructuredContent(), "spec", "template", "metadata", "labels") + if err != nil { + return err + } + if !ok || templateLabels == nil { + templateLabels = make(map[string]interface{}) + } + templateLabels[key] = val + err = unstructured.SetNestedMap(target.UnstructuredContent(), templateLabels, "spec", "template", "metadata", "labels") + if err != nil { + return err + } + // The following is a workaround for issue #335. In API version extensions/v1beta1 or + // apps/v1beta1, if a spec omits spec.selector then k8s will default the + // spec.selector.matchLabels to match spec.template.metadata.labels. This means Argo CD + // labels can potentially make their way into spec.selector.matchLabels, which is a bad + // thing. The following logic prevents this behavior. + switch target.GetAPIVersion() { + case "apps/v1beta1", "extensions/v1beta1": + selector, _, err := unstructured.NestedMap(target.UnstructuredContent(), "spec", "selector") + if err != nil { + return err + } + if len(selector) == 0 { + // If we get here, user did not set spec.selector in their manifest. We do not want + // our Argo CD labels to get defaulted by kubernetes, so we explicitly set the labels + // for them (minus the Argo CD labels). + delete(templateLabels, key) + err = unstructured.SetNestedMap(target.UnstructuredContent(), templateLabels, "spec", "selector", "matchLabels") + if err != nil { + return err + } + } + } + } + case "batch": + switch gvk.Kind { + case kube.JobKind: + templateLabels, ok, err := unstructured.NestedMap(target.UnstructuredContent(), "spec", "template", "metadata", "labels") + if err != nil { + return err + } + if !ok || templateLabels == nil { + templateLabels = make(map[string]interface{}) + } + templateLabels[key] = val + err = unstructured.SetNestedMap(target.UnstructuredContent(), templateLabels, "spec", "template", "metadata", "labels") + if err != nil { + return err + } + } + } return nil } // SetAppInstanceAnnotation the recommended app.kubernetes.io/instance annotation against an unstructured object +// Uses the legacy labeling if environment variable is set func SetAppInstanceAnnotation(target *unstructured.Unstructured, key, val string) error { annotations, _, err := nestedNullableStringMap(target.Object, "metadata", "annotations") if err != nil { diff --git a/util/kube/kube_test.go b/util/kube/kube_test.go index 389a2d0f9870a..51474359bde5e 100644 --- a/util/kube/kube_test.go +++ b/util/kube/kube_test.go @@ -84,6 +84,56 @@ func TestSetLabels(t *testing.T) { } } +func TestSetLegacyLabels(t *testing.T) { + for _, yamlStr := range []string{depWithoutSelector, depWithSelector} { + var obj unstructured.Unstructured + err := yaml.Unmarshal([]byte(yamlStr), &obj) + require.NoError(t, err) + + err = SetAppInstanceLabel(&obj, common.LabelKeyLegacyApplicationName, "my-app") + require.NoError(t, err) + + manifestBytes, err := json.MarshalIndent(obj.Object, "", " ") + require.NoError(t, err) + log.Println(string(manifestBytes)) + + var depV1Beta1 extv1beta1.Deployment + err = json.Unmarshal(manifestBytes, &depV1Beta1) + require.NoError(t, err) + assert.Len(t, depV1Beta1.Spec.Selector.MatchLabels, 1) + assert.Equal(t, "nginx", depV1Beta1.Spec.Selector.MatchLabels["app"]) + assert.Len(t, depV1Beta1.Spec.Template.Labels, 2) + assert.Equal(t, "nginx", depV1Beta1.Spec.Template.Labels["app"]) + assert.Equal(t, "my-app", depV1Beta1.Spec.Template.Labels[common.LabelKeyLegacyApplicationName]) + } +} + +func TestSetLegacyJobLabel(t *testing.T) { + yamlBytes, err := os.ReadFile("testdata/job.yaml") + require.NoError(t, err) + var obj unstructured.Unstructured + err = yaml.Unmarshal(yamlBytes, &obj) + require.NoError(t, err) + err = SetAppInstanceLabel(&obj, common.LabelKeyLegacyApplicationName, "my-app") + require.NoError(t, err) + + manifestBytes, err := json.MarshalIndent(obj.Object, "", " ") + require.NoError(t, err) + log.Println(string(manifestBytes)) + + job := unstructured.Unstructured{} + err = json.Unmarshal(manifestBytes, &job) + require.NoError(t, err) + + labels := job.GetLabels() + assert.Equal(t, "my-app", labels[common.LabelKeyLegacyApplicationName]) + + templateLabels, ok, err := unstructured.NestedMap(job.UnstructuredContent(), "spec", "template", "metadata", "labels") + assert.True(t, ok) + require.NoError(t, err) + assert.Equal(t, "my-app", templateLabels[common.LabelKeyLegacyApplicationName]) +} + func TestSetSvcLabel(t *testing.T) { yamlBytes, err := os.ReadFile("testdata/svc.yaml") require.NoError(t, err) diff --git a/util/settings/settings.go b/util/settings/settings.go index 1f036ddeff7c6..8cb1c4df956f5 100644 --- a/util/settings/settings.go +++ b/util/settings/settings.go @@ -761,11 +761,6 @@ func (mgr *SettingsManager) GetAppInstanceLabelKey() (string, error) { if label == "" { return common.LabelKeyAppInstance, nil } - // return new label key if user is still using legacy key - if label == common.LabelKeyLegacyApplicationName { - log.Warnf("deprecated legacy application instance tracking key(%v) is present in configmap, new key(%v) will be used automatically", common.LabelKeyLegacyApplicationName, common.LabelKeyAppInstance) - return common.LabelKeyAppInstance, nil - } return label, nil }