Skip to content

Conversation

@xrstf
Copy link
Contributor

@xrstf xrstf commented Apr 30, 2025

Summary

kcp-dev/code-generator#107 reworks the kcp code-generator to use the generic-style code that was introduced recently (around kube 1.32) in upstream. To fully take advantage of this approach, it's necessary to also provide cluster-aware variants of ResourceIndexers, fake Clients etc.

This PR adds those by soft-forking client-go's gentype and listers package, and then adding cluster-aware types to it.


Additionally, this PR also includes all changes to k8s.io/client-go/testing that happened between Kubernetes 1.26 and 1.32, specifically this list of k/k commits (commits in italics happened in that range, but did not modify the testing package and so were skipped):

  • b0ce65df9: Generify fake clientsets
  • 93f82d25a: Merge pull request #127689 from mmorel-35/testifylint/[email protected]/client-go
  • 71ced25f4: fix: enable error-nil and nil-compare rules from testifylint in module k8s.io/client-go
  • 8286a6903: fix: enable expected-actual rule from testifylint in module k8s.io/client-go
  • 5784e5844: Use Fatalf for non-recoverable errors in test
  • 5f1c7ae63: Stamp fake client apply reuqests with name from action
  • 1095af88e: Remove test dependency on swwagger.json to fix client-go repo
  • 75d6f0243: Add field tracker support to client fake fixtures
  • 599f03c72: Support options for all client fake actions
  • 5300466a5: Use canonical json-patch v4 import

1.26 was chosen as the base because last time we touched our fork of the testing package was in #25.


NB: There is a hack/populate-copies.sh in this repo and it gives off the impression that some files in third_party/ are not meant to be modified, but copied wholesale 1:1 from k/k. However since that script was added, maaaaany changes have been done to those copied files and I think today, that script is pointless. But I'm not sure and so I kept it for ... sentimental reasons. That is also why I did not put the files from gentype and listers into the script.


All of these changes have been tested in conjunction with kcp-dev/code-generator#107 in kcp main branch and all unit/e2e tests pass.

@kcp-ci-bot kcp-ci-bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Apr 30, 2025
@kcp-ci-bot
Copy link
Contributor

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@kcp-ci-bot kcp-ci-bot added dco-signoff: yes Indicates the PR's author has signed the DCO. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Apr 30, 2025
@xrstf
Copy link
Contributor Author

xrstf commented Apr 30, 2025

/test all

@xrstf xrstf changed the title Add cluster-aware versions of gentype and listers ✨ Add cluster-aware versions of gentype and listers Apr 30, 2025
@xrstf
Copy link
Contributor Author

xrstf commented Apr 30, 2025

/test all

@xrstf xrstf changed the title ✨ Add cluster-aware versions of gentype and listers ✨ Add cluster-aware versions of gentype and listers, update testing May 9, 2025
@kcp-ci-bot kcp-ci-bot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels May 9, 2025
@xrstf
Copy link
Contributor Author

xrstf commented May 9, 2025

/test all

1 similar comment
@xrstf
Copy link
Contributor Author

xrstf commented May 9, 2025

/test all

@xrstf
Copy link
Contributor Author

xrstf commented May 9, 2025

/test all

@xrstf
Copy link
Contributor Author

xrstf commented May 9, 2025

/test all

@xrstf
Copy link
Contributor Author

xrstf commented May 9, 2025

/test all

@xrstf
Copy link
Contributor Author

xrstf commented May 9, 2025

/test all

@xrstf xrstf marked this pull request as ready for review May 9, 2025 13:19
@kcp-ci-bot kcp-ci-bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label May 9, 2025
@mjudeikis
Copy link
Contributor

/lgtm
/approve
🚢

@kcp-ci-bot kcp-ci-bot added the lgtm Indicates that a PR is ready to be merged. label May 12, 2025
@kcp-ci-bot
Copy link
Contributor

LGTM label has been added.

Git tree hash: 82b9a10177f2275bc0947fbdfafa3924a0276a5f

@kcp-ci-bot
Copy link
Contributor

[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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@kcp-ci-bot kcp-ci-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 12, 2025
@kcp-ci-bot kcp-ci-bot merged commit 91bd851 into kcp-dev:main May 12, 2025
10 checks passed
@mjudeikis
Copy link
Contributor

/revert

xrstf added a commit to xrstf/kcp-client-go that referenced this pull request May 12, 2025
This reverts commit 91bd851, reversing
changes made to 5ae6774.

Except for the .prow.yaml, which is a required upgrade in all branches.
@mjudeikis
Copy link
Contributor

/cherry-pick kcp-1.32.3

@kcp-ci-bot
Copy link
Contributor

@mjudeikis: new pull request created: #44

In response to this:

/cherry-pick kcp-1.32.3

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. dco-signoff: yes Indicates the PR's author has signed the DCO. lgtm Indicates that a PR is ready to be merged. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants