Fail loudly when `renew-certificate --restart` cannot restart the control plane; include scheduler by Mukuwul · Pull Request #1670 · dapr/cli

Mukuwul · 2026-07-18T18:11:55Z

Description

dapr mtls renew-certificate -k --restart had three defects that combined to swallow every failure mode of the post-rotation control-plane restart:

The error returned by restartControlPlaneService() was discarded, and the if err != nil beneath it tested a stale variable that is provably nil at that point (a non-nil value would already have exited via logErrorAndExit). A failed restart still printed "Certificate rotation is successful!" and exited 0.
The restart list omitted statefulsets/dapr-scheduler-server, which is part of the control plane since runtime 1.14 — so the scheduler kept running with pre-rotation certificates (previously reported from the field in dapr mtls renew-certificate -k --valid-until <days> --restart did not restart statefulsets/dapr-scheduler-server #1574, closed stale).
A GetDaprNamespace() failure was printed and then ignored, producing kubectl rollout restart -n "".

Changes:

Capture and handle the restartControlPlaneService() error: print to stderr and exit 1.
Return early on namespace lookup failure instead of continuing with an empty namespace.
Add statefulsets/dapr-scheduler-server to the restart list — probed with kubectl get first so clusters on runtime < 1.14 (no scheduler) are unaffected.

Issue reference

Fixes #1669
Related: #1574 (field report of the missing scheduler restart, closed by the stale bot), #807 (mTLS rotation reliability epic)

Checklist

Please make sure you've completed the relevant tasks for this PR, out of the following list:

Code compiles correctly
Created/updated tests (restart path shells out to kubectl; no existing harness for it — happy to add coverage if maintainers point at a preferred pattern)
Extended the documentation (n/a — behavior fix)

Copilot

Pull request overview

This PR fixes dapr mtls renew-certificate -k --restart so that control-plane restart failures are no longer silently ignored, and ensures the scheduler (introduced as a control-plane component in newer runtimes) is included in the restart when present.

Changes:

Propagates and handles restartControlPlaneService() errors, printing to stderr and exiting non-zero on failure.
Fails early if GetDaprNamespace() cannot be determined (instead of continuing with an empty namespace).
Adds a conditional restart for statefulsets/dapr-scheduler-server by probing for its presence first.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Mukuwul · 2026-07-18T18:41:36Z

-				if err != nil {
-					print.FailureStatusEvent(os.Stdout, err.Error())
+				if err := restartControlPlaneService(); err != nil {
+					print.FailureStatusEvent(os.Stderr, err.Error())


Fixed in 339975f - switched to a constant format string. Side note: go vet on Go 1.26 flags ~50 pre-existing call sites across cmd/ with this same pattern, so a separate cleanup PR might be worth doing at some point.

Mukuwul · 2026-07-18T18:41:40Z

+	// The scheduler control plane service only exists for runtime 1.14 onwards,
+	// so restart it only when it is present in the cluster.
+	if _, err := utils.RunCmdAndWait("kubectl", "get", "statefulsets/dapr-scheduler-server", "-n", namespace); err == nil {
+		controlPlaneServices = append(controlPlaneServices, "statefulsets/dapr-scheduler-server")
 	}


Done in 339975f - reworded both errors to be resource-agnostic since the list now includes statefulsets.

…ol plane dapr mtls renew-certificate --restart discarded the error returned by restartControlPlaneService and checked a stale err variable that is always nil at that point, so a failed control-plane restart still reported success with exit code 0. A namespace lookup failure was also ignored, producing 'kubectl rollout restart -n ""'. The restart list additionally omitted statefulsets/dapr-scheduler-server, which is part of the control plane since runtime 1.14 (reported in dapr#1574). The scheduler is restarted only when present so older clusters are unaffected. Fixes dapr#1669 Signed-off-by: Mukul <nmukul32@gmail.com>

…errors Signed-off-by: Mukul <nmukul32@gmail.com>

Twelve call sites printed FailureStatusEvent to stdout, and dapr list -k printed its namespace deprecation warning to stdout ahead of the requested output, which corrupts 'dapr list -k -o json' (#1015). Errors-to-stderr is existing project policy (#748); these sites crept back in over time. The renew-certificate sites are intentionally left out because #1670 already moves them. Fixes #1015 Signed-off-by: Mukul <nmukul32@gmail.com> Co-authored-by: Dapr Bot <56698301+dapr-bot@users.noreply.github.com>

Copilot

⚠️ Not ready to approve

The scheduler “presence probe” currently treats any kubectl get error as “scheduler absent”, which can silently skip restarting the scheduler under RBAC/transient failures and undermines the goal of failing loudly.

Review details

Files reviewed: 1/1 changed files
Comments generated: 1
Review effort level: Low

Note

Your feedback helps us improve the quality of this feature.
Please use 👍 or 👎 to tell us whether this assessment is correct.

acroca · 2026-07-27T10:56:29Z

+	// The scheduler control plane service only exists for runtime 1.14 onwards,
+	// so restart it only when it is present in the cluster.
+	if _, err := utils.RunCmdAndWait("kubectl", "get", "statefulsets/dapr-scheduler-server", "-n", namespace); err == nil {
+		controlPlaneServices = append(controlPlaneServices, "statefulsets/dapr-scheduler-server")
 	}


I think this is a good one, can you include it?

Good catch, thanks both - done in dc167c6. The probe now uses --ignore-not-found -o name, so absence is the only case that skips the scheduler; any other kubectl failure (RBAC, transient API error) propagates and fails the restart loudly.

The presence probe treated any kubectl get failure as 'scheduler absent', so an RBAC or transient API error would silently skip the scheduler restart. Use --ignore-not-found -o name so absence is the only non-error outcome with empty output, and propagate every other probe failure so --restart fails loudly. Signed-off-by: Mukul <nmukul32@gmail.com>

Mukuwul requested review from a team as code owners July 18, 2026 18:11

Mukuwul mentioned this pull request Jul 18, 2026

dapr mtls renew-certificate --restart silently ignores restart failures and never restarts the scheduler statefulset #1669

Open

JoshVanL requested a review from Copilot July 18, 2026 18:13

Copilot started reviewing on behalf of JoshVanL July 18, 2026 18:13 View session

Copilot AI reviewed Jul 18, 2026

View reviewed changes

Mukuwul added 2 commits July 19, 2026 03:02

Address review: constant format string and resource-agnostic restart …

339975f

…errors Signed-off-by: Mukul <nmukul32@gmail.com>

Mukuwul force-pushed the fix/renew-cert-restart-errors branch from bf7c9f8 to 339975f Compare July 18, 2026 21:33

Mukuwul mentioned this pull request Jul 19, 2026

Route error and warning status messages to stderr so -o json output stays parseable #1673

Merged

3 tasks

acroca requested a review from Copilot July 27, 2026 10:42

Copilot started reviewing on behalf of acroca July 27, 2026 10:42 View session

Copilot AI reviewed Jul 27, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fail loudly when `renew-certificate --restart` cannot restart the control plane; include scheduler - #1670

Fail loudly when `renew-certificate --restart` cannot restart the control plane; include scheduler#1670
Mukuwul wants to merge 3 commits into
dapr:masterfrom
Mukuwul:fix/renew-cert-restart-errors

Mukuwul commented Jul 18, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Mukuwul Jul 18, 2026 •

edited

Loading

Uh oh!

Mukuwul Jul 18, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

acroca Jul 27, 2026

Uh oh!

Mukuwul Jul 27, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

Mukuwul commented Jul 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Issue reference

Checklist

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Mukuwul Jul 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Mukuwul Jul 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

⚠️ Not ready to approve

Uh oh!

acroca Jul 27, 2026

Choose a reason for hiding this comment

Uh oh!

Mukuwul Jul 27, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Mukuwul commented Jul 18, 2026 •

edited

Loading

Mukuwul Jul 18, 2026 •

edited

Loading

Mukuwul Jul 18, 2026 •

edited

Loading