ConfigMaps From Host Sync: #2439
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- adds sync.fromHost.configMaps config options - parses namespaces in sync.fromHost.configMaps.selector.mappings and creates Roles and RoleBindings accordingly - implements translator based on the sync.fromHost.configMaps.selector.mappings - adds from host syncer for ConfigMap, it watches for ConfigMaps in additional namespaces in the host, so it uses also uncachedPhysicalClient - adds E2E test for syncing config maps from two different namespaces in the host to another one in vCluster, then creates a Pod that uses these ConfigMaps as EnvVar source Signed-off-by: Paweł Bojanowski <[email protected]>
✅ Deploy Preview for vcluster-docs canceled.Built without sensitive environment variables
|
- adds sync.fromHost.configMaps config options - parses namespaces in sync.fromHost.configMaps.selector.mappings and creates Roles and RoleBindings accordingly - implements translator based on the sync.fromHost.configMaps.selector.mappings - adds from host syncer for ConfigMap, it watches for ConfigMaps in additional namespaces in the host, so it uses also uncachedPhysicalClient - adds E2E test for syncing config maps from two different namespaces in the host to another one in vCluster, then creates a Pod that uses these ConfigMaps as EnvVar source - adds helm unittests Signed-off-by: Paweł Bojanowski <[email protected]>
Signed-off-by: Paweł Bojanowski <[email protected]>
Signed-off-by: Paweł Bojanowski <[email protected]>
johannesfrey
reviewed
Feb 3, 2025
| @@ -15,3 +15,49 @@ | |||
| {{ .repository }}:{{ .tag }} | |||
| {{- end -}} | |||
| {{- end -}} | |||
|
|
|||
| {{- define "extractNamespacesFromHostMappings" -}} | |||
There was a problem hiding this comment.
Just for my understanding:
- When a user adds a host namespace here that does not exist, this would lead to a helm error during install/update?
- What happens with the syncer when a user deletes a host namespace during runtime?
There was a problem hiding this comment.
- yes, it leads to helm error:
16:40:45 fatal error executing helm upgrade vcluster ./chart/ --create-namespace --kubeconfig /tmp/3659476771 --namespace vcluster --install --repository-config='' --values /tmp/3887008891 --values ./dist/commonValues.yaml --values ./test/e2e/values.yaml: Release "vcluster" does not exist. Installing it now.
Error: 2 errors occurred:
* namespaces "foobar2" not found
- Well, the vCluster will periodically log an error saying that it does not have enough permissions to watch for configmaps in
<deleted-namespace>. This is because the Role & RoleBinding gets too during namespace deletion. Synced ConfigMap will stay unchanged in the vCluster.
I also changed default error handler, so we can log a more helpful error message about it: https://github.com/loft-sh/vcluster/pull/2439/files#diff-098628cf79dfa68abeae1696afdb72b5c484c5f8638fb1a0b7db7e60faee8efaR140-R148
Thanks for bringing this up. I think it would be good to document these assumptions in the docs, gonna do it in the docs PR.
| return hostName == "kube-root-ca.crt" | ||
| } | ||
|
|
||
| func matchesHostObject(hostName, hostNamespace string, resourceMappings map[string]string, vClusterHostNamespace string, skippers ...skipHostObject) (types.NamespacedName, bool) { |
There was a problem hiding this comment.
When matched, should we also check if the host namespace still exists?
There was a problem hiding this comment.
I don't think we need to do this: in most cases, this method is called with pObj.GetName() & pObj.GetNamespace() as arguments, where pObj is fetched from host cluster by uncached client (so it will error out there if namespace does not exist)
Signed-off-by: Paweł Bojanowski <[email protected]>
Signed-off-by: Paweł Bojanowski <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What issue type does this pull request address? (keep at least one, remove the others)
/kind feature
/kind test
What does this pull request do? Which issues does it resolve? (use
resolves #<issue_number>if possible)resolves ENG-5653
Please provide a short message that should be published in the vcluster release notes
ConfigMaps From Host Sync
What else do we need to know?
Roles and RoleBindings created by helm assume that all Namespaces listed in the
sync.fromHost.configMaps.selector.mappingsalready exist in the host. I added creatingfoobarnamespace (which is used in e2e test) to Justfile & e2e CI (it has to happen before creating vCluster).In the following up PR, which implements from host syncing for Secret, I'll make the syncer more generic so only kind-specific details has to be implemented, but otherwise the logic can be re-used.