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

feat(charts): Set new migration values and use bootloader image #8843

Merged
merged 8 commits into from
Dec 21, 2021

Conversation

vnourdin
Copy link
Contributor

@vnourdin vnourdin commented Dec 16, 2021

What

since #8584, the server pod use some new variables, and we should use the bootloader image that manage db migrations.
This PR add everything needed in the helm chart.

How

  • Create a Bootloader Pod
  • add values for it
  • add env variables
  • update docs

@github-actions github-actions bot added area/platform issues related to the platform kubernetes labels Dec 16, 2021
@vnourdin vnourdin marked this pull request as ready for review December 16, 2021 15:51
Copy link
Member

@marcosmarxm marcosmarxm left a comment

Choose a reason for hiding this comment

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

thanks @vnourdin

@marcosmarxm
Copy link
Member

@vnourdin can you solve the conflict so I can merge your contirbution?

@vnourdin
Copy link
Contributor Author

@marcosmarxm it's done !

@alafanechere alafanechere self-assigned this Dec 21, 2021
@alafanechere
Copy link
Contributor

Thanks @vnourdin, ready to merge!

@alafanechere
Copy link
Contributor

@vnourdin we have another PR which is also adding the env var for flyway job and config minimum versions. In his PR @mohamagdy exposed these env var as helm values. It's also a valid approach but I'm under the impression that it's fine to hardcode them in the ConfigMap (as you did) because they won't probably be changed by users, but I might be wrong. @davinchia what's your opinion on this? I'll merge but I'd be interested if you have a guideline about what should be hardcoded into ConfigMap and what should rather be exposed as values (only in the env var context).

@alafanechere alafanechere merged commit d2d0205 into airbytehq:master Dec 21, 2021
@davinchia
Copy link
Contributor

@alafanechere I don't have a strong opinion on whether the values should come from the configmap or be plain env vars. As long as it's easily configured whether the helm chart, I think it's ok!

@vnourdin
Copy link
Contributor Author

I think it's always better to put everything that can change in values so that users only change one file.
If those version exists, I supposed they should sometimes change 🤷 I just did the simplest way 😅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/platform issues related to the platform community kubernetes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants