Update dependencies to kcp 0.29, Kube 1.34 #124
+796
−770
Conversation
This file contains hidden or 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
kcp-ci-bot
added
do-not-merge/work-in-progress
Contributor
|
Skipping CI for Draft Pull Request. |
kcp-ci-bot
added
the
dco-signoff: yes
Contributor
|
[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.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
kcp-ci-bot
added
do-not-merge/needs-kind
xrstf
force-pushed
the
kcp-0.29
branch
2 times, most recently
from
d4336a7 to
36b3e38
Compare
Contributor
Author
|
/test all |
On-behalf-of: @SAP [email protected]
On-behalf-of: @SAP [email protected]
kcp-ci-bot
added
size/XXL
Contributor
Author
|
/test all |
On-behalf-of: @SAP [email protected]
On-behalf-of: @SAP [email protected]
On-behalf-of: @SAP [email protected]
Since the mcprovider now watches the APIExportEndpointSlice itself, it does not make sense anymore to start it dynamically on demand. There is no point in having the syncmanager watch an APIExport or EndpointSlice itself, since the mcprovider does not take any URLs as input values anymore, it relies solely on watching the EndpointSlice itself. To accomodate this, I moved the manager and its provider up into the main(). Since the syncmanager controller however not just simply starts an mcmanager/provider, but also deals with dynamically started controllers, I decided to move that behaviour into a new DynamicMultiClusterManager (DMCM). The DMCM is capable of keeping track of all engaged clusters and allows you to add and start controllers at any time. It will automatically pre-seed new controller instances with the known clusters. There is one downside: Right now, if the controller fails to start, the syncmanager controller will not notice, re-reconcile and restore it. There's no back channel for that error from the sync controller's goroutine yet. --- Since the syncagent by design dynamically fills APIExports with resource schemas, when the DMCM is started, there are most likely (at least in cold start situations) no resource schemas yet in the APIExport. Even though the syncmanager waits for the apiresourceschema controller to be done, it currently does not wait for the apiexport controller to be done (in adding that ResourceSchema to the APIExport) (there is no condition or anything to tell when that controller has finished). This means the syncmanager will start new sync controllers at the same time as the apiexport controller is still filling the APIExport. However when the resources are not present yet, these sync controllers fail to boot up, and since there is currently no channel to report this back to the syncmanager, they do not get quickly restarted. To handle this (it might have been simpler to wait for the APIExport to be ready, but alas... I realized my oversight too late), I added an explicit API discovery step, where the sync controllers are only started if and when the primary resource is actually available. --- One major side effect of this whole refactoring is of course: the agent does not longer support watching APIExports for their deprecated virtual workspace URLs. When this is merged, you must provide an APIExportEndpointSlice. On-behalf-of: @SAP [email protected]
Contributor
Author
|
/test all |
Contributor
Author
|
/kind feature |
kcp-ci-bot
added
release-note
kcp-ci-bot
added
kind/api-change
kcp-ci-bot
removed
the
do-not-merge/work-in-progress
xrstf
changed the title
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.
Summary
This PR updates the syncagent to the latest and greatest dependencies, mostly in order to get some important bugfixes from multicluster-provider. This means kcp 0.29 wasn't good enough, we actually needed the tip of everything. Mostly because of Kubernetes 1.33 vs 1.34.
Due to the significant changes in multicluster-provider, the agent also had to be adjusted quite a bit and the most significant change for endusers:
--apiexport-refhad to be removed, so with this PR, the agent will only support APIExportEndpointSlices.I tried to split the branch into easy to review commits and left some more details in the individual commits.
What Type of PR Is This?
/kind feature
/kind api-change
Related Issue(s)
Fixes #126
Release Notes