Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

OAS-10007 Fix race condition in ArangoBackup #1708

Merged
merged 2 commits into from
Aug 28, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
13 changes: 11 additions & 2 deletions pkg/apis/deployment/v2alpha1/deployment_spec_gateway.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
jwierzbo marked this conversation as resolved.
Show resolved Hide resolved
// 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
Expand All @@ -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)
}
8 changes: 8 additions & 0 deletions pkg/handlers/backup/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
47 changes: 47 additions & 0 deletions pkg/handlers/backup/handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
})
}