Hi @inteon. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

/ok-to-test

Thanks @inteon , nice work!

It seems you have removed the Stop() from Controller, so how should users stop one or more specific controller?

@FillZpp what do you think about a more generic solution for canceling runnables:

// NewCancelRunnable returns a new Runnable and a function to cancel that runnable.
func NewCancelRunnable(r Runnable) (Runnable, context.CancelFunc) {
	mu := sync.Mutex{}
	cancel := (func())(nil)
	canceled := false

	return RunnableFunc(func(ctx context.Context) error {
			mu.Lock()
			ctx, cancel = context.WithCancel(ctx)
			if canceled {
				cancel()
			}
			mu.Unlock()

			return r.Start(ctx)
		}), func() {
			mu.Lock()
			defer mu.Unlock()
			canceled = true
			if cancel != nil {
				cancel()
			}
		}
}

Thanks @inteon , nice work!
It seems you have removed the Stop() from Controller, so how should users stop one or more specific controller?

@FillZpp what do you think about a more generic solution for canceling runnables:

// NewCancelRunnable returns a new Runnable and a function to cancel that runnable.
func NewCancelRunnable(r Runnable) (Runnable, context.CancelFunc) {
	mu := sync.Mutex{}
	cancel := (func())(nil)
	canceled := false

	return RunnableFunc(func(ctx context.Context) error {
			mu.Lock()
			ctx, cancel = context.WithCancel(ctx)
			if canceled {
				cancel()
			}
			mu.Unlock()

			return r.Start(ctx)
		}), func() {
			mu.Lock()
			defer mu.Unlock()
			canceled = true
			if cancel != nil {
				cancel()
			}
		}
}

Emm, I think it's alright. It may not be intuitive as the Stop method, but more generic as you said.

/label tide/merge-method-squash

fixes #1884

@vincepri are these breaking changes ok?: https://prow.k8s.io/view/gs/kubernetes-jenkins/pr-logs/pull/kubernetes-sigs_controller-runtime/2159/pull-controller-runtime-apidiff-master/1625461909174095872

@inteon Thank you for your contribution with this PR. I've noticed that this PR seems to have not been updated for a while. I believe this PR is very important for the enhancement of this project, and I also have use cases for this feature. If there is any way I can help, such as conducting code reviews or assisting with testing, I would be more than happy to contribute.

@lou-lan I'm not sure what is blocking this PR.
I resolved the remaining issues and challenges.
Looking for a final review from @vincepri @sbueringer or @FillZpp

Sorry, I don't understand this change. Contrary to what the title claims, it never stops any watch, it stops the source. How does that help? Please provide a real-world use-case.

If this is supposed to work towards allowing to stop watches by refcounting if they have eventhandlers or are otherwise used, I'd like to see a description somewhere as to how exactly this will work once all pieces are done.

Sorry, I don't understand this change. Contrary to what the title claims, it never stops any watch, it stops the source. How does that help? Please provide a real-world use-case.

If this is supposed to work towards allowing to stop watches by refcounting if they have eventhandlers or are otherwise used, I'd like to see a description somewhere as to how exactly this will work once all pieces are done.

@alvaroaleman I just added a StopWatch function to expose this functionality via the Controller API (see 6c16925).
This integration test should be a good example on how this PR can be used:
6c16925#diff-c1c23edcf40f177710028c953468fd5bf7aa0f32d67780354c609e127603bb04

/test pull-controller-runtime-test

/retest

@inteon: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

The Kubernetes project currently lacks enough contributors to adequately respond to all PRs.

This bot triages PRs according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the PR is closed

You can:

• Mark this PR as fresh with /remove-lifecycle stale
• Close this PR with /close
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

The Kubernetes project currently lacks enough active contributors to adequately respond to all PRs.

This bot triages PRs according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the PR is closed

You can:

• Mark this PR as fresh with /remove-lifecycle rotten
• Close this PR with /close
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle rotten

PR needs rebase.

I'm discontinuing this effort. Feel free to pick this up and continue this work in a new PR.