feat(operator): support restartability of KeptnApp (#544)

Signed-off-by: odubajDT <ondrej.dubaj@dynatrace.com>
Signed-off-by: odubajDT <93584209+odubajDT@users.noreply.github.com>

operator/api/v1alpha2/common/common_test.go

@@ -179,7 +179,7 @@ func Test_GeOverallState(t *testing.T) {
 			Want:    StateFailed,
 		},
 		{
-			Name:    "Deprecated",
+			Name:    "deprecated",
 			Summary: StatusSummary{0, 0, 0, 0, 0, 0, 1},
 			Want:    StateFailed,
 		},

operator/api/v1alpha2/keptnapp_test.go

@@ -12,22 +12,23 @@ import (
 func TestKeptnApp(t *testing.T) {
 	app := &KeptnApp{
 		ObjectMeta: metav1.ObjectMeta{
-			Name:      "app",
-			Namespace: "namespace",
+			Name:       "app",
+			Namespace:  "namespace",
+			Generation: 1,
 		},
 		Spec: KeptnAppSpec{
 			Version: "version",
 		},
 	}
 
 	appVersionName := app.GetAppVersionName()
-	require.Equal(t, "app-version", appVersionName)
+	require.Equal(t, "app-version-1", appVersionName)
 
 	appVersion := app.GenerateAppVersion("prev", map[string]string{})
 	require.Equal(t, KeptnAppVersion{
 		ObjectMeta: metav1.ObjectMeta{
 			Annotations: map[string]string{},
-			Name:        "app-version",
+			Name:        "app-version-1",
 			Namespace:   "namespace",
 		},
 		Spec: KeptnAppVersionSpec{

operator/api/v1alpha2/keptnapp_types.go

@@ -17,6 +17,7 @@ limitations under the License.
 package v1alpha2
 
 import (
+	"fmt"
 	"strings"
 
 	"github.com/keptn/lifecycle-toolkit/operator/api/v1alpha2/common"
@@ -32,7 +33,7 @@ import (
 type KeptnAppSpec struct {
 	Version string `json:"version"`
 	// +kubebuilder:default:=1
-	Revision                  int                `json:"revision,omitempty"`
+	Revision                  uint               `json:"revision,omitempty"`
 	Workloads                 []KeptnWorkloadRef `json:"workloads,omitempty"`
 	PreDeploymentTasks        []string           `json:"preDeploymentTasks,omitempty"`
 	PostDeploymentTasks       []string           `json:"postDeploymentTasks,omitempty"`
@@ -77,7 +78,7 @@ func init() {
 }
 
 func (a KeptnApp) GetAppVersionName() string {
-	return strings.ToLower(a.Name + "-" + a.Spec.Version)
+	return strings.ToLower(fmt.Sprintf("%s-%s-%d", a.Name, a.Spec.Version, a.Generation))
 }
 
 func (a KeptnApp) SetSpanAttributes(span trace.Span) {

operator/api/v1alpha2/keptnappversion_test.go

@@ -413,15 +413,29 @@ func TestKeptnAppVersionList(t *testing.T) {
 				ObjectMeta: metav1.ObjectMeta{
 					Name: "obj1",
 				},
+				Status: KeptnAppVersionStatus{
+					Status: common.StateSucceeded,
+				},
 			},
 			{
 				ObjectMeta: metav1.ObjectMeta{
 					Name: "obj2",
 				},
+				Status: KeptnAppVersionStatus{
+					Status: common.StateDeprecated,
+				},
 			},
 		},
 	}
 
+	// fetch the list items
 	got := list.GetItems()
 	require.Len(t, got, 2)
+
+	// remove deprecated items from the list
+	list.RemoveDeprecated()
+
+	// check that deprecated items are not present in the list anymore
+	got = list.GetItems()
+	require.Len(t, got, 1)
 }

operator/api/v1alpha2/keptnappversion_types.go

@@ -104,13 +104,23 @@ type KeptnAppVersionList struct {
 }
 
 func (a KeptnAppVersionList) GetItems() []client.Object {
-	var b []client.Object
+	b := make([]client.Object, 0, len(a.Items))
 	for _, i := range a.Items {
 		b = append(b, &i)
 	}
 	return b
 }
 
+func (a *KeptnAppVersionList) RemoveDeprecated() {
+	b := make([]KeptnAppVersion, 0, len(a.Items))
+	for _, i := range a.Items {
+		if i.Status.Status != common.StateDeprecated {
+			b = append(b, i)
+		}
+	}
+	a.Items = b
+}
+
 func init() {
 	SchemeBuilder.Register(&KeptnAppVersion{}, &KeptnAppVersionList{})
 }

operator/api/v1alpha2/keptnevaluationprovider_types.go

@@ -17,9 +17,10 @@ limitations under the License.
 package v1alpha2
 
 import (
+	"strings"
+
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"strings"
 )
 
 // EDIT THIS FILE!  THIS IS SCAFFOLDING FOR YOU TO OWN!

operator/controllers/common/test_utils.go

@@ -10,32 +10,42 @@ import (
 	"go.opentelemetry.io/otel/metric/unit"
 	"go.opentelemetry.io/otel/sdk/metric"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/types"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 )
 
 func AddApp(c client.Client, name string) error {
 	app := &lfcv1alpha2.KeptnApp{
 		TypeMeta: metav1.TypeMeta{},
 		ObjectMeta: metav1.ObjectMeta{
-			Name:      name,
-			Namespace: "default",
+			Name:       name,
+			Namespace:  "default",
+			Generation: 1,
 		},
 		Spec: lfcv1alpha2.KeptnAppSpec{
 			Version: "1.0.0",
 		},
 		Status: lfcv1alpha2.KeptnAppStatus{},
 	}
 	return c.Create(context.TODO(), app)
+}
 
+func UpdateAppRevision(c client.Client, name string, revision uint) error {
+	app := &lfcv1alpha2.KeptnApp{}
+	c.Get(context.TODO(), types.NamespacedName{Namespace: "default", Name: name}, app)
+	app.Spec.Revision = revision
+	app.Generation = int64(revision)
+	return c.Update(context.TODO(), app)
 }
 
 func AddAppVersion(c client.Client, namespace string, appName string, version string, workloads []lfcv1alpha2.KeptnWorkloadRef, status lfcv1alpha2.KeptnAppVersionStatus) error {
 	appVersionName := fmt.Sprintf("%s-%s", appName, version)
 	app := &lfcv1alpha2.KeptnAppVersion{
 		TypeMeta: metav1.TypeMeta{},
 		ObjectMeta: metav1.ObjectMeta{
-			Name:      appVersionName,
-			Namespace: namespace,
+			Name:       appVersionName,
+			Namespace:  namespace,
+			Generation: 1,
 		},
 		Spec: lfcv1alpha2.KeptnAppVersionSpec{
 			KeptnAppSpec: lfcv1alpha2.KeptnAppSpec{

operator/controllers/keptnapp/controller.go

@@ -19,9 +19,11 @@ package keptnapp
 import (
 	"context"
 	"fmt"
+	"strconv"
 
 	"github.com/go-logr/logr"
 	klcv1alpha2 "github.com/keptn/lifecycle-toolkit/operator/api/v1alpha2"
+	"github.com/keptn/lifecycle-toolkit/operator/api/v1alpha2/common"
 	controllererrors "github.com/keptn/lifecycle-toolkit/operator/controllers/errors"
 	"github.com/keptn/lifecycle-toolkit/operator/controllers/interfaces"
 	"go.opentelemetry.io/otel"
@@ -113,6 +115,9 @@ func (r *KeptnAppReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c
 			r.Log.Error(err, "could not update Current Version of App")
 			return ctrl.Result{}, err
 		}
+		if err := r.handleGenerationBump(ctx, app); err != nil {
+			return ctrl.Result{Requeue: true}, nil
+		}
 		return ctrl.Result{}, nil
 	}
 	if err != nil {
@@ -162,3 +167,36 @@ func (r *KeptnAppReconciler) createAppVersion(ctx context.Context, app *klcv1alp
 
 	return &appVersion, err
 }
+
+func (r *KeptnAppReconciler) handleGenerationBump(ctx context.Context, app *klcv1alpha2.KeptnApp) error {
+	if app.Generation != 1 {
+		if err := r.deprecateAppVersions(ctx, app); err != nil {
+			r.Log.Error(err, "could not deprecate appVersions for appVersion %s", app.GetAppVersionName())
+			r.Recorder.Event(app, "Warning", "AppVersionNotDeprecated", fmt.Sprintf("Could not deprecate KeptnAppVersions for KeptnAppVersion / Namespace: %s, Name: %s ", app.Namespace, app.GetAppVersionName()))
+			return err
+		}
+		r.Recorder.Event(app, "Normal", "AppVersionDeprecated", fmt.Sprintf("Deprecated KeptnAppVersions for KeptnAppVersion / Namespace: %s, Name: %s ", app.Namespace, app.GetAppVersionName()))
+	}
+	return nil
+}
+
+func (r *KeptnAppReconciler) deprecateAppVersions(ctx context.Context, app *klcv1alpha2.KeptnApp) error {
+	var lastResultErr error
+	lastResultErr = nil
+	for i := app.Generation - 1; i > 0; i-- {
+		deprecatedAppVersion := &klcv1alpha2.KeptnAppVersion{}
+		if err := r.Get(ctx, types.NamespacedName{Namespace: app.Namespace, Name: app.Name + "-" + app.Spec.Version + "-" + strconv.FormatInt(i, 10)}, deprecatedAppVersion); err != nil {
+			if !errors.IsNotFound(err) {
+				r.Log.Error(err, fmt.Sprintf("Could not get KeptnAppVersion: %s", app.Name+"-"+app.Spec.Version+"-"+strconv.FormatInt(i, 10)))
+				lastResultErr = err
+			}
+		} else if !deprecatedAppVersion.Status.Status.IsDeprecated() {
+			deprecatedAppVersion.DeprecateRemainingPhases(common.PhaseDeprecated)
+			if err := r.Client.Status().Update(ctx, deprecatedAppVersion); err != nil {
+				r.Log.Error(err, "could not update appVersion %s status", deprecatedAppVersion.Name)
+				lastResultErr = err
+			}
+		}
+	}
+	return lastResultErr
+}

operator/controllers/keptnapp/controller_test.go

@@ -2,6 +2,7 @@ package keptnapp
 
 import (
 	"context"
+	"fmt"
 	"reflect"
 	"testing"
 
@@ -27,8 +28,9 @@ func TestKeptnAppReconciler_createAppVersionSuccess(t *testing.T) {
 	app := &lfcv1alpha2.KeptnApp{
 		TypeMeta: metav1.TypeMeta{},
 		ObjectMeta: metav1.ObjectMeta{
-			Name:      "my-app",
-			Namespace: "default",
+			Name:       "my-app",
+			Namespace:  "default",
+			Generation: 1,
 		},
 		Spec: lfcv1alpha2.KeptnAppSpec{
 			Version: "1.0.0",
@@ -43,8 +45,7 @@ func TestKeptnAppReconciler_createAppVersionSuccess(t *testing.T) {
 	}
 	t.Log("Verifying created app")
 	assert.Equal(t, appVersion.Namespace, app.Namespace)
-	assert.Equal(t, appVersion.Name, app.Name+"-"+app.Spec.Version)
-
+	assert.Equal(t, appVersion.Name, fmt.Sprintf("%s-%s-%d", app.Name, app.Spec.Version, app.Generation))
 }
 
 func TestKeptnAppReconciler_reconcile(t *testing.T) {
@@ -96,7 +97,7 @@ func TestKeptnAppReconciler_reconcile(t *testing.T) {
 	require.Nil(t, err)
 	err = controllercommon.AddApp(r.Client, "myfinishedapp")
 	require.Nil(t, err)
-	err = controllercommon.AddAppVersion(r.Client, "default", "myfinishedapp", "1.0.0", nil, lfcv1alpha2.KeptnAppVersionStatus{Status: apicommon.StateSucceeded})
+	err = controllercommon.AddAppVersion(r.Client, "default", "myfinishedapp", "1.0.0-1", nil, lfcv1alpha2.KeptnAppVersionStatus{Status: apicommon.StateSucceeded})
 	require.Nil(t, err)
 
 	for _, tt := range tests {
@@ -121,12 +122,49 @@ func TestKeptnAppReconciler_reconcile(t *testing.T) {
 	// case 1 reconcile and create app ver
 	assert.Equal(t, tracer.StartCalls()[0].SpanName, "reconcile_app")
 	assert.Equal(t, tracer.StartCalls()[1].SpanName, "create_app_version")
-	assert.Equal(t, tracer.StartCalls()[2].SpanName, "myapp-1.0.0")
+	assert.Equal(t, tracer.StartCalls()[2].SpanName, "myapp-1.0.0-1")
 	//case 2 creates no span because notfound
 	//case 3 reconcile finished crd
 	assert.Equal(t, tracer.StartCalls()[3].SpanName, "reconcile_app")
 }
 
+func TestKeptnAppReconciler_deprecateAppVersions(t *testing.T) {
+	r, eventChannel, _ := setupReconciler(t)
+
+	err := controllercommon.AddApp(r.Client, "myapp")
+	require.Nil(t, err)
+
+	_, err = r.Reconcile(context.TODO(), ctrl.Request{
+		NamespacedName: types.NamespacedName{
+			Namespace: "default",
+			Name:      "myapp",
+		},
+	})
+
+	require.Nil(t, err)
+
+	event := <-eventChannel
+	assert.Matches(t, event, `Normal AppVersionCreated Created KeptnAppVersion / Namespace: default, Name: myapp-1.0.0-1`)
+
+	err = controllercommon.UpdateAppRevision(r.Client, "myapp", 2)
+	require.Nil(t, err)
+
+	_, err = r.Reconcile(context.TODO(), ctrl.Request{
+		NamespacedName: types.NamespacedName{
+			Namespace: "default",
+			Name:      "myapp",
+		},
+	})
+
+	require.Nil(t, err)
+
+	event = <-eventChannel
+	assert.Matches(t, event, `Normal AppVersionCreated Created KeptnAppVersion / Namespace: default, Name: myapp-1.0.0-2`)
+
+	event = <-eventChannel
+	assert.Matches(t, event, `Normal AppVersionDeprecated Deprecated KeptnAppVersions for KeptnAppVersion / Namespace: default, Name: myapp-1.0.0-2`)
+}
+
 func setupReconciler(t *testing.T) (*KeptnAppReconciler, chan string, *interfacesfake.ITracerMock) {
 	//setup logger
 	opts := zap.Options{

operator/controllers/keptnworkloadinstance/controller.go

@@ -273,6 +273,10 @@ func (r *KeptnWorkloadInstanceReconciler) getAppVersionForWorkloadInstance(ctx c
 		return false, klcv1alpha2.KeptnAppVersion{}, err
 	}
 
+	// due to effectivity reasons deprecated KeptnAppVersions are removed from the list, as there is
+	// no point in iterating through them in the next steps
+	apps.RemoveDeprecated()
+
 	workloadFound, latestVersion, err := getLatestAppVersion(apps, wli)
 	if err != nil {
 		r.Log.Error(err, "could not look up KeptnAppVersion for WorkloadInstance")