fix(appset): When Appset is deleted, the controller should reconcile applicationset #23723 (#23823)

Signed-off-by: Sangeetha Madamanchi <smadamanchi@expediagroup.com>
Co-authored-by: Sangeetha Madamanchi <smadamanchi@expediagroup.com>
This commit is contained in:
sangeer 2025-07-17 06:44:02 -05:00 committed by GitHub
parent d83ef2c224
commit ac4ae1779e
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 34 additions and 7 deletions

View file

@ -1696,24 +1696,21 @@ func shouldRequeueForApplicationSet(appSetOld, appSetNew *argov1alpha1.Applicati
}
// Requeue if any ApplicationStatus.Status changed for Progressive sync strategy
// Requeue if deletionTimestamp added
if enableProgressiveSyncs {
if !cmp.Equal(appSetOld.Status.ApplicationStatus, appSetNew.Status.ApplicationStatus, cmpopts.EquateEmpty()) {
return true
}
if !cmp.Equal(appSetOld.DeletionTimestamp, appSetNew.DeletionTimestamp, cmpopts.EquateEmpty()) {
return true
}
}
// only compare the applicationset spec, annotations, labels and finalizers, specifically avoiding
// only compare the applicationset spec, annotations, labels and finalizers, deletionTimestamp, specifically avoiding
// the status field. status is owned by the applicationset controller,
// and we do not need to requeue when it does bookkeeping
// NB: the ApplicationDestination comes from the ApplicationSpec being embedded
// in the ApplicationSetTemplate from the generators
if !cmp.Equal(appSetOld.Spec, appSetNew.Spec, cmpopts.EquateEmpty(), cmpopts.EquateComparable(argov1alpha1.ApplicationDestination{})) ||
!cmp.Equal(appSetOld.GetLabels(), appSetNew.GetLabels(), cmpopts.EquateEmpty()) ||
!cmp.Equal(appSetOld.GetFinalizers(), appSetNew.GetFinalizers(), cmpopts.EquateEmpty()) {
!cmp.Equal(appSetOld.GetFinalizers(), appSetNew.GetFinalizers(), cmpopts.EquateEmpty()) ||
!cmp.Equal(appSetOld.DeletionTimestamp, appSetNew.DeletionTimestamp, cmpopts.EquateEmpty()) {
return true
}

View file

@ -7126,7 +7126,7 @@ func TestApplicationSetOwnsHandlerUpdate(t *testing.T) {
},
},
enableProgressiveSyncs: false,
want: false,
want: true,
},
}
@ -7276,6 +7276,36 @@ func TestShouldRequeueForApplicationSet(t *testing.T) {
},
want: true,
},
{
name: "ApplicationSetWithDeletionTimestamp",
args: args{
appSetOld: &v1alpha1.ApplicationSet{
Status: v1alpha1.ApplicationSetStatus{
ApplicationStatus: []v1alpha1.ApplicationSetApplicationStatus{
{
Application: "app1",
Status: "Healthy",
},
},
},
},
appSetNew: &v1alpha1.ApplicationSet{
ObjectMeta: metav1.ObjectMeta{
DeletionTimestamp: &metav1.Time{Time: time.Now()},
},
Status: v1alpha1.ApplicationSetStatus{
ApplicationStatus: []v1alpha1.ApplicationSetApplicationStatus{
{
Application: "app1",
Status: "Waiting",
},
},
},
},
enableProgressiveSyncs: false,
},
want: true,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {