Drop support for `applications.argoproj.io/app-name`
Summary
Remove support for old tracking label, applications.argoproj.io/app-name
Motivation
"applications.argoproj.io/app-name" is found once in the argo-cd repo:
// LabelKeyLegacyApplicationName is the legacy label (v0.10 and below) and is superseded by 'app.kubernetes.io/instance'
LabelKeyLegacyApplicationName = "applications.argoproj.io/app-name"
That's very old.
The constant is used in one place: SetAppInstanceLabel.
That function is called in only one place: SetAppInstance.
That function is called by groupObjsForDiff, which uses argoSettings.AppLabelKey for the key param, and by GenerateManifests, which uses q.AppLabelKey for the key param.
q.AppLabelKey is set in 4 places:
-
getLocalObjectsString- called by
getLocalObjects- called by
findandPrintDiff, which usesargoSettings.AppLabelKey - called by
NewApplicationManifestsCommand, which usesargoSettings.AppLabelKey
- called by
- called by
NewApplicationSyncCommand, which usesargoSettings.AppLabelKey
- called by
-
getRepoObjs- called by
CompareAppState, which usesappStateManager.settingsMgr.GetAppInstanceLabelKey(), which pulls it directly fromapplication.instanceLabelKeyin argocd-cm
- called by
-
GetManifests, which usesServer.settingsMgr.GetAppInstanceLabelKey(), which gets the value directly from the configmap -
GetManifestsWithFiles, which does the same asGetManifests
I believe that is a full accounting of where the deprecated label was used. Basically, if that label isn't explicitly configured in argocd-cm, we're not using it in the code base.
Proposal
Let's remove support for this label in v3.0.
Some history/context about this, applications.argoproj.io/app-name is a little special. It was our first tracking method and IIRC, the label actually makes its way down to the pod spec, and not just the top-level managed resource. There are a handful of Argo CD instances that might use this legacy labeling at Intuit, but migrating away from it might actually cause a redeploy of Pods.
But I can confidently say the only people that might be affected by removing this label is internal argo cd instances at Intuit. It's also easy to check to see which ones are impacted by looking at the ConfigMap of who is using this legacy key.
@crenshaw-dev is there anything else we need to do for this issue apart from removing the extra code and tests that handle the case for LabelKeyLegacyApplicationName in SetAppInstanceLabel()? I do not see any other changes from a code perspective but we can also probably add a check to explicitly return app.kubernetes.io/instance in GetAppInstanceLabelKey() if the key in the configmap is applications.argoproj.io/app-name which would probably make it easier for migrating any user that's still using the old label? WDYT?
That seems reasonable. We'd also want to log warnings that the user should update their configured tracking method. And we should have complete release notes explaining the risk that the upgrade might cause pod restarts.
@crenshaw-dev this was reverted if I am not mistaken. Should we reopen as this will be related to https://github.com/argoproj/argo-cd/issues/13981
Yep!
Can we start by merging just a deprecation warning in the next 2.x release so that when 3.0 comes around it can be missing?
Removing from 3.0 as per discussion with @crenshaw-dev