-
Notifications
You must be signed in to change notification settings - Fork 183
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
Adjust fluentd names and app labels #573
Conversation
24fa00c
to
8c54065
Compare
@@ -3,11 +3,11 @@ kind: Service | |||
metadata: | |||
name: {{ template "sumologic.fullname" . }}-fluentd-headless | |||
labels: | |||
app: {{ template "sumologic.labels.app" . }} | |||
app: {{ printf "%s-fluentd-headless" (include "sumologic.labels.app" .) }} |
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 think we should stick to the templates :)
We could create templates for events, fluentd, otelcol and whatever else we have. for both names and labels
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.
That's a good point but let's do it in 1.1 and not change the 1.0 scope any further.
I've created an issue to keep track of that one: #574
e46d2f8
to
34c4fec
Compare
There's a few other places that we use |
Actually that's a very good question @samjsong ! |
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.
LGTM, with the remaining changes to be done in 1.1
Hmm, I'm afraid changing the names of those other resources might technically be a breaking change for non-helm users since we'd be leaving old resources behind. The user would have to manually delete the old but I don't believe it'll have any impact in terms of functionality, just some leftover resources... if this is okay then I'm +1 and we can proceed with this plan |
6602c2f
to
e956081
Compare
Actually |
4c6fa5b
to
2eb526c
Compare
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.
Thanks Perk!
6738358
to
dfc0d4f
Compare
Description
Fill in your description here.
Testing performed