Skip to content

[WIP] Feat/label driven yurtnode conversion#2533

Draft
Vacant-lot07734 wants to merge 19 commits intoopenyurtio:masterfrom
Vacant-lot07734:feat/label-driven-yurtnode-conversion
Draft

[WIP] Feat/label driven yurtnode conversion#2533
Vacant-lot07734 wants to merge 19 commits intoopenyurtio:masterfrom
Vacant-lot07734:feat/label-driven-yurtnode-conversion

Conversation

@Vacant-lot07734
Copy link
Copy Markdown
Contributor

What type of PR is this?

Uncomment only one /kind <> line, hit enter to put that in a new line, and remove leading whitespace from that line:
/kind feature

What this PR does / why we need it:

#2530
This is still a draft PR. It already contains the main production changes for the new label-driven YurtHub conversion flow, while the e2e migration is still in progress in the same PR.

This PR currently covers seven key parts:

  1. Extract reusable host-side YurtHub lifecycle helpers for binary installation, systemd management, health checks, and cleanup.
  2. Migrate node-servant convert/revert from the old static-pod-based path to the new systemd + host binary path.
  3. Refactor the node-servant Job contract so that convert and revert can be driven by the controller through one unified Job model.
  4. Add YurtNodeConversionController to yurt-manager to watch Node label changes and conversion Jobs, then orchestrate convert/revert rounds.
  5. Restart recreatable workload Pods after successful conversion by deleting Pods on the target node and letting upper-level controllers recreate them.
  6. Protect openyurt.io/is-edge-worker as a controller-managed label and only allow apps.openyurt.io/nodepool to be added or removed, not modified in place.
  7. Keep the e2e migration and default release wiring as the remaining draft work; the old GitHub e2e init flow is still being replaced by the new label-driven workflow.

Special notes for your reviewer:

Does this PR introduce a user-facing change?

- action required: node conversion is now driven by adding or removing the `apps.openyurt.io/nodepool` label.
- In-place modification of the nodepool label is not supported. The `openyurt.io/is-edge-worker` label is now controller-managed and should not be modified manually. 
- During convert or revert, the target node will be cordoned, and recreatable workload Pods on that node may be deleted and rebuilt.
- Do not trigger this workflow on production nodes without planning for workload disruption.

other Note

@Vacant-lot07734 Vacant-lot07734 requested a review from a team as a code owner March 17, 2026 10:13
@Vacant-lot07734 Vacant-lot07734 marked this pull request as draft March 17, 2026 10:13
@codecov
Copy link
Copy Markdown

codecov bot commented Mar 17, 2026

Codecov Report

❌ Patch coverage is 64.81994% with 381 lines in your changes missing coverage. Please review.
✅ Project coverage is 45.24%. Comparing base (7589921) to head (12d4cdb).

Files with missing lines Patch % Lines
...tnodeconversion/yurt_node_conversion_controller.go 70.08% 97 Missing and 37 partials ⚠️
pkg/node-servant/components/runtime.go 69.54% 55 Missing and 12 partials ⚠️
pkg/yurtadm/util/yurthub/yurthub.go 41.44% 50 Missing and 15 partials ⚠️
...anager/app/options/yurtnodeconversioncontroller.go 0.00% 21 Missing ⚠️
pkg/node-servant/convert/convert.go 48.14% 13 Missing and 1 partial ⚠️
pkg/node-servant/convert/options.go 57.14% 7 Missing and 5 partials ⚠️
pkg/node-servant/components/workload.go 79.16% 6 Missing and 4 partials ⚠️
pkg/node-servant/revert/revert.go 28.57% 8 Missing and 2 partials ⚠️
pkg/yurtadm/util/yurthub/host_config.go 77.77% 5 Missing and 5 partials ⚠️
pkg/node-servant/revert/options.go 0.00% 9 Missing ⚠️
... and 8 more
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2533      +/-   ##
==========================================
+ Coverage   43.95%   45.24%   +1.28%     
==========================================
  Files         400      404       +4     
  Lines       26541    27413     +872     
==========================================
+ Hits        11666    12402     +736     
- Misses      13809    13850      +41     
- Partials     1066     1161      +95     
Flag Coverage Δ
unittests 45.24% <64.81%> (+1.28%) ⬆️

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

☔ View full report in Codecov by Sentry.
📢 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.

if o.ConcurrentYurtNodeConversionWorkers <= 0 {
errs = append(errs, fmt.Errorf("concurrent-yurtnodeconversion-workers(%d) is invalid, should greater than 0", o.ConcurrentYurtNodeConversionWorkers))
}
if len(o.YurthubBinaryURL) == 0 && len(o.YurthubVersion) == 0 {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can YurthubBinaryURL and YurthubVersion be configured simultaneously?

kubeadmConfPaths: strings.Join(components.GetDefaultKubeadmConfPath(), ","),
openyurtDir: constants.OpenyurtDir,
kubeadmConfPaths: strings.Join(components.GetDefaultKubeadmConfPath(), ","),
namespace: constants.YurthubNamespace,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the purpose of a namespace?

"convert",
fmt.Sprintf("--%s=%s", constants.NodeName, nodeName),
fmt.Sprintf("--%s=%s", constants.Namespace, valueOrDefault(tmplCtx["namespace"], DefaultConversionJobNamespace)),
fmt.Sprintf("--%s=%s", workingModeFlag, valueOrDefault(tmplCtx["workingMode"], defaultWorkingMode)),
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it still support working mode?

"k8s.io/klog/v2"
"sigs.k8s.io/controller-runtime/pkg/client"
)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can directly use crictl to delete the containers on the node.

// for inspection instead of recreating the Job indefinitely
if isJobFailed(job) {
klog.Info(Format("node(%s) observed failed %s job %s", node.Name, currentJobAction, job.Name))
if err := r.ensureNodeUnschedulable(ctx, node.Name, false); err != nil {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs to be considered: after a failure, scheduling may not still be possible.

@Vacant-lot07734 Vacant-lot07734 force-pushed the feat/label-driven-yurtnode-conversion branch 5 times, most recently from df0fe21 to d19f131 Compare March 29, 2026 07:53
…hub conversion and revert"

This reverts commit 94cef5a.
@Vacant-lot07734 Vacant-lot07734 force-pushed the feat/label-driven-yurtnode-conversion branch from cab2339 to 12d4cdb Compare April 5, 2026 05:37
@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud bot commented Apr 5, 2026

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants