Skip to content

Commit 820a8dc

Browse files
ColdsteelRailColdsteelRail
committed
refactor ApplyDecoration
1 parent 3587a4b commit 820a8dc

File tree

6 files changed

+83
-80
lines changed

6 files changed

+83
-80
lines changed

‎xset/api/xset_controller_types.go‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -105,12 +105,12 @@ type DecorationAdapter interface {
105105
WatchDecoration(c controller.Controller) error
106106
// GetDecorationGroupVersionKind returns decoration gvk.
107107
GetDecorationGroupVersionKind() metav1.GroupVersionKind
108-
// GetDecorationPatcherFromTarget returns patcher for decoration from target.
109-
GetDecorationPatcherFromTarget(ctx context.Context, target client.Object) func(client.Object) error
110-
// GetDecorationPatcherFromRevisions returns patcher for decoration from revisions.
111-
GetDecorationPatcherFromRevisions(ctx context.Context, revision ...string) func(client.Object) error
112-
// GetDecorationRevisionFromTarget returns decoration revision on target.
113-
GetDecorationRevisionFromTarget(ctx context.Context, target client.Object) (string, error)
114-
// IsDecorationChanged returns true if decoration on target is changed.
115-
IsDecorationChanged(ctx context.Context, target client.Object) (bool, error)
108+
// GetTargetCurrentDecorationRevisions returns decoration revision on target.
109+
GetTargetCurrentDecorationRevisions(ctx context.Context, c client.Client, target client.Object) (string, error)
110+
// GetTargetUpdatedDecorationRevisions returns decoration revision on target.
111+
GetTargetUpdatedDecorationRevisions(ctx context.Context, c client.Client, target client.Object) (string, error)
112+
// GetDecorationPatcherByRevisions returns patcher for decoration from revisions.
113+
GetDecorationPatcherByRevisions(ctx context.Context, c client.Client, target client.Object, revision string) (func(client.Object) error, error)
114+
// IsTargetDecorationChanged returns true if decoration on target is changed.
115+
IsTargetDecorationChanged(currentRevision, updatedRevision string) (bool, error)
116116
}

‎xset/synccontrols/sync_control.go‎

Lines changed: 34 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -220,6 +220,26 @@ func (r *RealSyncControl) SyncTargets(ctx context.Context, instance api.XSetObje
220220
}
221221
}
222222

223+
// sync decoration revisions
224+
var decorationInfo DecorationInfo
225+
if decorationAdapter, enabled := r.xsetController.(api.DecorationAdapter); enabled {
226+
if decorationInfo.DecorationCurrentRevisions, err = decorationAdapter.GetTargetCurrentDecorationRevisions(ctx, r.Client, target); err != nil {
227+
return false, err
228+
}
229+
if decorationInfo.DecorationUpdatedRevisions, err = decorationAdapter.GetTargetUpdatedDecorationRevisions(ctx, r.Client, target); err != nil {
230+
return false, err
231+
}
232+
if decorationInfo.DecorationChanged, err = decorationAdapter.IsTargetDecorationChanged(decorationInfo.DecorationCurrentRevisions, decorationInfo.DecorationUpdatedRevisions); err != nil {
233+
return false, err
234+
}
235+
}
236+
237+
// sync target ops priority
238+
var opsPriority *api.OpsPriority
239+
if opsPriority, err = r.xsetController.GetXOpsPriority(ctx, r.Client, target); err != nil {
240+
return false, err
241+
}
242+
223243
targetWrappers = append(targetWrappers, &TargetWrapper{
224244
Object: target,
225245
ID: id,
@@ -231,6 +251,9 @@ func (r *RealSyncControl) SyncTargets(ctx context.Context, instance api.XSetObje
231251

232252
IsDuringScaleInOps: opslifecycle.IsDuringOps(r.updateConfig.XsetLabelAnnoMgr, r.scaleInLifecycleAdapter, target),
233253
IsDuringUpdateOps: opslifecycle.IsDuringOps(r.updateConfig.XsetLabelAnnoMgr, r.updateLifecycleAdapter, target),
254+
255+
DecorationInfo: decorationInfo,
256+
OpsPriority: opsPriority,
234257
})
235258

236259
if id >= 0 {
@@ -395,7 +418,7 @@ func (r *RealSyncControl) Replace(ctx context.Context, xsetObject api.XSetObject
395418
syncContext.replacingMap = classifyTargetReplacingMapping(r.xsetLabelAnnoMgr, syncContext.activeTargets)
396419
}()
397420

398-
needReplaceOriginTargets, needCleanLabelTargets, targetsNeedCleanLabels, needDeleteTargets := r.dealReplaceTargets(ctx, syncContext.FilteredTarget)
421+
needReplaceOriginTargets, needCleanLabelTargets, targetsNeedCleanLabels, needDeleteTargets := r.dealReplaceTargets(ctx, syncContext.TargetWrappers)
399422

400423
// delete origin targets for replace
401424
err = r.BatchDeleteTargetsByLabel(ctx, r.xControl, needDeleteTargets)
@@ -516,18 +539,18 @@ func (r *RealSyncControl) Scale(ctx context.Context, xsetObject api.XSetObject,
516539
if decorationAdapter, ok := r.xsetController.(api.DecorationAdapter); ok {
517540
revisionsInfo, ok := r.resourceContextControl.Get(availableIDContext, api.EnumTargetDecorationRevisionKey)
518541
if !ok {
519-
needUpdateContext.Store(true)
520-
revisions, err := decorationAdapter.GetDecorationRevisionFromTarget(ctx, object)
521-
if err != nil {
542+
// get updated decoration revisions from target and write to resource context
543+
if revisionsInfo, err = decorationAdapter.GetTargetUpdatedDecorationRevisions(ctx, r.Client, object); err != nil {
522544
return err
523545
}
524-
r.resourceContextControl.Put(availableIDContext, api.EnumTargetDecorationRevisionKey, revisions)
525-
patcherFn := decorationAdapter.GetDecorationPatcherFromTarget(ctx, object)
526-
return patcherFn(object)
546+
r.resourceContextControl.Put(availableIDContext, api.EnumTargetDecorationRevisionKey, revisionsInfo)
547+
needUpdateContext.Store(true)
548+
}
549+
// get patcher from decoration revisions and patch target
550+
if fn, err := decorationAdapter.GetDecorationPatcherByRevisions(ctx, r.Client, object, revisionsInfo); err != nil {
551+
return err
527552
} else {
528-
// upgrade by recreate target case
529-
patcherFn := decorationAdapter.GetDecorationPatcherFromRevisions(ctx, revisionsInfo)
530-
return patcherFn(object)
553+
return fn(object)
531554
}
532555
}
533556
return nil
@@ -571,10 +594,6 @@ func (r *RealSyncControl) Scale(ctx context.Context, xsetObject api.XSetObject,
571594
}
572595

573596
if diff <= 0 {
574-
// get targets ops priority
575-
if err := r.getTargetsOpsPriority(ctx, r.Client, syncContext.activeTargets); err != nil {
576-
return false, recordedRequeueAfter, err
577-
}
578597
// chose the targets to scale in
579598
targetsToScaleIn := r.getTargetsToDelete(xsetObject, syncContext.activeTargets, syncContext.replacingMap, diff*-1)
580599
// filter out Targets need to trigger TargetOpsLifecycle
@@ -725,11 +744,6 @@ func (r *RealSyncControl) Update(ctx context.Context, xsetObject api.XSetObject,
725744
var err error
726745
var recordedRequeueAfter *time.Duration
727746

728-
// 0. get targets ops priority
729-
if err := r.getTargetsOpsPriority(ctx, r.Client, syncContext.TargetWrappers); err != nil {
730-
return false, recordedRequeueAfter, err
731-
}
732-
733747
// 1. scan and analysis targets update info for active targets and PlaceHolder targets
734748
targetUpdateInfos, err := r.attachTargetUpdateInfo(ctx, xsetObject, syncContext)
735749
if err != nil {
@@ -746,7 +760,7 @@ func (r *RealSyncControl) Update(ctx context.Context, xsetObject api.XSetObject,
746760
// 3. filter already updated revision,
747761
for i, targetInfo := range targetToUpdate {
748762
// TODO check decoration and pvc template changed
749-
if targetInfo.IsUpdatedRevision && !targetInfo.PvcTmpHashChanged {
763+
if targetInfo.IsUpdatedRevision && !targetInfo.PvcTmpHashChanged && !targetInfo.DecorationChanged {
750764
continue
751765
}
752766

@@ -1007,22 +1021,6 @@ func (r *RealSyncControl) reclaimOwnedIDs(
10071021
return nil
10081022
}
10091023

1010-
// getTargetsOpsPriority try to set targets' ops priority
1011-
func (r *RealSyncControl) getTargetsOpsPriority(ctx context.Context, c client.Client, targets []*TargetWrapper) error {
1012-
_, err := controllerutils.SlowStartBatch(len(targets), controllerutils.SlowStartInitialBatchSize, true, func(i int, _ error) error {
1013-
if targets[i].PlaceHolder || targets[i].Object == nil || targets[i].OpsPriority != nil {
1014-
return nil
1015-
}
1016-
var iErr error
1017-
targets[i].OpsPriority, iErr = r.xsetController.GetXOpsPriority(ctx, c, targets[i].Object)
1018-
if iErr != nil {
1019-
return fmt.Errorf("failed to get target %s/%s ops priority: %w", targets[i].Object.GetNamespace(), targets[i].Object.GetName(), iErr)
1020-
}
1021-
return nil
1022-
})
1023-
return err
1024-
}
1025-
10261024
// FilterOutActiveTargetWrappers filter out non placeholder targets
10271025
func FilterOutActiveTargetWrappers(targets []*TargetWrapper) []*TargetWrapper {
10281026
var filteredTargetWrappers []*TargetWrapper

‎xset/synccontrols/types.go‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,9 +63,20 @@ type TargetWrapper struct {
6363
IsDuringScaleInOps bool
6464
IsDuringUpdateOps bool
6565

66+
DecorationInfo
67+
6668
OpsPriority *api.OpsPriority
6769
}
6870

71+
type DecorationInfo struct {
72+
// indicate if the decoration changed
73+
DecorationChanged bool
74+
// updated revisions of decoration
75+
DecorationUpdatedRevisions string
76+
// current revisions of decoration
77+
DecorationCurrentRevisions string
78+
}
79+
6980
type TargetUpdateInfo struct {
7081
*TargetWrapper
7182

@@ -81,7 +92,6 @@ type TargetUpdateInfo struct {
8192
// carry the desired update revision
8293
UpdateRevision *appsv1.ControllerRevision
8394

84-
DecorationChanged bool
8595
SubResourcesChanged
8696

8797
// indicates operate is allowed for TargetOpsLifecycle.

‎xset/synccontrols/x_replace.go‎

Lines changed: 18 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -114,14 +114,15 @@ func (r *RealSyncControl) replaceOriginTargets(
114114
ctx context.Context,
115115
instance api.XSetObject,
116116
syncContext *SyncContext,
117-
needReplaceOriginTargets []client.Object,
117+
needReplaceOriginTargets []*TargetWrapper,
118118
ownedIDs map[int]*api.ContextDetail,
119119
availableContexts []*api.ContextDetail,
120120
) (int, error) {
121121
logger := logr.FromContext(ctx)
122122
mapNewToOriginTargetContext := r.mapReplaceNewToOriginTargetContext(ownedIDs)
123123
successCount, err := controllerutils.SlowStartBatch(len(needReplaceOriginTargets), controllerutils.SlowStartInitialBatchSize, false, func(i int, _ error) error {
124-
originTarget := needReplaceOriginTargets[i]
124+
originWrapper := needReplaceOriginTargets[i]
125+
originTarget := needReplaceOriginTargets[i].Object
125126
originTargetId, _ := GetInstanceID(r.xsetLabelAnnoMgr, originTarget)
126127

127128
if ownedIDs[originTargetId] == nil {
@@ -136,8 +137,12 @@ func (r *RealSyncControl) replaceOriginTargets(
136137
r.xsetController.GetXSetTemplatePatcher(instance),
137138
func(object client.Object) error {
138139
if decorationAdapter, ok := r.xsetController.(api.DecorationAdapter); ok {
139-
patcherFn := decorationAdapter.GetDecorationPatcherFromTarget(ctx, originTarget)
140-
return patcherFn(object)
140+
// get current decoration patcher from origin target, and patch new target
141+
if fn, err := decorationAdapter.GetDecorationPatcherByRevisions(ctx, r.Client, originTarget, originWrapper.DecorationUpdatedRevisions); err != nil {
142+
return err
143+
} else {
144+
return fn(object)
145+
}
141146
}
142147
return nil
143148
},
@@ -214,14 +219,15 @@ func (r *RealSyncControl) replaceOriginTargets(
214219
return successCount, err
215220
}
216221

217-
func (r *RealSyncControl) dealReplaceTargets(ctx context.Context, targets []client.Object) (
218-
needReplaceTargets, needCleanLabelTargets []client.Object, targetNeedCleanLabels [][]string, needDeleteTargets []client.Object,
222+
func (r *RealSyncControl) dealReplaceTargets(ctx context.Context, targets []*TargetWrapper) (
223+
needReplaceTargets []*TargetWrapper, needCleanLabelTargets []client.Object, targetNeedCleanLabels [][]string, needDeleteTargets []client.Object,
219224
) {
220225
logger := logr.FromContext(ctx)
221226
targetInstanceIdMap := make(map[string]client.Object)
222-
targetNameMap := make(map[string]client.Object)
227+
targetNameMap := make(map[string]*TargetWrapper)
228+
filteredTargets := FilterOutActiveTargetWrappers(targets)
223229

224-
for _, target := range targets {
230+
for _, target := range filteredTargets {
225231
targetLabels := target.GetLabels()
226232

227233
if instanceId, ok := r.xsetLabelAnnoMgr.Get(targetLabels, api.XInstanceIdLabelKey); ok {
@@ -231,7 +237,7 @@ func (r *RealSyncControl) dealReplaceTargets(ctx context.Context, targets []clie
231237
}
232238

233239
// deal need replace targets
234-
for _, target := range targets {
240+
for _, target := range filteredTargets {
235241
targetLabels := target.GetLabels()
236242

237243
// no replace indication label
@@ -262,7 +268,8 @@ func (r *RealSyncControl) dealReplaceTargets(ctx context.Context, targets []clie
262268
needReplaceTargets = append(needReplaceTargets, target)
263269
}
264270

265-
for _, target := range targets {
271+
for _, wrapper := range filteredTargets {
272+
target := wrapper.Object
266273
targetLabels := target.GetLabels()
267274
_, replaceByUpdate := r.xsetLabelAnnoMgr.Get(targetLabels, api.XReplaceByReplaceUpdateLabelKey)
268275
var needCleanLabels []string
@@ -280,7 +287,7 @@ func (r *RealSyncControl) dealReplaceTargets(ctx context.Context, targets []clie
280287
} else if !replaceByUpdate {
281288
// not replace update, delete origin target when new created target is service available
282289
if r.xsetController.CheckAvailable(target) {
283-
needDeleteTargets = append(needDeleteTargets, originTarget)
290+
needDeleteTargets = append(needDeleteTargets, originTarget.Object)
284291
}
285292
}
286293
}

‎xset/synccontrols/x_update.go‎

Lines changed: 4 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -53,15 +53,6 @@ func (r *RealSyncControl) attachTargetUpdateInfo(ctx context.Context, xsetObject
5353
TargetWrapper: syncContext.TargetWrappers[i],
5454
}
5555

56-
// check for decoration changed
57-
if decoration, ok := r.xsetController.(api.DecorationAdapter); ok {
58-
var err error
59-
updateInfo.DecorationChanged, err = decoration.IsDecorationChanged(ctx, updateInfo.TargetWrapper.Object)
60-
if err != nil {
61-
return nil, err
62-
}
63-
}
64-
6556
updateInfo.UpdateRevision = syncContext.UpdatedRevision
6657
// decide this target current revision, or nil if not indicated
6758
if target.GetLabels() != nil {
@@ -427,13 +418,10 @@ func (u *GenericTargetUpdater) FilterAllowOpsTargets(ctx context.Context, candid
427418
}
428419

429420
// add decoration revision to target context
430-
if decorationAdapter, ok := u.XsetController.(api.DecorationAdapter); ok && targetInfo.DecorationChanged {
431-
decorationRevision, err := decorationAdapter.GetDecorationRevisionFromTarget(ctx, targetInfo.Object)
432-
if err != nil {
433-
return recordedRequeueAfter, err
434-
}
435-
if val, ok := u.ResourceContextControl.Get(ownedIDs[targetInfo.ID], api.EnumTargetDecorationRevisionKey); !ok || val != decorationRevision {
436-
u.ResourceContextControl.Put(ownedIDs[targetInfo.ID], api.EnumTargetDecorationRevisionKey, decorationRevision)
421+
if targetInfo.DecorationChanged {
422+
if val, ok := u.ResourceContextControl.Get(ownedIDs[targetInfo.ID], api.EnumTargetDecorationRevisionKey); !ok || val != targetInfo.DecorationUpdatedRevisions {
423+
needUpdateContext = true
424+
u.ResourceContextControl.Put(ownedIDs[targetInfo.ID], api.EnumTargetDecorationRevisionKey, targetInfo.DecorationUpdatedRevisions)
437425
}
438426
}
439427

‎xset/xset_controller.go‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -182,14 +182,6 @@ func (r *xSetCommonReconciler) Reconcile(ctx context.Context, req reconcile.Requ
182182

183183
if instance.GetDeletionTimestamp() != nil {
184184
if controllerutil.ContainsFinalizer(instance, r.finalizerName) {
185-
// reclaim owner IDs in ResourceContextControl
186-
if err := r.resourceContextControl.UpdateToTargetContext(ctx, instance, nil); err != nil {
187-
return ctrl.Result{}, err
188-
}
189-
if cleaned, err := r.ensureReclaimTargetsDeletion(ctx, instance); !cleaned || err != nil {
190-
// reclaim targets deletion before remove finalizers
191-
return ctrl.Result{}, err
192-
}
193185
// reclaim target sub resources before remove finalizers
194186
if err := r.ensureReclaimTargetSubResources(ctx, instance); err != nil {
195187
return ctrl.Result{}, err
@@ -198,6 +190,14 @@ func (r *xSetCommonReconciler) Reconcile(ctx context.Context, req reconcile.Requ
198190
if err := r.ensureReclaimOwnerReferences(ctx, instance); err != nil {
199191
return ctrl.Result{}, err
200192
}
193+
if cleaned, err := r.ensureReclaimTargetsDeletion(ctx, instance); !cleaned || err != nil {
194+
// reclaim targets deletion before remove finalizers
195+
return ctrl.Result{}, err
196+
}
197+
// reclaim owner IDs in ResourceContextControl
198+
if err := r.resourceContextControl.UpdateToTargetContext(ctx, instance, nil); err != nil {
199+
return ctrl.Result{}, err
200+
}
201201
}
202202
return ctrl.Result{}, clientutil.RemoveFinalizerAndUpdate(ctx, r.Client, instance, r.finalizerName)
203203
}

0 commit comments

Comments
 (0)