Simplify construction of the fake dynamic client #102928
Merged
Conversation
This file contains 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
k8s-ci-robot
added
size/L
dprotaso
changed the title
dprotaso
changed the title
k8s-ci-robot
added
sig/api-machinery
|
/kind cleanup |
k8s-ci-robot
added
kind/cleanup
|
/assign @lavalamp |
|
I debated whether to move the logic to I wonder if that method is still necessary and could be marked deprecated |
|
/assign @deads2k related to kubernetes/client-go#914 specifically kubernetes/client-go#949 (comment) |
|
cc @n3wscott |
Interesting this PR won't fix #914 because in that example there's no scheme or resources being supplied during construction. |
|
/triage accepted |
k8s-ci-robot
added
triage/accepted
|
Does a priority need to be assigned prior to review? |
deads2k
reviewed
Jul 7, 2021
| unstructuredScheme.AddKnownTypeWithName(gvk, &unstructured.Unstructured{}) | ||
| } | ||
|
|
||
| objects, err := convertObjectsToUnstructured(scheme, objects) |
deads2k
reviewed
Jul 7, 2021
|
|
||
| // This test ensures list works when the fake dynamic client is seeded with a typed scheme and | ||
| // unstructured type fixtures | ||
| func TestListWithUnstructuredObjectsAndTypedScheme(t *testing.T) { |
There was a problem hiding this comment.
I'd like to see a test case where the list isn't seeded at all and the response needs to be fully constructed.
There was a problem hiding this comment.
Added TestListWithNoFixturesAndTypedScheme
I'm testing with a typed scheme - otherwise I'd expect consumers to use the other constructor that requires a mapping of gvrs to list kinds
|
/lgtm holding for squash. ping me on slack for a re-lgtm. |
k8s-ci-robot
added
do-not-merge/hold
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: deads2k, dprotaso 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 |
k8s-ci-robot
added
the
approved
With the introduction of GVK to the fake dynamic client it made using the fake much more cumbersome. Specifically: - requires manual registration of list types - mismatch between scheme types and passed in fixtures would result in errors The PR changes the constructor method NewSimpleDynamicClient to do the following: - rewire the schemes to unstructured types - typed fixtures are converted to unstructured types - automatically register fixture gvks with the scheme This should make the dynamic client 'flexible' with it's inputs like it was before
dprotaso
force-pushed
the
dynamic-client-backwards-compatible
branch
from
cfbd3ff to
418fa71
Compare
k8s-ci-robot
removed
the
lgtm
|
Ok squashed |
|
/lgtm |
k8s-ci-robot
added
lgtm
|
/retest |
1 similar comment
|
/retest |
|
/cherrypick release-1.21 |
This was referenced Jul 9, 2021
k8s-ci-robot
added a commit
that referenced
this pull request
Jul 29, 2021
…2928-upstream-release-1.20 Automated cherry pick of #102928: Simplify use of the fake dynamic client
k8s-ci-robot
added a commit
that referenced
this pull request
Jul 29, 2021
…2928-upstream-release-1.21 Automated cherry pick of #102928: Simplify use of the fake dynamic client
halvards
added a commit
to halvards/skaffold
that referenced
this pull request
Mar 24, 2022
The new `ko` build option to produce SBOM is disabled until we have a design for how this should work across Skaffold builders. Additional transitive dependencies (mainly authentication libraries for various image registries) means that the Skaffold binary grows by about 4.8MB. The change to labels_test.go works around the tests failing due to this change in `client-go` between `v0.21.3` and `v0.21.4`: kubernetes/client-go@c6c0ca0 That commit came from this k/k PR: kubernetes/kubernetes#102928 Related: GoogleContainerTools#7193 Tracking: GoogleContainerTools#7131
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.
What type of PR is this?
/kind cleanup
What this PR does / why we need it:
With the introduction of GVK to the fake dynamic client it made using
the fake much more cumbersome.
Specifically:
The PR changes the constructor method NewSimpleDynamicClient to do the following:
This should make the dynamic client 'flexible' with it's inputs like it was
before
Which issue(s) this PR fixes:
Fixes N/A
Special notes for your reviewer:
Thanks!
Does this PR introduce a user-facing change?
Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.: