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

Fake ducks might not work with k8s 1.20 clients #5185

Closed
vaikas opened this issue Mar 31, 2021 · 4 comments
Closed

Fake ducks might not work with k8s 1.20 clients #5185

vaikas opened this issue Mar 31, 2021 · 4 comments
Assignees
@vaikas
Labels
area/test-and-release Test infrastructure, tests or release kind/bug Categorizes issue or PR as related to a bug. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release.
Milestone
v0.23.0

Comments

@vaikas
Copy link
Contributor

vaikas commented Mar 31, 2021

Describe the bug
Just jotting down this that might be a bug. While working on the rabbitmq broker, we noticed that if I used 1.20 k8s clients, I couldn't use ducks in our tests. Problem was a panic while doing a get for dynamic client. This results in a panic, and when reverting back to 1.19 it worked just fine.

@n3wscott for 👀 also.

Expected behavior
A clear and concise description of what you expected to happen.

To Reproduce
Steps to reproduce the behavior.

Knative release version

Additional context
Add any other context about the problem here such as proposed priority
@vaikas vaikas added the kind/bug Categorizes issue or PR as related to a bug. label Mar 31, 2021
@n3wscott n3wscott mentioned this issue Mar 31, 2021
Closed
@n3wscott
Copy link
Contributor

Ok I figured out how to fix this.

For every GVK that will be used in the test you also need to register a ListKind, for example the RabbitmqCluster and RabbitmqClusterList:

	scheme.AddKnownTypeWithName(schema.GroupVersionKind{Group: "rabbitmq.com", Version: "v1beta1", Kind: "RabbitmqCluster"}, &unstructured.Unstructured{})
	scheme.AddKnownTypeWithName(schema.GroupVersionKind{Group: "rabbitmq.com", Version: "v1beta1", Kind: "RabbitmqClusterList"}, &unstructured.UnstructuredList{})

@grantr grantr added this to the v0.23.0 milestone Apr 5, 2021
@grantr grantr added priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. area/test-and-release Test infrastructure, tests or release labels Apr 5, 2021
@dprotaso
Copy link
Member

dprotaso commented Jun 8, 2021

As a precursor this PR should setup the right schemes: #5477

This should enable my k8s bump (knative/pkg#2145) to re-wire these schemes to use unstructured.* types.when the fake dynamic client is setup (using injection).

@vaikas
Copy link
Contributor Author

vaikas commented Aug 10, 2021

/close

This was fixed.

@knative-prow-robot
Copy link
Contributor

@vaikas: Closing this issue.

In response to this:

/close

This was fixed.

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.

benmoss pushed a commit to benmoss/eventing-rabbitmq that referenced this issue Sep 23, 2021
Switch broker DLQ test to use a k8s Service
9a837cb 
With upcoming changes to eventing DLQs will be defaulted to the
namespace of their parent object if the namespace isn't specified in the
ref. For some reason this causes a panic from the fake dynamic client
that doesn't occur if the namespace isn't specified.

It happens because ksvcs have been registered with the fake dynamic
client as a type, but I'm not really sure why they only panic when a
namespace is present.

xref knative/eventing#5748 for the namespace
defaulting change
xref knative/eventing#5185 for more discussion of
the changed behavior of client-go fake dynamic clients.
@benmoss benmoss mentioned this issue Sep 23, 2021
Merged
knative-prow-robot pushed a commit to knative-extensions/eventing-rabbitmq that referenced this issue Sep 23, 2021
Switch broker DLQ test to use a k8s Service (#433)
1cc859e 
With upcoming changes to eventing DLQs will be defaulted to the
namespace of their parent object if the namespace isn't specified in the
ref. For some reason this causes a panic from the fake dynamic client
that doesn't occur if the namespace isn't specified.

It happens because ksvcs have been registered with the fake dynamic
client as a type, but I'm not really sure why they only panic when a
namespace is present.

xref knative/eventing#5748 for the namespace
defaulting change
xref knative/eventing#5185 for more discussion of
the changed behavior of client-go fake dynamic clients.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@vaikas vaikas

Labels
area/test-and-release Test infrastructure, tests or release kind/bug Categorizes issue or PR as related to a bug. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release.
Projects
None yet
Milestone
v0.23.0
Development

No branches or pull requests
5 participants
@grantr @dprotaso @vaikas @n3wscott @knative-prow-robot