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

Set manager as first container in deployment #409

Merged
merged 3 commits into from
Jun 14, 2024

Conversation

TheoBrigitte
Copy link
Contributor

This sets manager container as default container in the Deployment.

This allow for manager container to be the default container shown when using kubectl logs otherwise one need to add --container=manager in order to get logs from the manager, since kube-rbac-proxy is the current default.

This allow for manager container to be the default container when using
`kubectl logs` otherwise one need to add `-c manager` in order to get
logs from the manager.
@CLAassistant
Copy link

CLAassistant commented Jun 4, 2024

CLA assistant check
All committers have signed the CLA.

Copy link
Collaborator

@yorugac yorugac left a comment

Choose a reason for hiding this comment

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

Hi @TheoBrigitte, thanks for the PR. I've wondered if anyone needs that change with containers but it never came up as an issue before. Technically, it still didn't since this is a PR, not an issue 🙂

I think overall this change shouldn't be breaking but if there are concerns from anyone, please let me know.

On a PR itself: this change needs to be done in Helm chart as well, to remain consistent.

Copy link
Collaborator

@yorugac yorugac 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 update @TheoBrigitte! LGTM 🙌

I'll merge this change next week: if there are any concerns about it within a community, please comment here.

AFAIK, the only possible impact is if anyone is referencing containers by index somewhere, like containers[1]. That can happen in kustomizations, for instance.

@yorugac yorugac merged commit 5d88899 into grafana:main Jun 14, 2024
6 of 8 checks passed
@TheoBrigitte TheoBrigitte deleted the default-container branch June 14, 2024 14:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants