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

Revert enabled parameter #32

Merged
merged 2 commits into from
Oct 3, 2024
Merged

Revert enabled parameter #32

merged 2 commits into from
Oct 3, 2024

Conversation

marcelldls
Copy link
Contributor

This allows <service>.enabled to be applied from the deployment repo

  • In order to expose <service>.enabled in the root app to allow override - <service>.enabled must be present in apps chart values.yaml (Not currently enforced)
  • The current value of the parameter (including override) can be read using argocd app get <ns>/<app> -o yaml
  • Using parameter means that the sub app always shows <service>.enabled as having been overridden

@gilesknap
Copy link
Member

I'm still struggling with this a little - not sure of all of the implications. First I don't think that your approach needs to enforce 'enabled' flag as you can use the tertiary operator to turn 'not set' into the default 'true' .

There are too many variables to assess here for me to choose from:

  • do we separate out ec_services and leave others to fend for themselves
  • is deletion of the whole app acceptable for stopping it
  • should we even allow overrides or just do everything in the repo
  • if we allow both overrides and permanent stoppage:
    • should this be a separate flag (enabled vs removed for example)
    • how do we show in ec the difference (or even explain it to users)
    • should permanent stoppage be controlled at both root app and child app level (if so is that not confusing?) - reason to do this is DAQ want to not need to touch the deployment repo
    • likewise for overrides
  • should we let ec write to the deployment repo for us so its easy and less error prone
  • all of these questions should also be applied to DAQ requirements

@marcelldls
Copy link
Contributor Author

@gilesknap to clarify - if you dont have enabled in your values then they do not show in the rootapp parameters. Adding a default doesn't solve this

Copy link
Member

@gilesknap gilesknap left a comment

Choose a reason for hiding this comment

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

All good

* Create separate services, ec_services maps to not have to set the ec_service key explicitly

* Replace $currentScope for just $ as a code clean up step
@marcelldls marcelldls marked this pull request as ready for review October 3, 2024 12:19
@marcelldls marcelldls merged commit 083a775 into main Oct 3, 2024
3 checks passed
@marcelldls marcelldls deleted the handle-enabled-flag branch November 25, 2024 14:53
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.

2 participants