From 617d4f3fd45bd828511804dd21c1dda1f8ab263c Mon Sep 17 00:00:00 2001 From: ajanikow <12255597+ajanikow@users.noreply.github.com> Date: Tue, 30 Nov 2021 14:33:22 +0000 Subject: [PATCH 1/3] [Feature] Backup Backoff --- pkg/apis/backup/v1/backup.go | 2 - pkg/apis/backup/v1/backup_policy.go | 2 - pkg/apis/backup/v1/backup_policy_spec.go | 2 - pkg/apis/backup/v1/backup_policy_status.go | 2 - pkg/apis/backup/v1/backup_policy_validate.go | 2 - pkg/apis/backup/v1/backup_spec.go | 4 +- pkg/apis/backup/v1/backup_spec_backoff.go | 92 +++++++++++++++++++ .../backup/v1/backup_spec_backoff_test.go | 88 ++++++++++++++++++ pkg/apis/backup/v1/backup_state.go | 2 - pkg/apis/backup/v1/backup_status.go | 3 +- pkg/apis/backup/v1/backup_status_backoff.go | 59 ++++++++++++ .../backup/v1/backup_status_backoff_test.go | 40 ++++++++ pkg/apis/backup/v1/backup_validate.go | 2 - pkg/apis/backup/v1/doc.go | 2 - pkg/apis/backup/v1/register.go | 2 - pkg/apis/backup/v1/zz_generated.deepcopy.go | 58 ++++++++++++ .../handlers/arango/backup/state_upload.go | 1 + .../arango/backup/state_upload_test.go | 72 +++++++++++++++ .../arango/backup/state_uploaderror.go | 9 +- .../arango/backup/state_uploaderror_test.go | 4 +- .../handlers/arango/backup/state_uploading.go | 2 + pkg/backup/handlers/arango/backup/status.go | 12 +++ 22 files changed, 432 insertions(+), 30 deletions(-) create mode 100644 pkg/apis/backup/v1/backup_spec_backoff.go create mode 100644 pkg/apis/backup/v1/backup_spec_backoff_test.go create mode 100644 pkg/apis/backup/v1/backup_status_backoff.go create mode 100644 pkg/apis/backup/v1/backup_status_backoff_test.go diff --git a/pkg/apis/backup/v1/backup.go b/pkg/apis/backup/v1/backup.go index 0a72a60b2..5912e2ea7 100644 --- a/pkg/apis/backup/v1/backup.go +++ b/pkg/apis/backup/v1/backup.go @@ -17,8 +17,6 @@ // // Copyright holder is ArangoDB GmbH, Cologne, Germany // -// Author Adam Janikowski -// package v1 diff --git a/pkg/apis/backup/v1/backup_policy.go b/pkg/apis/backup/v1/backup_policy.go index f37b9a8b9..6a250d143 100644 --- a/pkg/apis/backup/v1/backup_policy.go +++ b/pkg/apis/backup/v1/backup_policy.go @@ -17,8 +17,6 @@ // // Copyright holder is ArangoDB GmbH, Cologne, Germany // -// Author Adam Janikowski -// package v1 diff --git a/pkg/apis/backup/v1/backup_policy_spec.go b/pkg/apis/backup/v1/backup_policy_spec.go index b75df558f..1ae36aa43 100644 --- a/pkg/apis/backup/v1/backup_policy_spec.go +++ b/pkg/apis/backup/v1/backup_policy_spec.go @@ -17,8 +17,6 @@ // // Copyright holder is ArangoDB GmbH, Cologne, Germany // -// Author Adam Janikowski -// package v1 diff --git a/pkg/apis/backup/v1/backup_policy_status.go b/pkg/apis/backup/v1/backup_policy_status.go index 4a055de33..869631c9d 100644 --- a/pkg/apis/backup/v1/backup_policy_status.go +++ b/pkg/apis/backup/v1/backup_policy_status.go @@ -17,8 +17,6 @@ // // Copyright holder is ArangoDB GmbH, Cologne, Germany // -// Author Adam Janikowski -// package v1 diff --git a/pkg/apis/backup/v1/backup_policy_validate.go b/pkg/apis/backup/v1/backup_policy_validate.go index 9dfa5f0f8..c00d0404e 100644 --- a/pkg/apis/backup/v1/backup_policy_validate.go +++ b/pkg/apis/backup/v1/backup_policy_validate.go @@ -17,8 +17,6 @@ // // Copyright holder is ArangoDB GmbH, Cologne, Germany // -// Author Adam Janikowski -// package v1 diff --git a/pkg/apis/backup/v1/backup_spec.go b/pkg/apis/backup/v1/backup_spec.go index 8520269d2..c2eff9fa2 100644 --- a/pkg/apis/backup/v1/backup_spec.go +++ b/pkg/apis/backup/v1/backup_spec.go @@ -17,8 +17,6 @@ // // Copyright holder is ArangoDB GmbH, Cologne, Germany // -// Author Adam Janikowski -// package v1 @@ -35,6 +33,8 @@ type ArangoBackupSpec struct { Upload *ArangoBackupSpecOperation `json:"upload,omitempty"` PolicyName *string `json:"policyName,omitempty"` + + Backoff *ArangoBackupSpecBackOff `json:"backoff,omitempty"` } type ArangoBackupSpecDeployment struct { diff --git a/pkg/apis/backup/v1/backup_spec_backoff.go b/pkg/apis/backup/v1/backup_spec_backoff.go new file mode 100644 index 000000000..61e8e405a --- /dev/null +++ b/pkg/apis/backup/v1/backup_spec_backoff.go @@ -0,0 +1,92 @@ +// +// DISCLAIMER +// +// Copyright 2016-2021 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 v1 + +import "time" + +type ArangoBackupSpecBackOff struct { + // MinDelay defines minimum delay in seconds. Default to 10 + MinDelay *int `json:"min_delay,omitempty"` + // MaxDelay defines maximum delay in seconds. Default to 60 + MaxDelay *int `json:"max_delay,omitempty"` + // Iterations defines number of iterations before reaching MaxDelay. Default to 5 + Iterations *int `json:"iterations,omitempty"` +} + +func (a *ArangoBackupSpecBackOff) GetMaxDelay() int { + if a == nil || a.MaxDelay == nil { + return 60 + } + + v := *a.MaxDelay + + if v < 0 { + return 0 + } + + return v +} + +func (a *ArangoBackupSpecBackOff) GetMinDelay() int { + if a == nil || a.MinDelay == nil { + return 10 + } + + v := *a.MinDelay + + if v < 0 { + return 0 + } + + if m := a.GetMaxDelay(); m < v { + return m + } + + return v +} + +func (a *ArangoBackupSpecBackOff) GetIterations() int { + if a == nil || a.Iterations == nil { + return 5 + } + + v := *a.Iterations + + if v < 1 { + return 1 + } + + return v +} + +func (a *ArangoBackupSpecBackOff) Backoff(iteration int) time.Duration { + if maxIterations := a.GetIterations(); maxIterations <= iteration { + return time.Duration(a.GetMaxDelay()) * time.Second + } else { + min, max := a.GetMinDelay(), a.GetMaxDelay() + + if min == max { + return time.Duration(min) * time.Second + } + + return time.Duration(min+int(float64(iteration)/float64(maxIterations)*float64(max-min))) * time.Second + } +} diff --git a/pkg/apis/backup/v1/backup_spec_backoff_test.go b/pkg/apis/backup/v1/backup_spec_backoff_test.go new file mode 100644 index 000000000..2275e2535 --- /dev/null +++ b/pkg/apis/backup/v1/backup_spec_backoff_test.go @@ -0,0 +1,88 @@ +// +// DISCLAIMER +// +// Copyright 2016-2021 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 v1 + +import ( + "testing" + "time" + + "github.com/arangodb/kube-arangodb/pkg/util" + "github.com/stretchr/testify/require" +) + +func TestArangoBackupSpecBackOff_Backoff(t *testing.T) { + t.Run("Default", func(t *testing.T) { + var b *ArangoBackupSpecBackOff + + require.Equal(t, 10*time.Second, b.Backoff(0)) + require.Equal(t, 20*time.Second, b.Backoff(1)) + require.Equal(t, 30*time.Second, b.Backoff(2)) + require.Equal(t, 40*time.Second, b.Backoff(3)) + require.Equal(t, 50*time.Second, b.Backoff(4)) + require.Equal(t, 60*time.Second, b.Backoff(5)) + require.Equal(t, 60*time.Second, b.Backoff(6)) + }) + t.Run("Custom", func(t *testing.T) { + b := &ArangoBackupSpecBackOff{ + MinDelay: util.NewInt(20), + MaxDelay: util.NewInt(120), + Iterations: util.NewInt(10), + } + + require.Equal(t, 20*time.Second, b.Backoff(0)) + require.Equal(t, 30*time.Second, b.Backoff(1)) + require.Equal(t, 40*time.Second, b.Backoff(2)) + require.Equal(t, 50*time.Second, b.Backoff(3)) + require.Equal(t, 60*time.Second, b.Backoff(4)) + require.Equal(t, 70*time.Second, b.Backoff(5)) + require.Equal(t, 80*time.Second, b.Backoff(6)) + require.Equal(t, 90*time.Second, b.Backoff(7)) + require.Equal(t, 100*time.Second, b.Backoff(8)) + require.Equal(t, 110*time.Second, b.Backoff(9)) + require.Equal(t, 120*time.Second, b.Backoff(10)) + require.Equal(t, 120*time.Second, b.Backoff(11)) + }) + + t.Run("Invalid", func(t *testing.T) { + b := &ArangoBackupSpecBackOff{ + MinDelay: util.NewInt(-1), + MaxDelay: util.NewInt(-1), + Iterations: util.NewInt(0), + } + + require.Equal(t, 0, b.GetMinDelay()) + require.Equal(t, 0, b.GetMaxDelay()) + require.Equal(t, 1, b.GetIterations()) + + require.Equal(t, 0*time.Second, b.Backoff(12345)) + }) + + t.Run("Max < Min", func(t *testing.T) { + b := &ArangoBackupSpecBackOff{ + MinDelay: util.NewInt(50), + MaxDelay: util.NewInt(20), + } + + require.Equal(t, 20, b.GetMinDelay()) + require.Equal(t, 20, b.GetMaxDelay()) + require.Equal(t, 5, b.GetIterations()) + }) +} diff --git a/pkg/apis/backup/v1/backup_state.go b/pkg/apis/backup/v1/backup_state.go index 47ebb56f8..6ce9082ed 100644 --- a/pkg/apis/backup/v1/backup_state.go +++ b/pkg/apis/backup/v1/backup_state.go @@ -17,8 +17,6 @@ // // Copyright holder is ArangoDB GmbH, Cologne, Germany // -// Author Adam Janikowski -// package v1 diff --git a/pkg/apis/backup/v1/backup_status.go b/pkg/apis/backup/v1/backup_status.go index 55d0f0efc..96e9afe64 100644 --- a/pkg/apis/backup/v1/backup_status.go +++ b/pkg/apis/backup/v1/backup_status.go @@ -17,8 +17,6 @@ // // Copyright holder is ArangoDB GmbH, Cologne, Germany // -// Author Adam Janikowski -// package v1 @@ -33,6 +31,7 @@ type ArangoBackupStatus struct { ArangoBackupState `json:",inline"` Backup *ArangoBackupDetails `json:"backup,omitempty"` Available bool `json:"available"` + Backoff *ArangoBackupStatusBackOff `json:"backoff,omitempty"` } func (a *ArangoBackupStatus) Equal(b *ArangoBackupStatus) bool { diff --git a/pkg/apis/backup/v1/backup_status_backoff.go b/pkg/apis/backup/v1/backup_status_backoff.go new file mode 100644 index 000000000..53586cef3 --- /dev/null +++ b/pkg/apis/backup/v1/backup_status_backoff.go @@ -0,0 +1,59 @@ +// +// DISCLAIMER +// +// Copyright 2016-2021 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 v1 + +import ( + "time" + + meta "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +type ArangoBackupStatusBackOff struct { + Iterations int `json:"iterations,omitempty"` + Next meta.Time `json:"next,omitempty"` +} + +func (a *ArangoBackupStatusBackOff) GetIterations() int { + if a == nil { + return 0 + } + + if a.Iterations < 0 { + return 0 + } + + return a.Iterations +} + +func (a *ArangoBackupStatusBackOff) GetNext() meta.Time { + if a == nil { + return meta.Time{} + } + + return a.Next +} + +func (a *ArangoBackupStatusBackOff) Backoff(spec *ArangoBackupSpecBackOff) *ArangoBackupStatusBackOff { + return &ArangoBackupStatusBackOff{ + Iterations: a.GetIterations() + 1, + Next: meta.Time{Time: time.Now().Add(spec.Backoff(a.GetIterations()))}, + } +} diff --git a/pkg/apis/backup/v1/backup_status_backoff_test.go b/pkg/apis/backup/v1/backup_status_backoff_test.go new file mode 100644 index 000000000..c35d88cdd --- /dev/null +++ b/pkg/apis/backup/v1/backup_status_backoff_test.go @@ -0,0 +1,40 @@ +// +// DISCLAIMER +// +// Copyright 2016-2021 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 v1 + +import ( + "testing" + "time" + + "github.com/stretchr/testify/require" +) + +func TestArangoBackupStatusBackOff_Backoff(t *testing.T) { + t.Run("Default", func(t *testing.T) { + var spec *ArangoBackupSpecBackOff + var status *ArangoBackupStatusBackOff + + n := status.Backoff(spec) + + require.Equal(t, 1, n.GetIterations()) + require.True(t, n.GetNext().After(time.Now().Add(time.Duration(9.9*float64(time.Second))))) + }) +} diff --git a/pkg/apis/backup/v1/backup_validate.go b/pkg/apis/backup/v1/backup_validate.go index a450cf1d9..4d8c6ec3a 100644 --- a/pkg/apis/backup/v1/backup_validate.go +++ b/pkg/apis/backup/v1/backup_validate.go @@ -17,8 +17,6 @@ // // Copyright holder is ArangoDB GmbH, Cologne, Germany // -// Author Adam Janikowski -// package v1 diff --git a/pkg/apis/backup/v1/doc.go b/pkg/apis/backup/v1/doc.go index 2fad81560..70d83fd3d 100644 --- a/pkg/apis/backup/v1/doc.go +++ b/pkg/apis/backup/v1/doc.go @@ -17,8 +17,6 @@ // // Copyright holder is ArangoDB GmbH, Cologne, Germany // -// Author Adam Janikowski -// // +k8s:deepcopy-gen=package // +groupName=backup.arangodb.com diff --git a/pkg/apis/backup/v1/register.go b/pkg/apis/backup/v1/register.go index 649759307..284ef934a 100644 --- a/pkg/apis/backup/v1/register.go +++ b/pkg/apis/backup/v1/register.go @@ -17,8 +17,6 @@ // // Copyright holder is ArangoDB GmbH, Cologne, Germany // -// Author Adam Janikowski -// package v1 diff --git a/pkg/apis/backup/v1/zz_generated.deepcopy.go b/pkg/apis/backup/v1/zz_generated.deepcopy.go index 342712329..877893dc6 100644 --- a/pkg/apis/backup/v1/zz_generated.deepcopy.go +++ b/pkg/apis/backup/v1/zz_generated.deepcopy.go @@ -273,6 +273,11 @@ func (in *ArangoBackupSpec) DeepCopyInto(out *ArangoBackupSpec) { *out = new(string) **out = **in } + if in.Backoff != nil { + in, out := &in.Backoff, &out.Backoff + *out = new(ArangoBackupSpecBackOff) + (*in).DeepCopyInto(*out) + } return } @@ -286,6 +291,37 @@ func (in *ArangoBackupSpec) DeepCopy() *ArangoBackupSpec { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *ArangoBackupSpecBackOff) DeepCopyInto(out *ArangoBackupSpecBackOff) { + *out = *in + if in.MinDelay != nil { + in, out := &in.MinDelay, &out.MinDelay + *out = new(int) + **out = **in + } + if in.MaxDelay != nil { + in, out := &in.MaxDelay, &out.MaxDelay + *out = new(int) + **out = **in + } + if in.Iterations != nil { + in, out := &in.Iterations, &out.Iterations + *out = new(int) + **out = **in + } + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ArangoBackupSpecBackOff. +func (in *ArangoBackupSpecBackOff) DeepCopy() *ArangoBackupSpecBackOff { + if in == nil { + return nil + } + out := new(ArangoBackupSpecBackOff) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ArangoBackupSpecDeployment) DeepCopyInto(out *ArangoBackupSpecDeployment) { *out = *in @@ -392,6 +428,11 @@ func (in *ArangoBackupStatus) DeepCopyInto(out *ArangoBackupStatus) { *out = new(ArangoBackupDetails) (*in).DeepCopyInto(*out) } + if in.Backoff != nil { + in, out := &in.Backoff, &out.Backoff + *out = new(ArangoBackupStatusBackOff) + (*in).DeepCopyInto(*out) + } return } @@ -405,6 +446,23 @@ func (in *ArangoBackupStatus) DeepCopy() *ArangoBackupStatus { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *ArangoBackupStatusBackOff) DeepCopyInto(out *ArangoBackupStatusBackOff) { + *out = *in + in.Next.DeepCopyInto(&out.Next) + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ArangoBackupStatusBackOff. +func (in *ArangoBackupStatusBackOff) DeepCopy() *ArangoBackupStatusBackOff { + if in == nil { + return nil + } + out := new(ArangoBackupStatusBackOff) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ArangoBackupTemplate) DeepCopyInto(out *ArangoBackupTemplate) { *out = *in diff --git a/pkg/backup/handlers/arango/backup/state_upload.go b/pkg/backup/handlers/arango/backup/state_upload.go index 3cdd4ee4b..0a06215ff 100644 --- a/pkg/backup/handlers/arango/backup/state_upload.go +++ b/pkg/backup/handlers/arango/backup/state_upload.go @@ -62,6 +62,7 @@ func stateUploadHandler(h *handler, backup *backupApi.ArangoBackup) (*backupApi. cleanStatusJob(), updateStatusBackupUpload(nil), updateStatusAvailable(true), + addBackOff(backup.Spec), ) } diff --git a/pkg/backup/handlers/arango/backup/state_upload_test.go b/pkg/backup/handlers/arango/backup/state_upload_test.go index 8e2101d61..4fb3538c8 100644 --- a/pkg/backup/handlers/arango/backup/state_upload_test.go +++ b/pkg/backup/handlers/arango/backup/state_upload_test.go @@ -178,6 +178,9 @@ func Test_State_Upload_TemporaryUploadFailed(t *testing.T) { // Assert newObj := refreshArangoBackup(t, handler, obj) checkBackup(t, newObj, backupApi.ArangoBackupStateUploadError, true) + + require.NotNil(t, newObj.Status.Backoff) + require.Equal(t, 1,newObj.Status.Backoff.Iterations) } func Test_State_Upload_FatalUploadFailed(t *testing.T) { @@ -205,4 +208,73 @@ func Test_State_Upload_FatalUploadFailed(t *testing.T) { // Assert newObj := refreshArangoBackup(t, handler, obj) checkBackup(t, newObj, backupApi.ArangoBackupStateUploadError, true) + + require.NotNil(t, newObj.Status.Backoff) + require.Equal(t, 1,newObj.Status.Backoff.Iterations) } + +func Test_State_Upload_TemporaryUploadFailed_Backoff(t *testing.T) { + // Arrange + error := newTemporaryErrorf("error") + handler, mock := newErrorsFakeHandler(mockErrorsArangoClientBackup{ + uploadError: error, + }) + + obj, deployment := newObjectSet(backupApi.ArangoBackupStateUpload) + + createResponse, err := mock.Create() + require.NoError(t, err) + + obj.Status.Backup = createBackupFromMeta(driver.BackupMeta{ + ID: createResponse.ID, + }, nil) + obj.Status.Backoff = &backupApi.ArangoBackupStatusBackOff{ + Iterations: 3, + } + + // Act + createArangoDeployment(t, handler, deployment) + createArangoBackup(t, handler, obj) + + require.NoError(t, handler.Handle(newItemFromBackup(operation.Update, obj))) + + // Assert + newObj := refreshArangoBackup(t, handler, obj) + checkBackup(t, newObj, backupApi.ArangoBackupStateUploadError, true) + + require.NotNil(t, newObj.Status.Backoff) + require.Equal(t, 4,newObj.Status.Backoff.Iterations) +} + +func Test_State_Upload_FatalUploadFailed_Backoff(t *testing.T) { + // Arrange + error := newFatalErrorf("error") + handler, mock := newErrorsFakeHandler(mockErrorsArangoClientBackup{ + uploadError: error, + }) + + obj, deployment := newObjectSet(backupApi.ArangoBackupStateUpload) + + createResponse, err := mock.Create() + require.NoError(t, err) + + obj.Status.Backup = createBackupFromMeta(driver.BackupMeta{ + ID: createResponse.ID, + }, nil) + obj.Status.Backoff = &backupApi.ArangoBackupStatusBackOff{ + Iterations: 3, + } + + // Act + createArangoDeployment(t, handler, deployment) + createArangoBackup(t, handler, obj) + + require.NoError(t, handler.Handle(newItemFromBackup(operation.Update, obj))) + + // Assert + newObj := refreshArangoBackup(t, handler, obj) + checkBackup(t, newObj, backupApi.ArangoBackupStateUploadError, true) + + require.NotNil(t, newObj.Status.Backoff) + require.Equal(t, 4,newObj.Status.Backoff.Iterations) +} \ No newline at end of file diff --git a/pkg/backup/handlers/arango/backup/state_uploaderror.go b/pkg/backup/handlers/arango/backup/state_uploaderror.go index 5f2e9e410..b1859ad2d 100644 --- a/pkg/backup/handlers/arango/backup/state_uploaderror.go +++ b/pkg/backup/handlers/arango/backup/state_uploaderror.go @@ -23,17 +23,12 @@ package backup import ( - "time" - backupApi "github.com/arangodb/kube-arangodb/pkg/apis/backup/v1" -) - -const ( - uploadDelay = time.Minute + "time" ) func stateUploadErrorHandler(h *handler, backup *backupApi.ArangoBackup) (*backupApi.ArangoBackupStatus, error) { - if backup.Spec.Upload == nil || backup.Status.Time.Time.Add(uploadDelay).Before(time.Now()) { + if backup.Spec.Upload == nil || !backup.Status.Backoff.GetNext().After(time.Now()) { return wrapUpdateStatus(backup, updateStatusState(backupApi.ArangoBackupStateReady, ""), cleanStatusJob(), diff --git a/pkg/backup/handlers/arango/backup/state_uploaderror_test.go b/pkg/backup/handlers/arango/backup/state_uploaderror_test.go index 6a160c88f..71d00dbb8 100644 --- a/pkg/backup/handlers/arango/backup/state_uploaderror_test.go +++ b/pkg/backup/handlers/arango/backup/state_uploaderror_test.go @@ -92,8 +92,10 @@ func Test_State_UploadError_Wait(t *testing.T) { CreationTimestamp: meta.Now(), Uploaded: util.NewBool(true), } + obj.Status.Backoff = &backupApi.ArangoBackupStatusBackOff{ + Next: meta.Time{Time: time.Now().Add(5*time.Second)}, + } - obj.Status.Time.Time = time.Now().Add(2 * downloadDelay) obj.Status.Message = "message" // Act diff --git a/pkg/backup/handlers/arango/backup/state_uploading.go b/pkg/backup/handlers/arango/backup/state_uploading.go index e9d4c349f..75c8cece5 100644 --- a/pkg/backup/handlers/arango/backup/state_uploading.go +++ b/pkg/backup/handlers/arango/backup/state_uploading.go @@ -69,6 +69,7 @@ func stateUploadingHandler(h *handler, backup *backupApi.ArangoBackup) (*backupA "Upload failed with error: %s", details.FailMessage), cleanStatusJob(), updateStatusAvailable(true), + addBackOff(backup.Spec), ) } @@ -78,6 +79,7 @@ func stateUploadingHandler(h *handler, backup *backupApi.ArangoBackup) (*backupA cleanStatusJob(), updateStatusBackupUpload(util.NewBool(true)), updateStatusAvailable(true), + cleanBackOff(), ) } diff --git a/pkg/backup/handlers/arango/backup/status.go b/pkg/backup/handlers/arango/backup/status.go index 06d79d490..00af57618 100644 --- a/pkg/backup/handlers/arango/backup/status.go +++ b/pkg/backup/handlers/arango/backup/status.go @@ -67,6 +67,18 @@ func updateStatusAvailable(available bool) updateStatusFunc { } } +func cleanBackOff() updateStatusFunc { + return func(status *backupApi.ArangoBackupStatus) { + status.Backoff = nil + } +} + +func addBackOff(spec backupApi.ArangoBackupSpec) updateStatusFunc { + return func(status *backupApi.ArangoBackupStatus) { + status.Backoff = status.Backoff.Backoff(spec.Backoff) + } +} + func updateStatusJob(id, progress string) updateStatusFunc { return func(status *backupApi.ArangoBackupStatus) { status.Progress = &backupApi.ArangoBackupProgress{ From 47417d41657f65efa3f817401df03d0a3b2f7de8 Mon Sep 17 00:00:00 2001 From: ajanikow <12255597+ajanikow@users.noreply.github.com> Date: Thu, 2 Dec 2021 12:28:37 +0000 Subject: [PATCH 2/3] Increase backoff --- pkg/apis/backup/v1/backup_spec_backoff.go | 8 +-- .../backup/v1/backup_spec_backoff_test.go | 55 ++++++++++--------- pkg/apis/backup/v1/backup_status.go | 6 +- .../arango/backup/state_upload_test.go | 10 ++-- .../arango/backup/state_uploaderror.go | 3 +- .../arango/backup/state_uploaderror_test.go | 2 +- 6 files changed, 43 insertions(+), 41 deletions(-) diff --git a/pkg/apis/backup/v1/backup_spec_backoff.go b/pkg/apis/backup/v1/backup_spec_backoff.go index 61e8e405a..a5a5104d7 100644 --- a/pkg/apis/backup/v1/backup_spec_backoff.go +++ b/pkg/apis/backup/v1/backup_spec_backoff.go @@ -23,9 +23,9 @@ package v1 import "time" type ArangoBackupSpecBackOff struct { - // MinDelay defines minimum delay in seconds. Default to 10 + // MinDelay defines minimum delay in seconds. Default to 30 MinDelay *int `json:"min_delay,omitempty"` - // MaxDelay defines maximum delay in seconds. Default to 60 + // MaxDelay defines maximum delay in seconds. Default to 600 MaxDelay *int `json:"max_delay,omitempty"` // Iterations defines number of iterations before reaching MaxDelay. Default to 5 Iterations *int `json:"iterations,omitempty"` @@ -33,7 +33,7 @@ type ArangoBackupSpecBackOff struct { func (a *ArangoBackupSpecBackOff) GetMaxDelay() int { if a == nil || a.MaxDelay == nil { - return 60 + return 600 } v := *a.MaxDelay @@ -47,7 +47,7 @@ func (a *ArangoBackupSpecBackOff) GetMaxDelay() int { func (a *ArangoBackupSpecBackOff) GetMinDelay() int { if a == nil || a.MinDelay == nil { - return 10 + return 30 } v := *a.MinDelay diff --git a/pkg/apis/backup/v1/backup_spec_backoff_test.go b/pkg/apis/backup/v1/backup_spec_backoff_test.go index 2275e2535..a16ace980 100644 --- a/pkg/apis/backup/v1/backup_spec_backoff_test.go +++ b/pkg/apis/backup/v1/backup_spec_backoff_test.go @@ -24,21 +24,22 @@ import ( "testing" "time" + "github.com/stretchr/testify/assert" + "github.com/arangodb/kube-arangodb/pkg/util" - "github.com/stretchr/testify/require" ) func TestArangoBackupSpecBackOff_Backoff(t *testing.T) { t.Run("Default", func(t *testing.T) { var b *ArangoBackupSpecBackOff - require.Equal(t, 10*time.Second, b.Backoff(0)) - require.Equal(t, 20*time.Second, b.Backoff(1)) - require.Equal(t, 30*time.Second, b.Backoff(2)) - require.Equal(t, 40*time.Second, b.Backoff(3)) - require.Equal(t, 50*time.Second, b.Backoff(4)) - require.Equal(t, 60*time.Second, b.Backoff(5)) - require.Equal(t, 60*time.Second, b.Backoff(6)) + assert.Equal(t, 30*time.Second, b.Backoff(0)) + assert.Equal(t, 144*time.Second, b.Backoff(1)) + assert.Equal(t, 258*time.Second, b.Backoff(2)) + assert.Equal(t, 372*time.Second, b.Backoff(3)) + assert.Equal(t, 486*time.Second, b.Backoff(4)) + assert.Equal(t, 600*time.Second, b.Backoff(5)) + assert.Equal(t, 600*time.Second, b.Backoff(6)) }) t.Run("Custom", func(t *testing.T) { b := &ArangoBackupSpecBackOff{ @@ -47,18 +48,18 @@ func TestArangoBackupSpecBackOff_Backoff(t *testing.T) { Iterations: util.NewInt(10), } - require.Equal(t, 20*time.Second, b.Backoff(0)) - require.Equal(t, 30*time.Second, b.Backoff(1)) - require.Equal(t, 40*time.Second, b.Backoff(2)) - require.Equal(t, 50*time.Second, b.Backoff(3)) - require.Equal(t, 60*time.Second, b.Backoff(4)) - require.Equal(t, 70*time.Second, b.Backoff(5)) - require.Equal(t, 80*time.Second, b.Backoff(6)) - require.Equal(t, 90*time.Second, b.Backoff(7)) - require.Equal(t, 100*time.Second, b.Backoff(8)) - require.Equal(t, 110*time.Second, b.Backoff(9)) - require.Equal(t, 120*time.Second, b.Backoff(10)) - require.Equal(t, 120*time.Second, b.Backoff(11)) + assert.Equal(t, 20*time.Second, b.Backoff(0)) + assert.Equal(t, 30*time.Second, b.Backoff(1)) + assert.Equal(t, 40*time.Second, b.Backoff(2)) + assert.Equal(t, 50*time.Second, b.Backoff(3)) + assert.Equal(t, 60*time.Second, b.Backoff(4)) + assert.Equal(t, 70*time.Second, b.Backoff(5)) + assert.Equal(t, 80*time.Second, b.Backoff(6)) + assert.Equal(t, 90*time.Second, b.Backoff(7)) + assert.Equal(t, 100*time.Second, b.Backoff(8)) + assert.Equal(t, 110*time.Second, b.Backoff(9)) + assert.Equal(t, 120*time.Second, b.Backoff(10)) + assert.Equal(t, 120*time.Second, b.Backoff(11)) }) t.Run("Invalid", func(t *testing.T) { @@ -68,11 +69,11 @@ func TestArangoBackupSpecBackOff_Backoff(t *testing.T) { Iterations: util.NewInt(0), } - require.Equal(t, 0, b.GetMinDelay()) - require.Equal(t, 0, b.GetMaxDelay()) - require.Equal(t, 1, b.GetIterations()) + assert.Equal(t, 0, b.GetMinDelay()) + assert.Equal(t, 0, b.GetMaxDelay()) + assert.Equal(t, 1, b.GetIterations()) - require.Equal(t, 0*time.Second, b.Backoff(12345)) + assert.Equal(t, 0*time.Second, b.Backoff(12345)) }) t.Run("Max < Min", func(t *testing.T) { @@ -81,8 +82,8 @@ func TestArangoBackupSpecBackOff_Backoff(t *testing.T) { MaxDelay: util.NewInt(20), } - require.Equal(t, 20, b.GetMinDelay()) - require.Equal(t, 20, b.GetMaxDelay()) - require.Equal(t, 5, b.GetIterations()) + assert.Equal(t, 20, b.GetMinDelay()) + assert.Equal(t, 20, b.GetMaxDelay()) + assert.Equal(t, 5, b.GetIterations()) }) } diff --git a/pkg/apis/backup/v1/backup_status.go b/pkg/apis/backup/v1/backup_status.go index 96e9afe64..329bfc1a2 100644 --- a/pkg/apis/backup/v1/backup_status.go +++ b/pkg/apis/backup/v1/backup_status.go @@ -29,9 +29,9 @@ import ( // an ArangoBackup. type ArangoBackupStatus struct { ArangoBackupState `json:",inline"` - Backup *ArangoBackupDetails `json:"backup,omitempty"` - Available bool `json:"available"` - Backoff *ArangoBackupStatusBackOff `json:"backoff,omitempty"` + Backup *ArangoBackupDetails `json:"backup,omitempty"` + Available bool `json:"available"` + Backoff *ArangoBackupStatusBackOff `json:"backoff,omitempty"` } func (a *ArangoBackupStatus) Equal(b *ArangoBackupStatus) bool { diff --git a/pkg/backup/handlers/arango/backup/state_upload_test.go b/pkg/backup/handlers/arango/backup/state_upload_test.go index 4fb3538c8..1a5e0d2a2 100644 --- a/pkg/backup/handlers/arango/backup/state_upload_test.go +++ b/pkg/backup/handlers/arango/backup/state_upload_test.go @@ -180,7 +180,7 @@ func Test_State_Upload_TemporaryUploadFailed(t *testing.T) { checkBackup(t, newObj, backupApi.ArangoBackupStateUploadError, true) require.NotNil(t, newObj.Status.Backoff) - require.Equal(t, 1,newObj.Status.Backoff.Iterations) + require.Equal(t, 1, newObj.Status.Backoff.Iterations) } func Test_State_Upload_FatalUploadFailed(t *testing.T) { @@ -210,7 +210,7 @@ func Test_State_Upload_FatalUploadFailed(t *testing.T) { checkBackup(t, newObj, backupApi.ArangoBackupStateUploadError, true) require.NotNil(t, newObj.Status.Backoff) - require.Equal(t, 1,newObj.Status.Backoff.Iterations) + require.Equal(t, 1, newObj.Status.Backoff.Iterations) } func Test_State_Upload_TemporaryUploadFailed_Backoff(t *testing.T) { @@ -243,7 +243,7 @@ func Test_State_Upload_TemporaryUploadFailed_Backoff(t *testing.T) { checkBackup(t, newObj, backupApi.ArangoBackupStateUploadError, true) require.NotNil(t, newObj.Status.Backoff) - require.Equal(t, 4,newObj.Status.Backoff.Iterations) + require.Equal(t, 4, newObj.Status.Backoff.Iterations) } func Test_State_Upload_FatalUploadFailed_Backoff(t *testing.T) { @@ -276,5 +276,5 @@ func Test_State_Upload_FatalUploadFailed_Backoff(t *testing.T) { checkBackup(t, newObj, backupApi.ArangoBackupStateUploadError, true) require.NotNil(t, newObj.Status.Backoff) - require.Equal(t, 4,newObj.Status.Backoff.Iterations) -} \ No newline at end of file + require.Equal(t, 4, newObj.Status.Backoff.Iterations) +} diff --git a/pkg/backup/handlers/arango/backup/state_uploaderror.go b/pkg/backup/handlers/arango/backup/state_uploaderror.go index b1859ad2d..d3e11297b 100644 --- a/pkg/backup/handlers/arango/backup/state_uploaderror.go +++ b/pkg/backup/handlers/arango/backup/state_uploaderror.go @@ -23,8 +23,9 @@ package backup import ( - backupApi "github.com/arangodb/kube-arangodb/pkg/apis/backup/v1" "time" + + backupApi "github.com/arangodb/kube-arangodb/pkg/apis/backup/v1" ) func stateUploadErrorHandler(h *handler, backup *backupApi.ArangoBackup) (*backupApi.ArangoBackupStatus, error) { diff --git a/pkg/backup/handlers/arango/backup/state_uploaderror_test.go b/pkg/backup/handlers/arango/backup/state_uploaderror_test.go index 71d00dbb8..11fadfeae 100644 --- a/pkg/backup/handlers/arango/backup/state_uploaderror_test.go +++ b/pkg/backup/handlers/arango/backup/state_uploaderror_test.go @@ -93,7 +93,7 @@ func Test_State_UploadError_Wait(t *testing.T) { Uploaded: util.NewBool(true), } obj.Status.Backoff = &backupApi.ArangoBackupStatusBackOff{ - Next: meta.Time{Time: time.Now().Add(5*time.Second)}, + Next: meta.Time{Time: time.Now().Add(5 * time.Second)}, } obj.Status.Message = "message" From 9eee9649d0f00cbcdeba0c33ef63f197e6d86fc3 Mon Sep 17 00:00:00 2001 From: ajanikow <12255597+ajanikow@users.noreply.github.com> Date: Thu, 2 Dec 2021 12:29:10 +0000 Subject: [PATCH 3/3] Changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3e213da70..af765ddc1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,7 @@ # Change Log ## [master](https://github.com/arangodb/kube-arangodb/tree/master) (N/A) +- Add ArangoBackup backoff functionality ## [1.2.5](https://github.com/arangodb/kube-arangodb/tree/1.2.5) (2021-10-25) - Split & Unify Lifecycle management functionality