Skip to content
This repository has been archived by the owner on Oct 21, 2020. It is now read-only.

ProvisionController support for SharedInformers #556

Merged
merged 2 commits into from
Feb 9, 2018

Conversation

zcahana
Copy link
Contributor

@zcahana zcahana commented Jan 14, 2018

This PR adds support for running the ProvisionController with SharedInformers.

This is achieved by introducing a new constructor, NewProvisionControllerWithConfig() that enables to optionally pass in shared informers; The existing constructor, NewProvisionController(), as well as its behavior, are left as they were.

Resolves #546.

@k8s-ci-robot
Copy link
Contributor

Thanks for your pull request. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please follow instructions at https://git.k8s.io/community/CLA.md#the-contributor-license-agreement to sign the CLA.

It may take a couple minutes for the CLA signature to be fully registered; after that, please reply here with a new comment and we'll verify. Thanks.


  • If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
  • If you signed the CLA as a corporation, please sign in with your organization's credentials at https://identity.linuxfoundation.org/projects/cncf to be authorized.
  • If you have done the above and are still having issues with the CLA being reported as unsigned, please email the CNCF helpdesk: [email protected]

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. I understand the commands that are listed here.

@k8s-ci-robot k8s-ci-robot added cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Jan 14, 2018
@zcahana
Copy link
Contributor Author

zcahana commented Jan 14, 2018

@k8s-ci-robot I've signed the CLA.

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. and removed cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. labels Jan 14, 2018
@zcahana
Copy link
Contributor Author

zcahana commented Jan 14, 2018

/cc @wongma7

@zcahana
Copy link
Contributor Author

zcahana commented Jan 21, 2018

I've refactored the code to be consistent with the functional options pattern:

  • Introduced 3 new option functions: ClaimsInformer(), VolumesInformer(), and ClassesInformer(),
    to be used with NewProvisionController().
  • Removed the NewProvisionControllerWithConfig() constructor mentioned in the beginning of this PR.

@zcahana
Copy link
Contributor Author

zcahana commented Jan 28, 2018

@wongma7 did you get a chance to look at this?

@rootfs
Copy link
Contributor

rootfs commented Feb 7, 2018

/assign @jsafrane
wait till #578 is resolved

@k8s-ci-robot k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Feb 8, 2018
@zcahana
Copy link
Contributor Author

zcahana commented Feb 8, 2018

I've removed the commit which vendored the additional client-go/informers/* packages (as these were added as part of #578), and rebased.

@jsafrane
Copy link
Contributor

jsafrane commented Feb 9, 2018

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Feb 9, 2018
@jsafrane
Copy link
Contributor

jsafrane commented Feb 9, 2018

/approve no-issue
Checking if there is merge bot...

@childsb childsb merged commit 9ff8c10 into kubernetes-retired:master Feb 9, 2018
@zcahana zcahana deleted the shared_informers branch October 7, 2021 07:28
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm 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
Development

Successfully merging this pull request may close these issues.

5 participants