NO-JIRA: certregenerationcontroller: Improve goroutine mgmt

[tchap]

Also use typed queues, the untyped are deprecated.

This is just refactoring and handling some todos.
I took the same approach as implemented in core k8s controllers.

[openshift-ci-robot]

@tchap: This pull request explicitly references no jira issue.

Details

In response to this:

Also use typed queues, the untyped are deprecated.

This is just refactoring and handling some todos.

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 openshift-eng/jira-lifecycle-plugin repository.

[coderabbitai]

Walkthrough

The changes introduce synchronization primitives (WaitGroup) and context-aware lifecycle management to the certificate regeneration controller. A work queue type is refactored to use typed generics, and graceful shutdown mechanisms are added to ensure proper goroutine coordination and context cancellation handling across multiple controller components.

Changes

Cohort / File(s)	Summary
CA Bundle Controller Synchronization pkg/cmd/certregenerationcontroller/cabundlesyncer.go	Added sync import and WaitGroup for goroutine coordination. Replaced crash-on-panic with HandleCrashWithContext(ctx). Refactored work queue from RateLimitingInterface to TypedRateLimitingInterface[string] with NewTypedRateLimitingQueueWithConfig. Modified run loop to start worker in goroutine with WaitGroup tracking and graceful shutdown via wg.Wait().
Command Handler Lifecycle Management pkg/cmd/certregenerationcontroller/cmd.go	Added sync import and WaitGroup for tracking goroutines. Established cancellable context with defer cancellation. Wrapped configInformers, kubeAPIServerCertRotationController.Run, and caBundleController.Run in WaitGroup-managed goroutines with proper wg.Done() calls. Added wg.Wait() to ensure all goroutines terminate before returning.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

• Review the WaitGroup initialization and synchronization logic to ensure no race conditions or deadlocks exist
• Verify the context cancellation order and that deferred cancel() is called before wg.Wait() to avoid deadlocks
• Confirm the typed queue refactoring (RateLimitingInterface to TypedRateLimitingInterface[string]) maintains functional equivalence
• Check that all goroutine starts include wg.Add(1) and defer wg.Done() pairs consistently across both files
• Validate that HandleCrashWithContext usage properly handles context cancellation during panic scenarios

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[openshift-ci]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: tchap
Once this PR has been reviewed and has the lgtm label, please assign dinhxuanvu for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (1)

pkg/cmd/certregenerationcontroller/cmd.go (1)

127-127: Minor: Duplicate configInformers.Start() call.

configInformers.Start(ctx.Done()) is called at line 127 and again at line 168. While SharedInformerFactory.Start() is idempotent (safe to call multiple times), the first call could be removed since line 168 starts all informers after resources are set up.

-	configInformers.Start(ctx.Done())
-
 	wg.Add(1)
 	go func() {
 		defer wg.Done()
 		featureGateAccessor.Run(ctx)
 	}()

Also applies to: 168-168

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Cache: Disabled due to data retention organization setting

Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting

📥 Commits

Reviewing files that changed from the base of the PR and between 2c58d2f and d93c607.

📒 Files selected for processing (2)

• pkg/cmd/certregenerationcontroller/cabundlesyncer.go (4 hunks)
• pkg/cmd/certregenerationcontroller/cmd.go (3 hunks)

🧰 Additional context used

📓 Path-based instructions (1)

**

⚙️ CodeRabbit configuration file

-Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity.

Files:

• pkg/cmd/certregenerationcontroller/cabundlesyncer.go
• pkg/cmd/certregenerationcontroller/cmd.go

🔇 Additional comments (4)

pkg/cmd/certregenerationcontroller/cabundlesyncer.go (2)

40-40: LGTM! Correct migration to typed queue.

The refactor from the deprecated RateLimitingInterface to TypedRateLimitingInterface[string] is properly implemented with the corresponding constructor and config changes.

Also applies to: 52-57

---

82-105: LGTM! Well-structured goroutine lifecycle management.

The shutdown sequence is correct:

1. c.queue.ShutDown() signals the worker to exit (causes Get() to return quit=true)
2. wg.Wait() ensures the worker completes before returning

The HandleCrashWithContext(ctx) is a good improvement for context-aware crash handling.

pkg/cmd/certregenerationcontroller/cmd.go (2)

121-133: LGTM! Correct defer ordering for graceful shutdown.

The pattern is correct: defer cancel() after defer wg.Wait() ensures cancel executes first (LIFO), signaling goroutines to stop before waiting. The comment at line 123 accurately documents this requirement.

---

170-180: LGTM! Proper WaitGroup tracking for controller goroutines.

Both controller goroutines are correctly managed with wg.Add(1) before launch and defer wg.Done() inside, ensuring graceful shutdown coordination.

[openshift-ci]

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

Test name	Commit	Details	Required	Rerun command
ci/prow/e2e-gcp-operator-serial-ote	d93c607	link	false	/test e2e-gcp-operator-serial-ote

Full PR test history. Your PR dashboard.

Details

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