pr review

Signed-off-by: Florian Bacher <florian.bacher@dynatrace.com>

operator/controllers/common/config/config.go

@@ -13,24 +13,24 @@ type IConfig interface {
 	GetCreationRequestTimeout() time.Duration
 }
 
-type Config struct {
+type ControllerConfig struct {
 	keptnAppCreationRequestTimeout time.Duration
 }
 
-var instance *Config
+var instance *ControllerConfig
 var once = sync.Once{}
 
-func Instance() *Config {
+func Instance() *ControllerConfig {
 	once.Do(func() {
-		instance = &Config{keptnAppCreationRequestTimeout: defaultKeptnAppCreationRequestTimeout}
+		instance = &ControllerConfig{keptnAppCreationRequestTimeout: defaultKeptnAppCreationRequestTimeout}
 	})
 	return instance
 }
 
-func (o *Config) SetCreationRequestTimeout(value time.Duration) {
+func (o *ControllerConfig) SetCreationRequestTimeout(value time.Duration) {
 	o.keptnAppCreationRequestTimeout = value
 }
 
-func (o *Config) GetCreationRequestTimeout() time.Duration {
+func (o *ControllerConfig) GetCreationRequestTimeout() time.Duration {
 	return o.keptnAppCreationRequestTimeout
 }

operator/controllers/lifecycle/keptnappcreationrequest/controller.go

@@ -110,16 +110,9 @@ func (r *KeptnAppCreationRequestReconciler) Reconcile(ctx context.Context, req c
 	// check if discovery deadline has expired or if the application is a single service app
 	if !r.shouldCreateApp(creationRequest) {
 		r.Log.Info("Discovery deadline not expired yet", "KeptnAppCreationRequest", creationRequest)
-		return ctrl.Result{RequeueAfter: 10 * time.Second}, nil
+		return ctrl.Result{RequeueAfter: r.getCreationRequestExpirationDuration(creationRequest)}, nil
 	}
 
-	// at this point we know that the creation request will be deleted at the end
-	defer func() {
-		if err := r.Delete(ctx, creationRequest); err != nil {
-			r.Log.Error(err, "Could not delete", "KeptnAppCreationRequest", creationRequest)
-		}
-	}()
-
 	// look up all the KeptnWorkloads referencing the KeptnApp
 	workloads := &lifecycle.KeptnWorkloadList{}
 	if err := r.Client.List(ctx, workloads, client.InNamespace(creationRequest.Namespace), client.MatchingFields{
@@ -138,12 +131,27 @@ func (r *KeptnAppCreationRequestReconciler) Reconcile(ctx context.Context, req c
 	if err != nil {
 		return ctrl.Result{}, fmt.Errorf("could not update: %w", err)
 	}
-
+	if err := r.Delete(ctx, creationRequest); err != nil {
+		r.Log.Error(err, "Could not delete", "KeptnAppCreationRequest", creationRequest)
+	}
 	return ctrl.Result{}, nil
 }
 
+func (r *KeptnAppCreationRequestReconciler) getCreationRequestExpirationDuration(cr *lifecycle.KeptnAppCreationRequest) time.Duration {
+	creationRequestTimeout := r.config.GetCreationRequestTimeout()
+	deadline := cr.CreationTimestamp.Add(creationRequestTimeout)
+
+	duration := deadline.Sub(r.clock.Now())
+
+	// make sure we return a non-negative duration
+	if duration >= 0 {
+		return duration
+	}
+	return 0
+}
+
 func (r *KeptnAppCreationRequestReconciler) shouldCreateApp(creationRequest *lifecycle.KeptnAppCreationRequest) bool {
-	discoveryDeadline := config.Instance().GetCreationRequestTimeout()
+	discoveryDeadline := r.config.GetCreationRequestTimeout()
 	return creationRequest.IsSingleService() || r.clock.Now().After(creationRequest.CreationTimestamp.Add(discoveryDeadline))
 }
 

operator/controllers/lifecycle/keptnappcreationrequest/controller_test.go

@@ -2,6 +2,7 @@ package keptnappcreationrequest
 
 import (
 	"context"
+	"github.com/keptn/lifecycle-toolkit/operator/controllers/common/config/fake"
 	"testing"
 	"time"
 
@@ -20,6 +21,7 @@ import (
 
 func TestKeptnAppCreationRequestReconciler_CreateAppAfterTimeout(t *testing.T) {
 	r, fakeClient, theClock := setupReconcilerAndClient(t)
+
 	const namespace = "my-namespace"
 	const appName = "my-app"
 	kacr := &klcv1alpha3.KeptnAppCreationRequest{
@@ -73,7 +75,7 @@ func TestKeptnAppCreationRequestReconciler_CreateAppAfterTimeout(t *testing.T) {
 	res, err := r.Reconcile(context.TODO(), request)
 
 	require.Nil(t, err)
-	require.NotZero(t, res.RequeueAfter)
+	require.Equal(t, 30*time.Second, res.RequeueAfter)
 
 	// turn the clock forward
 	theClock.Add(1 * time.Minute)
@@ -329,6 +331,11 @@ func setupReconcilerAndClient(t *testing.T) (*KeptnAppCreationRequestReconciler,
 		Scheme: fakeClient.Scheme(),
 		Log:    logr.Logger{},
 		clock:  theClock,
+		config: &fake.MockConfig{
+			GetCreationRequestTimeoutFunc: func() time.Duration {
+				return 30 * time.Second
+			},
+		},
 	}
 	return r, fakeClient, theClock
 }