Repository: argoproj-labs/applicationset
Pull request #11 (2020-07-11 10:45:46)

Update
Refactor the create application method so it will also do update and delete.

Not sure about the name of the function, does doing delete in an apply function is ok?

dgoodwin: LGTM. I'll merge it in.

Patch:
diff --git a/pkg/controllers/applicationset_controller.go b/pkg/controllers/applicationset_controller.go
index 191ddaf..1fccb1e 100644
--- a/pkg/controllers/applicationset_controller.go
+++ b/pkg/controllers/applicationset_controller.go
@@ -21,6 +21,7 @@ import (
 	"fmt"
 	"github.com/argoproj-labs/applicationset/pkg/generators"
 	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/client-go/tools/record"
 	"k8s.io/kubernetes/pkg/apis/core"
 	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
@@ -78,17 +79,34 @@ func (r *ApplicationSetReconciler) Reconcile(req ctrl.Request) (ctrl.Result, err
 
 	}
 
-	if err := r.createApplications(ctx, applicationSetInfo, desiredApplications); err != nil {
-		log.Infof("Unable to create applications applicationSetInfo %e", err)
-		return ctrl.Result{}, err
-	}
+	r.createOrUpdateInCluster(ctx, applicationSetInfo, desiredApplications)
+	r.deleteInCluster(ctx, applicationSetInfo, desiredApplications)
 
 	return ctrl.Result{}, nil
 }
 
 func (r *ApplicationSetReconciler) SetupWithManager(mgr ctrl.Manager) error {
+	if err := mgr.GetFieldIndexer().IndexField(&argov1alpha1.Application{}, ".metadata.controller", func(rawObj runtime.Object) []string {
+		// grab the job object, extract the owner...
+		app := rawObj.(*argov1alpha1.Application)
+		owner := metav1.GetControllerOf(app)
+		if owner == nil {
+			return nil
+		}
+		// ...make sure it's a application set...
+		if owner.APIVersion != argoprojiov1alpha1.GroupVersion.String() || owner.Kind != "ApplicationSet" {
+			return nil
+		}
+
+		// ...and if so, return it
+		return []string{owner.Name}
+	}); err != nil {
+		return err
+	}
+
 	return ctrl.NewControllerManagedBy(mgr).
 		For(&argoprojiov1alpha1.ApplicationSet{}).
+		Owns(&argov1alpha1.Application{}).
 		Watches(
 			&source.Kind{Type: &corev1.Secret{}},
 			&clusterSecretEventHandler{
@@ -99,34 +117,62 @@ func (r *ApplicationSetReconciler) SetupWithManager(mgr ctrl.Manager) error {
 		Complete(r)
 }
 
-func (r *ApplicationSetReconciler) createApplications(ctx context.Context, applicationSetInfo argoprojiov1alpha1.ApplicationSet, appList []argov1alpha1.Application) error {
+// createOrUpdateInCluster will create / update application resources in the cluster.
+// For new application it will call create
+// For application that need to update it will call update
+// The function also adds owner reference to all applications, and uses it for delete them.
+func (r *ApplicationSetReconciler) createOrUpdateInCluster(ctx context.Context, applicationSet argoprojiov1alpha1.ApplicationSet, desiredApplications []argov1alpha1.Application) {
 
-	for _, app := range appList {
-		app.Namespace = applicationSetInfo.Namespace
+	//create or updates the application in appList
+	for _, app := range desiredApplications {
+		appLog := log.WithFields(log.Fields{"app": app.Name, "appSet": applicationSet.Name})
+		app.Namespace = applicationSet.Namespace
 
 		found := app
 		action, err := ctrl.CreateOrUpdate(ctx, r.Client, &found, func() error {
 			found.Spec = app.Spec
-			return controllerutil.SetControllerReference(&applicationSetInfo, &found, r.Scheme)
+			return controllerutil.SetControllerReference(&applicationSet, &found, r.Scheme)
 		})
 
 		if err != nil {
-			log.Error(err, fmt.Sprintf("failed to get Application %s resource for applicationSet %s", app.Name, applicationSetInfo.Name))
+			appLog.WithError(err).Errorf("failed to %s Application", action)
 			continue
 		}
 
-		r.Recorder.Eventf(&applicationSetInfo, core.EventTypeNormal, "Created", "Created Application %q", app.Name)
-		log.Infof("%s Application %s resource for applicationSet %s", action, app.Name, applicationSetInfo.Name)
+		r.Recorder.Eventf(&applicationSet, core.EventTypeNormal, fmt.Sprint(action), "%s Application %q", action, app.Name)
+		appLog.Logf(log.InfoLevel, "%s Application", action)
 	}
+}
 
-	return nil
+// deleteInCluster will delete application that are current in the cluster but not in appList.
+// The function must be called after all generators had been called and generated applications
+func (r *ApplicationSetReconciler) deleteInCluster(ctx context.Context, applicationSet argoprojiov1alpha1.ApplicationSet, desiredApplications []argov1alpha1.Application) {
 
-}
+	// Save current applications to be able to delete the ones that are not in appList
+	var current argov1alpha1.ApplicationList
+	_ = r.Client.List(context.Background(), &current, client.MatchingFields{".metadata.controller": applicationSet.Name})
 
-func (r *ApplicationSetReconciler) getApplications(ctx context.Context, applicationSetInfo argoprojiov1alpha1.ApplicationSet) ([]argov1alpha1.Application, error) {
+	m := make(map[string]bool) // Will holds the app names in appList for the deletion process
 
-	return nil, nil
+	for _, app := range desiredApplications {
+		m[app.Name] = true
+	}
 
+	// Delete apps that are not in m[string]bool
+	for _, app := range current.Items {
+		appLog := log.WithFields(log.Fields{"app": app.Name, "appSet": applicationSet.Name})
+		_, exists := m[app.Name]
+
+		if exists == false {
+			err := r.Client.Delete(ctx, &app)
+			if err != nil {
+				appLog.WithError(err).Error("failed to delete Application")
+				continue
+			}
+			r.Recorder.Eventf(&applicationSet, core.EventTypeNormal, "Deleted", "Deleted Application %q", app.Name)
+			appLog.Log(log.InfoLevel, "Deleted application")
+		}
+	}
 }
 
 var _ handler.EventHandler = &clusterSecretEventHandler{}
diff --git a/pkg/utils/util.go b/pkg/utils/util.go
index 334892e..a3bf3ab 100644
--- a/pkg/utils/util.go
+++ b/pkg/utils/util.go
@@ -36,6 +36,11 @@ func RenderTemplateParams(tmpl *argov1alpha1.Application, params map[string]stri
 		return nil, err
 	}
 
+	if replacedTmpl.ObjectMeta.Finalizers == nil {
+		replacedTmpl.ObjectMeta.Finalizers = []string{}
+	}
+	replacedTmpl.ObjectMeta.Finalizers = append(replacedTmpl.ObjectMeta.Finalizers, "resources-finalizer.argocd.argoproj.io")
+
 	return &replacedTmpl, nil
 }
 
@@ -64,5 +69,6 @@ func Replace(fstTmpl *fasttemplate.Template, replaceMap map[string]string, allow
 	if unresolvedErr != nil {
 		return "", unresolvedErr
 	}
+
 	return replacedTmpl, nil
 }


Test patch:
diff --git a/pkg/controllers/applicationset_controller_test.go b/pkg/controllers/applicationset_controller_test.go
index cd050e6..a545731 100644
--- a/pkg/controllers/applicationset_controller_test.go
+++ b/pkg/controllers/applicationset_controller_test.go
@@ -2,6 +2,7 @@ package controllers
 
 import (
 	"context"
+	"fmt"
 	argov1alpha1 "github.com/argoproj/argo-cd/pkg/apis/application/v1alpha1"
 	"github.com/stretchr/testify/assert"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -15,7 +16,7 @@ import (
 	argoprojiov1alpha1 "github.com/argoproj-labs/applicationset/api/v1alpha1"
 )
 
-func TestCreateApplications(t *testing.T) {
+func TestCreateOrUpdateApplications(t *testing.T) {
 
 	scheme := runtime.NewScheme()
 	argoprojiov1alpha1.AddToScheme(scheme)
@@ -113,6 +114,63 @@ func TestCreateApplications(t *testing.T) {
 				},
 			},
 		},
+		{
+			appSet: argoprojiov1alpha1.ApplicationSet{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "name",
+					Namespace: "namespace",
+				},
+				Spec: argoprojiov1alpha1.ApplicationSetSpec{
+					Template: argoprojiov1alpha1.ApplicationSetTemplate{
+						Spec: argov1alpha1.ApplicationSpec{
+							Project: "project",
+						},
+					},
+				},
+			},
+			existsApps: []argov1alpha1.Application{
+				argov1alpha1.Application{
+					TypeMeta: metav1.TypeMeta{
+						Kind:       "Application",
+						APIVersion: "argoproj.io/v1alpha1",
+					},
+					ObjectMeta: metav1.ObjectMeta{
+						Name:            "app1",
+						Namespace:       "namespace",
+						ResourceVersion: "2",
+					},
+					Spec: argov1alpha1.ApplicationSpec{
+						Project: "test",
+					},
+				},
+			},
+			apps: []argov1alpha1.Application{
+				{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: "app2",
+					},
+					Spec: argov1alpha1.ApplicationSpec{
+						Project: "project",
+					},
+				},
+			},
+			expected: []argov1alpha1.Application{
+				{
+					TypeMeta: metav1.TypeMeta{
+						Kind:       "Application",
+						APIVersion: "argoproj.io/v1alpha1",
+					},
+					ObjectMeta: metav1.ObjectMeta{
+						Name:            "app2",
+						Namespace:       "namespace",
+						ResourceVersion: "1",
+					},
+					Spec: argov1alpha1.ApplicationSpec{
+						Project: "project",
+					},
+				},
+			},
+		},
 	} {
 		initObjs := []runtime.Object{&c.appSet}
 		for _, a := range c.existsApps {
@@ -125,10 +183,143 @@ func TestCreateApplications(t *testing.T) {
 		r := ApplicationSetReconciler{
 			Client:   client,
 			Scheme:   scheme,
-			Recorder: record.NewFakeRecorder(1),
+			Recorder: record.NewFakeRecorder(len(initObjs) + len(c.expected)),
+		}
+
+		r.createOrUpdateInCluster(context.TODO(), c.appSet, c.apps)
+
+		for _, obj := range c.expected {
+			got := &argov1alpha1.Application{}
+			_ = client.Get(context.Background(), crtclient.ObjectKey{
+				Namespace: obj.Namespace,
+				Name:      obj.Name,
+			}, got)
+
+			controllerutil.SetControllerReference(&c.appSet, &obj, r.Scheme)
+
+			assert.Equal(t, obj, *got)
+		}
+	}
+
+}
+
+func TestDeleteApplications(t *testing.T) {
+
+	scheme := runtime.NewScheme()
+	argoprojiov1alpha1.AddToScheme(scheme)
+	argov1alpha1.AddToScheme(scheme)
+
+	for _, c := range []struct {
+		appSet      argoprojiov1alpha1.ApplicationSet
+		existsApps  []argov1alpha1.Application
+		apps        []argov1alpha1.Application
+		expected    []argov1alpha1.Application
+		notExpected []argov1alpha1.Application
+	}{
+		{
+			appSet: argoprojiov1alpha1.ApplicationSet{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "name",
+					Namespace: "namespace",
+				},
+				Spec: argoprojiov1alpha1.ApplicationSetSpec{
+					Template: argoprojiov1alpha1.ApplicationSetTemplate{
+						Spec: argov1alpha1.ApplicationSpec{
+							Project: "project",
+						},
+					},
+				},
+			},
+			existsApps: []argov1alpha1.Application{
+				{
+					TypeMeta: metav1.TypeMeta{
+						Kind:       "Application",
+						APIVersion: "argoproj.io/v1alpha1",
+					},
+					ObjectMeta: metav1.ObjectMeta{
+						Name:            "delete",
+						Namespace:       "namespace",
+						ResourceVersion: "2",
+					},
+					Spec: argov1alpha1.ApplicationSpec{
+						Project: "project",
+					},
+				},
+				{
+					TypeMeta: metav1.TypeMeta{
+						Kind:       "Application",
+						APIVersion: "argoproj.io/v1alpha1",
+					},
+					ObjectMeta: metav1.ObjectMeta{
+						Name:            "keep",
+						Namespace:       "namespace",
+						ResourceVersion: "2",
+					},
+					Spec: argov1alpha1.ApplicationSpec{
+						Project: "project",
+					},
+				},
+			},
+			apps: []argov1alpha1.Application{
+				{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: "keep",
+					},
+					Spec: argov1alpha1.ApplicationSpec{
+						Project: "project",
+					},
+				},
+			},
+			expected: []argov1alpha1.Application{
+				{
+					TypeMeta: metav1.TypeMeta{
+						Kind:       "Application",
+						APIVersion: "argoproj.io/v1alpha1",
+					},
+					ObjectMeta: metav1.ObjectMeta{
+						Name:            "keep",
+						Namespace:       "namespace",
+						ResourceVersion: "2",
+					},
+					Spec: argov1alpha1.ApplicationSpec{
+						Project: "project",
+					},
+				},
+			},
+			notExpected: []argov1alpha1.Application{
+				{
+					TypeMeta: metav1.TypeMeta{
+						Kind:       "Application",
+						APIVersion: "argoproj.io/v1alpha1",
+					},
+					ObjectMeta: metav1.ObjectMeta{
+						Name:            "delete",
+						Namespace:       "namespace",
+						ResourceVersion: "1",
+					},
+					Spec: argov1alpha1.ApplicationSpec{
+						Project: "project",
+					},
+				},
+			},
+		},
+	} {
+		initObjs := []runtime.Object{&c.appSet}
+		for _, a := range c.existsApps {
+			temp := a
+			controllerutil.SetControllerReference(&c.appSet, &temp, scheme)
+			initObjs = append(initObjs, &temp)
+		}
+
+		client := fake.NewFakeClientWithScheme(scheme, initObjs...)
+
+		r := ApplicationSetReconciler{
+			Client:   client,
+			Scheme:   scheme,
+			Recorder: record.NewFakeRecorder(len(initObjs) + len(c.expected)),
 		}
 
-		r.createApplications(context.TODO(), c.appSet, c.apps)
+		r.deleteInCluster(context.TODO(), c.appSet, c.apps)
 
 		for _, obj := range c.expected {
 			got := &argov1alpha1.Application{}
@@ -141,6 +332,16 @@ func TestCreateApplications(t *testing.T) {
 
 			assert.Equal(t, obj, *got)
 		}
+
+		for _, obj := range c.notExpected {
+			got := &argov1alpha1.Application{}
+			err := client.Get(context.Background(), crtclient.ObjectKey{
+				Namespace: obj.Namespace,
+				Name:      obj.Name,
+			}, got)
+
+			assert.EqualError(t, err, fmt.Sprintf("applications.argoproj.io \"%s\" not found", obj.Name))
+		}
 	}
 
 }