Skip to content

chore: preserve legacy config-manager during controller upgrade#10081

Merged
leon-ape merged 2 commits intomainfrom
support/compatible-config-manager
Mar 19, 2026
Merged

chore: preserve legacy config-manager during controller upgrade#10081
leon-ape merged 2 commits intomainfrom
support/compatible-config-manager

Conversation

@leon-ape
Copy link
Copy Markdown
Contributor

No description provided.

@leon-ape leon-ape added this to the Release 1.2.0 milestone Mar 12, 2026
@leon-ape leon-ape requested a review from kizuna-lek March 12, 2026 07:23
@leon-ape leon-ape added the nopick Not auto cherry-pick when PR merged label Mar 12, 2026
@github-actions github-actions bot added the size/XL Denotes a PR that changes 500-999 lines. label Mar 12, 2026
@apecloud-bot
Copy link
Copy Markdown
Collaborator

Auto Cherry-pick Instructions

Usage:
  - /nopick: Not auto cherry-pick when PR merged.
  - /pick: release-x.x [release-x.x]: Auto cherry-pick to the specified branch when PR merged.

Example:
  - /nopick
  - /pick release-1.1

@leon-ape leon-ape force-pushed the support/compatible-config-manager branch 2 times, most recently from d498c04 to 485f1c4 Compare March 12, 2026 08:21
@leon-ape leon-ape marked this pull request as ready for review March 12, 2026 08:22
@leon-ape leon-ape requested a review from a team as a code owner March 12, 2026 08:22
@codecov
Copy link
Copy Markdown

codecov bot commented Mar 12, 2026

Codecov Report

❌ Patch coverage is 68.40000% with 79 lines in your changes missing coverage. Please review.
✅ Project coverage is 51.69%. Comparing base (badbaba) to head (ffb1949).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
...s/apps/component/transformer_component_workload.go 68.54% 67 Missing and 11 partials ⚠️
pkg/constant/annotations.go 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #10081      +/-   ##
==========================================
+ Coverage   51.51%   51.69%   +0.18%     
==========================================
  Files         525      525              
  Lines       58146    58393     +247     
==========================================
+ Hits        29955    30189     +234     
- Misses      25232    25261      +29     
+ Partials     2959     2943      -16     
Flag Coverage Δ
unittests 51.69% <68.40%> (+0.18%) ⬆️

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.

@apecloud-bot apecloud-bot added the approved PR Approved Test label Mar 13, 2026
@leon-ape
Copy link
Copy Markdown
Contributor Author

/approve

@leon-ape leon-ape force-pushed the support/compatible-config-manager branch from 485f1c4 to 3463395 Compare March 16, 2026 08:37
@apecloud-bot apecloud-bot removed the approved PR Approved Test label Mar 16, 2026
@leon-ape leon-ape force-pushed the support/compatible-config-manager branch from 3463395 to e0e7f84 Compare March 16, 2026 09:01
@github-actions github-actions bot added size/XXL Denotes a PR that changes 1000+ lines. and removed size/XL Denotes a PR that changes 500-999 lines. labels Mar 16, 2026
@leon-ape leon-ape force-pushed the support/compatible-config-manager branch from e0e7f84 to 77228e1 Compare March 16, 2026 11:51
// Clean up only when the configured policy will recreate Pods. Otherwise we keep the live template aligned
// with old Pods that still carry the legacy sidecar and avoid creating an extra rollout only for cleanup.
if hasPodUpgradeTemplateChanges(oldTemplate, newTemplate) {
return desiredITS.Spec.PodUpgradePolicy == appsv1.ReCreatePodUpdatePolicyType
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 about PreferInPlace policy? If ITS controller decides to recreate pod, then we should also clean up config manager.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes. PreferInPlace is only a preference, not a guarantee.

The cleanup logic has been updated to follow whether this rollout would actually recreate Pods, rather than matching the policy name literally. So if ITS still ends up recreating Pods under PreferInPlace, legacy config-manager will be cleaned up in the same rollout; otherwise it will be preserved.

if comp == nil || comp.Annotations == nil {
return legacyConfigManagerPolicyKeep
}
value, ok := comp.Annotations[constant.LegacyConfigManagerRequiredAnnotationKey]
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 annotaion will be set by user?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It is not intended to be user-managed. It is written by the parameters controller on the Cluster and then inherited by the Component as an internal compatibility marker.

@apecloud-bot apecloud-bot added the approved PR Approved Test label Mar 19, 2026
@leon-ape leon-ape merged commit 6c6da58 into main Mar 19, 2026
43 checks passed
@leon-ape leon-ape deleted the support/compatible-config-manager branch March 19, 2026 11:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved PR Approved Test nopick Not auto cherry-pick when PR merged size/XXL Denotes a PR that changes 1000+ lines.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants