Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[HELM] added flag for proxy.prometheus.enabled in helm chart #14688

Merged
merged 1 commit into from
Feb 2, 2021
Merged

[HELM] added flag for proxy.prometheus.enabled in helm chart #14688

merged 1 commit into from
Feb 2, 2021

Conversation

yuriydzobak
Copy link
Contributor

@yuriydzobak yuriydzobak commented Jan 21, 2021 •
edited by kaworu

By default the service is crated and has annotation of scrape metrics, I don't use envoy, Istio and I have auto discovery for all annotation about prometheus. The endpoint is created in promehtues and spam alerts about it. I want have ability to disable the service. I have set value true because it was enabled by default, any users are not effected who wants to disable they should change the flag to false

Added flag `proxy.prometheus.enabled` to helm chart for disabling service

@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Jan 21, 2021
@maintainer-s-little-helper maintainer-s-little-helper bot added this to In progress in 1.10.0 Jan 21, 2021
@maintainer-s-little-helper
Copy link

Commit be867428749b388153000b3dcef3de66617dd5f8 does not contain "Signed-off-by".

Please follow instructions provided in https://docs.cilium.io/en/stable/contributing/development/contributing_guide/#developer-s-certificate-of-origin

@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. label Jan 21, 2021
@yuriydzobak yuriydzobak changed the title added flag for proxy.prometheus.enabled in helm chart [HELM] added flag for proxy.prometheus.enabled in helm chart Jan 21, 2021
@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. label Jan 21, 2021
@yuriydzobak yuriydzobak marked this pull request as ready for review January 21, 2021 17:32
@yuriydzobak yuriydzobak requested review from a team as code owners January 21, 2021 17:32
@yuriydzobak yuriydzobak requested review from a team, joestringer and kaworu January 21, 2021 17:32
@kaworu kaworu added area/helm Impacts helm charts and user deployment experience release-note/misc This PR makes changes that have no direct user impact. labels Jan 22, 2021
@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Jan 22, 2021
@kaworu
Copy link
Member

kaworu commented Jan 22, 2021

Hello @yuriydzobak and thank you for the PR!

I'll defer to @errordeveloper and @seanmwinn here as I lack context about prometheus.

@kaworu kaworu requested review from errordeveloper and seanmwinn and removed request for kaworu January 22, 2021 10:39
aanm
aanm approved these changes Jan 25, 2021
View reviewed changes
Copy link
Member

@aanm aanm left a comment

Choose a reason for hiding this comment

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

Changes LGTM, please remove the new lines added.

install/kubernetes/cilium/templates/cilium-agent-daemonset.yaml Outdated Show resolved Hide resolved
install/kubernetes/cilium/templates/cilium-configmap.yaml Outdated Show resolved Hide resolved
@aanm
Copy link
Member

aanm commented Jan 25, 2021

@jrajahalme can you take a look as well?

@maintainer-s-little-helper
Copy link

Commit 03129a0aeff048959ca62dc46c79aa9b5d917c6d does not contain "Signed-off-by".

Please follow instructions provided in https://docs.cilium.io/en/stable/contributing/development/contributing_guide/#developer-s-certificate-of-origin

@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. label Jan 25, 2021
@maintainer-s-little-helper
Copy link

Commit 03129a0aeff048959ca62dc46c79aa9b5d917c6d does not contain "Signed-off-by".

Please follow instructions provided in https://docs.cilium.io/en/stable/contributing/development/contributing_guide/#developer-s-certificate-of-origin

@maintainer-s-little-helper
Copy link

Commits 03129a0aeff048959ca62dc46c79aa9b5d917c6d, 1fa10d532c30bbf66483ca06d1383428ef393277 do not contain "Signed-off-by".

Please follow instructions provided in https://docs.cilium.io/en/stable/contributing/development/contributing_guide/#developer-s-certificate-of-origin

@maintainer-s-little-helper
Copy link

Commit c521deeaa91240d61736cd7fff96bbb32fd8cc9c does not contain "Signed-off-by".

Please follow instructions provided in https://docs.cilium.io/en/stable/contributing/development/contributing_guide/#developer-s-certificate-of-origin

1 similar comment
@maintainer-s-little-helper
Copy link

Commit c521deeaa91240d61736cd7fff96bbb32fd8cc9c does not contain "Signed-off-by".

Please follow instructions provided in https://docs.cilium.io/en/stable/contributing/development/contributing_guide/#developer-s-certificate-of-origin

christarazi
christarazi requested changes Jan 27, 2021
View reviewed changes
Copy link
Member

@christarazi christarazi left a comment

Choose a reason for hiding this comment

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

Please squash all commits into the first

@yuriydzobak
added flag for proxy.prometheus.enabled in helm chart
b1be125 
Signed-off-by: yuriydzobak <yurii.dzobak@lotusflare.com>
@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. label Jan 28, 2021
@yuriydzobak
Copy link
Contributor Author

Please squash all commits into the first

Done

@aanm aanm requested a review from christarazi January 30, 2021 15:04
@aanm
Copy link
Member

aanm commented Jan 30, 2021

test-me-please

fristonio
fristonio approved these changes Jan 31, 2021
View reviewed changes
Copy link
Member

@fristonio fristonio left a comment

Choose a reason for hiding this comment

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

LGTM Thanks 💯

@aanm
Copy link
Member

aanm commented Feb 1, 2021

retest-gke

@aanm aanm merged commit ab24b93 into cilium:master Feb 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nebril nebril nebril approved these changes

@christarazi christarazi christarazi approved these changes

@aanm aanm aanm approved these changes

@fristonio fristonio fristonio approved these changes

@errordeveloper errordeveloper Awaiting requested review from errordeveloper

@seanmwinn seanmwinn Awaiting requested review from seanmwinn

Assignees

@errordeveloper errordeveloper

@seanmwinn seanmwinn

Labels
area/helm Impacts helm charts and user deployment experience release-note/misc This PR makes changes that have no direct user impact.
Projects
No open projects
1.10.0
Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
9 participants
@yuriydzobak @kaworu @aanm @nebril @christarazi @fristonio @errordeveloper @joestringer @seanmwinn