Add annotation on config change #1001
Conversation
When updating the Vault config (and corresponding) configmap, we now generate a checksum of the config and set it as an annotation on both the configmap and the Vault StatefulSet pod template. This allows the deployer to know what pods need to be restarted to pick up the a changed config. We still recommend using the standard upgrade [method for Vault on Kubernetes](https://developer.hashicorp.com/vault/tutorials/kubernetes/kubernetes-raft-deployment-guide#upgrading-vault-on-kubernetes), i.e., using the `OnDelete` strategy for the Vault StatefulSet, so updating the config and doing a `helm upgrade` should not trigger the pods to restart, and then deleting pods one at a time, starting with the standby pods. With `kubectl` and `jq`, you can check check which pods need to be updated by first getting the value of the current configmap checksum: ```shell kubectl get pods -o json | jq -r ".items[] | select(.metadata.annotations.\"config/checksum\" != $(kubectl get configmap vault-config -o json | jq '.metadata.annotations."config/checksum"') ) | .metadata.name" ``` Fixes #748.
There was a problem hiding this comment.
I like the approach. I think we definitely want make including the dynamic annotation optional. Also we will probably want to mention something about setting up the the strategy to control how best to handle the update whenever the configuration changes.
In addition, we should also extend the bats tests to cover the new features being added here.
templates/_helpers.tpl
Outdated
| annotations: | ||
| config/checksum: {{ include "vault.config" . | sha256sum }} |
There was a problem hiding this comment.
I think we probably want add a configurable that controls inclusion/exclusion of this annotation, since changing a configuration could cause an unexpected restart of Vault. Users may currently have their own strategy for dealing Vault config updates.
|
We may also want to prefix the annotation something like |
And update Test.dockerfile to use virtualenv pip install, as the global pip install no longer works.
|
Updated. I also fixed the |
Co-authored-by: Tom Proctor <[email protected]>
| --set 'server.configAnnotation=true' \ | ||
| . | tee /dev/stderr | | ||
| yq '.spec.template.metadata.annotations["vault.hashicorp.com/config-checksum"] == null' | tee /dev/stderr) | ||
| [ "${actual}" = "false" ] |
There was a problem hiding this comment.
Should we also test the value here? Maybe assert something about its length, or expect a specific hash?
There was a problem hiding this comment.
I did that initially, but realized that the value might be something noisy and annoying to fix every time we change the config file.
There was a problem hiding this comment.
We are doing those checks as part of the other tests for this kind of feature:
vault-helm/test/unit/prometheus-servicemonitor.bats
Lines 84 to 85 in 0407431
I think it is worth it to catch any potential regression where we could the expected defaults.
| --set 'server.configAnnotation=true' \ | ||
| . | tee /dev/stderr | | ||
| yq '.spec.template.metadata.annotations["vault.hashicorp.com/config-checksum"] == null' | tee /dev/stderr) | ||
| [ "${actual}" = "false" ] |
There was a problem hiding this comment.
We are doing those checks as part of the other tests for this kind of feature:
vault-helm/test/unit/prometheus-servicemonitor.bats
Lines 84 to 85 in 0407431
I think it is worth it to catch any potential regression where we could the expected defaults.
|
Thanks! |
When updating the Vault config (and corresponding) configmap, we now generate a checksum of the config and set it as an annotation on both the configmap
and the Vault StatefulSet pod template.
This allows the deployer to know what pods need to be restarted to pick up the a changed config.
We still recommend using the standard upgrade
method for Vault on Kubernetes, i.e., using the
OnDeletestrategyfor the Vault StatefulSet, so updating the config
and doing a
helm upgradeshould not trigger thepods to restart, and then deleting pods one
at a time, starting with the standby pods.
With
kubectlandjq, you can check check which pods need to be updated by first getting the value of the current configmap checksum:Fixes #748.