[Feature] Fix backup retries (#1435)

CHANGELOG.md

@@ -7,6 +7,7 @@
 - (Maintenance) Remove support for RELATED_IMAGE_UBI, RELATED_IMAGE_DATABASE and RELATED_IMAGE_METRICSEXPORTER env vars
 - (Bugfix) Fix numactl options
 - (Maintenance) Bump Go to 1.20.10
+- (Bugfix) Fix ArangoBackup Create Backoff & ArangoBackupPolicy propagation
  
 ## [1.2.33](https://github.com/arangodb/kube-arangodb/tree/1.2.33) (2023-09-27)
 - (Maintenance) Bump golang.org/x/net to v0.13.0

Makefile

@@ -352,6 +352,7 @@ update-vendor:
 	@rm -Rf $(VENDORDIR)/k8s.io/code-generator
 	@git clone --branch "kubernetes-1.$(KUBERNETES_VERSION_MINOR).$(KUBERNETES_VERSION_PATCH)" https://github.com/kubernetes/code-generator.git $(VENDORDIR)/k8s.io/code-generator
 	@rm -Rf $(VENDORDIR)/k8s.io/code-generator/.git
+	@(cd "$(VENDORDIR)/k8s.io/code-generator"; go mod download; go mod vendor)
 
 
 .PHONY: update-generated

pkg/apis/backup/v1/backup_policy.go

@@ -1,7 +1,7 @@
 //
 // DISCLAIMER
 //
-// Copyright 2016-2022 ArangoDB GmbH, Cologne, Germany
+// Copyright 2016-2023 ArangoDB GmbH, Cologne, Germany
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -60,6 +60,8 @@ func (a *ArangoBackupPolicy) NewBackup(d *deployment.ArangoDeployment) *ArangoBa
 		},
 		Upload:     a.Spec.BackupTemplate.Upload.DeepCopy(),
 		Options:    a.Spec.BackupTemplate.Options.DeepCopy(),
+		Backoff:    a.Spec.BackupTemplate.Backoff.DeepCopy(),
+		Lifetime:   a.Spec.BackupTemplate.Lifetime.DeepCopy(),
 		PolicyName: &policyName,
 	}
 

pkg/apis/backup/v1/backup_policy_spec.go

@@ -49,4 +49,9 @@ type ArangoBackupTemplate struct {
 	Options *ArangoBackupSpecOptions `json:"options,omitempty"`
 
 	Upload *ArangoBackupSpecOperation `json:"upload,omitempty"`
+
+	Backoff *ArangoBackupSpecBackOff `json:"backoff,omitempty"`
+
+	// Lifetime is the time after which the backup will be deleted. Format: "1.5h" or "2h45m".
+	Lifetime *meta.Duration `json:"lifetime,omitempty"`
 }

pkg/apis/backup/v1/backup_spec_backoff.go

@@ -92,3 +92,7 @@ func (a *ArangoBackupSpecBackOff) Backoff(iteration int) time.Duration {
 		return time.Duration(min+int(float64(iteration)/float64(maxIterations)*float64(max-min))) * time.Second
 	}
 }
+
+func (a *ArangoBackupSpecBackOff) Enabled() bool {
+	return a != nil && a.MaxIterations != nil
+}

pkg/apis/backup/v1/zz_generated.deepcopy.go

@@ -492,6 +492,16 @@ func (in *ArangoBackupTemplate) DeepCopyInto(out *ArangoBackupTemplate) {
 		*out = new(ArangoBackupSpecOperation)
 		**out = **in
 	}
+	if in.Backoff != nil {
+		in, out := &in.Backoff, &out.Backoff
+		*out = new(ArangoBackupSpecBackOff)
+		(*in).DeepCopyInto(*out)
+	}
+	if in.Lifetime != nil {
+		in, out := &in.Lifetime, &out.Lifetime
+		*out = new(metav1.Duration)
+		**out = **in
+	}
 	return
 }
 

pkg/handlers/backup/state_create.go

@@ -21,8 +21,6 @@
 package backup
 
 import (
-	"time"
-
 	"github.com/arangodb/go-driver"
 
 	backupApi "github.com/arangodb/kube-arangodb/pkg/apis/backup/v1"
@@ -69,22 +67,3 @@ func stateCreateHandler(h *handler, backup *backupApi.ArangoBackup) (*backupApi.
 		cleanBackOff(),
 	)
 }
-
-func stateCreateErrorHandler(h *handler, backup *backupApi.ArangoBackup) (*backupApi.ArangoBackupStatus, error) {
-	// no more retries - move to failed state
-	if !backup.Status.Backoff.ShouldBackoff(backup.Spec.Backoff) {
-		return wrapUpdateStatus(backup,
-			updateStatusState(backupApi.ArangoBackupStateFailed, "out of Create retries"),
-			cleanStatusJob())
-	}
-
-	// if we should retry - move to create state
-	if backup.Status.Backoff.ShouldBackoff(backup.Spec.Backoff) && !backup.Status.Backoff.GetNext().After(time.Now()) {
-		return wrapUpdateStatus(backup,
-			updateStatusState(backupApi.ArangoBackupStateCreate, ""),
-			cleanStatusJob())
-	}
-
-	// no ready to retry - wait (do not change state)
-	return wrapUpdateStatus(backup)
-}

pkg/handlers/backup/state_create_test.go

@@ -22,10 +22,8 @@ package backup
 
 import (
 	"testing"
-	"time"
 
 	"github.com/stretchr/testify/require"
-	meta "k8s.io/apimachinery/pkg/apis/meta/v1"
 
 	"github.com/arangodb/go-driver"
 
@@ -143,71 +141,3 @@ func Test_State_Create_CreateError(t *testing.T) {
 	require.Nil(t, newObj.Status.Backup)
 	require.False(t, newObj.Status.Available)
 }
-
-func Test_State_CreateError_Retry(t *testing.T) {
-	// Arrange
-	handler, mock := newErrorsFakeHandler(mockErrorsArangoClientBackup{})
-
-	obj, deployment := newObjectSet(backupApi.ArangoBackupStateCreateError)
-
-	backupMeta, err := mock.Create()
-	require.NoError(t, err)
-
-	obj.Status.Backup = &backupApi.ArangoBackupDetails{
-		ID:                string(backupMeta.ID),
-		Version:           backupMeta.Version,
-		CreationTimestamp: meta.Now(),
-	}
-
-	obj.Status.Time.Time = time.Now().Add(-2 * downloadDelay)
-
-	// Act
-	createArangoDeployment(t, handler, deployment)
-	createArangoBackup(t, handler, obj)
-
-	require.NoError(t, handler.Handle(newItemFromBackup(operation.Update, obj)))
-
-	// Assert
-	newObj := refreshArangoBackup(t, handler, obj)
-	require.Equal(t, newObj.Status.State, backupApi.ArangoBackupStateCreate)
-	require.False(t, newObj.Status.Available)
-	require.NotNil(t, newObj.Status.Backup)
-	require.Equal(t, obj.Status.Backup, newObj.Status.Backup)
-}
-
-func Test_State_CreateError_Transfer_To_Failed(t *testing.T) {
-	// Arrange
-	handler, mock := newErrorsFakeHandler(mockErrorsArangoClientBackup{})
-
-	obj, deployment := newObjectSet(backupApi.ArangoBackupStateCreateError)
-
-	backupMeta, err := mock.Create()
-	require.NoError(t, err)
-
-	obj.Status.Backup = &backupApi.ArangoBackupDetails{
-		ID:                string(backupMeta.ID),
-		Version:           backupMeta.Version,
-		CreationTimestamp: meta.Now(),
-	}
-	obj.Status.Backoff = &backupApi.ArangoBackupStatusBackOff{
-		Iterations: 2,
-	}
-
-	obj.Spec.Backoff = &backupApi.ArangoBackupSpecBackOff{
-		Iterations:    util.NewType[int](1),
-		MaxIterations: util.NewType[int](2),
-	}
-
-	obj.Status.Time.Time = time.Now().Add(-2 * downloadDelay)
-
-	// Act
-	createArangoDeployment(t, handler, deployment)
-	createArangoBackup(t, handler, obj)
-
-	require.NoError(t, handler.Handle(newItemFromBackup(operation.Update, obj)))
-
-	// Assert
-	newObj := refreshArangoBackup(t, handler, obj)
-	require.Equal(t, newObj.Status.State, backupApi.ArangoBackupStateFailed)
-	require.Equal(t, newObj.Status.Message, "out of Create retries")
-}

pkg/handlers/backup/state_createerror.go

@@ -0,0 +1,52 @@
+//
+// DISCLAIMER
+//
+// Copyright 2023 ArangoDB GmbH, Cologne, Germany
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+//
+// Copyright holder is ArangoDB GmbH, Cologne, Germany
+//
+
+package backup
+
+import (
+	"time"
+
+	backupApi "github.com/arangodb/kube-arangodb/pkg/apis/backup/v1"
+)
+
+func stateCreateErrorHandler(h *handler, backup *backupApi.ArangoBackup) (*backupApi.ArangoBackupStatus, error) {
+	if !backup.Spec.Backoff.Enabled() {
+		return wrapUpdateStatus(backup,
+			updateStatusState(backupApi.ArangoBackupStateFailed, "retries are disabled"),
+			cleanStatusJob())
+	}
+
+	// no more retries - move to failed state
+	if !backup.Status.Backoff.ShouldBackoff(backup.Spec.Backoff) {
+		return wrapUpdateStatus(backup,
+			updateStatusState(backupApi.ArangoBackupStateFailed, "out of Create retries"),
+			cleanStatusJob())
+	}
+
+	// if we should retry - move to create state
+	if backup.Status.Backoff.ShouldBackoff(backup.Spec.Backoff) && !backup.Status.Backoff.GetNext().After(time.Now()) {
+		return wrapUpdateStatus(backup,
+			updateStatusState(backupApi.ArangoBackupStateCreate, ""),
+			cleanStatusJob())
+	}
+
+	// no ready to retry - wait (do not change state)
+	return wrapUpdateStatus(backup)
+}

pkg/handlers/backup/state_createerror_test.go

@@ -0,0 +1,165 @@
+//
+// DISCLAIMER
+//
+// Copyright 2023 ArangoDB GmbH, Cologne, Germany
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+//
+// Copyright holder is ArangoDB GmbH, Cologne, Germany
+//
+
+package backup
+
+import (
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/require"
+	meta "k8s.io/apimachinery/pkg/apis/meta/v1"
+
+	backupApi "github.com/arangodb/kube-arangodb/pkg/apis/backup/v1"
+	"github.com/arangodb/kube-arangodb/pkg/operatorV2/operation"
+	"github.com/arangodb/kube-arangodb/pkg/util"
+)
+
+func Test_State_CreateError_Retry_WhenBackoffEnabled(t *testing.T) {
+	// Arrange
+	handler, mock := newErrorsFakeHandler(mockErrorsArangoClientBackup{})
+
+	obj, deployment := newObjectSet(backupApi.ArangoBackupStateCreateError)
+
+	backupMeta, err := mock.Create()
+	require.NoError(t, err)
+
+	obj.Status.Backup = &backupApi.ArangoBackupDetails{
+		ID:                string(backupMeta.ID),
+		Version:           backupMeta.Version,
+		CreationTimestamp: meta.Now(),
+	}
+
+	obj.Spec.Backoff = &backupApi.ArangoBackupSpecBackOff{
+		MaxIterations: util.NewType(1),
+	}
+
+	obj.Status.Time.Time = time.Now().Add(-2 * downloadDelay)
+
+	// Act
+	createArangoDeployment(t, handler, deployment)
+	createArangoBackup(t, handler, obj)
+
+	require.NoError(t, handler.Handle(newItemFromBackup(operation.Update, obj)))
+
+	// Assert
+	newObj := refreshArangoBackup(t, handler, obj)
+	require.Equal(t, newObj.Status.State, backupApi.ArangoBackupStateCreate)
+	require.False(t, newObj.Status.Available)
+	require.NotNil(t, newObj.Status.Backup)
+	require.Equal(t, obj.Status.Backup, newObj.Status.Backup)
+}
+
+func Test_State_CreateError_Retry_WhenBackoffDisabled_C1(t *testing.T) {
+	// Arrange
+	handler, mock := newErrorsFakeHandler(mockErrorsArangoClientBackup{})
+
+	obj, deployment := newObjectSet(backupApi.ArangoBackupStateCreateError)
+
+	backupMeta, err := mock.Create()
+	require.NoError(t, err)
+
+	obj.Status.Backup = &backupApi.ArangoBackupDetails{
+		ID:                string(backupMeta.ID),
+		Version:           backupMeta.Version,
+		CreationTimestamp: meta.Now(),
+	}
+
+	obj.Spec.Backoff = &backupApi.ArangoBackupSpecBackOff{}
+
+	obj.Status.Time.Time = time.Now().Add(-2 * downloadDelay)
+
+	// Act
+	createArangoDeployment(t, handler, deployment)
+	createArangoBackup(t, handler, obj)
+
+	require.NoError(t, handler.Handle(newItemFromBackup(operation.Update, obj)))
+
+	// Assert
+	newObj := refreshArangoBackup(t, handler, obj)
+	require.Equal(t, newObj.Status.State, backupApi.ArangoBackupStateFailed)
+	require.Equal(t, newObj.Status.Message, "retries are disabled")
+}
+
+func Test_State_CreateError_Retry_WhenBackoffDisabled_C2(t *testing.T) {
+	// Arrange
+	handler, mock := newErrorsFakeHandler(mockErrorsArangoClientBackup{})
+
+	obj, deployment := newObjectSet(backupApi.ArangoBackupStateCreateError)
+
+	backupMeta, err := mock.Create()
+	require.NoError(t, err)
+
+	obj.Status.Backup = &backupApi.ArangoBackupDetails{
+		ID:                string(backupMeta.ID),
+		Version:           backupMeta.Version,
+		CreationTimestamp: meta.Now(),
+	}
+
+	obj.Status.Time.Time = time.Now().Add(-2 * downloadDelay)
+
+	// Act
+	createArangoDeployment(t, handler, deployment)
+	createArangoBackup(t, handler, obj)
+
+	require.NoError(t, handler.Handle(newItemFromBackup(operation.Update, obj)))
+
+	// Assert
+	newObj := refreshArangoBackup(t, handler, obj)
+	require.Equal(t, newObj.Status.State, backupApi.ArangoBackupStateFailed)
+	require.Equal(t, newObj.Status.Message, "retries are disabled")
+}
+
+func Test_State_CreateError_Transfer_To_Failed(t *testing.T) {
+	// Arrange
+	handler, mock := newErrorsFakeHandler(mockErrorsArangoClientBackup{})
+
+	obj, deployment := newObjectSet(backupApi.ArangoBackupStateCreateError)
+
+	backupMeta, err := mock.Create()
+	require.NoError(t, err)
+
+	obj.Status.Backup = &backupApi.ArangoBackupDetails{
+		ID:                string(backupMeta.ID),
+		Version:           backupMeta.Version,
+		CreationTimestamp: meta.Now(),
+	}
+	obj.Status.Backoff = &backupApi.ArangoBackupStatusBackOff{
+		Iterations: 2,
+	}
+
+	obj.Spec.Backoff = &backupApi.ArangoBackupSpecBackOff{
+		Iterations:    util.NewType[int](1),
+		MaxIterations: util.NewType[int](2),
+	}
+
+	obj.Status.Time.Time = time.Now().Add(-2 * downloadDelay)
+
+	// Act
+	createArangoDeployment(t, handler, deployment)
+	createArangoBackup(t, handler, obj)
+
+	require.NoError(t, handler.Handle(newItemFromBackup(operation.Update, obj)))
+
+	// Assert
+	newObj := refreshArangoBackup(t, handler, obj)
+	require.Equal(t, newObj.Status.State, backupApi.ArangoBackupStateFailed)
+	require.Equal(t, newObj.Status.Message, "out of Create retries")
+}