Skip to content

Stopping individual controllers in a manager #730

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

Closed
suskin opened this issue Dec 17, 2019 · 7 comments · Fixed by #863
Closed

Stopping individual controllers in a manager #730

suskin opened this issue Dec 17, 2019 · 7 comments · Fixed by #863
Labels
help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines.

Comments

@suskin
Copy link

suskin commented Dec 17, 2019

Hi controller-runtime friends! Sorry if this is a dumb question; I'm a bit new to Kubernetes and controller patterns!

I'm experimenting with ways to configure and set up controllers dynamically for user-specified Kinds (as part of the crossplane project). I'm interested in stopping a running controller if, for example, the desired Kind no longer exists. I'm using a manager; a single manager may be managing multiple controllers, each of which watches a different Kind.

I already have some code which creates a controller and adds it to an already-started manager, which appears to work. For stopping that controller, I've looked into the manager logic, and the controller logic, and there appears to be support for stopping a controller, but it also appears to not be accessible if I am using a manager. Because a manager reuses a single stop channel for all of its controllers, and because it starts all of the controllers without checking whether they're already started, and because controllers appear to not check whether they've already been started when Start is called, I haven't found a way to do this with today's managers. It seems like one of the easier ways to add support for this would be to add another variation of the manager's Add function. The variation would take in a stop channel as a parameter.

Am I doing a Weird Thing that I shouldn't be doing? Is there a recommended, better way of doing this that I am missing? If possible, it seems like it'd be the most convenient to continue to use the manager pattern and be able to just stop an individual controller under that manager, rather than having to reimplement some of the nice management code in order to call into a controller or into a source directly.

Thanks in advance for your help!

@DirectXMan12
Copy link
Contributor

It's not a Weird Thing -- we just didn't design around this use case. It's come up a couple times though, so I think it's worth digging into.

/help

a design or prototype for this is wanted. When we fit in context support (replacing the stop channel stuff) a reasonable design might fall out of that.

@k8s-ci-robot
Copy link
Contributor

@DirectXMan12:
This request has been marked as needing help from a contributor.

Please ensure the request meets the requirements listed here.

If this request no longer meets these requirements, the label can be removed
by commenting with the /remove-help command.

In response to this:

It's not a Weird Thing -- we just didn't design around this use case. It's come up a couple times though, so I think it's worth digging into.

/help

a design or prototype for this is wanted. When we fit in context support (replacing the stop channel stuff) a reasonable design might fall out of that.

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.

@k8s-ci-robot k8s-ci-robot added the help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. label Jan 6, 2020
@negz
Copy link
Contributor

negz commented Jan 14, 2020

@DirectXMan12 Do you have some context (hurr) for the context support? Is there an issue or design for that?

@DirectXMan12
Copy link
Contributor

I thought we had one, but I can't seem to find it. Feel free to file a new one. Basically, we want to eventually replace all of our uses of stop channels with context objects instead.

@negz
Copy link
Contributor

negz commented Jan 14, 2020

Thanks! Myself (or someone else on Crossplane) may be interested in working on this sometime in the next few weeks. We have a handful of use cases in which we'd like to spin up and down controllers dynamically - e.g. we want to submit CR A, which describes how CRs of kind: B should be reconciled. Let me know if there are any great examples of prior designs (issues? design docs?) that highlight what you're looking for from a design!

@vincepri
Copy link
Member

Closing in favor of #764

/close

@k8s-ci-robot
Copy link
Contributor

@vincepri: Closing this issue.

In response to this:

Closing in favor of #764

/close

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.

negz added a commit to negz/controller-runtime that referenced this issue Mar 17, 2020
As far as I can tell the main reason we add controllers to the manager is to
ensure that we've been elected leader before we start any controllers. The
downside of this design is that it's not possible to stop individual
controllers, or remove controllers from the manager.

I believe this commit is the minimum possible change necessary to allow
controllers to be started and stopped on-demand. It allows a controller to be
created, started, and stopped without ever being added to the manager. Any
controller that is started separately from the manager must handle its own
leader election.

kubernetes-sigs#730

Signed-off-by: Nic Cope <[email protected]>
negz added a commit to negz/controller-runtime that referenced this issue Mar 26, 2020
As far as I can tell the main reason we add controllers to the manager is to
ensure that we've been elected leader before we start any controllers. The
downside of this design is that it's not possible to stop individual
controllers, or remove controllers from the manager.

I believe this commit is the minimum possible change necessary to allow
controllers to be started and stopped on-demand. It allows a controller to be
created, started, and stopped without ever being added to the manager. Any
controller that is started separately from the manager must handle its own
leader election.

kubernetes-sigs#730

Signed-off-by: Nic Cope <[email protected]>
negz added a commit to negz/controller-runtime that referenced this issue Apr 22, 2020
As far as I can tell the main reason we add controllers to the manager is to
ensure that we've been elected leader before we start any controllers. The
downside of this design is that it's not possible to stop individual
controllers, or remove controllers from the manager.

I believe this commit is the minimum possible change necessary to allow
controllers to be started and stopped on-demand. It allows a controller to be
created, started, and stopped without ever being added to the manager. Any
controller that is started separately from the manager must handle its own
leader election.

kubernetes-sigs#730

Signed-off-by: Nic Cope <[email protected]>
kevindelgado pushed a commit to kevindelgado/controller-runtime that referenced this issue Sep 15, 2020
As far as I can tell the main reason we add controllers to the manager is to
ensure that we've been elected leader before we start any controllers. The
downside of this design is that it's not possible to stop individual
controllers, or remove controllers from the manager.

I believe this commit is the minimum possible change necessary to allow
controllers to be started and stopped on-demand. It allows a controller to be
created, started, and stopped without ever being added to the manager. Any
controller that is started separately from the manager must handle its own
leader election.

kubernetes-sigs#730

Signed-off-by: Nic Cope <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants