-
Notifications
You must be signed in to change notification settings - Fork 360
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Helm chart] Refactoring Kubernetes labels #2259
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Nicolas Lamirault <nicolas.lamirault@gmail.com>
@@ -39,7 +39,12 @@ helm.sh/chart: {{ include "eg.chart" . }} | |||
{{- if .Chart.AppVersion }} | |||
app.kubernetes.io/version: {{ .Chart.AppVersion | quote }} | |||
{{- end }} | |||
app.kubernetes.io/component: gateway-api |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i am not sure whether refer gateway-api
as a component is accurate, any thoughts?
cc @envoyproxy/gateway-maintainers
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMO, default shouldn't
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
eg is built on top of gateway-api and envoy.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
agree
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do you have an idea for the component label ?
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2259 +/- ##
==========================================
- Coverage 66.89% 64.30% -2.59%
==========================================
Files 163 112 -51
Lines 23453 15728 -7725
==========================================
- Hits 15689 10114 -5575
+ Misses 6838 4974 -1864
+ Partials 926 640 -286 ☔ View full report in Codecov by Sentry. |
This pull request has been automatically marked as stale because it has not had activity in the last 30 days. Please feel free to give a status update now, ping for review, when it's ready. Thank you for your contributions! |
This pull request has been automatically marked as stale because it has not had activity in the last 30 days. Please feel free to give a status update now, ping for review, when it's ready. Thank you for your contributions! |
Signed-off-by: Nicolas Lamirault <nicolas.lamirault@gmail.com>
@@ -1,3 +1,7 @@ | |||
## Additional labels to add to all resources | |||
additionalLabels: {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
any idea if other projects also call this top level field additionalLabels
?
@@ -44,7 +45,8 @@ metadata: | |||
name: {{ include "eg.fullname" . }}-leader-election-rolebinding | |||
namespace: '{{ .Release.Namespace }}' | |||
labels: | |||
{{- include "eg.labels" . | nindent 4 }} | |||
{{- include "eg.labels" . | nindent 4 }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks like the idententation is changing for all these files, was it broken earlier ?
This pull request has been automatically marked as stale because it has not had activity in the last 30 days. Please feel free to give a status update now, ping for review, when it's ready. Thank you for your contributions! |
This pull request has been automatically marked as stale because it has not had activity in the last 30 days. Please feel free to give a status update now, ping for review, when it's ready. Thank you for your contributions! |
What type of PR is this?
What this PR does / why we need it:
Which issue(s) this PR fixes:
Fixes #