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

Generate reva configuration from Helm Values #48

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

mirekys
Copy link
Member

@mirekys mirekys commented Feb 20, 2023

This PR allows chart users to modify individual settings of Reva daemon directly either
by Helm values.yaml or by helm upgrade ... --set config.some-reva-setting=value method, rather
than by rewriting the whole revad.toml file. It also allows for inheritance of common Reva settings in subcharts,
leveraging built-in Helm values inheritance.

Currently used way of configuring Reva using static toml files:

configFiles:
  revad.toml: |
    [grpc]
    ...

remains still possible.

Contributing a Chart / update to an existing Chart

  • Run helm lint on the chart dir.
  • (Update) Bump the Chart.yaml version before merging, to release it as a new version.
  • (Update) Replace the annotations on the Chart.yaml with:
    • artifacthub.io/changes - the changes introduced on the PR with the appropiate format.
    • artifacthub.io/images - the updated tag on the cs3org/revad image.
  • (Update) If the PR includes new configurable parameters in the chart's values.yaml. Add documentation in the appropiate README.

@mirekys mirekys changed the title Generate reva configuration from Helm Values [WIP] Generate reva configuration from Helm Values Feb 20, 2023
@mirekys mirekys changed the title [WIP] Generate reva configuration from Helm Values Generate reva configuration from Helm Values Feb 21, 2023
Copy link
Contributor

@SamuAlfageme SamuAlfageme left a comment

Choose a reason for hiding this comment

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

@mirekys very nice idea behind this PR, I love it!

However, I think as both a backwards-compatibility mechanism and to provide some flexibility in terms of being able to inject the .toml file directly (e.g. when migrating from existing non-kubernetes reva installations), it could be good to keep this logic as well as fallback.

For instance, when passing a file rather than individual config parameters - cc/ @labkode we discussed this a couple of years ago, what do you think?

@mirekys
Copy link
Member Author

mirekys commented Mar 27, 2023

However, I think as both a backwards-compatibility mechanism and to provide some flexibility in terms of being able to inject the .toml file directly (e.g. when migrating from existing non-kubernetes reva installations), it could be good to keep this logic as well as fallback.

Hi @SamuAlfageme, this change is already backwards-compatible. If someone wishes to inject toml file as a reva config,
he could still do: helm ... --set-file .configFiles.revad\\.toml=custom-gateway.toml .... Doing so completely overrides this template with custom-gateway.toml file contents. In addition, you can also use helm template tags in that toml file (basically in any file under configFiles) and now it gets resolved (here).

@labkode
Copy link
Member

labkode commented Mar 27, 2023

@dagl please review

@mirekys
Copy link
Member Author

mirekys commented May 26, 2023

Any news on this @dagl @labkode? It has some potential to help with simplifying reva site configurations

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.

3 participants