fix(GcpNfsVolumeBackup): Short circuit reconciliation loop if completed

api/cloud-resources/v1beta1/gcpnfsvolumebackup_types.go

@@ -41,6 +41,9 @@ const (
 
 	// GcpNfsBackupDeleted signifies backup delete operation is complete.
 	GcpNfsBackupDeleted GcpNfsBackupState = "Deleted"
+
+	// GcpNfsBackupFailed signifies backup operation failed, and it will not be retried again.
+	GcpNfsBackupFailed GcpNfsBackupState = "Failed"
 )
 
 type GcpNfsVolumeBackupSource struct {

pkg/skr/gcpnfsvolumebackup/markFailed.go

@@ -0,0 +1,75 @@
+package gcpnfsvolumebackup
+
+import (
+	"context"
+	"fmt"
+	"github.com/kyma-project/cloud-manager/api/cloud-resources/v1beta1"
+	"github.com/kyma-project/cloud-manager/pkg/composed"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+)
+
+func markFailed(ctx context.Context, st composed.State) (error, context.Context) {
+	state := st.(*State)
+
+	//If deletion, continue.
+	if composed.MarkedForDeletionPredicate(ctx, st) {
+		return nil, nil
+	}
+
+	backup := state.ObjAsGcpNfsVolumeBackup()
+	backupState := backup.Status.State
+
+	//If not in error state, continue
+	if backupState != v1beta1.GcpNfsBackupError {
+		return nil, ctx
+	}
+
+	//If this backup doesn't belong to a schedule, continue
+	scheduleName, exists := backup.GetLabels()[v1beta1.LabelScheduleName]
+	if !exists {
+		return nil, ctx
+	}
+
+	//createdOn := backup.GetCreationTimestamp().Format(time.RFC3339)
+	list := &v1beta1.GcpNfsVolumeBackupList{}
+
+	//List subsequent backups for this schedule.
+	err := state.SkrCluster.K8sClient().List(
+		ctx,
+		list,
+		client.MatchingLabels{
+			v1beta1.LabelScheduleName:      scheduleName,
+			v1beta1.LabelScheduleNamespace: backup.GetNamespace(),
+		},
+		client.InNamespace(backup.GetNamespace()),
+	)
+
+	if err != nil {
+		return composed.PatchStatus(backup).
+			SetExclusiveConditions(metav1.Condition{
+				Type:    v1beta1.ConditionTypeError,
+				Status:  metav1.ConditionTrue,
+				Reason:  v1beta1.ReasonBackupListFailed,
+				Message: fmt.Sprintf("Error listing subsequent backup(s) : %s", err.Error()),
+			}).
+			SuccessError(composed.StopWithRequeue).
+			Run(ctx, state)
+	}
+
+	//If there are subsequent backups exist,
+	//mark this backup object state as failed.
+	for _, item := range list.Items {
+
+		if item.CreationTimestamp.Time.After(backup.CreationTimestamp.Time) {
+			backup.Status.State = v1beta1.GcpNfsBackupFailed
+			return composed.PatchStatus(backup).
+				SuccessLogMsg("GcpNfsVolumeBackup status updated with Failed state. ").
+				SuccessError(composed.StopAndForget).
+				Run(ctx, state)
+		}
+	}
+
+	//continue if there are no subsequent backups exist
+	return nil, ctx
+}

pkg/skr/gcpnfsvolumebackup/markFailed_test.go

@@ -0,0 +1,227 @@
+package gcpnfsvolumebackup
+
+import (
+	"context"
+	"github.com/go-logr/logr"
+	"github.com/kyma-project/cloud-manager/api/cloud-resources/v1beta1"
+	"github.com/kyma-project/cloud-manager/pkg/composed"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/suite"
+	v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/types"
+	"net/http"
+	"net/http/httptest"
+	"sigs.k8s.io/controller-runtime/pkg/log"
+	"testing"
+	"time"
+)
+
+type markFailedSuite struct {
+	suite.Suite
+	ctx context.Context
+}
+
+func (suite *markFailedSuite) SetupTest() {
+	suite.ctx = log.IntoContext(context.Background(), logr.Discard())
+}
+
+func (suite *markFailedSuite) TestWhenBackupIsDeleting() {
+	fakeHttpServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		assert.Fail(suite.T(), "unexpected request: "+r.URL.String())
+	}))
+	obj := deletingGpNfsVolumeBackup.DeepCopy()
+	factory, err := newTestStateFactoryWithObj(fakeHttpServer, obj)
+	suite.Nil(err)
+
+	ctx, cancel := context.WithCancel(context.Background())
+	defer cancel()
+	//Get state object with GcpNfsVolume
+	state, err := factory.newStateWith(obj)
+	suite.Nil(err)
+
+	err, _ctx := markFailed(ctx, state)
+
+	//validate expected return values
+	suite.Nil(err)
+	suite.Nil(_ctx)
+}
+
+func (suite *markFailedSuite) TestWhenBackupIsReady() {
+	fakeHttpServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		assert.Fail(suite.T(), "unexpected request: "+r.URL.String())
+	}))
+	obj := gcpNfsVolumeBackup.DeepCopy()
+	obj.Status.State = v1beta1.GcpNfsBackupReady
+	factory, err := newTestStateFactoryWithObj(fakeHttpServer, obj)
+	suite.Nil(err)
+
+	ctx, cancel := context.WithCancel(context.Background())
+	defer cancel()
+	//Get state object with GcpNfsVolume
+	state, err := factory.newStateWith(obj)
+	suite.Nil(err)
+
+	err, _ctx := markFailed(ctx, state)
+
+	//validate expected return values
+	suite.Nil(err)
+	suite.Equal(ctx, _ctx)
+
+	fromK8s := &v1beta1.GcpNfsVolumeBackup{}
+	err = factory.skrCluster.K8sClient().Get(ctx,
+		types.NamespacedName{Name: gcpNfsVolumeBackup.Name,
+			Namespace: gcpNfsVolumeBackup.Namespace},
+		fromK8s)
+	suite.Nil(err)
+
+	suite.Equal(v1beta1.GcpNfsBackupReady, fromK8s.Status.State)
+}
+
+func (suite *markFailedSuite) TestWhenBackupIsFailed() {
+	fakeHttpServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		assert.Fail(suite.T(), "unexpected request: "+r.URL.String())
+	}))
+	obj := gcpNfsVolumeBackup.DeepCopy()
+	obj.Status.State = v1beta1.GcpNfsBackupFailed
+	factory, err := newTestStateFactoryWithObj(fakeHttpServer, obj)
+	suite.Nil(err)
+
+	ctx, cancel := context.WithCancel(context.Background())
+	defer cancel()
+	//Get state object with GcpNfsVolume
+	state, err := factory.newStateWith(obj)
+	suite.Nil(err)
+
+	err, _ctx := markFailed(ctx, state)
+
+	//validate expected return values
+	suite.Nil(err)
+	suite.Equal(ctx, _ctx)
+
+	fromK8s := &v1beta1.GcpNfsVolumeBackup{}
+	err = factory.skrCluster.K8sClient().Get(ctx,
+		types.NamespacedName{Name: gcpNfsVolumeBackup.Name,
+			Namespace: gcpNfsVolumeBackup.Namespace},
+		fromK8s)
+	suite.Nil(err)
+
+	suite.Equal(v1beta1.GcpNfsBackupFailed, fromK8s.Status.State)
+}
+
+func (suite *markFailedSuite) TestWhenBackupIsCreating() {
+	fakeHttpServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		assert.Fail(suite.T(), "unexpected request: "+r.URL.String())
+	}))
+	obj := gcpNfsVolumeBackup.DeepCopy()
+	obj.Status.State = v1beta1.GcpNfsBackupCreating
+	factory, err := newTestStateFactoryWithObj(fakeHttpServer, obj)
+	suite.Nil(err)
+
+	ctx, cancel := context.WithCancel(context.Background())
+	defer cancel()
+	//Get state object with GcpNfsVolume
+	state, err := factory.newStateWith(obj)
+	suite.Nil(err)
+
+	err, _ctx := markFailed(ctx, state)
+
+	//validate expected return values
+	suite.Nil(err)
+	suite.Equal(ctx, _ctx)
+
+	fromK8s := &v1beta1.GcpNfsVolumeBackup{}
+	err = factory.skrCluster.K8sClient().Get(ctx,
+		types.NamespacedName{Name: gcpNfsVolumeBackup.Name,
+			Namespace: gcpNfsVolumeBackup.Namespace},
+		fromK8s)
+	suite.Nil(err)
+
+	suite.Equal(v1beta1.GcpNfsBackupCreating, fromK8s.Status.State)
+}
+
+func (suite *markFailedSuite) TestWhenBackupIsLatestAndInError() {
+	fakeHttpServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		assert.Fail(suite.T(), "unexpected request: "+r.URL.String())
+	}))
+	obj := gcpNfsVolumeBackup.DeepCopy()
+	obj.Status.State = v1beta1.GcpNfsBackupError
+	factory, err := newTestStateFactoryWithObj(fakeHttpServer, obj)
+	suite.Nil(err)
+
+	ctx, cancel := context.WithCancel(context.Background())
+	defer cancel()
+	//Get state object with GcpNfsVolume
+	state, err := factory.newStateWith(obj)
+	suite.Nil(err)
+
+	err, _ctx := markFailed(ctx, state)
+
+	//validate expected return values
+	suite.Nil(err)
+	suite.Equal(ctx, _ctx)
+
+	fromK8s := &v1beta1.GcpNfsVolumeBackup{}
+	err = factory.skrCluster.K8sClient().Get(ctx,
+		types.NamespacedName{Name: gcpNfsVolumeBackup.Name,
+			Namespace: gcpNfsVolumeBackup.Namespace},
+		fromK8s)
+	suite.Nil(err)
+
+	suite.Equal(v1beta1.GcpNfsBackupError, fromK8s.Status.State)
+}
+
+func (suite *markFailedSuite) TestWhenBackupIsNotLatestAndInError() {
+	fakeHttpServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		assert.Fail(suite.T(), "unexpected request: "+r.URL.String())
+	}))
+	labels := map[string]string{
+		v1beta1.LabelScheduleName:      "test-schedule",
+		v1beta1.LabelScheduleNamespace: "test",
+	}
+
+	obj := gcpNfsVolumeBackup.DeepCopy()
+	obj.CreationTimestamp = v1.Time{Time: time.Now().Add(-1 * time.Minute)}
+	obj.Labels = labels
+	factory, err := newTestStateFactoryWithObj(fakeHttpServer, obj)
+	suite.Nil(err)
+
+	ctx, cancel := context.WithCancel(context.Background())
+	defer cancel()
+
+	obj.Status.State = v1beta1.GcpNfsBackupError
+	err = factory.skrCluster.K8sClient().Status().Update(ctx, obj)
+	suite.Nil(err)
+
+	//Get state object with GcpNfsVolume
+	state, err := factory.newStateWith(obj)
+	suite.Nil(err)
+
+	//Create another backup object for the same schedule
+	obj2 := gcpNfsVolumeBackup.DeepCopy()
+	obj2.Name = "test-backup-02"
+	obj2.Namespace = "test"
+	obj2.CreationTimestamp = v1.Time{Time: time.Now()}
+	obj2.Labels = labels
+	obj2.Status.State = v1beta1.GcpNfsBackupReady
+	err = factory.skrCluster.K8sClient().Create(ctx, obj2)
+	suite.Nil(err)
+
+	err, _ctx := markFailed(ctx, state)
+
+	//validate expected return values
+	suite.Equal(composed.StopAndForget, err)
+	suite.Equal(ctx, _ctx)
+
+	fromK8s := &v1beta1.GcpNfsVolumeBackup{}
+	err = factory.skrCluster.K8sClient().Get(ctx,
+		types.NamespacedName{Name: obj.Name,
+			Namespace: obj.Namespace},
+		fromK8s)
+	suite.Nil(err)
+
+	suite.Equal(v1beta1.GcpNfsBackupFailed, fromK8s.Status.State)
+}
+
+func TestMarkFailed(t *testing.T) {
+	suite.Run(t, new(markFailedSuite))
+}

pkg/skr/gcpnfsvolumebackup/shortCircuit.go

@@ -0,0 +1,25 @@
+package gcpnfsvolumebackup
+
+import (
+	"context"
+	"github.com/kyma-project/cloud-manager/api/cloud-resources/v1beta1"
+	"github.com/kyma-project/cloud-manager/pkg/composed"
+)
+
+func shortCircuitCompleted(ctx context.Context, st composed.State) (error, context.Context) {
+	state := st.(*State)
+
+	//If deletion, continue.
+	if composed.MarkedForDeletionPredicate(ctx, st) {
+		return nil, nil
+	}
+
+	backup := state.ObjAsGcpNfsVolumeBackup()
+	backupState := backup.Status.State
+	if backupState == v1beta1.GcpNfsBackupReady || backupState == v1beta1.GcpNfsBackupFailed {
+		composed.LoggerFromCtx(ctx).Info("NfsVolumeBackup is complete , short-circuiting into StopAndForget")
+		return composed.StopAndForget, nil
+	}
+
+	return nil, ctx
+}