-
Notifications
You must be signed in to change notification settings - Fork 366
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
✨ Create one DNS nameserver per workspace #2293
✨ Create one DNS nameserver per workspace #2293
Conversation
Hi @lionelvillard. Thanks for your PR. I'm waiting for a kcp-dev member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. 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. |
afde7e9
to
c3cba76
Compare
@davidfestal @jmprusi this is now ready for a first round of reviews. Keep in mind that I haven't updated the tests yet. |
4d07bdd
to
0b89ddc
Compare
/ok-to-test |
0b89ddc
to
397f094
Compare
397f094
to
0137e5c
Compare
0137e5c
to
03f68fa
Compare
63577ba
to
f48e41c
Compare
Additional, is it possible to create a followup PR for the update, and most importantly for the cleanup case, and add it into the related EPIC: kcp-dev/contrib-tmc#95 ? |
f48e41c
to
3a8cc42
Compare
2b43311
to
5923ad3
Compare
5923ad3
to
1f53d1b
Compare
bc4836e
to
4210ebc
Compare
This PR is now ready @davidfestal @jmprusi. It needs some follow-up PRs which will be captured in issues. |
4210ebc
to
530fab8
Compare
/test e2e-multiple-runs |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: davidfestal The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
do we always need to sync role/rolebinding after this is merged? should we add role/rolebinding by default to the virtual workspace? |
@qiujian16 yes we always need to sync role/rolebinding. Role restricts the DNS pod to only access the configmap associated to the workspace. See https://github.com/kcp-dev/kcp/blob/main/pkg/syncer/spec/dns/role_dns.yaml#L11. There is also one ServiceAccount per workspace. |
cc @davidfestal shouldn't we add role/rolebinding as default api resources in syncer virtual workspace and let syncer always sync them? |
Summary
Dynamically deploy DNS nameserver k8s resources, one set per workspace. Each set of resources is composed of:
The spec controller checks for DNS endpoint readiness before syncing to pclusters.
All DNS objects associated to all workspaces bound to the workload ws are deployed in the synctarget namespace. Initial network policies design document review with @davidfestal shows this approach allows workspaces segmentation.
This PR does not support:
Related issue(s)
Fixes #1987