Skip to content

Commit

Permalink
chore: revert "feat: removed legacy app tracking label (#13203)" (che…
Browse files Browse the repository at this point in the history
…rry-pick #19196)

This reverts commit 4d8436b.

Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
Co-authored-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
  • Loading branch information
gcp-cherry-pick-bot[bot] and crenshaw-dev committed Jul 24, 2024
1 parent cec8504 commit d926310
Show file tree
Hide file tree
Showing 4 changed files with 123 additions and 6 deletions.
4 changes: 3 additions & 1 deletion util/argo/resource_tracking.go
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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 <application-name>;<group>/<kind>/<namespace>/<name>
Expand Down
70 changes: 70 additions & 0 deletions util/kube/kube.go
Original file line number Diff line number Diff line change
Expand Up @@ -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])?$")
Expand All @@ -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 {
Expand All @@ -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 {
Expand Down
50 changes: 50 additions & 0 deletions util/kube/kube_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
5 changes: 0 additions & 5 deletions util/settings/settings.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down

0 comments on commit d926310

Please # to comment.