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

Update deprecated function call in k8s providers #5241

Merged
merged 3 commits into from
Nov 27, 2019
Merged

Conversation

Wagum
Copy link
Contributor

@Wagum Wagum commented Aug 22, 2019

What does this PR do?

Remove the usage of deprecated SharedInformerFactory in kubernetes providers.

Motivation

The kubernetes providers should use NewSharedInformerFactoryWithOptions(), due to the fact that NewFilteredSharedInformerFactory() is deprecated and may get removed in future releases.

@Wagum
Copy link
Contributor Author

Wagum commented Aug 22, 2019

I see that the crd client is also using this function call. Should this be also refactored?
And there is a deprecation management issue on 2.0 milestone (#2212). Maybe just add this to the list in this issue.

@dtomcej
Copy link
Contributor

dtomcej commented Aug 23, 2019

Hello @Wagum,

Thanks for this!

You are correct, we should also investigate using informers.WithTweakListOptions to use labelselectors instead of filtering later on.

We should also look at updating the CRD provider to use NewSharedInformerFactoryWithOptions too. Ideally this would be in this PR for consistency.

@Wagum
Copy link
Contributor Author

Wagum commented Aug 23, 2019

Hey @dtomcej ,

im going to add the call on NewSharedInformerFactoryWithOptions to the k8s CRD provider as well.

I was concerned about adding informers.WithTweakListOptions to the factory, because the ListOption from TweakListOptionsFunc will apply to all objects, that the factory provides.
(i could not identify kubernetes object types inside the TweakListOptionsFunc, becuase only ListOptions are passed into it).

This means, that you would have to add the ingressLabelSelector to the corresponding secrets, endpoints and services inside the k8s cluster as well.

An other option would be using two factories. One for ingresses including the TweakListOption for labelSelectors, and one further factory for secrets, endpoints and services. This second factory should not use the TweakListOption. I think you implemented something similar in the crd factory for Kubernetes.

Im going to rethink about this and propose a solution to you asap.
Please let me know, what you think about this after investigating with your team :)

@Wagum Wagum changed the title Remove deprecated function call in k8s ingress provider Update deprecated function call in k8s ingress provider Aug 23, 2019
@Wagum Wagum changed the title Update deprecated function call in k8s ingress provider Update deprecated function call in k8s providers Aug 23, 2019
@ldez ldez modified the milestones: 2.0, next Aug 27, 2019
@ldez ldez removed this from the 2.1 milestone Nov 10, 2019
@ldez ldez changed the base branch from v2.0 to master November 27, 2019 13:59
@ldez ldez removed the bot/no-merge label Nov 27, 2019
@ldez ldez added this to the next milestone Nov 27, 2019
Copy link
Contributor

@ldez ldez left a comment

Choose a reason for hiding this comment

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

Thanks 👍

Copy link
Member

@jbdoumenjou jbdoumenjou left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@dtomcej dtomcej left a comment

Choose a reason for hiding this comment

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

LGTM
:shipit:

remove deprecated function call.
updated CRD client
use csKube as clientset.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants