feat: preventing zombie filestore backups and patching status instead…

… of update

.github/workflows/pr-checks.yml

@@ -9,6 +9,7 @@ jobs:
     strategy:
       matrix:
         ipRangeAutomaticCidrAllocation: [ "false", "true" ]
+        gcpNfsVolumeAutomaticLocationAllocation: [ "false", "true" ]
     runs-on: ubuntu-latest
     steps:
       - name: Checkout repository
@@ -22,6 +23,7 @@ jobs:
       - name: Build and test
         env:
           FF_IP_RANGE_AUTOMATIC_CIDR_ALLOCATION: ${{ matrix.ipRangeAutomaticCidrAllocation }}
+          FF_GCP_NFS_VOLUME_AUTOMATIC_LOCATION_ALLOCATION: ${{ matrix.gcpNfsVolumeAutomaticLocationAllocation }}
         run: |
           ./config/sync.sh
           go mod tidy

api/cloud-resources/v1beta1/gcpnfsbackupschedule_types.go

@@ -20,6 +20,7 @@ import (
 	featuretypes "github.com/kyma-project/cloud-manager/pkg/feature/types"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"sigs.k8s.io/controller-runtime/pkg/client"
 )
 
 // EDIT THIS FILE!  THIS IS SCAFFOLDING FOR YOU TO OWN!
@@ -280,3 +281,17 @@ type GcpNfsBackupScheduleList struct {
 func init() {
 	SchemeBuilder.Register(&GcpNfsBackupSchedule{}, &GcpNfsBackupScheduleList{})
 }
+
+func (sc *GcpNfsBackupSchedule) CloneForPatchStatus() client.Object {
+	return &GcpNfsBackupSchedule{
+		TypeMeta: metav1.TypeMeta{
+			Kind:       "GcpNfsBackupSchedule",
+			APIVersion: GroupVersion.String(),
+		},
+		ObjectMeta: metav1.ObjectMeta{
+			Namespace: sc.Namespace,
+			Name:      sc.Name,
+		},
+		Status: sc.Status,
+	}
+}

api/cloud-resources/v1beta1/gcpnfsvolumebackup_types.go

@@ -20,6 +20,7 @@ import (
 	featuretypes "github.com/kyma-project/cloud-manager/pkg/feature/types"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
+	"sigs.k8s.io/controller-runtime/pkg/client"
 )
 
 type GcpNfsBackupState string
@@ -32,9 +33,6 @@ const (
 	// GcpNfsBackupError signifies the backup operation resulted in error.
 	GcpNfsBackupError GcpNfsBackupState = "Error"
 
-	// GcpNfsBackupProcessing signifies backup operation is in-progress.
-	GcpNfsBackupProcessing GcpNfsBackupState = "Processing"
-
 	// GcpNfsBackupCreating signifies backup create operation is in-progress.
 	GcpNfsBackupCreating GcpNfsBackupState = "Creating"
 
@@ -62,7 +60,7 @@ type GcpNfsVolumeRef struct {
 	Namespace string `json:"namespace,omitempty"`
 }
 
-func (v GcpNfsVolumeRef) ToNamespacedName(fallbackNamespace string) types.NamespacedName {
+func (v *GcpNfsVolumeRef) ToNamespacedName(fallbackNamespace string) types.NamespacedName {
 	ns := v.Namespace
 	if len(ns) == 0 {
 		ns = fallbackNamespace
@@ -158,3 +156,17 @@ type GcpNfsVolumeBackupList struct {
 func init() {
 	SchemeBuilder.Register(&GcpNfsVolumeBackup{}, &GcpNfsVolumeBackupList{})
 }
+
+func (in *GcpNfsVolumeBackup) CloneForPatchStatus() client.Object {
+	return &GcpNfsVolumeBackup{
+		TypeMeta: metav1.TypeMeta{
+			Kind:       "GcpNfsVolumeBackup",
+			APIVersion: GroupVersion.String(),
+		},
+		ObjectMeta: metav1.ObjectMeta{
+			Namespace: in.Namespace,
+			Name:      in.Name,
+		},
+		Status: in.Status,
+	}
+}

api/cloud-resources/v1beta1/gcpnfsvolumerestore_types.go

@@ -20,6 +20,7 @@ import (
 	featuretypes "github.com/kyma-project/cloud-manager/pkg/feature/types"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
+	"sigs.k8s.io/controller-runtime/pkg/client"
 )
 
 type GcpNfsVolumeBackupRef struct {
@@ -31,7 +32,7 @@ type GcpNfsVolumeBackupRef struct {
 	Namespace string `json:"namespace"`
 }
 
-func (v GcpNfsVolumeBackupRef) ToNamespacedName(fallbackNamespace string) types.NamespacedName {
+func (v *GcpNfsVolumeBackupRef) ToNamespacedName(fallbackNamespace string) types.NamespacedName {
 	ns := v.Namespace
 	if len(ns) == 0 {
 		ns = fallbackNamespace
@@ -125,3 +126,17 @@ type GcpNfsVolumeRestoreList struct {
 func init() {
 	SchemeBuilder.Register(&GcpNfsVolumeRestore{}, &GcpNfsVolumeRestoreList{})
 }
+
+func (in *GcpNfsVolumeRestore) CloneForPatchStatus() client.Object {
+	return &GcpNfsVolumeRestore{
+		TypeMeta: metav1.TypeMeta{
+			Kind:       "GcpNfsVolumeRestore",
+			APIVersion: GroupVersion.String(),
+		},
+		ObjectMeta: metav1.ObjectMeta{
+			Namespace: in.Namespace,
+			Name:      in.Name,
+		},
+		Status: in.Status,
+	}
+}

internal/controller/cloud-resources/gcpnfsbackupschedule_test.go

@@ -24,8 +24,8 @@ var _ = Describe("Feature: SKR GcpNfsBackupSchedule", func() {
 		timeout = time.Second * 20
 	)
 	now := time.Now().UTC()
-	skrIpRangeName := "gcp-iprange-1"
-	skrNfsVolumeName := "gcp-nfs-1"
+	skrIpRangeName := "gcp-iprange-1-bs"
+	skrNfsVolumeName := "gcp-nfs-1-bs"
 	skrNfsVolume := &cloudresourcesv1beta1.GcpNfsVolume{}
 	scope := &cloudcontrolv1beta1.Scope{}
 
@@ -83,7 +83,7 @@ var _ = Describe("Feature: SKR GcpNfsBackupSchedule", func() {
 		nfsBackupHourlySchedule := "0 * * * *"
 		nfsBackupLocation := "us-west1"
 
-		nfsBackup1Name := "gcp-nfs-backup-1"
+		nfsBackup1Name := "gcp-nfs-backup-1-bs"
 
 		expectedTimes := []time.Time{
 			time.Date(now.Year(), now.Month(), now.Day(), now.Hour()+1, 0, 0, 0, now.Location()).UTC(),

internal/controller/cloud-resources/gcpnfsvolume_test.go

@@ -1,8 +1,9 @@
 package cloudresources
 
 import (
+	"context"
 	"fmt"
-	"os"
+	"github.com/kyma-project/cloud-manager/pkg/feature"
 	"time"
 
 	cloudcontrolv1beta1 "github.com/kyma-project/cloud-manager/api/cloud-control/v1beta1"
@@ -26,14 +27,19 @@ var _ = Describe("Feature: SKR GcpNfsVolume", func() {
 		timeout = time.Second * 20
 	)
 
-	skrIpRangeName := "gcp-iprange-1"
+	skrIpRangeName := "gcp-iprange-1-v"
 	skrIpRange := &cloudresourcesv1beta1.IpRange{}
 	kcpIpRangeName := "513f20b4-7b73-4246-9397-f8dd55344479"
 	kcpIpRange := &cloudcontrolv1beta1.IpRange{}
 
+	shouldSkipIfGcpNfsVolumeAutomaticLocationAllocationDisabled := func() (bool, string) {
+		if feature.GcpNfsVolumeAutomaticLocationAllocation.Value(context.Background()) {
+			return false, ""
+		}
+		return true, "gcpNfsVolumeAutomaticLocationAllocation is disabled"
+	}
+
 	BeforeEach(func() {
-		err := os.Unsetenv("FF_GCP_NFS_VOLUME_AUTOMATIC_LOCATION_ALLOCATION")
-		Expect(err).ToNot(HaveOccurred())
 		By("And Given SKR namespace exists", func() {
 			//Create namespace if it doesn't exist.
 			Eventually(CreateNamespace).
@@ -100,9 +106,9 @@ var _ = Describe("Feature: SKR GcpNfsVolume", func() {
 		pv := &corev1.PersistentVolume{}
 
 		gcpNfsVolumeName := "gcp-nfs-volume-1"
-		nfsIpAddress := "10.11.12.14"
+		nfsIpAddress := "10.11.12.11"
 		pvSpec := &cloudresourcesv1beta1.GcpNfsVolumePvSpec{
-			Name: "gcp-nfs-pv",
+			Name: "gcp-nfs-pv-1",
 			Labels: map[string]string{
 				"app": "gcp-nfs",
 			},
@@ -301,7 +307,7 @@ var _ = Describe("Feature: SKR GcpNfsVolume", func() {
 		pv := &corev1.PersistentVolume{}
 
 		gcpNfsVolumeName := "gcp-nfs-volume-2"
-		nfsIpAddress := "10.11.12.16"
+		nfsIpAddress := "10.11.12.12"
 		updatedCapacityGb := 1024
 
 		prevPv := &corev1.PersistentVolume{}
@@ -551,7 +557,7 @@ var _ = Describe("Feature: SKR GcpNfsVolume", func() {
 		pvc := &corev1.PersistentVolumeClaim{}
 
 		gcpNfsVolumeName := "gcp-nfs-volume-3"
-		nfsIpAddress := "10.11.12.16"
+		nfsIpAddress := "10.11.12.13"
 
 		BeforeEach(func() {
 			By("And Given SKR GcpNfsVolume exists", func() {
@@ -678,14 +684,12 @@ var _ = Describe("Feature: SKR GcpNfsVolume", func() {
 		kcpNfsInstance := &cloudcontrolv1beta1.NfsInstance{}
 		pv := &corev1.PersistentVolume{}
 		scope := &cloudcontrolv1beta1.Scope{}
-		err := os.Setenv("FF_GCP_NFS_VOLUME_AUTOMATIC_LOCATION_ALLOCATION", "true")
-		Expect(err).ToNot(HaveOccurred())
 		gcpNfsVolumeName := "gcp-nfs-volume-4"
 		nfsIpAddress := "10.11.12.14"
 		pvSpec := &cloudresourcesv1beta1.GcpNfsVolumePvSpec{
-			Name: "gcp-nfs-pv-3",
+			Name: "gcp-nfs-pv-4",
 			Labels: map[string]string{
-				"app": "gcp-nfs-3",
+				"app": "gcp-nfs-4",
 			},
 			Annotations: map[string]string{
 				"volume": "gcp-nfs-volume-4",
@@ -694,7 +698,7 @@ var _ = Describe("Feature: SKR GcpNfsVolume", func() {
 
 		pvc := &corev1.PersistentVolumeClaim{}
 		pvcSpec := &cloudresourcesv1beta1.GcpNfsVolumePvcSpec{
-			Name: "gcp-nfs-pvc-3",
+			Name: "gcp-nfs-pvc-4",
 			Labels: map[string]string{
 				"foo": "bar",
 			},
@@ -703,6 +707,10 @@ var _ = Describe("Feature: SKR GcpNfsVolume", func() {
 			},
 		}
 		BeforeEach(func() {
+			shouldSkip, msg := shouldSkipIfGcpNfsVolumeAutomaticLocationAllocationDisabled()
+			if shouldSkip {
+				Skip(msg)
+			}
 			By("Given KCP Scope exists", func() {
 
 				// Given Scope exists
@@ -725,7 +733,7 @@ var _ = Describe("Feature: SKR GcpNfsVolume", func() {
 					infra.Ctx(), infra.SKR().Client(), gcpNfsVolume,
 					WithName(gcpNfsVolumeName),
 					WithGcpNfsVolumeIpRange(skrIpRange.Name),
-					WithGcpNfsVolumeLocation(""),
+					WithGcpNfsVolumeSpecLocation(""),
 					WithPvSpec(pvSpec),
 					WithPvcSpec(pvcSpec),
 				).
@@ -898,18 +906,16 @@ var _ = Describe("Feature: SKR GcpNfsVolume", func() {
 		kcpNfsInstance := &cloudcontrolv1beta1.NfsInstance{}
 		pv := &corev1.PersistentVolume{}
 		scope := &cloudcontrolv1beta1.Scope{}
-		err := os.Setenv("FF_GCP_NFS_VOLUME_AUTOMATIC_LOCATION_ALLOCATION", "true")
-		Expect(err).ToNot(HaveOccurred())
 
 		gcpNfsVolumeName := "gcp-nfs-volume-5"
-		nfsIpAddress := "10.11.12.16"
+		nfsIpAddress := "10.11.12.15"
 		updatedCapacityGb := 1024
 
 		prevPv := &corev1.PersistentVolume{}
 		pvSpec := &cloudresourcesv1beta1.GcpNfsVolumePvSpec{
-			Name: "gcp-nfs-pv-4",
+			Name: "gcp-nfs-pv-5",
 			Labels: map[string]string{
-				"app": "gcp-nfs-4",
+				"app": "gcp-nfs-5",
 			},
 			Annotations: map[string]string{
 				"volume": "gcp-nfs-volume-5",
@@ -918,7 +924,7 @@ var _ = Describe("Feature: SKR GcpNfsVolume", func() {
 
 		pvc := &corev1.PersistentVolumeClaim{}
 		prevPvcSpec := &cloudresourcesv1beta1.GcpNfsVolumePvcSpec{
-			Name: "gcp-nfs-pvc-4",
+			Name: "gcp-nfs-pvc-5",
 			Labels: map[string]string{
 				"foo": "bar",
 			},
@@ -927,7 +933,7 @@ var _ = Describe("Feature: SKR GcpNfsVolume", func() {
 			},
 		}
 		pvcSpec := &cloudresourcesv1beta1.GcpNfsVolumePvcSpec{
-			Name: "gcp-nfs-pvc-4",
+			Name: "gcp-nfs-pvc-5",
 			Labels: map[string]string{
 				"foo":  "bar-changed",
 				"foo2": "bar2",
@@ -939,6 +945,10 @@ var _ = Describe("Feature: SKR GcpNfsVolume", func() {
 		}
 
 		BeforeEach(func() {
+			shouldSkip, msg := shouldSkipIfGcpNfsVolumeAutomaticLocationAllocationDisabled()
+			if shouldSkip {
+				Skip(msg)
+			}
 			By("Given KCP Scope exists", func() {
 
 				// Given Scope exists
@@ -1022,7 +1032,6 @@ var _ = Describe("Feature: SKR GcpNfsVolume", func() {
 		})
 
 		It("When SKR GcpNfsVolume Update is called ", func() {
-
 			Eventually(Update).
 				WithArguments(
 					infra.Ctx(), infra.SKR().Client(), gcpNfsVolume,
@@ -1165,13 +1174,15 @@ var _ = Describe("Feature: SKR GcpNfsVolume", func() {
 		pv := &corev1.PersistentVolume{}
 		pvc := &corev1.PersistentVolumeClaim{}
 		scope := &cloudcontrolv1beta1.Scope{}
-		err := os.Setenv("FF_GCP_NFS_VOLUME_AUTOMATIC_LOCATION_ALLOCATION", "true")
-		Expect(err).ToNot(HaveOccurred())
 
 		gcpNfsVolumeName := "gcp-nfs-volume-6"
 		nfsIpAddress := "10.11.12.16"
 
 		BeforeEach(func() {
+			shouldSkip, msg := shouldSkipIfGcpNfsVolumeAutomaticLocationAllocationDisabled()
+			if shouldSkip {
+				Skip(msg)
+			}
 			By("Given KCP Scope exists", func() {
 
 				// Given Scope exists
@@ -1253,7 +1264,6 @@ var _ = Describe("Feature: SKR GcpNfsVolume", func() {
 			})
 		})
 		It("When SKR GcpNfsVolume Delete is called ", func() {
-
 			//Delete SKR GcpNfsVolume
 			Eventually(Delete).
 				WithArguments(