Skip to content

feature: use annotation for app name if longer than max len#4123

Closed
darshanime wants to merge 1 commit intoargoproj:masterfrom
darshanime:label_annotation
Closed

feature: use annotation for app name if longer than max len#4123
darshanime wants to merge 1 commit intoargoproj:masterfrom
darshanime:label_annotation

Conversation

@darshanime
Copy link
Member

@darshanime darshanime commented Aug 19, 2020

Signed-off-by: darshanime deathbullet@gmail.com

closes #4045
blocked on argoproj/gitops-engine#121

Checklist:

  • Either (a) I've created an enhancement proposal and discussed it with the community, (b) this is a bug fix, or (c) this does not need to be in the release notes.
  • The title of the PR states what changed and the related issues number (used for the release note).
  • I've updated both the CLI and UI to expose my feature, or I plan to submit a second PR with them.
  • Optional. My organization is added to USERS.md.
  • I've signed the CLA and my build is green (troubleshooting builds).

@sbose78
Copy link
Contributor

sbose78 commented Aug 25, 2020

@darshanime Could you update the PR description with the annotation you are proposing ?

@NargiT
Copy link

NargiT commented Sep 22, 2020

any update on this?

@jessesuen jessesuen added this to the v1.8 milestone Oct 1, 2020
@mayzhang2000 mayzhang2000 self-assigned this Oct 1, 2020
@mayzhang2000
Copy link
Contributor

@darshanime, please let me know when this is ready to be reviewed. Thank you.

@alexmt alexmt closed this Oct 29, 2020
@alexmt alexmt reopened this Oct 30, 2020
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please see comment in argoproj/gitops-engine#121 (comment)

Instead of using GetAppInstanceIdentifier from gitops engine let's move it into Argo CD repo.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Annotation is preferable in some other cases, not only when the application name is too long.

Instead of automatically switching to annotation let's introduce an application-level setting that allows switching from label to annotation. I would suggest using the argocd.argoproj.io/annotation-marker annotation . E.g. if application has argocd.argoproj.io/annotation-marker annotation then argocd should make resources with annotation:

apiVersion: argoproj.io/v1alpha1
kind: Application
metadata:
  name: guestbook
  annotations:
    # instruct argocd to use annotation instead of label
    argocd.argoproj.io/annotation-marker: 'true'

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please make sure to check for annotation and label in

appName := kube.GetAppInstanceLabel(un, cacheSettings.appInstanceLabelKey)

Controller should cache whole resource manifest if either annotation or label present.

@alexmt alexmt modified the milestones: v1.8, v1.9 Nov 5, 2020
Signed-off-by: darshanime <deathbullet@gmail.com>
@NargiT
Copy link

NargiT commented Jun 21, 2021

hello any updates for this ?

@alexmt alexmt modified the milestones: v2.1, v2.2 Jul 2, 2021
@alexmt
Copy link
Collaborator

alexmt commented Oct 4, 2021

The same changes were implemented by #7251

@alexmt alexmt closed this Oct 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support Application names more than 63 chars long

6 participants