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

Ability to override values on per-chart basis #14

Open
rsotnychenko opened this issue Jan 30, 2019 · 6 comments
Open

Ability to override values on per-chart basis #14

rsotnychenko opened this issue Jan 30, 2019 · 6 comments
Labels
enhancement New feature or request

Comments

@rsotnychenko
Copy link

rsotnychenko commented Jan 30, 2019

Hello there! @hagaibarel @maorfr :)

First of all, I want to say that Orca looks great! It's fascinating to see how much it evolved since the very beginning of Nuvo's CI/CD.

While using it lately, I came up with an idea of what can be improved, and I was wondering what do you think about the usefulness of having this feature in the mainline repo.

Orca supports two ways of overriding default chart values:

  1. using command line param --set;
  2. via a YAML file packaged together with a chart.

However, neither of them covers a major case: when you need to change values only for some charts without knowing values during charts packaging (e.g., number of replicas of a service). So I was thinking about adding an ability to override values during deployment on a per-chart basis.

Currently, I see a couple of ways to implement this feature:

  1. Change the structure of Orca's charts.yaml
  2. Add a CLI param that takes keys with chart name as a prefix (e.g. --chart-set service1.replicas=10)
  3. Add a CLI param (e.g. --global-set) that will set values for all charts (current behavior of --set) and change the logic of --set to accept keys with prefixes like in option (2)

All of the options have their advantages and drawbacks. So I was wondering what do you guys think? Which option looks the best to you? Or would this feature be welcomed at all?

Thanks in advance!

@maorfr
Copy link
Contributor

maorfr commented Jan 31, 2019

Hey @rsotnychenko,

Thanks for your kind words, we have put a lot of work in this tool ;)

Your idea is actually something we have considered for a while, but never got around to do. It will make a great addition.

My thoughts -
We want to stay as close to helm as we can, so we will try to stay with --set only. To make things more flexible, I think that a good direction can be:

To set a value for a specific chart:
---set service1.replicas ...
To set a value for all charts:
--set global.replicas ...

This will make the behavior very similar to helm's subchart concept.

What do you think?
And if this sounds good, would yoy like to get involved and take this on? ;)

@hagaibarel FYI, this will break your existing pipelines.

@hagaibarel
Copy link
Contributor

I like the idea, but would prefer to avoid breaking chanages if possible.
We can take a differnet approch here - leave the --set flag as is, and intoduce a --setChart that looks like chart.key.value which takes precedence over the --set flag.
It will mean we'll need to parse the flags before passing to helm but it will keep exisiting functionality intact

@maorfr
Copy link
Contributor

maorfr commented Jan 31, 2019

Maybe this would be a good situation for a feature flag, which is essentially what @rsotnychenko proposed as option 3.

Imagine this -
We change the behavior of --set to what was proposed, which is consistent with helm (and I think is bettet in the long term). We add a --global-set, which is boolean and defaults to true, and indicates that every parameter defined using --set is global.

Anyone who is already using --set will not be harmed, and anyone who wishes to use the new global/specific --set, will set this flag to false.

We should add a warning for future change to the --set behavior when this flag is used.

Win win.

@hagaibarel, Thoughts?

@maorfr maorfr closed this as completed Jan 31, 2019
@maorfr maorfr reopened this Jan 31, 2019
@maorfr
Copy link
Contributor

maorfr commented Jan 31, 2019

Clicked close by mistake ;)

@maorfr maorfr added the enhancement New feature or request label Feb 5, 2019
@maorfr maorfr self-assigned this Feb 5, 2019
@hagaibarel
Copy link
Contributor

Have fun, don't forget to add relevant sections in the docs explaining the mechanism

@maorfr maorfr removed their assignment Feb 6, 2019
@rsotnychenko
Copy link
Author

@maorfr sounds good to me.

I should be able to start working on this on the next week :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants