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.

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

upgrade controller-runtime to support LabelSelector in fakeClient List func #1606

Closed
andy2046 opened this issue Jun 27, 2019 · 2 comments · Fixed by #1612
Closed

upgrade controller-runtime to support LabelSelector in fakeClient List func #1606

andy2046 opened this issue Jun 27, 2019 · 2 comments · Fixed by #1612

Comments

@andy2046
Copy link
Contributor

Feature Request

Is your feature request related to a problem? Please describe.

Currently the sdk is using controller-runtime v0.1.10, which does not support LabelSelector filter in fakeClient List func, only support Namespace filter

operator-sdk/Gopkg.toml

Lines 33 to 35 in 3cf985d

[[constraint]]
name = "sigs.k8s.io/controller-runtime"
version = "=v0.1.10"

https://github.com/kubernetes-sigs/controller-runtime/blob/12d98582e72927b6cd0123e2b4e819f9341ce62c/pkg/client/fake/client.go#L91-L114

func (c *fakeClient) List(ctx context.Context, opts *client.ListOptions, list runtime.Object) error {
	gvk, err := getGVKFromList(list, c.scheme)
	if err != nil {
		// The old fake client required GVK info in Raw.TypeMeta, so check there
		// before giving up
		if opts.Raw == nil || opts.Raw.TypeMeta.APIVersion == "" || opts.Raw.TypeMeta.Kind == "" {
			return err
		}
		gvk = opts.Raw.TypeMeta.GroupVersionKind()
	}

	gvr, _ := meta.UnsafeGuessKindToResource(gvk)
	**o, err := c.tracker.List(gvr, gvk, opts.Namespace)**
	if err != nil {
		return err
	}
	j, err := json.Marshal(o)
	if err != nil {
		return err
	}
	decoder := scheme.Codecs.UniversalDecoder()
	_, _, err = decoder.Decode(j, nil, list)
	return err
}

Describe the solution you'd like

here I request to upgrade to at least v0.1.11 to support the LabelSelector in fakeClient List

https://github.com/kubernetes-sigs/controller-runtime/blob/477bf4f046c31c351b46fa00262bc814ac0bbca1/pkg/client/fake/client.go#L93-L123

func (c *fakeClient) List(ctx context.Context, opts *client.ListOptions, list runtime.Object) error {
	gvk, err := getGVKFromList(list, c.scheme)
	if err != nil {
		// The old fake client required GVK info in Raw.TypeMeta, so check there
		// before giving up
		if opts.Raw == nil || opts.Raw.TypeMeta.APIVersion == "" || opts.Raw.TypeMeta.Kind == "" {
			return err
		}
		gvk = opts.Raw.TypeMeta.GroupVersionKind()
	}

	gvr, _ := meta.UnsafeGuessKindToResource(gvk)
	o, err := c.tracker.List(gvr, gvk, opts.Namespace)
	if err != nil {
		return err
	}
	j, err := json.Marshal(o)
	if err != nil {
		return err
	}
	decoder := scheme.Codecs.UniversalDecoder()
	_, _, err = decoder.Decode(j, nil, list)
	if err != nil {
		return err
	}

	**if opts.LabelSelector != nil {
		return filterListItems(list, opts.LabelSelector)
	}**
	return nil
}

I am willing to help submitting a PR for this upgrade if required, thanks

@andy2046 andy2046 changed the title upgrade controller-runtime version to support LabelSelector in fakeClient List func upgrade controller-runtime to support LabelSelector in fakeClient List func Jun 27, 2019
@lilic
Copy link
Member

lilic commented Jun 27, 2019

@andy2046 We are already in the middle of upgrading to controller-runtime v0.2.0 in PR #1512 We are blocked until controller-runtime releases a stable v0.2.0 release.

here I request to upgrade to at least v0.1.11 to support the LabelSelector in fakeClient List

I guess depending on when the v0.2.0 release will happen, bumping to v0.1.11 would be good to do. cc @estroz @camilamacedo86 @joelanford What do you think?

@joelanford
Copy link
Member

@lilic 0.1.12 was actually released last week, so if we are going to bump to a 0.1.x version, we should shoot for that.

If it's just a matter of bumping that version (and the Kubernetes version) in our deps, and none of the function signatures we use in our scaffolds change, then we can probably go ahead and do this since 0.2.0 isn't released yet.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants