Skip to content

[velero] Refactoring labels and annotations#281

Open
nlamirault wants to merge 16 commits intovmware-tanzu:mainfrom
nlamirault:feat/labels
Open

[velero] Refactoring labels and annotations#281
nlamirault wants to merge 16 commits intovmware-tanzu:mainfrom
nlamirault:feat/labels

Conversation

@nlamirault
Copy link
Copy Markdown

@nlamirault nlamirault commented Jun 30, 2021

Special notes for your reviewer:

  • support for custom annotations and custom labels for all resources
  • remove name label on 2 resources
  • split labels and selector labels (Helm recommendation)

Checklist

[Place an '[x]' (no spaces) in all applicable fields. Please remove unrelated fields.]

  • DCO signed
  • Chart Version bumped
  • Variables are documented in the values.yaml or README.md
  • Title of the PR starts with chart name (e.g. [velero])

Signed-off-by: Nicolas Lamirault <nicolas.lamirault@gmail.com>
Signed-off-by: Nicolas Lamirault <nicolas.lamirault@gmail.com>
Signed-off-by: Nicolas Lamirault <nicolas.lamirault@gmail.com>
Signed-off-by: Nicolas Lamirault <nicolas.lamirault@gmail.com>
Signed-off-by: Nicolas Lamirault <nicolas.lamirault@gmail.com>
Copy link
Copy Markdown
Collaborator

@jenting jenting left a comment

Choose a reason for hiding this comment

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

Thank you for your improvement to make the chart easier to read and configure.
But the CI is not passed, could you please take a look?

@jenting jenting added velero enhancement New feature or request labels Jul 5, 2021
Signed-off-by: Nicolas Lamirault <nicolas.lamirault@gmail.com>
@nlamirault
Copy link
Copy Markdown
Author

@jenting i don't understard CI error. Node not ready ?

@jenting
Copy link
Copy Markdown
Collaborator

jenting commented Jul 5, 2021

Okay, I'll look at it later.

Signed-off-by: JenTing Hsiao <jenting.hsiao@suse.com>
@jenting
Copy link
Copy Markdown
Collaborator

jenting commented Jul 6, 2021

I push a commit to test it :)

Comment on lines 20 to 22
{{- with .Values.labels }}
{{- toYaml . | nindent 4 }}
{{- end }}
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

With removing these lines, will this make the labels duplicated?

Comment on lines +9 to +11
{{- if .Values.customAnnotations }}
{{- toYaml .Values.customAnnotations | nindent 4 }}
{{- end }}
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Personally, I'd prefer.

    {{- With .Values.customAnnotations }}
    {{- toYaml . | nindent 4 }}
    {{- end }}

This makes the code simpler.

Signed-off-by: Nicolas Lamirault <nicolas.lamirault@gmail.com>
Signed-off-by: Nicolas Lamirault <nicolas.lamirault@gmail.com>
Signed-off-by: Nicolas Lamirault <nicolas.lamirault@gmail.com>
Signed-off-by: Nicolas Lamirault <nicolas.lamirault@gmail.com>
@jenting
Copy link
Copy Markdown
Collaborator

jenting commented Jul 19, 2021

CI was failed, could you please take a look?

Error:  templates/: template: velero/templates/schedule.yaml:11:20: executing "velero/templates/schedule.yaml" at <.Values.customAnnotations>: nil pointer evaluating interface {}.customAnnotations

Signed-off-by: Nicolas Lamirault <nicolas.lamirault@gmail.com>
@jenting jenting removed the request for review from carlisia August 31, 2021 08:43
Signed-off-by: Nicolas Lamirault <nicolas.lamirault@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request velero

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants