-
Notifications
You must be signed in to change notification settings - Fork 424
Rebase to kube 1.33.3 #3511
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
Rebase to kube 1.33.3 #3511
Conversation
77913c5 to
616e645
Compare
|
/retest https://public-prow.kcp.k8c.io/view/s3/prow-public-data/pr-logs/pull/kcp-dev_kcp/3511/pull-kcp-test-e2e-sharded/1952381142900412416 |
822e6f6 to
4ab6231
Compare
embik
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mostly looking good, thanks for all the hard work! I mostly have concerns around the Go versioning happening in the GitHub workflows and the Dockerfile.
Signed-off-by: Nelo-T. Wallus <[email protected]>
See kubernetes/kubernetes#129416 Signed-off-by: Nelo-T. Wallus <[email protected]>
Signed-off-by: Nelo-T. Wallus <[email protected]>
Signed-off-by: Nelo-T. Wallus <[email protected]>
Signed-off-by: Nelo-T. Wallus <[email protected]>
Signed-off-by: Nelo-T. Wallus <[email protected]>
Signed-off-by: Nelo-T. Wallus <[email protected]>
Signed-off-by: Nelo-T. Wallus <[email protected]>
Signed-off-by: Nelo-T. Wallus <[email protected]>
Upstream disabled list-via-watch in kubernetes/kubernetes#131359 Reenabling would be possible through a feature gate but upstream doesn't seem keen on continuing the feature. Signed-off-by: Nelo-T. Wallus <[email protected]>
The sed call would always leave a "go.mod''" since the expansion was quoting the quotes again. Signed-off-by: Nelo-T. Wallus <[email protected]>
Signed-off-by: Nelo-T. Wallus <[email protected]>
Signed-off-by: Nelo-T. Wallus <[email protected]>
Signed-off-by: Nelo-T. Wallus <[email protected]>
|
/lgtm |
|
LGTM label has been added. Git tree hash: e331463d6a05d8a040503c9e34d61db78e36ccf7
|
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: mjudeikis 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 |
|
Nothing blocking this from my perspective. |
|
/hold cancel |
Summary
Rebase to 1.33.3
Diff PRs:
Notable Changes:
When starting to watch resources the initial state of objects will no longer be sent, instead watchers only receive updates from when the watch was started
As a result one test had to be adjusted to remove the expectation of the initial objects:
f3a4fee(#3511)While this can be reenabled I think we should follow upstream with this. Clients that need it can still explicitly call list.
This causes goreleaser to fail on the free actions runners due to running out of disk space: https://github.com/kcp-dev/kcp/actions/runs/16750243000/job/47418386309?pr=3511
Letting goreleaser build sequentially alleviates the issue and keeps the same runtime: https://github.com/kcp-dev/kcp/actions/runs/16751809185/job/47423741078?pr=3511
Most likely because it can reuse the built archives of previous builds.
@embik and I discussed the possibility of moving goreleaser to a prow job in the future but for this rebase this is fine.
Some planned post-steps/discussion points:
KCP can technically use protobuf (and has no problems doing so when the client sends protobuf [or CBOR], see e.g. https://github.com/ntnn/kcp-cbor-proto-tester/) - however having
PrefersProtobufenabled in kcp-dev/kubernetes errors in the apiextension server where it is trying to decode protobuf as JSON. I'm not sure why as selecting the correct decoder should have been handled by the headers.While protobuf is less interesting as it only benefits kube native types CBOR is more interesting and our patches currently do not take CBOR in account, e.g. here: https://github.com/kcp-dev/kubernetes/pull/170/files#diff-39e2ff2eeff49f0b9c7d72720178f54e0c233226f912db61d3d08dcd551278de
I made some tests with both and ultimately decided that it is better to leave this is a separate issue.
Kube switched to using a lock-less GC in 1.32 which was patched back in for KCP; when talking about performance problems it came up that we could just drop that patch and see if that produces any problems. Having just tagged 0.28 this might be a good opportunity to do that.
The converter has many changes and the patches for it do not apply without conflicts even though there were no changes upstream. Also three of four helper functions are identical to upstream helpers, one only has minor changes. Much of the logic in the converter is also present in the webhook_converter. This patch could probably be rewritten to be less intrusive and to merge easier for future rebases.
In KCPs hack/update-codegen-clients.sh is a comment about using
generate-groups.shonce Add applyconfiguration generator to code-generator script kubernetes/kubernetes#114987 is merged. The PR has been merged a while ago, maybe this can be addressed.See UPSTREAM: <carry>: Drop validation-gen kubernetes#169 and specifically the commit kcp-dev/kubernetes@
934d467(#170) for details - in essence k/k has added validation-gen for declarative field validation from struct tags rather than the handwritten validation. But that fails in the kcp-dev/k fork as the go.mod rewrites do not work in the fork.From what I gather this is only intended for kube-native types.
What Type of PR Is This?
/kind feature
Related Issue(s)
Fixes #
Release Notes