From 3727351b8e1cd117d11e69ea3f31ee4726ea7129 Mon Sep 17 00:00:00 2001 From: jwierzbo Date: Tue, 27 Aug 2024 14:19:42 +0200 Subject: [PATCH 1/2] OAS-10007 Fix race condition in ArangoBackup --- CHANGELOG.md | 1 + pkg/handlers/backup/handler.go | 8 +++++ pkg/handlers/backup/handler_test.go | 47 +++++++++++++++++++++++++++++ 3 files changed, 56 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5f848f077..1c7a3407d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,7 @@ - (Improvement) Better panic handling - (Feature) PongV1 Integration Service - (Feature) Custom Gateway image +- (Bugfix) Fix race condition in ArangoBackup ## [1.2.42](https://github.com/arangodb/kube-arangodb/tree/1.2.42) (2024-07-23) - (Maintenance) Go 1.22.4 & Kubernetes 1.29.6 libraries diff --git a/pkg/handlers/backup/handler.go b/pkg/handlers/backup/handler.go index 8ca1ba83a..1659167ec 100644 --- a/pkg/handlers/backup/handler.go +++ b/pkg/handlers/backup/handler.go @@ -123,6 +123,14 @@ func (h *handler) refreshDeployment(deployment *database.ArangoDeployment) error return err } + for _, backup := range backups.Items { + switch backup.GetStatus().ArangoBackupState.State { + case backupApi.ArangoBackupStateCreate, backupApi.ArangoBackupStateCreating: + // Skip refreshing backups if they are in creation state + return nil + } + } + existingBackups, err := client.List() if err != nil { return err diff --git a/pkg/handlers/backup/handler_test.go b/pkg/handlers/backup/handler_test.go index 509c9e426..c8ee85a93 100644 --- a/pkg/handlers/backup/handler_test.go +++ b/pkg/handlers/backup/handler_test.go @@ -129,4 +129,51 @@ func Test_Refresh_Cleanup(t *testing.T) { require.NoError(t, err) require.Len(t, backups.Items, 0) }) + + t.Run("Do not refresh if backup is creating", func(t *testing.T) { + // Arrange + fakeId := driver.BackupID(uuid.NewUUID()) + createBackup := backupApi.ArangoBackup{ + + ObjectMeta: meta.ObjectMeta{ + Name: "backup", + }, + Status: backupApi.ArangoBackupStatus{ + ArangoBackupState: backupApi.ArangoBackupState{ + State: backupApi.ArangoBackupStateCreating, + }, + Backup: &backupApi.ArangoBackupDetails{ + ID: string(fakeId), + }, + }, + } + b, err := handler.client.BackupV1().ArangoBackups(tests.FakeNamespace).Create(context.Background(), &createBackup, meta.CreateOptions{}) + require.NoError(t, err) + require.NotNil(t, b) + require.Equal(t, backupApi.ArangoBackupStateCreating, b.Status.State) + + t.Run("Refresh should not happen if there is Backup in creation state", func(t *testing.T) { + require.NoError(t, handler.refreshDeployment(arangoDeployment)) + + backups, err := handler.client.BackupV1().ArangoBackups(tests.FakeNamespace).List(context.Background(), meta.ListOptions{}) + require.NoError(t, err) + require.Len(t, backups.Items, 1) + require.NotNil(t, backups.Items[0].Status.Backup) + require.EqualValues(t, fakeId, backups.Items[0].Status.Backup.ID) + }) + + createBackup.Status.State = backupApi.ArangoBackupStateReady + b, err = handler.client.BackupV1().ArangoBackups(tests.FakeNamespace).UpdateStatus(context.Background(), &createBackup, meta.UpdateOptions{}) + require.NoError(t, err) + require.NotNil(t, b) + require.Equal(t, backupApi.ArangoBackupStateReady, b.Status.State) + + t.Run("Refresh should happen if there is Backup in ready state", func(t *testing.T) { + require.NoError(t, handler.refreshDeployment(arangoDeployment)) + + backups, err := handler.client.BackupV1().ArangoBackups(tests.FakeNamespace).List(context.Background(), meta.ListOptions{}) + require.NoError(t, err) + require.Len(t, backups.Items, 2) + }) + }) } From aace96b20c9c1701e7bd8fe0d94b15c167c72540 Mon Sep 17 00:00:00 2001 From: jwierzbo Date: Wed, 28 Aug 2024 11:40:45 +0200 Subject: [PATCH 2/2] Update --- .../deployment/v2alpha1/deployment_spec_gateway.go | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/pkg/apis/deployment/v2alpha1/deployment_spec_gateway.go b/pkg/apis/deployment/v2alpha1/deployment_spec_gateway.go index 48d4437ce..5909fecf1 100644 --- a/pkg/apis/deployment/v2alpha1/deployment_spec_gateway.go +++ b/pkg/apis/deployment/v2alpha1/deployment_spec_gateway.go @@ -23,10 +23,17 @@ package v2alpha1 import "github.com/arangodb/kube-arangodb/pkg/util" type DeploymentSpecGateway struct { - Enabled *bool `json:"enabled,omitempty"` - Image *string `json:"image"` + // Enabled setting enables/disables support for gateway in the cluster. + // When enabled, the cluster will contain a number of `gateway` servers. + // +doc/default: false + Enabled *bool `json:"enabled,omitempty"` + + // Image is the image to use for the gateway. + // By default, the image is determined by the operator. + Image *string `json:"image"` } +// IsEnabled returns whether the gateway is enabled. func (d *DeploymentSpecGateway) IsEnabled() bool { if d == nil || d.Enabled == nil { return false @@ -35,10 +42,12 @@ func (d *DeploymentSpecGateway) IsEnabled() bool { return *d.Enabled } +// Validate the given spec func (d *DeploymentSpecGateway) Validate() error { return nil } +// GetImage returns the image to use for the gateway. func (d *DeploymentSpecGateway) GetImage() string { return util.TypeOrDefault[string](d.Image) }