Skip to content

Conversation

@pmjacinto
Copy link

What this PR does / why we need it:

  • Adds support for storing spring profiles as kubernetes secrets (e.g. database connection strings)
  • Enables automatic deployment rollout when the configuration changes (using helm)
  • Moves store.yaml into a new configmap as it does not require a pod restart to reload

Which issue(s) this PR fixes:

General reliability improvements.

Does this PR introduce a user-facing change?:

NONE

feast-*.springSecretProfiles and feast-*.springSecretProfilesActive were added to values.yaml. No helm chart values update required in order to keep the current behaviour.

To compare the Helm output of these changes with the previous version, these commands can be used.

helm install --dry-run --debug feast-charts/feast --name demo -f infra/charts/feast/values-demo.yaml > scratch/helm-install.demo.old.yaml

helm install --dry-run --debug ./infra/charts/feast --name demo -f infra/charts/feast/values-demo.yaml > scratch/helm-install.demo.new.yaml

Enable deployment rollout on configmap / secret changes
Move store.yaml into a new configmap as it is already reloaded at runtime
@feast-ci-bot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: pmjacinto
To complete the pull request process, please assign pradithya
You can assign the PR to them by writing /assign @pradithya in a comment when ready.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@feast-ci-bot
Copy link
Collaborator

Hi @pmjacinto. Thanks for your PR.

I'm waiting for a gojek member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@woop
Copy link
Member

woop commented Feb 25, 2020

Thanks for the submission @pmjacinto!

Quick question: Have you signed the CLA?

@woop
Copy link
Member

woop commented Feb 26, 2020

/ok-to-test

@pmjacinto
Copy link
Author

Thanks for the submission @pmjacinto!

Quick question: Have you signed the CLA?

Hey @woop

Yes, I've signed the google CLA.

@pmjacinto
Copy link
Author

/assign @pradithya

@woop
Copy link
Member

woop commented Mar 5, 2020

Haven't had time to review this yet. Please bear with us :)

@woop
Copy link
Member

woop commented Apr 29, 2020

@pmjacinto would be great to get your take on #533. Especially because it now tries to solve the secrets problem as well.

@woop
Copy link
Member

woop commented May 3, 2020

@pmjacinto I believe this functionality is now covered by #533. Please let us know if you think it should be reopened. Really do appreciate the effort you put in here.

@woop woop closed this May 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants