Skip to content

Commit fdc654d

Browse files
committed
Merge remote-tracking branch 'upstream/release-3.3' into ant-release-3.3
2 parents a13dd6f + 998fb59 commit fdc654d

34 files changed

+566
-171
lines changed

.github/workflows/ci-build.yaml

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -418,14 +418,14 @@ jobs:
418418
# latest: true means that this version mush upload the coverage report to codecov.io
419419
# We designate the latest version because we only collect code coverage for that version.
420420
k3s:
421-
- version: v1.34.2
421+
- version: v1.35.0
422422
latest: true
423+
- version: v1.34.2
424+
latest: false
423425
- version: v1.33.1
424426
latest: false
425427
- version: v1.32.1
426428
latest: false
427-
- version: v1.31.0
428-
latest: false
429429
needs:
430430
- build-go
431431
- changes

VERSION

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
3.3.4
1+
3.3.6

controller/appcontroller.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1846,7 +1846,7 @@ func (ctrl *ApplicationController) processAppRefreshQueueItem() (processNext boo
18461846
logCtx = logCtx.WithField(k, v.Milliseconds())
18471847
}
18481848

1849-
ctrl.normalizeApplication(origApp, app)
1849+
ctrl.normalizeApplication(app)
18501850
ts.AddCheckpoint("normalize_application_ms")
18511851

18521852
tree, err := ctrl.setAppManagedResources(destCluster, app, compareResult)
@@ -2085,7 +2085,8 @@ func (ctrl *ApplicationController) refreshAppConditions(app *appv1.Application)
20852085
}
20862086

20872087
// normalizeApplication normalizes an application.spec and additionally persists updates if it changed
2088-
func (ctrl *ApplicationController) normalizeApplication(orig, app *appv1.Application) {
2088+
func (ctrl *ApplicationController) normalizeApplication(app *appv1.Application) {
2089+
orig := app.DeepCopy()
20892090
app.Spec = *argo.NormalizeApplicationSpec(&app.Spec)
20902091
logCtx := log.WithFields(applog.GetAppLogFields(app))
20912092

controller/hook.go

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,21 @@ func isPostDeleteHook(obj *unstructured.Unstructured) bool {
7676
return isHookOfType(obj, PostDeleteHookType)
7777
}
7878

79+
// hasGitOpsEngineSyncPhaseHook is true when gitops-engine would run the resource during a sync
80+
// phase (PreSync, Sync, PostSync, SyncFail). PreDelete/PostDelete are not sync phases;
81+
// without this check, state reconciliation drops such resources
82+
// entirely because isPreDeleteHook/isPostDeleteHook match any comma-separated value.
83+
// HookTypeSkip is omitted as it is not a sync phase.
84+
func hasGitOpsEngineSyncPhaseHook(obj *unstructured.Unstructured) bool {
85+
for _, t := range hook.Types(obj) {
86+
switch t {
87+
case common.HookTypePreSync, common.HookTypeSync, common.HookTypePostSync, common.HookTypeSyncFail:
88+
return true
89+
}
90+
}
91+
return false
92+
}
93+
7994
// executeHooks is a generic function to execute hooks of a specified type
8095
func (ctrl *ApplicationController) executeHooks(hookType HookType, app *appv1.Application, proj *appv1.AppProject, liveObjs map[kube.ResourceKey]*unstructured.Unstructured, config *rest.Config, logCtx *log.Entry) (bool, error) {
8196
appLabelKey, err := ctrl.settingsMgr.GetAppInstanceLabelKey()

controller/hook_test.go

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -192,6 +192,92 @@ func TestIsPostDeleteHook(t *testing.T) {
192192
}
193193
}
194194

195+
// TestPartitionTargetObjsForSync covers partitionTargetObjsForSync in state.go.
196+
func TestPartitionTargetObjsForSync(t *testing.T) {
197+
newObj := func(name string, annot map[string]string) *unstructured.Unstructured {
198+
u := &unstructured.Unstructured{}
199+
u.SetName(name)
200+
u.SetAnnotations(annot)
201+
return u
202+
}
203+
204+
tests := []struct {
205+
name string
206+
in []*unstructured.Unstructured
207+
wantNames []string
208+
wantPreDelete bool
209+
wantPostDelete bool
210+
}{
211+
{
212+
name: "PostSync with PreDelete and PostDelete in same annotation stays in sync set",
213+
in: []*unstructured.Unstructured{
214+
newObj("combined", map[string]string{"argocd.argoproj.io/hook": "PostSync,PreDelete,PostDelete"}),
215+
},
216+
wantNames: []string{"combined"},
217+
wantPreDelete: true,
218+
wantPostDelete: true,
219+
},
220+
{
221+
name: "PreDelete-only manifest excluded from sync",
222+
in: []*unstructured.Unstructured{
223+
newObj("pre-del", map[string]string{"argocd.argoproj.io/hook": "PreDelete"}),
224+
},
225+
wantNames: nil,
226+
wantPreDelete: true,
227+
wantPostDelete: false,
228+
},
229+
{
230+
name: "PostDelete-only manifest excluded from sync",
231+
in: []*unstructured.Unstructured{
232+
newObj("post-del", map[string]string{"argocd.argoproj.io/hook": "PostDelete"}),
233+
},
234+
wantNames: nil,
235+
wantPreDelete: false,
236+
wantPostDelete: true,
237+
},
238+
{
239+
name: "Helm pre-delete only excluded from sync",
240+
in: []*unstructured.Unstructured{
241+
newObj("helm-pre-del", map[string]string{"helm.sh/hook": "pre-delete"}),
242+
},
243+
wantNames: nil,
244+
wantPreDelete: true,
245+
wantPostDelete: false,
246+
},
247+
{
248+
name: "Helm pre-install with pre-delete stays in sync (sync-phase hook wins)",
249+
in: []*unstructured.Unstructured{
250+
newObj("helm-mixed", map[string]string{"helm.sh/hook": "pre-install,pre-delete"}),
251+
},
252+
wantNames: []string{"helm-mixed"},
253+
wantPreDelete: true,
254+
wantPostDelete: false,
255+
},
256+
{
257+
name: "Non-hook resource unchanged",
258+
in: []*unstructured.Unstructured{
259+
newObj("pod", map[string]string{"app": "x"}),
260+
},
261+
wantNames: []string{"pod"},
262+
wantPreDelete: false,
263+
wantPostDelete: false,
264+
},
265+
}
266+
267+
for _, tt := range tests {
268+
t.Run(tt.name, func(t *testing.T) {
269+
got, hasPre, hasPost := partitionTargetObjsForSync(tt.in)
270+
var names []string
271+
for _, o := range got {
272+
names = append(names, o.GetName())
273+
}
274+
assert.Equal(t, tt.wantNames, names)
275+
assert.Equal(t, tt.wantPreDelete, hasPre, "hasPreDeleteHooks")
276+
assert.Equal(t, tt.wantPostDelete, hasPost, "hasPostDeleteHooks")
277+
})
278+
}
279+
}
280+
195281
func TestMultiHookOfType(t *testing.T) {
196282
tests := []struct {
197283
name string

controller/state.go

Lines changed: 23 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -536,6 +536,28 @@ func isManagedNamespace(ns *unstructured.Unstructured, app *v1alpha1.Application
536536
return ns != nil && ns.GetKind() == kubeutil.NamespaceKind && ns.GetName() == app.Spec.Destination.Namespace && app.Spec.SyncPolicy != nil && app.Spec.SyncPolicy.ManagedNamespaceMetadata != nil
537537
}
538538

539+
// partitionTargetObjsForSync returns the manifest subset passed to gitops-engine sync, and whether
540+
// the full manifest set declared PreDelete and/or PostDelete hooks (for finalizer handling).
541+
// Uses isPreDeleteHook / isPostDeleteHook / hasGitOpsEngineSyncPhaseHook from hook.go.
542+
func partitionTargetObjsForSync(targetObjs []*unstructured.Unstructured) (syncObjs []*unstructured.Unstructured, hasPreDeleteHooks, hasPostDeleteHooks bool) {
543+
for _, obj := range targetObjs {
544+
if isPreDeleteHook(obj) {
545+
hasPreDeleteHooks = true
546+
if !hasGitOpsEngineSyncPhaseHook(obj) {
547+
continue
548+
}
549+
}
550+
if isPostDeleteHook(obj) {
551+
hasPostDeleteHooks = true
552+
if !hasGitOpsEngineSyncPhaseHook(obj) {
553+
continue
554+
}
555+
}
556+
syncObjs = append(syncObjs, obj)
557+
}
558+
return syncObjs, hasPreDeleteHooks, hasPostDeleteHooks
559+
}
560+
539561
// CompareAppState compares application git state to the live app state, using the specified
540562
// revision and supplied source. If revision or overrides are empty, then compares against
541563
// revision and overrides in the app spec.
@@ -763,24 +785,7 @@ func (m *appStateManager) CompareAppState(app *v1alpha1.Application, project *v1
763785
}
764786
}
765787
}
766-
hasPreDeleteHooks := false
767-
hasPostDeleteHooks := false
768-
// Filter out PreDelete and PostDelete hooks from targetObjs since they should not be synced
769-
// as regular resources. They are only executed during deletion.
770-
var targetObjsForSync []*unstructured.Unstructured
771-
for _, obj := range targetObjs {
772-
if isPreDeleteHook(obj) {
773-
hasPreDeleteHooks = true
774-
// Skip PreDelete hooks - they are not synced, only executed during deletion
775-
continue
776-
}
777-
if isPostDeleteHook(obj) {
778-
hasPostDeleteHooks = true
779-
// Skip PostDelete hooks - they are not synced, only executed after deletion
780-
continue
781-
}
782-
targetObjsForSync = append(targetObjsForSync, obj)
783-
}
788+
targetObjsForSync, hasPreDeleteHooks, hasPostDeleteHooks := partitionTargetObjsForSync(targetObjs)
784789

785790
reconciliation := sync.Reconcile(targetObjsForSync, liveObjByKey, app.Spec.Destination.Namespace, infoProvider)
786791
ts.AddCheckpoint("live_ms")
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
| Argo CD version | Kubernetes versions |
22
|-----------------|---------------------|
3-
| 3.3 | v1.34, v1.33, v1.32, v1.31 |
3+
| 3.3 | v1.35, v1.34, v1.33, v1.32 |
44
| 3.2 | v1.34, v1.33, v1.32, v1.31 |
55
| 3.1 | v1.34, v1.33, v1.32, v1.31 |

0 commit comments

Comments
 (0)