Skip to content

fix(controller): use TLS for upgrade hooks - #886

Merged
furykerry merged 3 commits into
openkruise:masterfrom
RedZapdos123:fix-controller-runtime-tls
Aug 31, 2026
Merged

fix(controller): use TLS for upgrade hooks#886
furykerry merged 3 commits into
openkruise:masterfrom
RedZapdos123:fix-controller-runtime-tls

Conversation

@RedZapdos123

@RedZapdos123 RedZapdos123 commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

Description:

Thread runtime TLS through upgrade lifecycle hook execution.

Before this change, lifecycle hooks called RunCommandWithRuntime without the transport resolved by runtime.TransportOptionsFor, and they still gated execution on plaintext GetRuntimeURL. As a result, a sandbox advertising the runtime TLS capability could not execute upgrade hooks over the TLS path and failed before the TLS transport had a chance to resolve.

This PR keeps the legacy plaintext behavior for non-TLS sandboxes, but binds the controller's existing RuntimeTLSBundle into lifecycle hook execution and adds regression tests for the missing-bundle and configured-bundle paths.

Validation Tests run:

  • go test ./pkg/controller/sandbox/core -run 'TestExecuteLifecycleHook|TestNewLifecycleHookFunc_UsesRuntimeTLSBundle' -count=1
  • go test ./pkg/controller/sandbox/core -count=1
  • go test ./pkg/controller/sandbox/... -count=1

Notes:

@kruise-bot
kruise-bot requested review from furykerry and zmberg August 26, 2026 19:59
@kruise-bot

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign zmberg for approval by writing /assign @zmberg in a comment. For more information see:The Kubernetes 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:

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

@kruise-bot

Copy link
Copy Markdown

Welcome @RedZapdos123! It looks like this is your first PR to openkruise/agents 🎉

@codecov

codecov Bot commented Aug 26, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 82.39%. Comparing base (3d45fb4) to head (f127940).
⚠️ Report is 8 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #886      +/-   ##
==========================================
- Coverage   82.48%   82.39%   -0.09%     
==========================================
  Files         248      254       +6     
  Lines       20416    21465    +1049     
==========================================
+ Hits        16840    17686     +846     
- Misses       2927     3080     +153     
- Partials      649      699      +50     
Flag Coverage Δ
unittests 82.39% <100.00%> (-0.09%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Thread runtime TLS through lifecycle hook execution so TLS-stamped sandboxes do not fall back to plaintext runtime routing during upgrade.

Signed-off-by: Mridankan Mandal <xerontitan90@gmail.com>
@RedZapdos123
RedZapdos123 force-pushed the fix-controller-runtime-tls branch from 7845b92 to be2f81f Compare August 27, 2026 10:29
@kruise-bot kruise-bot added size/M and removed size/L labels Aug 27, 2026
@RedZapdos123 RedZapdos123 changed the title fix(controller): honor runtime TLS in hooks fix(controller): use TLS for upgrade hooks Aug 27, 2026
The variable was left over from an abandoned test seam and is never
referenced; the unused linter rejects it. Lifecycle hooks call
agentsruntime.RunCommandWithRuntime directly.

Signed-off-by: 守辰 <shouchen.zz@alibaba-inc.com>
…diness

ExecuteLifecycleHook has no production callers left after the TLS
bundle threading, so drop the exported wrapper and drive all call
sites (including tests) through NewLifecycleHookFunc(nil).

Add a symmetric readiness gate for the TLS transport: when transport
options resolve, the hook now fails fast with a Pod-IP readiness
error instead of surfacing the misleading plaintext 'runtime url not
found' message from the runtime client.

Signed-off-by: 守辰 <shouchen.zz@alibaba-inc.com>
@kruise-bot kruise-bot added size/L and removed size/M labels Aug 28, 2026
@furykerry
furykerry merged commit f14aedf into openkruise:master Aug 31, 2026
29 of 30 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants