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

Introduce extraInitContainers to helm chart #3393

Merged
merged 3 commits into from
May 20, 2021

Conversation

strowk
Copy link

@strowk strowk commented May 14, 2021

New extraInitContainers configuration added.
It allows to pass template with a list of containers to execute before
main code-server container started. Main container will only start when
all init containers are completed (exited with 0 code).

Additionally changes the way extraContainers is used - instead of
toYaml use tpl, because this allows to
reference any values from extraContainers string.

close #3392

@codecov
Copy link

codecov bot commented May 14, 2021

Codecov Report

Merging #3393 (43d72c5) into main (8f3de91) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #3393   +/-   ##
=======================================
  Coverage   59.21%   59.21%           
=======================================
  Files          35       35           
  Lines        1709     1709           
  Branches      379      379           
=======================================
  Hits         1012     1012           
  Misses        559      559           
  Partials      138      138           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8f3de91...43d72c5. Read the comment docs.

@jsjoeio jsjoeio requested a review from a team May 14, 2021 22:50
@jsjoeio jsjoeio added the enhancement Some improvement that isn't a feature label May 14, 2021
@jsjoeio jsjoeio added this to the v3.11.0 milestone May 14, 2021
@jsjoeio jsjoeio added the new contributor For PRs by first-time contributor label May 14, 2021
@repo-ranger
Copy link
Contributor

repo-ranger bot commented May 14, 2021

Thanks for making your first contribution! 🙂

@jsjoeio
Copy link
Contributor

jsjoeio commented May 14, 2021

Thanks for opening an issue and raising a PR @strowk 🎉

This sounds like a great addition but would love to get some extra eyes on it from the other code-server team members.

In the meantime, can you:

  • rebase so your branch is up to date with main
  • update the CHANGELOG

@jsjoeio jsjoeio requested a review from bpmct May 14, 2021 23:12
@code-asher code-asher modified the milestones: v3.10.1, 3.11.0 May 17, 2021
@jsjoeio jsjoeio removed the request for review from bpmct May 18, 2021 17:51
@jsjoeio
Copy link
Contributor

jsjoeio commented May 18, 2021

cc @alexgorbatchev and @Matthew-Beckett - any chance either of you would be able to help review this?

ci/helm-chart/README.md Show resolved Hide resolved
ci/helm-chart/README.md Show resolved Hide resolved
@strowk strowk force-pushed the extra-init-containers branch 2 times, most recently from d4cbc69 to 2d8b07a Compare May 20, 2021 15:36
@strowk
Copy link
Author

strowk commented May 20, 2021

@jsjoeio , rebased on main and also updated changelog. I guess that next version should be 3.11.0, since this is kinda new feature (even though only for helm chart)

New extraInitContainers configuration added.
It allows to pass template with a list of containers to execute before
main code-server container started. Main container will only start when
all init containers are  completed (exited with 0 code).

 Additionally changes the way extraContainers is used - instead of
 toYaml use tpl, because this allows to
 reference any values from extraContainers string.
@strowk
Copy link
Author

strowk commented May 20, 2021

Upd: changed entry in changelog to "Next Version" for now, guess there could be more stuff in there before next release.. (sorry for after review change)

@Matthew-Beckett
Copy link
Contributor

@jsjoeio can you advise on the CHANGELOG before I approve? Not sure on the rules for adding changes being a part time maintainer. This pull has a milestone associated with it so is @strowk correct to put it in "Next Update"?

@jsjoeio
Copy link
Contributor

jsjoeio commented May 20, 2021

@strowk thanks for making those changes! Using "Next Version" in the CHANGELOG is correct (apologies, it's new and we need to add more documentation around that #3412).

@Matthew-Beckett looks good from our side. Thanks so much for reviewing this so quickly and helping out with maintaining code-server! We really appreciate it.

@Matthew-Beckett
Copy link
Contributor

Matthew-Beckett commented May 20, 2021

@strowk nice work!

@jsjoeio it's my pleasure, any time.

@jsjoeio jsjoeio enabled auto-merge May 20, 2021 19:12
@jsjoeio jsjoeio merged commit ac96517 into coder:main May 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Some improvement that isn't a feature new contributor For PRs by first-time contributor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add extraInitContainers to helm chart
5 participants