✨ Fakeclient: Add apply support #2981

alvaroaleman · 2024-10-14T03:14:23Z

This change adds apply support into the fake client.

This relies on the upstream support for this which is implemented in a new FieldManagedObjectTracker. In order to support many types, a custom multiTypeConverter is added.

The FieldManagedObjectTracker results in ManagedFields being set after any operations. As that would be a very breaking change, the fake client will by default unset them and allows to optionally leave them.

Based on all existing tests still passing, I believe this change is fully backwards-compatible (But depends on breaking changes such as a very new client-go and apimachinery and #3228).

Fixes #2341

k8s-ci-robot · 2024-10-14T03:14:25Z

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

k8s-triage-robot · 2025-02-08T22:37:05Z

The Kubernetes project currently lacks enough contributors to adequately respond to all PRs.

This bot triages PRs according to the following rules:

After 90d of inactivity, lifecycle/stale is applied
After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
After 30d of inactivity since lifecycle/rotten was applied, the PR is closed

You can:

Mark this PR as fresh with /remove-lifecycle stale
Close this PR with /close
Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

alvaroaleman · 2025-02-08T22:51:59Z

/lifecycle frozen
This is a requested feature so I'd like to keep this open to demonstrate the challenges. I personally will not be working further on it though.

k8s-ci-robot · 2025-02-08T22:52:01Z

@alvaroaleman: The lifecycle/frozen label cannot be applied to Pull Requests.

In response to this:

/lifecycle frozen
This is a requested feature so I'd like to keep this open to demonstrate the challenges. I personally will not be working further on it though.

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-sigs/prow repository.

k8s-triage-robot · 2025-02-09T01:43:05Z

The lifecycle/frozen label can not be applied to PRs.

This bot removes lifecycle/frozen from PRs because:

Commenting /lifecycle frozen on a PR has not worked since March 2021
PRs that remain open for >150 days are unlikely to be easily rebased

You can:

Rebase this PR and attempt to get it merged
Close this PR with /close

Please send feedback to sig-contributor-experience at kubernetes/community.

/remove-lifecycle frozen

jpbetz · 2025-02-25T16:26:22Z

pkg/client/fake/typeconverter.go

+type multiTypeConverter struct {
+	upstream []managedfields.TypeConverter
+}


Providing this composite type converter seems fine.

We have a different use case for a composite type converter in the apiserver for admission: https://github.com/kubernetes/kubernetes/blob/25e11cd1c143ef136418c33bfbbbd4f24e32e529/staging/src/k8s.io/apiserver/pkg/admission/plugin/policy/mutating/patch/typeconverter.go#L37. But the use case is different, it takes a single static type converter and an openapi client, instead of multiple underlying type converters, so it is not useful here.

I'd like to ensure we keep this implementation unexported. Maybe add some godoc to that effect?

Yep makes sense, will do that

jpbetz · 2025-02-25T16:30:15Z

pkg/client/fake/client.go

@@ -119,6 +123,7 @@ type ClientBuilder struct {
 	withStatusSubresource []client.Object
 	objectTracker         testing.ObjectTracker
 	interceptorFuncs      *interceptor.Funcs
+	typeConverters        []managedfields.TypeConverter


nit: Can this simply be typeConverter managedfields.TypeConverter? When multiTypeConverter is needed, it should implement the managedfields.TypeConverter interface and so can be initialized and used here?

This is on the builder and often times it will be needed to have more than one, as both in-tree and CRs are used

pkg/client/fake/client.go

jpbetz

This looks like a good approach. Left some minor feedback then LGTM.

k8s-triage-robot · 2025-04-07T04:23:22Z

The Kubernetes project currently lacks enough active contributors to adequately respond to all PRs.

This bot triages PRs according to the following rules:

After 90d of inactivity, lifecycle/stale is applied
After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
After 30d of inactivity since lifecycle/rotten was applied, the PR is closed

You can:

Mark this PR as fresh with /remove-lifecycle rotten
Close this PR with /close
Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle rotten

pkg/client/fake/client.go

sbueringer · 2025-06-19T05:50:01Z

pkg/client/fake/client.go

@@ -228,6 +237,29 @@ func (f *ClientBuilder) WithInterceptorFuncs(interceptorFuncs interceptor.Funcs)
 	return f
 }

+// WithTypeConverters sets the type converters for the fake client. The list is ordered and the first
+// non-erroring converter is used. A type converter must be provided for all types the client is used


Under which exact circumstances must these be provided?

Only if SSA with custom types is used with the fake client? (based on the unit test it looks like Unstructured also works without setting TypeConverters)

It is always requiered if the FieldManagedObjectTracker is used, it seems to error out on all of its methods if the typeConverter doesn't handle a given type

I think I'm missing something.

So if I'm using the fake client today with my own CRDs, this will now break if I don't start setting type converters for them? This would break a lot of people

(FieldManagedObjectTracker is always used if objectTracker is not explicitly set, right?)

So if I'm using the fake client today with my own CRDs, this will now break if I don't start setting type converters for them? This would break a lot of people

No it will not break. SSA might not work correctly due to the inferencing type converter, but that is all.

Okay, can we make this slightly clearer please? The current comment is

A type converter must be provided for all types the client is used for, otherwise it will error.

I'm not sure if folks will infer that correctly otherwise

pkg/client/fake/client.go

pkg/client/fake/typeconverter.go

pkg/client/fake/client_test.go

sbueringer · 2025-06-19T06:32:37Z

Nice! Thx for working on this.

My comments are mostly around improving test coverage and godoc a bit

alvaroaleman · 2025-06-19T16:21:31Z

@tomasaschan @fsommar could you please find a different place for your comments that are not about feedback to the code changes in here to avoid polluting this? Thanks!

sbueringer · 2025-06-26T14:36:11Z

@alvaroaleman Last one from my side: #2981 (comment)

This change adds apply support into the fake client. This relies on the upstream support for this which is implemented in a new [FieldManagedObjectTracker][0]. In order to support many types, a custom `multiTypeConverter` is added. [0]: https://github.com/kubernetes/kubernetes/blob/4dc7a48ac6fb631a84e1974772bf7b8fd0bb9c59/staging/src/k8s.io/client-go/testing/fixture.go#L643

sbueringer · 2025-06-26T17:59:39Z

/lgtm
/approve

k8s-ci-robot · 2025-06-26T17:59:46Z

LGTM label has been added.

Git tree hash: 2b9d7c67fa6bf77671bde55014a360b49b49e0fd

k8s-ci-robot · 2025-06-26T17:59:46Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: alvaroaleman, sbueringer

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:

~~OWNERS~~ [alvaroaleman,sbueringer]

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

k8s-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Oct 14, 2024

k8s-ci-robot requested review from inteon and vincepri October 14, 2024 03:14

k8s-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Oct 14, 2024

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 10, 2024

k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Feb 8, 2025

alvaroaleman added the lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. label Feb 8, 2025

k8s-ci-robot removed the lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. label Feb 9, 2025

alvaroaleman force-pushed the apply-fake branch from ac5705d to b8202b9 Compare February 23, 2025 17:56

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 23, 2025

alvaroaleman changed the title ~~✨ Fakeclient: Add apply support~~ ⚠️ : Fakeclient: Add apply support Feb 23, 2025

k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Feb 23, 2025

alvaroaleman force-pushed the apply-fake branch from b8202b9 to 71e305a Compare February 23, 2025 17:57

alvaroaleman changed the title ~~⚠️ : Fakeclient: Add apply support~~ ⚠️ Fakeclient: Add apply support Feb 23, 2025

jpbetz reviewed Feb 25, 2025

View reviewed changes

pkg/client/fake/client.go Show resolved Hide resolved

jpbetz reviewed Feb 25, 2025

View reviewed changes

pkg/client/fake/client.go Show resolved Hide resolved

jpbetz reviewed Feb 25, 2025

View reviewed changes

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 8, 2025

sbueringer mentioned this pull request Mar 30, 2025

Feature: Native SSA Support #3183

Open

9 tasks

alvaroaleman force-pushed the apply-fake branch from 46e8e54 to 3e3e290 Compare June 6, 2025 23:33

alvaroaleman changed the title ~~⚠️ Fakeclient: Add apply support~~ ✨ Fakeclient: Add apply support Jun 6, 2025

alvaroaleman removed the lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. label Jun 6, 2025

alvaroaleman force-pushed the apply-fake branch 3 times, most recently from d6585af to e087e01 Compare June 7, 2025 20:25

This comment was marked as off-topic.

Sign in to view

sbueringer reviewed Jun 19, 2025

View reviewed changes

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 19, 2025

alvaroaleman force-pushed the apply-fake branch from e087e01 to 5f2b677 Compare June 19, 2025 16:03

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 19, 2025

alvaroaleman force-pushed the apply-fake branch 3 times, most recently from c33ac89 to 04c0089 Compare June 20, 2025 17:36

alvaroaleman force-pushed the apply-fake branch 2 times, most recently from b83e359 to 2f94e55 Compare June 26, 2025 16:54

alvaroaleman force-pushed the apply-fake branch from 2f94e55 to f4014f5 Compare June 26, 2025 16:55

k8s-ci-robot assigned sbueringer Jun 26, 2025

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 26, 2025

k8s-ci-robot merged commit cacd627 into kubernetes-sigs:main Jun 26, 2025
9 checks passed

sbueringer mentioned this pull request Jun 27, 2025

🌱Bump to k8s.io/* v0.34.0-alpha.2 kubernetes-sigs/controller-tools#1225

Merged

✨ Fakeclient: Add apply support #2981

✨ Fakeclient: Add apply support #2981

Conversation

alvaroaleman commented Oct 14, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

k8s-ci-robot commented Oct 14, 2024

Uh oh!

k8s-triage-robot commented Feb 8, 2025

Uh oh!

alvaroaleman commented Feb 8, 2025

Uh oh!

k8s-ci-robot commented Feb 8, 2025

Uh oh!

k8s-triage-robot commented Feb 9, 2025

Uh oh!

jpbetz Feb 25, 2025

Choose a reason for hiding this comment

Uh oh!

alvaroaleman Feb 25, 2025

Choose a reason for hiding this comment

Uh oh!

jpbetz Feb 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alvaroaleman Jun 6, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

jpbetz left a comment

Choose a reason for hiding this comment

Uh oh!

k8s-triage-robot commented Apr 7, 2025

Uh oh!

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

Uh oh!

sbueringer Jun 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alvaroaleman Jun 19, 2025

Choose a reason for hiding this comment

Uh oh!

sbueringer Jun 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alvaroaleman Jun 19, 2025

Choose a reason for hiding this comment

Uh oh!

sbueringer Jun 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sbueringer commented Jun 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

alvaroaleman commented Jun 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sbueringer commented Jun 26, 2025

Uh oh!

sbueringer commented Jun 26, 2025

Uh oh!

k8s-ci-robot commented Jun 26, 2025

Uh oh!

k8s-ci-robot commented Jun 26, 2025

Uh oh!

alvaroaleman commented Oct 14, 2024 •

edited

Loading

jpbetz Feb 25, 2025 •

edited

Loading

sbueringer Jun 19, 2025 •

edited

Loading

sbueringer Jun 19, 2025 •

edited

Loading

sbueringer Jun 26, 2025 •

edited

Loading

sbueringer commented Jun 19, 2025 •

edited

Loading

alvaroaleman commented Jun 19, 2025 •

edited

Loading