WIP 🌱 Syncer: Cleanup of DNS-related resources

[davidfestal]

Summary

Cleanup of DNS-related resources when no more downstream namespaces exist for a given upstream workspace.

Disclaimer:

• based on PRs ✨ Syncer: support multiple shards #2675 and ✨ Make the tunneler shard-aware #2946. Should be merged as soon as those have been.
• only the last commit is really part of this PR

Related issue(s)

Fixes kcp-dev/contrib-tmc#80

[stevekuznetsov]

Can we do this with actual garbage collection and owner refs?

[lionelvillard]

Can we do this with actual garbage collection and owner refs?

I think yes. Namespaced dependents can have cluster-scoped owners (Namespaces in this case).

[davidfestal]

Can we do this with actual garbage collection and owner refs?

I think yes. Namespaced dependents can have cluster-scoped owners (Namespaces in this case).

Not sure about how you would use garbage collection in the specific case.
Do you mean that every DNS-related resource for a given KCP workspace should have as many owner references as the number of downstream namespaces originating from the given KCP workspace ?

In order to do this, in the EnsureDNSUpAndReady function we would need to:

• check that every DNS-related resource has the namespace of the currently-synced resource in its owner refs,
• update every DNS-related resource if necessary to add the namespace owner ref.
  This would be required during every resource sync. I'd rather do more work during cleanup, and keep the resource syncing main reconciliation loop as quick as possible.

Then I assume that the DNS-related resources deletion would be triggered by cascading deletion when all the namespaces related to the given KCP workspace would be deleted. However we wouldn't control when this cleaning would happen, which would make it hard to synchronize this cleaning with the on-the-fly creation of those DNS-related resources when they are necessary (in the EnsureDNSUpAndReady function). Such a synchronization is for now still missing from this PR, but might be necessary and should be thought of.

Last point: when we start implementing the network isolation through NetworkPolicies on the physical cluster, we will need this concept of KCP tenant on downstream (== a originating KCP workspace + related SyncTarget). That seemed quite natural to use it here as well.

So, unless I'm missing something, applying the standard Kubernetes garbage collection and owner refs mechanism here is not as simple as it may seem, and may bring more slowness and complexity.

Let me finally just point to a previous PR comment answer to remind that we are not in the standard Kubernetes case, because everything downstream is finally driven by the individual resources synced from upstream.

[davidfestal]

/test e2e-sharded

[ncdc]

@davidfestal are you still interested in moving forward with this?

[davidfestal]

/hold

on hold until PR #2675 is merged.

[openshift-ci]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign ncdc for approval. For more information see the Kubernetes Code Review Process.

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

Needs approval from an approver in each of these files:

• OWNERS
• sdk/apis/OWNERS

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

[davidfestal]

/hold

Wait until PRs #2675 and #2946 are merged.

[kcp-ci-bot]

PR needs rebase.

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.

[kcp-ci-bot]

Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it.

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.

[mjudeikis]

/close
TMC

[kcp-ci-bot]

@mjudeikis: Closed this PR.

In response to this:

/close
TMC

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.