Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Add scheme support to fake client #213

Merged
merged 4 commits into from
Nov 21, 2018
Merged

Add scheme support to fake client #213

merged 4 commits into from
Nov 21, 2018

Conversation

grantr
Copy link
Contributor

@grantr grantr commented Nov 14, 2018

Adds a NewFakeClientWithScheme function that takes a *runtime.Scheme as its first argument. NewFakeClient is now a wrapper around NewFakeClientWithScheme, passing it the default scheme.Scheme.

Fixes #137.

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Nov 14, 2018
@k8s-ci-robot k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Nov 14, 2018
@grantr
Add scheme support to fake client
8f3407b 
Adds a NewFakeClientWithScheme function that takes a *runtime.Scheme as
its first argument. NewFakeClient is now a wrapper around
NewFakeClientWithScheme, passing it the default scheme.Scheme.

As part of this, the scheme can now be used to derive the correct List
GVK. This eliminates the need to use a metav1.List and
opts.Raw.TypeMeta. Now it's possible to pass in a typed List, same as
with the real client.
@grantr
Update fake client tests to use typed Lists
8ade6d7 
This is enabled by new type support in fake client List. Other tests
may also be updated to use this style.
@grantr
Copy link
Contributor Author

grantr commented Nov 14, 2018

As a bonus, I added the GVK support from https://github.com/kubernetes-sigs/controller-runtime/pull/106/files#diff-62fbcb10a0dc0bc5b0a1bba07b8eefb0R82 to this PR. This allows the fake client to work with typed Lists (e.g. appsv1.DeploymentList) instead of requiring metav1.List and opts.Raw.TypeMeta.The old style still works, but the new style is cleaner. I updated the fake client tests as an example.

@grantr
Don't derive GVK for generic List type
bf4afaa 
metav1.List and corev1.List don't conform to the TypeList standard
expected by getGVKFromList.
grantr
grantr commented Nov 15, 2018
View reviewed changes
pkg/client/fake/client.go
}

if gvk.Kind == "List" {
return schema.GroupVersionKind{}, fmt.Errorf("cannot derive GVK for generic List type %T (kind %q)", list, gvk)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This check is necessary to detect the corev1.List and metav1.List type, which don't conform to the SingularTypeList format expected here. Without this check the returned GVK has an empty Kind.

It seems worthwhile to consolidate this derivation into a single helper function, since it's also used in varying forms in these places:

controller-runtime/pkg/client/client_cache.go

Line 60 in 2ef903b

if strings.HasSuffix(gvk.Kind, "List") && meta.IsListType(obj) {

controller-runtime/pkg/client/unstructured_client.go

Line 115 in df7c11e

if strings.HasSuffix(gvk.Kind, "List") {

controller-runtime/pkg/cache/informer_cache.go

Line 67 in df7c11e

if !strings.HasSuffix(gvk.Kind, "List") {

@grantr grantr mentioned this pull request Nov 19, 2018
Closed
estroz
estroz reviewed Nov 21, 2018
View reviewed changes
pkg/client/fake/client.go Outdated Show resolved Hide resolved
@estroz @grantr
Check for nil Raw value
5558165 
Co-Authored-By: grantr <grantr@gmail.com>
@droot droot added lgtm "Looks good to me", indicates that a PR is ready to be merged. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Nov 21, 2018
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

Approval requirements bypassed by manually added approval.

This pull-request has been approved by: grantr

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

@k8s-ci-robot k8s-ci-robot merged commit 0e09a0d into kubernetes-sigs:master Nov 21, 2018
@grantr grantr deleted the fake-client-scheme branch November 21, 2018 21:54
@grantr grantr mentioned this pull request Nov 22, 2018
Merged
justinsb pushed a commit to justinsb/controller-runtime that referenced this pull request Dec 7, 2018
@k8s-ci-robot
Merge pull request kubernetes-sigs#213 from grantr/fake-client-scheme
3bdfa82 
Add scheme support to fake client
manno pushed a commit to cloudfoundry-incubator/quarks-operator that referenced this pull request Dec 11, 2018
Update controller-runtime to 1.8
4c29632 
* update generated code via bin/gen-kube

This should help with

* kubernetes-sigs/controller-runtime#168
* kubernetes-sigs/controller-runtime#213
manno pushed a commit to cloudfoundry-incubator/quarks-operator that referenced this pull request Dec 11, 2018
Update controller-runtime to 1.8
bf3b302 
* `go get -u ...@version`
* `go mod tidy`
* update generated code via bin/gen-kube

This should help with

* kubernetes-sigs/controller-runtime#168
* kubernetes-sigs/controller-runtime#213
@hasbro17 hasbro17 mentioned this pull request Jan 2, 2019
Closed
@zreigz zreigz mentioned this pull request Feb 8, 2019
Merged
@mszostok mszostok mentioned this pull request Apr 1, 2019
Merged
@jsanda jsanda mentioned this pull request Aug 17, 2019
Merged
manno pushed a commit to cloudfoundry-incubator/quarks-job that referenced this pull request Nov 11, 2019
Update controller-runtime to 1.8
b37a340 
* `go get -u ...@version`
* `go mod tidy`
* update generated code via bin/gen-kube

This should help with

* kubernetes-sigs/controller-runtime#168
* kubernetes-sigs/controller-runtime#213
manno pushed a commit to cloudfoundry-incubator/quarks-secret that referenced this pull request Jun 9, 2020
Update controller-runtime to 1.8
ced8a54 
* `go get -u ...@version`
* `go mod tidy`
* update generated code via bin/gen-kube

This should help with

* kubernetes-sigs/controller-runtime#168
* kubernetes-sigs/controller-runtime#213
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@estroz estroz estroz left review comments

@DirectXMan12 DirectXMan12 Awaiting requested review from DirectXMan12

@pwittrock pwittrock Awaiting requested review from pwittrock

Assignees
No one assigned
Labels
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. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@grantr @k8s-ci-robot @estroz @droot