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

Read aso-controller-settings ConfigMap via volume rather than env variables #4168

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

matthchr
Copy link
Member

Moving from a model where we have to explicitly set each secret in the pod env to a model where we just mount the whole secret into the pod. This has the added benefit of allowing us to autodetect secret changes and restart the pod AND means that users can totally omit the secret now as opposed to having to create the secret with empty values.

If applicable:

  • this PR contains documentation
  • this PR contains tests
  • this PR contains YAML Samples

@theunrepentantgeek
Copy link
Member

/ok-to-test sha=27873f2

@matthchr
Copy link
Member Author

I may hold off on merging this until we re-enable KIND tests as they're the primary test of this change. While I've tested it locally and it works it may be safer to wait to merge until those tests are re-enabled.

@matthchr matthchr added the blocked 🚧 Blocked by external factors label Jul 31, 2024
@theunrepentantgeek
Copy link
Member

/ok-to-test sha=27873f2

v2/internal/config/watcher.go Outdated Show resolved Hide resolved
v2/internal/config/watcher.go Show resolved Hide resolved
@matthchr matthchr force-pushed the feature/config-reading-changes branch from 27873f2 to 7c36469 Compare August 16, 2024 23:37
@matthchr matthchr modified the milestones: v2.9.0, v2.10.0 Aug 19, 2024
@matthchr matthchr changed the title Change how we read configuration Read azureserviceoperator-settings ConfigMap via volume rather than env variables Oct 4, 2024
@matthchr matthchr changed the title Read azureserviceoperator-settings ConfigMap via volume rather than env variables Read aso-controller-settings ConfigMap via volume rather than env variables Oct 4, 2024
@matthchr
Copy link
Member Author

matthchr commented Oct 4, 2024

This has some possible disadvantages during upgrades that change the configmap, as there's just one ConfigMap which means the old pod may exit early (due to configmap change).

Need to do some testing around that.

Moving from a model where we have to explicitly set each secret in the
pod env to a model where we just mount the whole secret into the pod.
This has the added benefit of allowing us to autodetect secret changes
and restart the pod AND means that users can totally omit the secret now
as opposed to having to create the secret with empty values.
@matthchr matthchr force-pushed the feature/config-reading-changes branch from 7c36469 to 6783a9f Compare October 4, 2024 16:07
@matthchr matthchr modified the milestones: v2.10.0, v2.11.0 Oct 18, 2024
@matthchr matthchr modified the milestones: v2.11.0, v2.12.0 Oct 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocked 🚧 Blocked by external factors
Projects
Development

Successfully merging this pull request may close these issues.

3 participants