⚠️ 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.

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-ci-robot · 2024-10-14T03:14:28Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: alvaroaleman

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]

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

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

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

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?

jpbetz · 2025-02-25T16:31:50Z

pkg/client/fake/client.go

@@ -160,6 +165,8 @@ func (f *ClientBuilder) WithRuntimeObjects(initRuntimeObjs ...runtime.Object) *C
 }

 // WithObjectTracker can be optionally used to initialize this fake client with testing.ObjectTracker.
+// Setting this is incompatible with setting WithTypeConverters, as they are a setting on the


Enforce this in code with an assertion?

jpbetz · 2025-02-25T16:31:56Z

pkg/client/fake/client.go

@@ -216,6 +223,18 @@ 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.
+// This setting is incompatible with WithObjectTracker, as the type converters are a setting on the tracker.


Enforce this in code with an assertion?

jpbetz

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

k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Oct 14, 2024

k8s-ci-robot added 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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

⚠️ Fakeclient: Add apply support #2981

⚠️ Fakeclient: Add apply support #2981

alvaroaleman commented Oct 14, 2024 •

edited

Loading

k8s-ci-robot commented Oct 14, 2024

k8s-ci-robot commented Oct 14, 2024

k8s-triage-robot commented Feb 8, 2025

alvaroaleman commented Feb 8, 2025

k8s-ci-robot commented Feb 8, 2025

k8s-triage-robot commented Feb 9, 2025

jpbetz Feb 25, 2025

alvaroaleman Feb 25, 2025

jpbetz Feb 25, 2025 •

edited

Loading

jpbetz Feb 25, 2025

jpbetz Feb 25, 2025

jpbetz left a comment

⚠️ Fakeclient: Add apply support #2981

Are you sure you want to change the base?

⚠️ Fakeclient: Add apply support #2981

Conversation

alvaroaleman commented Oct 14, 2024 • edited Loading

k8s-ci-robot commented Oct 14, 2024

k8s-ci-robot commented Oct 14, 2024

k8s-triage-robot commented Feb 8, 2025

alvaroaleman commented Feb 8, 2025

k8s-ci-robot commented Feb 8, 2025

k8s-triage-robot commented Feb 9, 2025

jpbetz Feb 25, 2025

Choose a reason for hiding this comment

alvaroaleman Feb 25, 2025

Choose a reason for hiding this comment

jpbetz Feb 25, 2025 • edited Loading

Choose a reason for hiding this comment

jpbetz Feb 25, 2025

Choose a reason for hiding this comment

jpbetz Feb 25, 2025

Choose a reason for hiding this comment

jpbetz left a comment

Choose a reason for hiding this comment

alvaroaleman commented Oct 14, 2024 •

edited

Loading

jpbetz Feb 25, 2025 •

edited

Loading