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

add option for sidecar containers in helm chart #2650

Merged
merged 3 commits into from
Jul 31, 2023

Conversation

lu1as
Copy link
Contributor

@lu1as lu1as commented Jul 26, 2023

What this PR does

Adds support for defining extra containers which run as sidecar alongside the celery and engine containers

Which issue(s) this PR fixes

Checklist

  • Unit, integration, and e2e (if applicable) tests updated
  • Documentation added (or pr:no public docs PR label added if not required)
  • CHANGELOG.md updated (or pr:no changelog PR label added if not required)

@lu1as lu1as requested a review from a team July 26, 2023 16:07
@lu1as lu1as force-pushed the add-extra-containers-to-helm-chart branch 2 times, most recently from c782157 to 6b5ccd9 Compare July 26, 2023 16:11
@CLAassistant
Copy link

CLAassistant commented Jul 26, 2023

CLA assistant check
All committers have signed the CLA.

@iskhakov
Copy link
Contributor

thank you for the contribution! Would you please add some helm unit tests

@lu1as lu1as force-pushed the add-extra-containers-to-helm-chart branch from 6b5ccd9 to edcedba Compare July 27, 2023 10:27
@lu1as
Copy link
Contributor Author

lu1as commented Jul 27, 2023

@iskhakov I added unit tests and already found an issue 😄 It's fixed now.
I hope the tests are sufficient, it's the first time for me using this framework.

@lu1as
Copy link
Contributor Author

lu1as commented Jul 28, 2023

I just added the option for extraContainers also for the migrate-job.

@lu1as lu1as force-pushed the add-extra-containers-to-helm-chart branch from c9545d0 to 0e9394c Compare July 28, 2023 15:13
@joeyorlando joeyorlando added the pr:no public docs Added to a PR that does not require public documentation updates label Jul 31, 2023
@joeyorlando joeyorlando enabled auto-merge July 31, 2023 07:17
@joeyorlando joeyorlando added this pull request to the merge queue Jul 31, 2023
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jul 31, 2023
@lu1as lu1as force-pushed the add-extra-containers-to-helm-chart branch from 0e9394c to bd17795 Compare July 31, 2023 14:13
@lu1as lu1as force-pushed the add-extra-containers-to-helm-chart branch from bd17795 to eddc456 Compare July 31, 2023 14:16
@lu1as lu1as force-pushed the add-extra-containers-to-helm-chart branch from eddc456 to 4acf553 Compare July 31, 2023 14:17
@joeyorlando joeyorlando enabled auto-merge July 31, 2023 14:20
@lu1as
Copy link
Contributor Author

lu1as commented Jul 31, 2023

I rebased the source branch to resolve merge conflicts.
In the meantime I also noticed that using the cloud-sql-proxy does not work for this helm chart, because there is an init container which waits for the DB connection and prevents the sidecar from starting. Nonetheless, the option to add sidecars is useful, I guess.

@joeyorlando joeyorlando disabled auto-merge July 31, 2023 14:27
@joeyorlando joeyorlando merged commit e1598d7 into grafana:dev Jul 31, 2023
@lu1as lu1as deleted the add-extra-containers-to-helm-chart branch July 31, 2023 15:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr:no public docs Added to a PR that does not require public documentation updates
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants