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.

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: configurable annotations for agent and operator pods #12189

Merged
merged 1 commit into from
Jun 19, 2020

Conversation

mvisonneau
Copy link
Contributor

This change allows the enduser to specify arbitrary annotations for the agent and operator pods. This is not necessarily Cilium related but is for example a prerequisite to get Datadog autodiscovery configured. Thanks to this change, it gets as "simple" as:

---
agent:
  podsAnnotations:
    ad.datadoghq.com/cilium-agent.check_names: '["cilium"]'
    ad.datadoghq.com/cilium-agent.init_configs: '[{}]'
    ad.datadoghq.com/cilium-agent.instances: '[{"agent_endpoint": "http://%%host%%:9090/metrics"}]'

operator:
  podsAnnotations:
    ad.datadoghq.com/cilium-operator.check_names: '["cilium"]'
    ad.datadoghq.com/cilium-operator.init_configs: '[{}]'
    ad.datadoghq.com/cilium-operator.instances: '[{"operator_endpoint": "http://%%host%%:6942/metrics"}]'

@mvisonneau mvisonneau requested review from a team as code owners June 18, 2020 20:15
@mvisonneau mvisonneau requested a review from a team June 18, 2020 20:15
@maintainer-s-little-helper
Copy link

Please set the appropriate release note label.

@coveralls
Copy link

coveralls commented Jun 18, 2020

Coverage Status

Coverage decreased (-0.02%) to 37.125% when pulling 8e32f4a on mvisonneau:helm/podsAnnotations into 2f04afd on cilium:master.

@joestringer joestringer added the release-note/minor This PR changes functionality that users may find relevant to operating Cilium. label Jun 18, 2020
Copy link
Member

@joestringer joestringer left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution, one minor comment below.

install/kubernetes/cilium/values.yaml Outdated Show resolved Hide resolved
Signed-off-by: Maxime VISONNEAU <maxime.visonneau@gmail.com>
@nebril
Copy link
Member

nebril commented Jun 19, 2020

smoke tests passed, merging

@nebril nebril merged commit 9a027ee into cilium:master Jun 19, 2020
1.8.0 automation moved this from In progress to Merged Jun 19, 2020
@mvisonneau mvisonneau deleted the helm/podsAnnotations branch June 19, 2020 15:15
@mvisonneau
Copy link
Contributor Author

mvisonneau commented Sep 24, 2020

is there any plan to backport this PR in 1.8 or 1.9 at some point? 🤔

@dominik318
Copy link

@mvisonneau just wanted to let you know that the prevailing spelling for most helm charts out there seems to be podAnnotations, not podsAnnotations. Do we think it is too late to change?

@mvisonneau
Copy link
Contributor Author

oh indeed @dominik318, I'll submit another PR to amend! I reckon it is still fine to change it given that it hasn't been backported yet 🤷

@joestringer joestringer removed this from Merged in 1.8.0 Oct 9, 2020
@joestringer joestringer added this to In Progress in 1.9.0 Oct 9, 2020
@joestringer
Copy link
Member

Glad we caught the typo before pushing it into a release :-) We discussed on #13458 to make these changes available in a subsequent v1.8.x release, assuming @mvisonneau prepares the changes against the v1.8 branch.

mvisonneau added a commit to mvisonneau/cilium that referenced this pull request Oct 10, 2020
This change follows-up onto cilium#12189 (comment)

Signed-off-by: Maxime VISONNEAU <maxime.visonneau@gmail.com>
aanm pushed a commit that referenced this pull request Oct 13, 2020
This change follows-up onto #12189 (comment)

Signed-off-by: Maxime VISONNEAU <maxime.visonneau@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-note/minor This PR changes functionality that users may find relevant to operating Cilium.
Projects
No open projects
1.9.0
  
In Progress
Development

Successfully merging this pull request may close these issues.

None yet

5 participants