From b855a72d2d07001f14842ac84d2c94ad1a587964 Mon Sep 17 00:00:00 2001 From: George Kechagias Date: Tue, 11 Nov 2025 13:35:09 +0200 Subject: [PATCH 1/2] K8SPS-616 image not required for backups when they are disabled --- api/v1/perconaservermysql_types.go | 36 +++++++++++---------- api/v1/perconaservermysql_types_test.go | 43 +++++++++++++++++++++++-- 2 files changed, 59 insertions(+), 20 deletions(-) diff --git a/api/v1/perconaservermysql_types.go b/api/v1/perconaservermysql_types.go index b7c35e62c..f9d8fd038 100644 --- a/api/v1/perconaservermysql_types.go +++ b/api/v1/perconaservermysql_types.go @@ -754,24 +754,26 @@ func (cr *PerconaServerMySQL) CheckNSetDefaults(_ context.Context, serverVersion cr.Spec.Backup = new(BackupSpec) } - if len(cr.Spec.Backup.Image) == 0 { - return errors.New("backup.image can't be empty") - } - - scheduleNames := make(map[string]struct{}, len(cr.Spec.Backup.Schedule)) - for _, sch := range cr.Spec.Backup.Schedule { - if _, ok := scheduleNames[sch.Name]; ok { - return errors.Errorf("scheduled backups should have different names: %s name is used by multiple schedules", sch.Name) + if cr.Spec.Backup.Enabled { + if len(cr.Spec.Backup.Image) == 0 { + return errors.New("backup.image can't be empty") } - scheduleNames[sch.Name] = struct{}{} - _, ok := cr.Spec.Backup.Storages[sch.StorageName] - if !ok { - return errors.Errorf("storage %s doesn't exist", sch.StorageName) - } - if sch.Schedule != "" { - _, err := cron.ParseStandard(sch.Schedule) - if err != nil { - return errors.Wrap(err, "invalid schedule format") + + scheduleNames := make(map[string]struct{}, len(cr.Spec.Backup.Schedule)) + for _, sch := range cr.Spec.Backup.Schedule { + if _, ok := scheduleNames[sch.Name]; ok { + return errors.Errorf("scheduled backups should have different names: %s name is used by multiple schedules", sch.Name) + } + scheduleNames[sch.Name] = struct{}{} + _, ok := cr.Spec.Backup.Storages[sch.StorageName] + if !ok { + return errors.Errorf("storage %s doesn't exist", sch.StorageName) + } + if sch.Schedule != "" { + _, err := cron.ParseStandard(sch.Schedule) + if err != nil { + return errors.Wrap(err, "invalid schedule format") + } } } } diff --git a/api/v1/perconaservermysql_types_test.go b/api/v1/perconaservermysql_types_test.go index 7c5c34a9a..a0f81d89c 100644 --- a/api/v1/perconaservermysql_types_test.go +++ b/api/v1/perconaservermysql_types_test.go @@ -10,8 +10,18 @@ import ( ) func TestCheckNSetDefaults(t *testing.T) { - t.Run("empty cr", func(t *testing.T) { + t.Run("with invalid cluster type", func(t *testing.T) { cr := new(PerconaServerMySQL) + cr.Spec.MySQL.ClusterType = "invalid" + + err := cr.CheckNSetDefaults(t.Context(), nil) + assert.EqualError(t, err, "invalid is not a valid clusterType, valid options are group-replication and async") + }) + t.Run("empty cr with backups enabled", func(t *testing.T) { + cr := new(PerconaServerMySQL) + cr.Spec.Backup = &BackupSpec{ + Enabled: true, + } err := cr.CheckNSetDefaults(t.Context(), nil) assert.EqualError(t, err, "backup.image can't be empty") @@ -19,16 +29,43 @@ func TestCheckNSetDefaults(t *testing.T) { t.Run("with backup image", func(t *testing.T) { cr := new(PerconaServerMySQL) cr.Spec.Backup = &BackupSpec{ - Image: "backup-image", + Enabled: true, + Image: "backup-image", } err := cr.CheckNSetDefaults(t.Context(), nil) assert.EqualError(t, err, "reconcile mysql volumeSpec: volumeSpec provided is nil") }) + t.Run("scheduled backups should have different names", func(t *testing.T) { + cr := new(PerconaServerMySQL) + cr.Spec.Backup = &BackupSpec{ + Enabled: true, + Image: "backup-image", + Schedule: []BackupSchedule{ + { + Name: "backup-schedule-1", + StorageName: "name", + Schedule: "* * * * *", + }, + { + Name: "backup-schedule-1", + StorageName: "name", + Schedule: "* * * * *", + }, + }, + } + cr.Spec.Backup.Storages = map[string]*BackupStorageSpec{ + "name": {}, + } + + err := cr.CheckNSetDefaults(t.Context(), nil) + assert.EqualError(t, err, "scheduled backups should have different names: backup-schedule-1 name is used by multiple schedules") + }) t.Run("with backup image and volume spec", func(t *testing.T) { cr := new(PerconaServerMySQL) cr.Spec.Backup = &BackupSpec{ - Image: "backup-image", + Enabled: true, + Image: "backup-image", } cr.Spec.MySQL.VolumeSpec = &VolumeSpec{ PersistentVolumeClaim: &corev1.PersistentVolumeClaimSpec{ From b96dc4c14f14be577e2a1c4b9ab7145f8a0c9a54 Mon Sep 17 00:00:00 2001 From: George Kechagias Date: Tue, 11 Nov 2025 13:53:03 +0200 Subject: [PATCH 2/2] add test case for missing backup spec --- api/v1/perconaservermysql_types_test.go | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/api/v1/perconaservermysql_types_test.go b/api/v1/perconaservermysql_types_test.go index a0f81d89c..1d3e239ea 100644 --- a/api/v1/perconaservermysql_types_test.go +++ b/api/v1/perconaservermysql_types_test.go @@ -80,6 +80,24 @@ func TestCheckNSetDefaults(t *testing.T) { }, } + err := cr.CheckNSetDefaults(t.Context(), nil) + assert.NoError(t, err) + }) + t.Run("without backup image, with volume spec", func(t *testing.T) { + cr := new(PerconaServerMySQL) + cr.Spec.MySQL.VolumeSpec = &VolumeSpec{ + PersistentVolumeClaim: &corev1.PersistentVolumeClaimSpec{ + Resources: corev1.VolumeResourceRequirements{ + Limits: corev1.ResourceList{ + corev1.ResourceStorage: resource.MustParse("1G"), + }, + Requests: corev1.ResourceList{ + corev1.ResourceStorage: resource.MustParse("1G"), + }, + }, + }, + } + err := cr.CheckNSetDefaults(t.Context(), nil) assert.NoError(t, err) })