Add logical cluster migration#4191
Conversation
Signed-off-by: Nelo-T. Wallus <red.brush9525@fastmail.com> Signed-off-by: Nelo-T. Wallus <n.wallus@sap.com>
Signed-off-by: Nelo-T. Wallus <red.brush9525@fastmail.com> Signed-off-by: Nelo-T. Wallus <n.wallus@sap.com>
Signed-off-by: Nelo-T. Wallus <red.brush9525@fastmail.com> Signed-off-by: Nelo-T. Wallus <n.wallus@sap.com>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
Signed-off-by: Nelo-T. Wallus <red.brush9525@fastmail.com> Signed-off-by: Nelo-T. Wallus <n.wallus@sap.com>
Signed-off-by: Nelo-T. Wallus <red.brush9525@fastmail.com> Signed-off-by: Nelo-T. Wallus <n.wallus@sap.com>
Signed-off-by: Nelo-T. Wallus <red.brush9525@fastmail.com> Signed-off-by: Nelo-T. Wallus <n.wallus@sap.com>
Signed-off-by: Nelo-T. Wallus <red.brush9525@fastmail.com> Signed-off-by: Nelo-T. Wallus <n.wallus@sap.com>
Signed-off-by: Nelo-T. Wallus <red.brush9525@fastmail.com> Signed-off-by: Nelo-T. Wallus <n.wallus@sap.com>
Signed-off-by: Nelo-T. Wallus <red.brush9525@fastmail.com> Signed-off-by: Nelo-T. Wallus <n.wallus@sap.com>
Signed-off-by: Nelo-T. Wallus <red.brush9525@fastmail.com> Signed-off-by: Nelo-T. Wallus <n.wallus@sap.com>
Signed-off-by: Nelo-T. Wallus <red.brush9525@fastmail.com> Signed-off-by: Nelo-T. Wallus <n.wallus@sap.com>
Signed-off-by: Nelo-T. Wallus <red.brush9525@fastmail.com> Signed-off-by: Nelo-T. Wallus <n.wallus@sap.com>
Signed-off-by: Nelo-T. Wallus <red.brush9525@fastmail.com> Signed-off-by: Nelo-T. Wallus <n.wallus@sap.com>
Signed-off-by: Nelo-T. Wallus <red.brush9525@fastmail.com> Signed-off-by: Nelo-T. Wallus <n.wallus@sap.com>
Signed-off-by: Nelo-T. Wallus <red.brush9525@fastmail.com> Signed-off-by: Nelo-T. Wallus <n.wallus@sap.com>
Signed-off-by: Nelo-T. Wallus <red.brush9525@fastmail.com> Signed-off-by: Nelo-T. Wallus <n.wallus@sap.com>
Signed-off-by: Nelo-T. Wallus <red.brush9525@fastmail.com> Signed-off-by: Nelo-T. Wallus <n.wallus@sap.com>
Signed-off-by: Nelo-T. Wallus <red.brush9525@fastmail.com> Signed-off-by: Nelo-T. Wallus <n.wallus@sap.com>
Signed-off-by: Nelo-T. Wallus <red.brush9525@fastmail.com> Signed-off-by: Nelo-T. Wallus <n.wallus@sap.com>
Signed-off-by: Nelo-T. Wallus <red.brush9525@fastmail.com> Signed-off-by: Nelo-T. Wallus <n.wallus@sap.com>
Signed-off-by: Nelo-T. Wallus <red.brush9525@fastmail.com> Signed-off-by: Nelo-T. Wallus <n.wallus@sap.com>
Signed-off-by: Nelo-T. Wallus <red.brush9525@fastmail.com> Signed-off-by: Nelo-T. Wallus <n.wallus@sap.com>
Signed-off-by: Nelo-T. Wallus <red.brush9525@fastmail.com> Signed-off-by: Nelo-T. Wallus <n.wallus@sap.com>
The paths aren't equal and making a shared list for two entries is excessive.
|
/test pull-kcp-test-e2e flake |
| s.blockDeltas.Lock() | ||
| defer s.blockDeltas.Unlock() | ||
|
|
||
| if s.ignoreFunc != nil { |
There was a problem hiding this comment.
Why doesn't handleBatchDeltas need to special-case ReplacedAllInfo deltas, similar to how handleDeltas does?
| continue | ||
| } | ||
| for _, obj := range objs { | ||
| if err := store.Delete(obj); err != nil { |
There was a problem hiding this comment.
Is it possible the handlers that use this informer are still processing this object? It sounds a bit racy :D I wonder if it could be moved into the controller (the caller, using this informer), purge outside of the reconcile func while holding a lock. So that all threads can observe the same events.
| } | ||
|
|
||
| // ForceRelist causes the underlying reflector to perform a full relist | ||
| // on the next list/watch cycle. |
There was a problem hiding this comment.
... causes the underlying reflector to perform a full relist IF the said reflector also implements this magical forceRelister interface.
I think more context could be added into the comment. With the single user of this being the LC migration controller, it's not super obvious (without further digging) WHY is this even here.
| // lastSyncResourceVersionMutex guards read/write access to lastSyncResourceVersion | ||
| lastSyncResourceVersionMutex sync.RWMutex | ||
| // activeWatch holds the current watch so ForceRelist can stop it. | ||
| activeWatch watch.Interface |
There was a problem hiding this comment.
A nit, but could this be thisWatch, just watch or something like that? :D While reading, I was wondering that if this is THE active watch, I was looking for where is the other, inactive watch.
Summary
This PR adds migrating logical clusters between shards.
The intent of an admin to migrate an LC is expressed using
LogicalClusterMigration.migration.kcp.io.The object is replicated to the cache-server and used to synchronize the migration between the origin and destination shard.
The destination shards pulls the data of the migrating LC using the
LogicalClusterDumptype through the migrating VW.The migrating VW was added only to not put the pressure of the migration on the front-proxy as well.
More details on the process are in the reconciler package docs.
The happy path works but it still needs some resilience and we'll probably hit some real blockers once we try it on real infrastructure.
Also I haven't put in guard rails yet for who can actually create lcmigration etcpp. But I don't want the PR to get even bigger than it is.
What Type of PR Is This?
/kind feature
Related Issue(s)
Fixes #
Release Notes