diff --git a/argocd-operator/controllers/argocd/util.go b/argocd-operator/controllers/argocd/util.go index 354b36a46c6..e308a7aa8b5 100644 --- a/argocd-operator/controllers/argocd/util.go +++ b/argocd-operator/controllers/argocd/util.go @@ -734,7 +734,8 @@ func (r *ReconcileArgoCD) deleteClusterResources(cr *argoproj.ArgoCD) error { return fmt.Errorf("failed to filter APIServices for %s: %w", cr.Name, err) } - if err := gitopspromoter.DeleteAPIServices(r.Client, apiSvcList); err != nil { + apiServerCompName := string(argoproj.PromoterComponentTypeAPIServer) + if err := gitopspromoter.DeleteAPIServices(r.Client, apiSvcList, apiServerCompName, cr); err != nil { return err } @@ -826,7 +827,6 @@ func removeString(slice []string, s string) []string { // setResourceWatches will register Watches for each of the supported Resources. func (r *ReconcileArgoCD) setResourceWatches(bldr *builder.Builder, clusterResourceMapper, tlsSecretMapper, namespaceResourceMapper, clusterSecretResourceMapper, applicationSetGitlabSCMTLSConfigMapMapper, nmMapper, systemCATrustMapper, imagePullSecretMapper handler.MapFunc) *builder.Builder { - // Add new predicate to delete Notifications Resources. The predicate watches the Argo CD CR for changes to the `.spec.Notifications.Enabled` // field. When a change is detected that results in notifications being disabled, we trigger deletion of notifications resources deleteNotificationsPred := predicate.Funcs{ diff --git a/argocd-operator/controllers/gitopspromoter/apiservice.go b/argocd-operator/controllers/gitopspromoter/apiservice.go index 83035d9589f..bb97849641f 100644 --- a/argocd-operator/controllers/gitopspromoter/apiservice.go +++ b/argocd-operator/controllers/gitopspromoter/apiservice.go @@ -56,10 +56,17 @@ func ReconcilePromoterAPIServerAPIService(client client.Client, compName string, } if exists { + // Check to see if referenced service was created by the CR if it is not do not do any operations on it + // This safe guards from another ArgoCD CR instance from deleting the APIService even if the promoter is disabled + // If the service field is for some reason nil be safe and skip deleting anyway + if !ownsAPISvc(compName, cr, apiSvc) { + return apiSvc, nil + } + if !cr.Spec.Promoter.IsEnabled() || !enabled || !allowed { argoutil.LogResourceDeletion(log, apiSvc, fmt.Sprintf("promoter apiservice for component %s is being deleted due to being disabled", compName)) if err := client.Delete(context.Background(), apiSvc); err != nil { - return nil, fmt.Errorf("failed to delete promoter service %s: %v", apiSvc.Name, err) + return nil, fmt.Errorf("failed to delete promoter api service %s: %v", apiSvc.Name, err) } return apiSvc, nil } @@ -149,8 +156,12 @@ func buildAPIServiceSpec(client client.Client, compName string, cr *argoproj.Arg } // DeleteAPIServices deletes a list of API Services -func DeleteAPIServices(c client.Client, apiSvcList *apiregistrationv1.APIServiceList) error { +func DeleteAPIServices(c client.Client, apiSvcList *apiregistrationv1.APIServiceList, compName string, cr *argoproj.ArgoCD) error { for _, apiSvc := range apiSvcList.Items { + if !ownsAPISvc(compName, cr, &apiSvc) { + continue + } + argoutil.LogResourceDeletion(log, &apiSvc, "cleaning up cluster resources") if err := c.Delete(context.TODO(), &apiSvc); err != nil { return fmt.Errorf("failed to delete APIService %s during cleanup: %w", apiSvc.Name, err) @@ -158,3 +169,7 @@ func DeleteAPIServices(c client.Client, apiSvcList *apiregistrationv1.APIService } return nil } + +func ownsAPISvc(compName string, cr *argoproj.ArgoCD, apiSvc *apiregistrationv1.APIService) bool { + return apiSvc.Spec.Service != nil && cr.Namespace == apiSvc.Spec.Service.Namespace && generatePromoterResourceName(compName, cr) == apiSvc.Spec.Service.Name +} diff --git a/argocd-operator/controllers/gitopspromoter/apiservice_test.go b/argocd-operator/controllers/gitopspromoter/apiservice_test.go index 7db5f17b3f7..afeb5461b84 100644 --- a/argocd-operator/controllers/gitopspromoter/apiservice_test.go +++ b/argocd-operator/controllers/gitopspromoter/apiservice_test.go @@ -370,8 +370,6 @@ func TestReconcilePromoterAPIServerAPIService_Exists_Update(t *testing.T) { cr := makeTestArgoCD(withPromoterEnabled(true), withPromoterAPIServerEnabled(true)) existingAPIService := makeExistingAPIService(cr) - existingAPIService.Spec.Service.Name = "not-a-real-service" - existingAPIService.Spec.Service.Namespace = "not-a-real-namespace" existingAPIService.Spec.Service.Port = ptr.To(int32(25565)) resObjs := []client.Object{cr, existingAPIService} @@ -392,3 +390,38 @@ func TestReconcilePromoterAPIServerAPIService_Exists_Update(t *testing.T) { assert.Equal(t, cr.Namespace, retrievedAPIService.Spec.Service.Namespace) assert.Equal(t, ptr.To(int32(APIServerPort)), retrievedAPIService.Spec.Service.Port) } + +func TestReconcilePromoterAPIServerAPIService_MultipleCRsPresent(t *testing.T) { + // Test case: APIService gets reconciled then another CR with the promoter disabled gets reconciled + // Expected behavior: the APIService should not get deleted + + crEnabled := makeTestArgoCD(withPromoterEnabled(true), withPromoterAPIServerEnabled(true)) + crDisabled := makeTestArgoCD(withPromoterEnabled(false), withPromoterAPIServerEnabled(false)) + crDisabled.Name = "disabled-cr" + crDisabled.Namespace = "different-namespace" + + resObjs := []client.Object{crEnabled, crDisabled} + sch := makeTestReconcilerScheme() + client := makeTestReconcilerClient(sch, resObjs) + + apiService, err := ReconcilePromoterAPIServerAPIService(client, testCompName, crEnabled) + assert.NoError(t, err) + assert.NotNil(t, apiService) + + // Make sure APIService as created + retrievedAPIService := &apiregistrationv1.APIService{} + err = client.Get(context.Background(), types.NamespacedName{ + Name: "v1alpha1.view.promoter.argoproj.io", + }, retrievedAPIService) + assert.NoError(t, err) + + apiService, err = ReconcilePromoterAPIServerAPIService(client, testCompName, crDisabled) + assert.NoError(t, err) + assert.NotNil(t, apiService) + + // Get APIService again to make sure it is not deleted + err = client.Get(context.Background(), types.NamespacedName{ + Name: "v1alpha1.view.promoter.argoproj.io", + }, retrievedAPIService) + assert.NoError(t, err) +}