Skip to content

Comments

Fix: prevent panic in YurtAppSet status for StatefulSet workloads#2503

Open
Yashika0724 wants to merge 1 commit intoopenyurtio:masterfrom
Yashika0724:fix-yurtappset-statefulset-panic
Open

Fix: prevent panic in YurtAppSet status for StatefulSet workloads#2503
Yashika0724 wants to merge 1 commit intoopenyurtio:masterfrom
Yashika0724:fix-yurtappset-statefulset-panic

Conversation

@Yashika0724
Copy link

Summary

This PR fixes a runtime panic in the YurtAppSet controller that occurs when calculating status
for YurtAppSets configured with a StatefulSet workload template.

The controller previously assumed all workloads were Deployments and performed an unsafe type
assertion, which caused a panic when workloads were actually StatefulSets. As a result,
StatefulSet-based YurtAppSets would crash the controller during reconciliation.

This fix updates the status calculation logic to safely handle both Deployment and StatefulSet
workloads.


Root Cause

In conciliateYurtAppSetStatus, workloads are iterated as []metav1.Object and cast like this:

go workloadObj := workload.(*appsv1.Deployment)

However, the workload manager returns different types depending on the workload template:
• Deployment template → *appsv1.Deployment
• StatefulSet template → *appsv1.StatefulSet

When a YurtAppSet uses statefulSetTemplate, the controller attempts to cast a
*appsv1.StatefulSet to *appsv1.Deployment, resulting in:

interface conversion: metav1.Object is *v1.StatefulSet, not *v1.Deployment

This causes the YurtAppSet controller to crash during reconciliation and enter a crash loop.

Steps to Reproduce

1.	Deploy OpenYurt and ensure the YurtAppSet controller is running
2.	Create a YurtAppSet that uses a StatefulSet workload template
3.	Create a NodePool with at least one node so workloads get created
4.	Wait for the controller to reconcile and calculate YurtAppSet status
5.	The controller panics while processing workload status and enters a crash loop

Fix Applied

The unsafe type assertion was replaced with a type switch that supports both workload types:
• *appsv1.Deployment
• *appsv1.StatefulSet

For each workload, the controller now correctly extracts:
• replicas
• ready replicas
• updated replicas
• workload revision hash

Unknown workload types are skipped safely with a warning log instead of crashing the controller.

This preserves existing behavior for Deployments and correctly enables StatefulSet support.


Impact

•	Prevents YurtAppSet controller crashes
•	Enables StatefulSet-based YurtAppSets as documented in the API
•	Correct status reporting for all supported workload types
•	No behavior change for Deployment-based YurtAppSets
•	Improves reliability for stateful edge workloads

Testing

•	Logic verified for both Deployment and StatefulSet workload managers
•	No API or behavior changes introduced
•	CI will validate controller integration behavior

Important: This + Your Previous Fix = STRONG Profile

You now have:

  1. Node lifecycle controller crash fix (zone label race)
  2. YurtAppSet controller crash fix (StatefulSet status)

These are core controller reliability fixes — not docs, not trivial bugs.

For CNCF LFX Mentorship, this is exactly the kind of contribution mentors look for.


If you want, next I can help you with:

  • PR review request comment (for this PR)
  • Message to maintainer mentioning your interest in contributing
  • How to mention these PRs in your LFX application answers
  • What to pick as your next OpenYurt issue

Signed-off-by: Yashika <ssyashika1311@gmail.com>
@Yashika0724 Yashika0724 requested a review from a team as a code owner January 21, 2026 10:21
@sonarqubecloud
Copy link

@Yashika0724
Copy link
Author

Hello @zyjhtangtang , I’ve submitted a fix for a controller panic related to StatefulSet-based
YurtAppSets and would appreciate a review when convenient. Happy to update the PR based on
any feedback. Thank you!

@codecov
Copy link

codecov bot commented Jan 26, 2026

Codecov Report

❌ Patch coverage is 50.00000% with 9 lines in your changes missing coverage. Please review.
✅ Project coverage is 44.08%. Comparing base (ed4eb0a) to head (fb9f53f).

Files with missing lines Patch % Lines
...r/controller/yurtappset/yurt_app_set_controller.go 50.00% 8 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2503      +/-   ##
==========================================
- Coverage   44.09%   44.08%   -0.02%     
==========================================
  Files         399      399              
  Lines       26554    26568      +14     
==========================================
+ Hits        11710    11713       +3     
- Misses      13782    13792      +10     
- Partials     1062     1063       +1     
Flag Coverage Δ
unittests 44.08% <50.00%> (-0.02%) ⬇️

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.

updatedReplicas = w.Status.UpdatedReplicas
hash = workloadmanager.GetWorkloadHash(w)
default:
klog.Warningf("YurtAppSet[%s/%s] unknown workload type: %T", yas.GetNamespace(), yas.GetName(), workload)
Copy link
Member

Choose a reason for hiding this comment

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

Would it be better to include the workload's name and namespace in the logs?

Copy link
Author

Choose a reason for hiding this comment

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

Would it be better to include the workload's name and namespace in the logs?

That makes sense, including the workload name and namespace would improve debugging and log clarity. I’ll update the warning log to include those fields.
Thanks for the suggestion!

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