From 78b865304b8604dcb5267d150315b47ed44bb34e Mon Sep 17 00:00:00 2001 From: Kacper Rzetelski Date: Thu, 3 Oct 2024 11:58:52 +0200 Subject: [PATCH 1/3] Add Labels to manager tasks' statuses in ScyllaCluster API --- pkg/api/scylla/v1/types_cluster.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/pkg/api/scylla/v1/types_cluster.go b/pkg/api/scylla/v1/types_cluster.go index 4d1dba2ac89..1222f608dee 100644 --- a/pkg/api/scylla/v1/types_cluster.go +++ b/pkg/api/scylla/v1/types_cluster.go @@ -632,6 +632,10 @@ type TaskStatus struct { // +optional ID *string `json:"id,omitempty"` + // labels reflects the labels of a task. + // +optional + Labels map[string]string `json:"labels,omitempty"` + // error holds the task error, if any. // +optional Error *string `json:"error,omitempty"` From 82a78bc3cc8bcf35b40983b463b43413cb05895c Mon Sep 17 00:00:00 2001 From: Kacper Rzetelski Date: Mon, 7 Oct 2024 11:17:30 +0200 Subject: [PATCH 2/3] Set managed hash labels in manager tasks and only update tasks when managed hash changes --- pkg/controller/manager/status.go | 4 +- pkg/controller/manager/sync.go | 8 +- pkg/controller/manager/sync_action.go | 160 ++++----- pkg/controller/manager/sync_test.go | 434 +++++++++++++++-------- pkg/controller/manager/types_old.go | 98 ++--- pkg/controller/manager/types_old_test.go | 44 ++- 6 files changed, 417 insertions(+), 331 deletions(-) diff --git a/pkg/controller/manager/status.go b/pkg/controller/manager/status.go index bde47fbe5cb..26d7f36a431 100644 --- a/pkg/controller/manager/status.go +++ b/pkg/controller/manager/status.go @@ -31,7 +31,7 @@ func (c *Controller) calculateStatus(sc *scyllav1.ScyllaCluster, managerState *s managerTaskStatus, isInManagerState := managerState.RepairTasks[rt.Name] if isInManagerState { - repairTaskStatus = scyllav1.RepairTaskStatus(managerTaskStatus) + repairTaskStatus = managerTaskStatus } else { // Retain the error from client. err, hasClientError := repairTaskClientErrorMap[rt.Name] @@ -62,7 +62,7 @@ func (c *Controller) calculateStatus(sc *scyllav1.ScyllaCluster, managerState *s managerTaskStatus, isInManagerState := managerState.BackupTasks[bt.Name] if isInManagerState { - backupTaskStatus = scyllav1.BackupTaskStatus(managerTaskStatus) + backupTaskStatus = managerTaskStatus } else { // Retain the error from client. err, hasClientError := backupTaskClientErrorMap[bt.Name] diff --git a/pkg/controller/manager/sync.go b/pkg/controller/manager/sync.go index db25ebc05a5..ffefe7e5d11 100644 --- a/pkg/controller/manager/sync.go +++ b/pkg/controller/manager/sync.go @@ -30,8 +30,8 @@ func (c *Controller) getManagerState(ctx context.Context, clusterID string) (*st return nil, err } var ( - repairTasks map[string]RepairTaskStatus - backupTasks map[string]BackupTaskStatus + repairTasks map[string]scyllav1.RepairTaskStatus + backupTasks map[string]scyllav1.BackupTaskStatus ) if clusterID != "" { @@ -48,7 +48,7 @@ func (c *Controller) getManagerState(ctx context.Context, clusterID string) (*st return nil, err } - repairTasks = make(map[string]RepairTaskStatus, len(managerRepairTasks.TaskListItemSlice)) + repairTasks = make(map[string]scyllav1.RepairTaskStatus, len(managerRepairTasks.TaskListItemSlice)) for _, managerRepairTask := range managerRepairTasks.TaskListItemSlice { rts, err := NewRepairStatusFromManager(managerRepairTask) if err != nil { @@ -62,7 +62,7 @@ func (c *Controller) getManagerState(ctx context.Context, clusterID string) (*st return nil, err } - backupTasks = make(map[string]BackupTaskStatus, len(managerBackupTasks.TaskListItemSlice)) + backupTasks = make(map[string]scyllav1.BackupTaskStatus, len(managerBackupTasks.TaskListItemSlice)) for _, managerBackupTask := range managerBackupTasks.TaskListItemSlice { bts, err := NewBackupStatusFromManager(managerBackupTask) if err != nil { diff --git a/pkg/controller/manager/sync_action.go b/pkg/controller/manager/sync_action.go index 56a482fe19f..5c072a788a9 100644 --- a/pkg/controller/manager/sync_action.go +++ b/pkg/controller/manager/sync_action.go @@ -5,7 +5,6 @@ package manager import ( "context" "fmt" - "reflect" "strings" "github.com/pkg/errors" @@ -21,8 +20,8 @@ import ( type state struct { Clusters []*managerclient.Cluster - RepairTasks map[string]RepairTaskStatus - BackupTasks map[string]BackupTaskStatus + RepairTasks map[string]scyllav1.RepairTaskStatus + BackupTasks map[string]scyllav1.BackupTaskStatus } func runSync(ctx context.Context, cluster *scyllav1.ScyllaCluster, authToken string, state *state) ([]action, bool, error) { @@ -146,14 +145,26 @@ func syncBackupTasks(clusterID string, cluster *scyllav1.ScyllaCluster, managerS var actions []action for _, bt := range cluster.Spec.Backups { - action, err := syncBackupTask(clusterID, managerState, &bt) + taskStatusFunc := func() (*scyllav1.TaskStatus, bool) { + s, ok := managerState.BackupTasks[bt.Name] + if !ok { + return nil, false + } + + return &s.TaskStatus, true + } + + backupTaskSpecCopy := bt.DeepCopy() + backupTaskSpec := BackupTaskSpec(*backupTaskSpecCopy) + + a, err := syncTask(clusterID, &backupTaskSpec, taskStatusFunc) if err != nil { errs = append(errs, fmt.Errorf("can't sync backup task %q: %w", bt.Name, err)) continue } - if action != nil { - actions = append(actions, action) + if a != nil { + actions = append(actions, a) } } @@ -165,68 +176,31 @@ func syncBackupTasks(clusterID string, cluster *scyllav1.ScyllaCluster, managerS return actions, nil } -func syncBackupTask(clusterID string, managerState *state, backupTaskSpec *scyllav1.BackupTaskSpec) (action, error) { - backupTaskSpecCopy := backupTaskSpec.DeepCopy() - backupTask := BackupTaskSpec(*backupTaskSpecCopy) - - managerTaskStatus, ok := managerState.BackupTasks[backupTask.Name] - if !ok { - managerClientTask, err := backupTask.ToManager() - if err != nil { - return nil, fmt.Errorf("can't convert backup task to manager task: %w", err) - } - - return &addTaskAction{ - clusterID: clusterID, - task: managerClientTask, - }, nil - } - - if managerTaskStatus.ID == nil { - // Sanity check. - return nil, fmt.Errorf("manager task status is missing an id") - } - - evaluateDates(&backupTask, &managerTaskStatus) - - // FIXME: Task spec is converted to status to compare it with its current state in manager. - // This is a temporary workaround and should be replaced with hash comparison when manager allows for storing metadata. - // Ref: https://github.com/scylladb/scylla-operator/issues/1827. - if isBackupTaskDeepEqual(&backupTask, &managerTaskStatus) { - return nil, nil - } - - managerClientTask, err := backupTask.ToManager() - if err != nil { - return nil, fmt.Errorf("can't convert backup task to manager task: %w", err) - } - managerClientTask.ID = *managerTaskStatus.ID - - return &updateTaskAction{ - clusterID: clusterID, - task: managerClientTask, - }, nil -} - -func isBackupTaskDeepEqual(backupTaskSpec *BackupTaskSpec, managerBackupTaskStatus *BackupTaskStatus) bool { - backupTaskStatus := backupTaskSpec.ToStatus() - backupTaskStatus.ID = managerBackupTaskStatus.ID - return reflect.DeepEqual(backupTaskStatus, managerBackupTaskStatus) -} - func syncRepairTasks(clusterID string, cluster *scyllav1.ScyllaCluster, managerState *state) ([]action, error) { var errs []error var actions []action for _, rt := range cluster.Spec.Repairs { - action, err := syncRepairTask(clusterID, managerState, &rt) + taskStatusFunc := func() (*scyllav1.TaskStatus, bool) { + s, ok := managerState.RepairTasks[rt.Name] + if !ok { + return nil, false + } + + return &s.TaskStatus, true + } + + repairTaskSpecCopy := rt.DeepCopy() + repairTaskSpec := RepairTaskSpec(*repairTaskSpecCopy) + + a, err := syncTask(clusterID, &repairTaskSpec, taskStatusFunc) if err != nil { errs = append(errs, fmt.Errorf("can't sync repair task %q: %w", rt.Name, err)) continue } - if action != nil { - actions = append(actions, action) + if a != nil { + actions = append(actions, a) } } @@ -238,16 +212,20 @@ func syncRepairTasks(clusterID string, cluster *scyllav1.ScyllaCluster, managerS return actions, nil } -func syncRepairTask(clusterID string, managerState *state, repairTaskSpec *scyllav1.RepairTaskSpec) (action, error) { - repairTaskSpecCopy := repairTaskSpec.DeepCopy() - repairTask := RepairTaskSpec(*repairTaskSpecCopy) +func syncTask(clusterID string, spec taskSpecInterface, statusFunc func() (*scyllav1.TaskStatus, bool)) (action, error) { + managedHash, err := spec.GetObjectHash() + if err != nil { + return nil, fmt.Errorf("can't get object hash: %w", err) + } - managerTaskStatus, ok := managerState.RepairTasks[repairTask.Name] + var managerClientTask *managerclient.Task + status, ok := statusFunc() if !ok { - managerClientTask, err := repairTask.ToManager() + managerClientTask, err = spec.ToManager() if err != nil { - return nil, fmt.Errorf("can't convert repair task to manager task: %w", err) + return nil, fmt.Errorf("can't convert task to manager task: %w", err) } + setManagerClientTaskManagedHashLabel(managerClientTask, managedHash) return &addTaskAction{ clusterID: clusterID, @@ -255,25 +233,23 @@ func syncRepairTask(clusterID string, managerState *state, repairTaskSpec *scyll }, nil } - if managerTaskStatus.ID == nil { - // Sanity check. - return nil, fmt.Errorf("manager task status is missing an id") - } - - evaluateDates(&repairTask, &managerTaskStatus) - - // FIXME: Task spec is converted to status to compare it with its current state in manager. - // This is a temporary workaround and should be replaced with hash comparison when manager allows for storing metadata. - // Ref: https://github.com/scylladb/scylla-operator/issues/1827. - if isRepairTaskDeepEqual(&repairTask, &managerTaskStatus) { + if managedHash == status.Labels[naming.ManagedHash] { + // Tasks are equal, do nothing. return nil, nil } - managerClientTask, err := repairTask.ToManager() + evaluateDates(spec.GetTaskSpec(), status) + managerClientTask, err = spec.ToManager() if err != nil { - return nil, fmt.Errorf("can't convert repair task to manager task: %w", err) + return nil, fmt.Errorf("can't convert task to manager task: %w", err) + } + setManagerClientTaskManagedHashLabel(managerClientTask, managedHash) + + if status.ID == nil { + // Sanity check. + return nil, fmt.Errorf("manager task status is missing an id") } - managerClientTask.ID = *managerTaskStatus.ID + managerClientTask.ID = *status.ID return &updateTaskAction{ clusterID: clusterID, @@ -281,17 +257,20 @@ func syncRepairTask(clusterID string, managerState *state, repairTaskSpec *scyll }, nil } -func isRepairTaskDeepEqual(repairTaskSpec *RepairTaskSpec, managerRepairTaskStatus *RepairTaskStatus) bool { - repairTaskStatus := repairTaskSpec.ToStatus() - repairTaskStatus.ID = managerRepairTaskStatus.ID - return reflect.DeepEqual(repairTaskStatus, managerRepairTaskStatus) -} +func evaluateDates(spec *scyllav1.TaskSpec, taskStatus *scyllav1.TaskStatus) { + var specStartDate string + if spec.StartDate != nil { + specStartDate = *spec.StartDate + } -func evaluateDates(spec startDateGetterSetter, managerTask startDateGetter) { - startDate := spec.GetStartDateOrEmpty() // Keep special "now" value evaluated on task creation. - if len(startDate) == 0 || strings.HasPrefix(startDate, "now") { - spec.SetStartDate(managerTask.GetStartDateOrEmpty()) + if len(specStartDate) == 0 || strings.HasPrefix(specStartDate, "now") { + var statusStartDate string + if taskStatus.StartDate != nil { + statusStartDate = *taskStatus.StartDate + } + + spec.StartDate = &statusStartDate } } @@ -487,3 +466,10 @@ func updateBackupTaskStatusError(backupTaskStatuses *[]scyllav1.BackupTaskStatus *backupTaskStatuses = append(*backupTaskStatuses, backupTaskStatus) } + +func setManagerClientTaskManagedHashLabel(task *managerclient.Task, hash string) { + if task.Labels == nil { + task.Labels = map[string]string{} + } + task.Labels[naming.ManagedHash] = hash +} diff --git a/pkg/controller/manager/sync_test.go b/pkg/controller/manager/sync_test.go index f5fb0256d46..7acb09634de 100644 --- a/pkg/controller/manager/sync_test.go +++ b/pkg/controller/manager/sync_test.go @@ -211,7 +211,7 @@ func TestManagerSynchronization(t *testing.T) { Name: clusterName, AuthToken: clusterAuthToken, }}, - RepairTasks: map[string]RepairTaskStatus{ + RepairTasks: map[string]scyllav1.RepairTaskStatus{ "repair": { TaskStatus: scyllav1.TaskStatus{ SchedulerTaskStatus: scyllav1.SchedulerTaskStatus{ @@ -256,7 +256,7 @@ func TestManagerSynchronization(t *testing.T) { Name: clusterName, AuthToken: clusterAuthToken, }}, - RepairTasks: map[string]RepairTaskStatus{ + RepairTasks: map[string]scyllav1.RepairTaskStatus{ "repair": { TaskStatus: scyllav1.TaskStatus{ SchedulerTaskStatus: scyllav1.SchedulerTaskStatus{ @@ -265,6 +265,9 @@ func TestManagerSynchronization(t *testing.T) { }, Name: "repair", ID: pointer.Ptr("repair-id"), + Labels: map[string]string{ + "scylla-operator.scylladb.com/managed-hash": "9gAqa0Ngh483/n6qTDn3FMKGvyMXrUKqPS3Jp5RDp8RJ1/58h8p5oYrtP7r6rYmNoRY1neKQHvV1IIzmMr6hBg==", + }, }, FailFast: pointer.Ptr(false), Intensity: pointer.Ptr("666"), @@ -287,7 +290,7 @@ func TestManagerSynchronization(t *testing.T) { Name: clusterName, AuthToken: clusterAuthToken, }}, - RepairTasks: map[string]RepairTaskStatus{ + RepairTasks: map[string]scyllav1.RepairTaskStatus{ "other-repair": { TaskStatus: scyllav1.TaskStatus{ Name: "other-repair", @@ -330,7 +333,7 @@ func TestManagerSynchronization(t *testing.T) { Name: clusterName, AuthToken: clusterAuthToken, }}, - RepairTasks: map[string]RepairTaskStatus{ + RepairTasks: map[string]scyllav1.RepairTaskStatus{ "repair": { TaskStatus: scyllav1.TaskStatus{ SchedulerTaskStatus: scyllav1.SchedulerTaskStatus{ @@ -339,6 +342,9 @@ func TestManagerSynchronization(t *testing.T) { }, Name: "repair", ID: pointer.Ptr("repair-id"), + Labels: map[string]string{ + "scylla-operator.scylladb.com/managed-hash": "QAYSzOPRCIVGqS04NfnFslWujYbjmjwNjH//peQawBxry6I6M/scdCaHR2qgNOL9YJQsGjnD846eO0oULMyJ8A==", + }, }, FailFast: pointer.Ptr(false), Intensity: pointer.Ptr("666"), @@ -377,7 +383,7 @@ func TestManagerSynchronization(t *testing.T) { Name: clusterName, AuthToken: clusterAuthToken, }}, - RepairTasks: map[string]RepairTaskStatus{ + RepairTasks: map[string]scyllav1.RepairTaskStatus{ "repair": { TaskStatus: scyllav1.TaskStatus{ SchedulerTaskStatus: scyllav1.SchedulerTaskStatus{ @@ -397,6 +403,225 @@ func TestManagerSynchronization(t *testing.T) { Actions: []action{&updateTaskAction{clusterID: clusterID, task: &managerclient.Task{ID: "repair-id"}}}, }, + { + Name: "Task gets updated when managed hash is missing", + Spec: scyllav1.ScyllaClusterSpec{ + Repairs: []scyllav1.RepairTaskSpec{ + { + TaskSpec: scyllav1.TaskSpec{ + Name: "repair", + SchedulerTaskSpec: scyllav1.SchedulerTaskSpec{ + StartDate: pointer.Ptr("2021-01-01T11:11:11Z"), + Interval: pointer.Ptr("0"), + }, + }, + SmallTableThreshold: "1GiB", + Intensity: "666", + Parallel: 0, + }, + }, + }, + Status: scyllav1.ScyllaClusterStatus{ + ManagerID: pointer.Ptr(clusterID), + }, + State: state{ + Clusters: []*managerclient.Cluster{{ + ID: clusterID, + Name: clusterName, + AuthToken: clusterAuthToken, + }}, + RepairTasks: map[string]scyllav1.RepairTaskStatus{ + "repair": { + TaskStatus: scyllav1.TaskStatus{ + SchedulerTaskStatus: scyllav1.SchedulerTaskStatus{ + StartDate: pointer.Ptr("2021-01-01T11:11:11Z"), + Interval: pointer.Ptr("0"), + }, + Name: "repair", + ID: pointer.Ptr("repair-id"), + Labels: map[string]string{}, + }, + FailFast: pointer.Ptr(false), + Intensity: pointer.Ptr("666"), + Parallel: pointer.Ptr[int64](0), + SmallTableThreshold: pointer.Ptr("1GiB"), + }, + }, + }, + Actions: []action{&updateTaskAction{clusterID: clusterID, task: &managerclient.Task{ID: "repair-id"}}}, + }, + { + Name: "Task gets updated when managed hash is empty", + Spec: scyllav1.ScyllaClusterSpec{ + Repairs: []scyllav1.RepairTaskSpec{ + { + TaskSpec: scyllav1.TaskSpec{ + Name: "repair", + SchedulerTaskSpec: scyllav1.SchedulerTaskSpec{ + StartDate: pointer.Ptr("2021-01-01T11:11:11Z"), + Interval: pointer.Ptr("0"), + }, + }, + SmallTableThreshold: "1GiB", + Intensity: "666", + Parallel: 0, + }, + }, + }, + Status: scyllav1.ScyllaClusterStatus{ + ManagerID: pointer.Ptr(clusterID), + }, + State: state{ + Clusters: []*managerclient.Cluster{{ + ID: clusterID, + Name: clusterName, + AuthToken: clusterAuthToken, + }}, + RepairTasks: map[string]scyllav1.RepairTaskStatus{ + "repair": { + TaskStatus: scyllav1.TaskStatus{ + SchedulerTaskStatus: scyllav1.SchedulerTaskStatus{ + StartDate: pointer.Ptr("2021-01-01T11:11:11Z"), + Interval: pointer.Ptr("0"), + }, + Name: "repair", + ID: pointer.Ptr("repair-id"), + Labels: map[string]string{ + "scylla-operator.scylladb.com/managed-hash": "", + }, + }, + FailFast: pointer.Ptr(false), + Intensity: pointer.Ptr("666"), + Parallel: pointer.Ptr[int64](0), + SmallTableThreshold: pointer.Ptr("1GiB"), + }, + }, + }, + Actions: []action{&updateTaskAction{clusterID: clusterID, task: &managerclient.Task{ID: "repair-id"}}}, + }, + { + Name: "Task gets updated when managed hash does not match", + Spec: scyllav1.ScyllaClusterSpec{ + Repairs: []scyllav1.RepairTaskSpec{ + { + TaskSpec: scyllav1.TaskSpec{ + Name: "repair", + SchedulerTaskSpec: scyllav1.SchedulerTaskSpec{ + StartDate: pointer.Ptr("2021-01-01T11:11:11Z"), + Interval: pointer.Ptr("0"), + }, + }, + SmallTableThreshold: "1GiB", + Intensity: "666", + Parallel: 0, + }, + }, + }, + Status: scyllav1.ScyllaClusterStatus{ + ManagerID: pointer.Ptr(clusterID), + }, + State: state{ + Clusters: []*managerclient.Cluster{{ + ID: clusterID, + Name: clusterName, + AuthToken: clusterAuthToken, + }}, + RepairTasks: map[string]scyllav1.RepairTaskStatus{ + "repair": { + TaskStatus: scyllav1.TaskStatus{ + SchedulerTaskStatus: scyllav1.SchedulerTaskStatus{ + StartDate: pointer.Ptr("2021-01-01T11:11:11Z"), + Interval: pointer.Ptr("0"), + }, + Name: "repair", + ID: pointer.Ptr("repair-id"), + Labels: map[string]string{ + "scylla-operator.scylladb.com/managed-hash": "non-matching-hash", + }, + }, + FailFast: pointer.Ptr(false), + Intensity: pointer.Ptr("666"), + Parallel: pointer.Ptr[int64](0), + SmallTableThreshold: pointer.Ptr("1GiB"), + }, + }, + }, + Actions: []action{&updateTaskAction{clusterID: clusterID, task: &managerclient.Task{ID: "repair-id"}}}, + }, + { + Name: "Tasks do not get updated when in-manager state differs but managed hashes match specs", + Spec: scyllav1.ScyllaClusterSpec{ + Repairs: []scyllav1.RepairTaskSpec{ + { + TaskSpec: scyllav1.TaskSpec{ + Name: "repair", + SchedulerTaskSpec: scyllav1.SchedulerTaskSpec{ + StartDate: pointer.Ptr("2021-01-01T11:11:11Z"), + }, + }, + SmallTableThreshold: "10GiB", + Intensity: "666", + Parallel: 3, + }, + }, + Backups: []scyllav1.BackupTaskSpec{ + { + TaskSpec: scyllav1.TaskSpec{ + Name: "backup", + SchedulerTaskSpec: scyllav1.SchedulerTaskSpec{ + StartDate: pointer.Ptr("2021-01-01T11:11:11Z"), + }, + }, + Location: []string{"s3:backup"}, + Retention: 3, + }, + }, + }, + Status: scyllav1.ScyllaClusterStatus{ + ManagerID: pointer.Ptr(clusterID), + }, + State: state{ + Clusters: []*managerclient.Cluster{{ + ID: clusterID, + Name: clusterName, + AuthToken: clusterAuthToken, + }}, + RepairTasks: map[string]scyllav1.RepairTaskStatus{ + "repair": { + TaskStatus: scyllav1.TaskStatus{ + SchedulerTaskStatus: scyllav1.SchedulerTaskStatus{ + StartDate: pointer.Ptr("2006-01-02T15:04:05Z"), + }, + Name: "repair", + ID: pointer.Ptr("repair-id"), + Labels: map[string]string{ + "scylla-operator.scylladb.com/managed-hash": "JcrUPldfq9/FT/tAaXzdY2aclZFsjTlRsYDh7LEjM3K5XRbl8w+jUGvZdBHRRSZ28TdWu2dsa/L5LBxxWjIpHw==", + }, + }, + SmallTableThreshold: pointer.Ptr("1GiB"), + Intensity: pointer.Ptr("1"), + Parallel: pointer.Ptr[int64](1), + }, + }, + BackupTasks: map[string]scyllav1.BackupTaskStatus{ + "backup": { + TaskStatus: scyllav1.TaskStatus{ + SchedulerTaskStatus: scyllav1.SchedulerTaskStatus{ + StartDate: pointer.Ptr("2006-01-02T15:04:05Z"), + }, + Name: "backup", + ID: pointer.Ptr("backup-id"), + Labels: map[string]string{ + "scylla-operator.scylladb.com/managed-hash": "NpboZYiDmzZfS84omU0kgZxxDzg5p3IEhKCWU0sS6G0QpSh2KZFPHB7AlJohyqo+RjBG2aEIqbBW8GiDo18uxw==", + }, + }, + Location: []string{"s3:other-backup"}, + Retention: pointer.Ptr[int64](1), + }, + }, + }, + Actions: nil, + }, } for _, test := range tcs { @@ -446,198 +671,93 @@ func actionComparer(a action, b action) bool { return false } -func TestBackupTaskChanged(t *testing.T) { +func TestEvaluateDates(t *testing.T) { ts := []struct { - name string - spec *BackupTaskSpec - managerTask *BackupTaskStatus - expected *BackupTaskStatus + name string + spec *scyllav1.TaskSpec + status *scyllav1.TaskStatus + expected scyllav1.TaskStatus }{ { name: "Task startDate is changed to one from manager state when it's not provided", - spec: &BackupTaskSpec{ - TaskSpec: scyllav1.TaskSpec{ - SchedulerTaskSpec: scyllav1.SchedulerTaskSpec{}, - }, + spec: &scyllav1.TaskSpec{ + SchedulerTaskSpec: scyllav1.SchedulerTaskSpec{}, }, - managerTask: &BackupTaskStatus{ - TaskStatus: scyllav1.TaskStatus{ - SchedulerTaskStatus: scyllav1.SchedulerTaskStatus{ - StartDate: pointer.Ptr("2021-01-01T11:11:11Z"), - }, + status: &scyllav1.TaskStatus{ + SchedulerTaskStatus: scyllav1.SchedulerTaskStatus{ + StartDate: pointer.Ptr("2021-01-01T11:11:11Z"), }, }, - expected: &BackupTaskStatus{ - TaskStatus: scyllav1.TaskStatus{ - SchedulerTaskStatus: scyllav1.SchedulerTaskStatus{ - StartDate: pointer.Ptr("2021-01-01T11:11:11Z"), - }, + expected: scyllav1.TaskStatus{ + SchedulerTaskStatus: scyllav1.SchedulerTaskStatus{ + StartDate: pointer.Ptr("2021-01-01T11:11:11Z"), }, - Retention: pointer.Ptr[int64](0), }, }, { name: "Task startDate is changed to one from manager state when it's an empty string", - spec: &BackupTaskSpec{ - TaskSpec: scyllav1.TaskSpec{ - SchedulerTaskSpec: scyllav1.SchedulerTaskSpec{ - StartDate: pointer.Ptr(""), - }, - }}, - managerTask: &BackupTaskStatus{ - TaskStatus: scyllav1.TaskStatus{ - SchedulerTaskStatus: scyllav1.SchedulerTaskStatus{ - StartDate: pointer.Ptr("2021-01-01T11:11:11Z"), - }, - }}, - expected: &BackupTaskStatus{ - TaskStatus: scyllav1.TaskStatus{ - SchedulerTaskStatus: scyllav1.SchedulerTaskStatus{ - StartDate: pointer.Ptr("2021-01-01T11:11:11Z"), - }, - }, - Retention: pointer.Ptr[int64](0), - }, - }, - { - name: "Task startDate is changed to one from manager state when prefix is 'now'", - spec: &BackupTaskSpec{ - TaskSpec: scyllav1.TaskSpec{ - SchedulerTaskSpec: scyllav1.SchedulerTaskSpec{ - StartDate: pointer.Ptr("now"), - }, - }, - }, - managerTask: &BackupTaskStatus{ - TaskStatus: scyllav1.TaskStatus{ - SchedulerTaskStatus: scyllav1.SchedulerTaskStatus{ - StartDate: pointer.Ptr("2021-01-01T11:11:11Z"), - }, + spec: &scyllav1.TaskSpec{ + SchedulerTaskSpec: scyllav1.SchedulerTaskSpec{ + StartDate: pointer.Ptr(""), }, }, - expected: &BackupTaskStatus{ - TaskStatus: scyllav1.TaskStatus{ - SchedulerTaskStatus: scyllav1.SchedulerTaskStatus{ - StartDate: pointer.Ptr("2021-01-01T11:11:11Z"), - }, + status: &scyllav1.TaskStatus{ + SchedulerTaskStatus: scyllav1.SchedulerTaskStatus{ + StartDate: pointer.Ptr("2021-01-01T11:11:11Z"), }, - Retention: pointer.Ptr[int64](0), }, - }, - } - - for _, test := range ts { - t.Run(test.name, func(t *testing.T) { - t.Parallel() - - evaluateDates(test.spec, test.managerTask) - got := test.spec.ToStatus() - if !reflect.DeepEqual(got, test.expected) { - t.Errorf("expected and got repair task statuses differ: %s", cmp.Diff(test.expected, got)) - } - }) - } -} - -func TestRepairTaskChanged(t *testing.T) { - ts := []struct { - name string - spec *RepairTaskSpec - managerTask *RepairTaskStatus - expected *RepairTaskStatus - }{ - { - name: "Task startDate is changed to one from manager state when it's not provided", - spec: &RepairTaskSpec{ - TaskSpec: scyllav1.TaskSpec{ - SchedulerTaskSpec: scyllav1.SchedulerTaskSpec{}, - }, - }, - managerTask: &RepairTaskStatus{ - TaskStatus: scyllav1.TaskStatus{ - SchedulerTaskStatus: scyllav1.SchedulerTaskStatus{ - StartDate: pointer.Ptr("2021-01-01T11:11:11Z"), - }, - }, - }, - expected: &RepairTaskStatus{ - TaskStatus: scyllav1.TaskStatus{ - SchedulerTaskStatus: scyllav1.SchedulerTaskStatus{ - StartDate: pointer.Ptr("2021-01-01T11:11:11Z"), - }, + expected: scyllav1.TaskStatus{ + SchedulerTaskStatus: scyllav1.SchedulerTaskStatus{ + StartDate: pointer.Ptr("2021-01-01T11:11:11Z"), }, - FailFast: pointer.Ptr(false), - Intensity: pointer.Ptr(""), - Parallel: pointer.Ptr[int64](0), - SmallTableThreshold: pointer.Ptr(""), }, }, { - name: "Task startDate is changed to one from manager state when it's an empty string", - spec: &RepairTaskSpec{ - TaskSpec: scyllav1.TaskSpec{ - SchedulerTaskSpec: scyllav1.SchedulerTaskSpec{ - StartDate: pointer.Ptr(""), - }, + name: "Task startDate is changed to one from manager state when it's 'now' literal", + spec: &scyllav1.TaskSpec{ + SchedulerTaskSpec: scyllav1.SchedulerTaskSpec{ + StartDate: pointer.Ptr("now"), }, }, - managerTask: &RepairTaskStatus{ - TaskStatus: scyllav1.TaskStatus{ - SchedulerTaskStatus: scyllav1.SchedulerTaskStatus{ - StartDate: pointer.Ptr("2021-01-01T11:11:11Z"), - }, + status: &scyllav1.TaskStatus{ + SchedulerTaskStatus: scyllav1.SchedulerTaskStatus{ + StartDate: pointer.Ptr("2021-01-01T11:11:11Z"), }, }, - expected: &RepairTaskStatus{ - TaskStatus: scyllav1.TaskStatus{ - SchedulerTaskStatus: scyllav1.SchedulerTaskStatus{ - StartDate: pointer.Ptr("2021-01-01T11:11:11Z"), - }, + expected: scyllav1.TaskStatus{ + SchedulerTaskStatus: scyllav1.SchedulerTaskStatus{ + StartDate: pointer.Ptr("2021-01-01T11:11:11Z"), }, - FailFast: pointer.Ptr(false), - Intensity: pointer.Ptr(""), - Parallel: pointer.Ptr[int64](0), - SmallTableThreshold: pointer.Ptr(""), }, }, { name: "Task startDate is changed to one from manager state when prefix is 'now'", - spec: &RepairTaskSpec{ - TaskSpec: scyllav1.TaskSpec{ - SchedulerTaskSpec: scyllav1.SchedulerTaskSpec{ - StartDate: pointer.Ptr("now"), - }, + spec: &scyllav1.TaskSpec{ + SchedulerTaskSpec: scyllav1.SchedulerTaskSpec{ + StartDate: pointer.Ptr("now+3d2h10m"), }, }, - managerTask: &RepairTaskStatus{ - TaskStatus: scyllav1.TaskStatus{ - SchedulerTaskStatus: scyllav1.SchedulerTaskStatus{ - StartDate: pointer.Ptr("2021-01-01T11:11:11Z"), - }, + status: &scyllav1.TaskStatus{ + SchedulerTaskStatus: scyllav1.SchedulerTaskStatus{ + StartDate: pointer.Ptr("2021-01-01T11:11:11Z"), }, }, - expected: &RepairTaskStatus{ - TaskStatus: scyllav1.TaskStatus{ - SchedulerTaskStatus: scyllav1.SchedulerTaskStatus{ - StartDate: pointer.Ptr("2021-01-01T11:11:11Z"), - }, + expected: scyllav1.TaskStatus{ + SchedulerTaskStatus: scyllav1.SchedulerTaskStatus{ + StartDate: pointer.Ptr("2021-01-01T11:11:11Z"), }, - FailFast: pointer.Ptr(false), - Intensity: pointer.Ptr(""), - Parallel: pointer.Ptr[int64](0), - SmallTableThreshold: pointer.Ptr(""), }, }, } - for _, test := range ts { - t.Run(test.name, func(t *testing.T) { + for _, tc := range ts { + t.Run(tc.name, func(t *testing.T) { t.Parallel() - evaluateDates(test.spec, test.managerTask) - got := test.spec.ToStatus() - if !reflect.DeepEqual(got, test.expected) { - t.Errorf("expected and got backup task statuses differ: %s", cmp.Diff(test.expected, got)) + evaluateDates(tc.spec, tc.status) + got := taskSpecToStatus(tc.spec) + if !reflect.DeepEqual(got, tc.expected) { + t.Errorf("expected and got repair task statuses differ: %s", cmp.Diff(tc.expected, got)) } }) } diff --git a/pkg/controller/manager/types_old.go b/pkg/controller/manager/types_old.go index 12c70ff0817..64a3cc808a2 100644 --- a/pkg/controller/manager/types_old.go +++ b/pkg/controller/manager/types_old.go @@ -4,6 +4,7 @@ package manager import ( "fmt" + "maps" "math" "regexp" "strconv" @@ -17,20 +18,22 @@ import ( scyllav1 "github.com/scylladb/scylla-operator/pkg/api/scylla/v1" "github.com/scylladb/scylla-operator/pkg/pointer" "github.com/scylladb/scylla-operator/pkg/util/duration" + hashutil "github.com/scylladb/scylla-operator/pkg/util/hash" ) -type startDateGetter interface { - GetStartDateOrEmpty() string -} - -type startDateGetterSetter interface { - startDateGetter - SetStartDate(sd string) +type taskSpecInterface interface { + GetTaskSpec() *scyllav1.TaskSpec + ToManager() (*managerclient.Task, error) + GetObjectHash() (string, error) } type RepairTaskSpec scyllav1.RepairTaskSpec -var _ startDateGetterSetter = &RepairTaskSpec{} +var _ taskSpecInterface = &RepairTaskSpec{} + +func (r *RepairTaskSpec) GetTaskSpec() *scyllav1.TaskSpec { + return &r.TaskSpec +} func (r *RepairTaskSpec) ToManager() (*managerclient.Task, error) { t := &managerclient.Task{ @@ -83,8 +86,12 @@ func (r *RepairTaskSpec) ToManager() (*managerclient.Task, error) { return t, nil } -func (r *RepairTaskSpec) ToStatus() *RepairTaskStatus { - rts := &RepairTaskStatus{ +func (r *RepairTaskSpec) GetObjectHash() (string, error) { + return hashutil.HashObjects(r) +} + +func (r *RepairTaskSpec) ToStatus() *scyllav1.RepairTaskStatus { + rts := &scyllav1.RepairTaskStatus{ DC: r.DC, Keyspace: r.Keyspace, FailFast: pointer.Ptr(r.FailFast), @@ -102,24 +109,8 @@ func (r *RepairTaskSpec) ToStatus() *RepairTaskStatus { return rts } -func (r *RepairTaskSpec) GetStartDateOrEmpty() string { - if r.StartDate != nil { - return *r.StartDate - } - - return "" -} - -func (r *RepairTaskSpec) SetStartDate(sd string) { - r.StartDate = &sd -} - -type RepairTaskStatus scyllav1.RepairTaskStatus - -var _ startDateGetter = &RepairTaskStatus{} - -func NewRepairStatusFromManager(t *managerclient.TaskListItem) (*RepairTaskStatus, error) { - rts := &RepairTaskStatus{} +func NewRepairStatusFromManager(t *managerclient.TaskListItem) (*scyllav1.RepairTaskStatus, error) { + rts := &scyllav1.RepairTaskStatus{} rts.TaskStatus = newTaskStatusFromManager(t) @@ -133,15 +124,13 @@ func NewRepairStatusFromManager(t *managerclient.TaskListItem) (*RepairTaskStatu return rts, nil } -func (r *RepairTaskStatus) GetStartDateOrEmpty() string { - if r.StartDate != nil { - return *r.StartDate - } +type BackupTaskSpec scyllav1.BackupTaskSpec - return "" -} +var _ taskSpecInterface = &BackupTaskSpec{} -type BackupTaskSpec scyllav1.BackupTaskSpec +func (b *BackupTaskSpec) GetTaskSpec() *scyllav1.TaskSpec { + return &b.TaskSpec +} func (b *BackupTaskSpec) ToManager() (*managerclient.Task, error) { t := &managerclient.Task{ @@ -186,8 +175,12 @@ func (b *BackupTaskSpec) ToManager() (*managerclient.Task, error) { return t, nil } -func (b *BackupTaskSpec) ToStatus() *BackupTaskStatus { - bts := &BackupTaskStatus{ +func (b *BackupTaskSpec) GetObjectHash() (string, error) { + return hashutil.HashObjects(b) +} + +func (b *BackupTaskSpec) ToStatus() *scyllav1.BackupTaskStatus { + bts := &scyllav1.BackupTaskStatus{ DC: b.DC, Keyspace: b.Keyspace, Location: b.Location, @@ -202,26 +195,8 @@ func (b *BackupTaskSpec) ToStatus() *BackupTaskStatus { return bts } -func (b *BackupTaskSpec) GetStartDateOrEmpty() string { - if b.StartDate != nil { - return *b.StartDate - } - - return "" -} - -func (b *BackupTaskSpec) SetStartDate(sd string) { - b.StartDate = &sd -} - -var _ startDateGetterSetter = &BackupTaskSpec{} - -type BackupTaskStatus scyllav1.BackupTaskStatus - -var _ startDateGetter = &BackupTaskStatus{} - -func NewBackupStatusFromManager(t *managerclient.TaskListItem) (*BackupTaskStatus, error) { - bts := &BackupTaskStatus{} +func NewBackupStatusFromManager(t *managerclient.TaskListItem) (*scyllav1.BackupTaskStatus, error) { + bts := &scyllav1.BackupTaskStatus{} bts.TaskStatus = newTaskStatusFromManager(t) @@ -235,14 +210,6 @@ func NewBackupStatusFromManager(t *managerclient.TaskListItem) (*BackupTaskStatu return bts, nil } -func (b *BackupTaskStatus) GetStartDateOrEmpty() string { - if b.StartDate != nil { - return *b.StartDate - } - - return "" -} - func schedulerTaskSpecToManager(schedulerTaskSpec *scyllav1.SchedulerTaskSpec) (*managerclient.Schedule, error) { schedule := &managerclient.Schedule{} @@ -310,6 +277,7 @@ func newTaskStatusFromManager(t *managerclient.TaskListItem) scyllav1.TaskStatus taskStatus := scyllav1.TaskStatus{ SchedulerTaskStatus: scyllav1.SchedulerTaskStatus{}, Name: t.Name, + Labels: maps.Clone(t.Labels), ID: pointer.Ptr(t.ID), } diff --git a/pkg/controller/manager/types_old_test.go b/pkg/controller/manager/types_old_test.go index f0818feb1ff..bcb802fae6b 100644 --- a/pkg/controller/manager/types_old_test.go +++ b/pkg/controller/manager/types_old_test.go @@ -152,17 +152,20 @@ func TestRepairTask_FromManager(t *testing.T) { tt := []struct { name string managerTask *managerclient.TaskListItem - expected *RepairTaskStatus + expected *scyllav1.RepairTaskStatus expectedError error }{ { name: "fields and properties are propagated", managerTask: &managerclient.TaskListItem{ - ClusterID: "cluster_id", - Name: "repair_task_name", - Enabled: true, - ErrorCount: 1, - ID: "repair_task_id", + ClusterID: "cluster_id", + Name: "repair_task_name", + Enabled: true, + ErrorCount: 1, + ID: "repair_task_id", + Labels: map[string]string{ + "scylla-operator.scylladb.com/managed-hash": "managed-hash-value", + }, LastError: validDateTime, LastSuccess: validDateTime, NextActivation: validDateTime, @@ -192,7 +195,7 @@ func TestRepairTask_FromManager(t *testing.T) { Suspended: false, Type: managerclient.RepairTask, }, - expected: &RepairTaskStatus{ + expected: &scyllav1.RepairTaskStatus{ TaskStatus: scyllav1.TaskStatus{ Name: "repair_task_name", SchedulerTaskStatus: scyllav1.SchedulerTaskStatus{ @@ -204,6 +207,9 @@ func TestRepairTask_FromManager(t *testing.T) { }, ID: pointer.Ptr("repair_task_id"), Error: nil, + Labels: map[string]string{ + "scylla-operator.scylladb.com/managed-hash": "managed-hash-value", + }, }, DC: []string{"us-east1"}, FailFast: pointer.Ptr(true), @@ -334,19 +340,22 @@ func TestBackupTask_FromManager(t *testing.T) { tt := []struct { name string managerTask *managerclient.TaskListItem - expected *BackupTaskStatus + expected *scyllav1.BackupTaskStatus expectedError error }{ { name: "fields and properties are propagated", managerTask: &managerclient.TaskListItem{ - ClusterID: "cluster_id", - Enabled: true, - ErrorCount: 1, - ID: "backup_task_id", - LastError: validDateTime, - LastSuccess: validDateTime, - Name: "backup_task_name", + ClusterID: "cluster_id", + Enabled: true, + ErrorCount: 1, + ID: "backup_task_id", + LastError: validDateTime, + LastSuccess: validDateTime, + Name: "backup_task_name", + Labels: map[string]string{ + "scylla-operator.scylladb.com/managed-hash": "managed-hash-value", + }, NextActivation: validDateTime, Properties: map[string]interface{}{ "location": []string{ @@ -383,7 +392,7 @@ func TestBackupTask_FromManager(t *testing.T) { Suspended: false, Type: managerclient.BackupTask, }, - expected: &BackupTaskStatus{ + expected: &scyllav1.BackupTaskStatus{ TaskStatus: scyllav1.TaskStatus{ Name: "backup_task_name", SchedulerTaskStatus: scyllav1.SchedulerTaskStatus{ @@ -395,6 +404,9 @@ func TestBackupTask_FromManager(t *testing.T) { }, ID: pointer.Ptr("backup_task_id"), Error: nil, + Labels: map[string]string{ + "scylla-operator.scylladb.com/managed-hash": "managed-hash-value", + }, }, DC: []string{"us-east1"}, Keyspace: []string{"test"}, From e6eaa650a549eb79814160dc824561a74689c7de Mon Sep 17 00:00:00 2001 From: Kacper Rzetelski Date: Mon, 7 Oct 2024 11:18:55 +0200 Subject: [PATCH 3/3] Update generated --- deploy/operator.yaml | 10 ++++++ .../scylla.scylladb.com/scyllaclusters.rst | 34 +++++++++++++++++++ .../scylla.scylladb.com_scyllaclusters.yaml | 10 ++++++ pkg/api/scylla/v1/zz_generated.deepcopy.go | 7 ++++ 4 files changed, 61 insertions(+) diff --git a/deploy/operator.yaml b/deploy/operator.yaml index ae58100279c..bdec40d227c 100644 --- a/deploy/operator.yaml +++ b/deploy/operator.yaml @@ -3221,6 +3221,11 @@ spec: items: type: string type: array + labels: + additionalProperties: + type: string + description: labels reflects the labels of a task. + type: object location: description: location reflects a list of backup locations in the format [:]: ex. s3:my-bucket. items: @@ -3405,6 +3410,11 @@ spec: items: type: string type: array + labels: + additionalProperties: + type: string + description: labels reflects the labels of a task. + type: object name: description: name reflects the name of a task. type: string diff --git a/docs/source/api-reference/groups/scylla.scylladb.com/scyllaclusters.rst b/docs/source/api-reference/groups/scylla.scylladb.com/scyllaclusters.rst index d2b2592b88c..a3814fff40f 100755 --- a/docs/source/api-reference/groups/scylla.scylladb.com/scyllaclusters.rst +++ b/docs/source/api-reference/groups/scylla.scylladb.com/scyllaclusters.rst @@ -4801,6 +4801,9 @@ object * - keyspace - array (string) - keyspace reflects a list of keyspace/tables glob patterns, e.g. 'keyspace,!keyspace.table_prefix_*' used to include or exclude keyspaces from repair. + * - :ref:`labels` + - object + - labels reflects the labels of a task. * - location - array (string) - location reflects a list of backup locations in the format [:]: ex. s3:my-bucket. @@ -4829,6 +4832,20 @@ object - array (string) - uploadParallel reflects a list of upload parallelism limits in the format [:]. +.. _api-scylla.scylladb.com-scyllaclusters-v1-.status.backups[].labels: + +.status.backups[].labels +^^^^^^^^^^^^^^^^^^^^^^^^ + +Description +""""""""""" +labels reflects the labels of a task. + +Type +"""" +object + + .. _api-scylla.scylladb.com-scyllaclusters-v1-.status.conditions[]: .status.conditions[] @@ -4933,6 +4950,9 @@ object * - keyspace - array (string) - keyspace reflects a list of keyspace/tables glob patterns, e.g. 'keyspace,!keyspace.table_prefix_*' used to include or exclude keyspaces from repair. + * - :ref:`labels` + - object + - labels reflects the labels of a task. * - name - string - name reflects the name of a task. @@ -4952,6 +4972,20 @@ object - string - timezone reflects the timezone of cron field. +.. _api-scylla.scylladb.com-scyllaclusters-v1-.status.repairs[].labels: + +.status.repairs[].labels +^^^^^^^^^^^^^^^^^^^^^^^^ + +Description +""""""""""" +labels reflects the labels of a task. + +Type +"""" +object + + .. _api-scylla.scylladb.com-scyllaclusters-v1-.status.upgrade: .status.upgrade diff --git a/pkg/api/scylla/v1/scylla.scylladb.com_scyllaclusters.yaml b/pkg/api/scylla/v1/scylla.scylladb.com_scyllaclusters.yaml index 0c3233a4c9c..b572472e6f3 100644 --- a/pkg/api/scylla/v1/scylla.scylladb.com_scyllaclusters.yaml +++ b/pkg/api/scylla/v1/scylla.scylladb.com_scyllaclusters.yaml @@ -2180,6 +2180,11 @@ spec: items: type: string type: array + labels: + additionalProperties: + type: string + description: labels reflects the labels of a task. + type: object location: description: location reflects a list of backup locations in the format [:]: ex. s3:my-bucket. items: @@ -2364,6 +2369,11 @@ spec: items: type: string type: array + labels: + additionalProperties: + type: string + description: labels reflects the labels of a task. + type: object name: description: name reflects the name of a task. type: string diff --git a/pkg/api/scylla/v1/zz_generated.deepcopy.go b/pkg/api/scylla/v1/zz_generated.deepcopy.go index 7137ac8f5a2..851a30faa5d 100644 --- a/pkg/api/scylla/v1/zz_generated.deepcopy.go +++ b/pkg/api/scylla/v1/zz_generated.deepcopy.go @@ -1048,6 +1048,13 @@ func (in *TaskStatus) DeepCopyInto(out *TaskStatus) { *out = new(string) **out = **in } + if in.Labels != nil { + in, out := &in.Labels, &out.Labels + *out = make(map[string]string, len(*in)) + for key, val := range *in { + (*out)[key] = val + } + } if in.Error != nil { in, out := &in.Error, &out.Error *out = new(string)