Make manager tasks' interval and start date optional and non-defaulted #1868

rzetelskik · 2024-03-28T10:41:45Z

Description of your changes:
StartDate and Interval fields are made optional and non-defaulted to make the validation more reasonable, while not changing the default behaviour.

This PR also moves manager's scheduler task spec conversion to a shared func.

Which issue is resolved by this Pull Request:
Prerequisite for #1851.

/kind feature
/priority important-longterm
/cc

scylla-operator-bot · 2024-03-28T10:41:47Z

@rzetelskik: GitHub didn't allow me to request PR reviews from the following users: rzetelskik.

Note that only scylladb members and repo collaborators can review this PR, and authors cannot review their own PRs.

In response to this:

Description of your changes:

Which issue is resolved by this Pull Request:
Resolves #

/kind feature
/priority important-longterm
/cc

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

zimnx

lgtm, please squash last two non-generated commits, as "Move manager task validation to validating webhook" also contains changes to existing tests that "React to schema and validation changes in existing tests " should have based on the title.

zimnx · 2024-04-02T15:25:23Z

/cc tnozicka

rzetelskik · 2024-04-02T15:28:48Z

lgtm, please squash last two non-generated commits, as "Move manager task validation to validating webhook" also contains changes to existing tests that "React to schema and validation changes in existing tests " should have based on the title.

done, thanks

rzetelskik · 2024-04-03T15:54:03Z

Changing the scope of this PR due to #1867 (comment)

tnozicka

one nit, rest lgtm
/approve

tnozicka · 2024-04-15T13:20:00Z

test/e2e/set/scyllacluster/scyllamanager.go

@@ -136,7 +137,7 @@ var _ = g.Describe("Scylla Manager integration", func() {
 		repairTask := tasks.TaskListItemSlice[0]
 		o.Expect(repairTask.Name).To(o.Equal(sc.Status.Repairs[0].Name))
 		o.Expect(repairTask.ID).To(o.Equal(sc.Status.Repairs[0].ID))
-		o.Expect(repairTask.Schedule.Interval).To(o.Equal(sc.Status.Repairs[0].Interval))
+		o.Expect(repairTask.Schedule.Interval).To(o.Equal(*sc.Status.Repairs[0].Interval))


nit: expect non-nil to this can't panic (and the other case)

…ulted

…t to schema and validation changes

zimnx

/lgtm

scylla-operator-bot · 2024-04-15T14:10:19Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: rzetelskik, tnozicka, zimnx

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [tnozicka,zimnx]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

rzetelskik · 2024-04-15T16:23:26Z

@rzetelskik: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/e2e-gke-parallel-clusterip a3fed6a link true /test e2e-gke-parallel-clusterip
Full PR test history. Your PR dashboard.

https://prow.scylla-operator.scylladb.com/view/gs/scylla-operator-prow/pr-logs/pull/scylladb_scylla-operator/1868/pull-scylla-operator-master-e2e-gke-parallel-clusterip/1779871557737254912#1:test-build-log.txt%3A1619

• [FAILED] [1020.830 seconds]
Scylla Manager integration [It] should register cluster and sync repair tasks
github.com/scylladb/scylla-operator/test/e2e/set/scyllacluster/scyllamanager.go:29
...
  STEP: Waiting for repair to finish @ 04/15/24 14:25:57.016
  [FAILED] in [It] - github.com/scylladb/scylla-operator/test/e2e/set/scyllacluster/scyllamanager.go:190 @ 04/15/24 14:35:57.023

Controller logs:

2024-04-15T14:25:55.960499577Z I0415 14:25:55.960239       1 manager/sync.go:134] "Executing action" action="add task &{ClusterID: Enabled:true ID: Name:weekly Properties:map[intensity:1 parallel:123 small_table_threshold:1073741824] Schedule:0xc000483340 Tags:[] Type:repair}"
...
2024-04-15T14:25:56.175142345Z I0415 14:25:56.172692       1 manager/controller.go:148] "Hit conflict, will retry in a bit" Key="e2e-test-scyllacluster-psxjg-q2tff/basic-hl65b" Error="Operation cannot be fulfilled on scyllaclusters.scylla.scylladb.com \"basic-hl65b\": the object has been modified; please apply your changes to the latest version and try again"
...
2024-04-15T14:25:56.253308196Z I0415 14:25:56.253239       1 manager/sync.go:134] "Executing action" action="delete task \"b3dc792a-587a-43c8-818f-cad6bf869934\""
...
2024-04-15T14:25:56.361381890Z I0415 14:25:56.361067       1 manager/sync.go:134] "Executing action" action="add task &{ClusterID: Enabled:true ID: Name:weekly Properties:map[intensity:1 parallel:123 small_table_threshold:1073741824] Schedule:0xc00081e310 Tags:[] Type:repair}"

Known issue: #1752

#1694 (comment)
/retest

rzetelskik · 2024-04-15T16:44:50Z

@rzetelskik: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/e2e-gke-parallel-clusterip a3fed6a link true /test e2e-gke-parallel-clusterip
Full PR test history. Your PR dashboard.

image was already gone

/test images
/retest

rzetelskik force-pushed the manager-validation branch from 5467977 to 87ca7ea Compare March 28, 2024 10:51

rzetelskik mentioned this pull request Mar 28, 2024

Support cron and timezone in manager task integration #1851

Merged

rzetelskik changed the title ~~[WIP] Move all manager task validation to validating webhook and make interval and start date optional and non-defaulted~~ Move all manager task validation to validating webhook and make interval and start date optional and non-defaulted Mar 28, 2024

scylla-operator-bot bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Mar 28, 2024

rzetelskik requested a review from zimnx March 28, 2024 11:08

rzetelskik force-pushed the manager-validation branch from 87ca7ea to 1e74791 Compare March 28, 2024 11:15

rzetelskik requested a review from tnozicka March 28, 2024 11:15

rzetelskik force-pushed the manager-validation branch 5 times, most recently from 6e0c1c3 to 561cf58 Compare March 29, 2024 10:30

zimnx reviewed Apr 2, 2024

View reviewed changes

rzetelskik force-pushed the manager-validation branch from 561cf58 to 1eaae5d Compare April 2, 2024 15:27

rzetelskik assigned tnozicka Apr 2, 2024

rzetelskik changed the title ~~Move all manager task validation to validating webhook and make interval and start date optional and non-defaulted~~ [WIP] Move all manager task validation to validating webhook and make interval and start date optional and non-defaulted Apr 3, 2024

scylla-operator-bot bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Apr 3, 2024

rzetelskik changed the title ~~[WIP] Move all manager task validation to validating webhook and make interval and start date optional and non-defaulted~~ [WIP] Make manager tasks' interval and start date optional and non-defaulted Apr 3, 2024

rzetelskik force-pushed the manager-validation branch from 1eaae5d to 8be250e Compare April 3, 2024 15:49

scylla-operator-bot bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Apr 3, 2024

rzetelskik force-pushed the manager-validation branch from 8be250e to 6c91453 Compare April 3, 2024 15:51

rzetelskik requested a review from zimnx April 3, 2024 15:53

rzetelskik changed the title ~~[WIP] Make manager tasks' interval and start date optional and non-defaulted~~ Make manager tasks' interval and start date optional and non-defaulted Apr 3, 2024

scylla-operator-bot bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Apr 3, 2024

tnozicka approved these changes Apr 15, 2024

View reviewed changes

scylla-operator-bot bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 15, 2024

rzetelskik force-pushed the manager-validation branch from 6c91453 to 144060d Compare April 15, 2024 13:42

rzetelskik requested a review from tnozicka April 15, 2024 13:43

rzetelskik added 3 commits April 15, 2024 15:55

Make manager task spec's interval and startDate optional and non-defa…

ce254ca

…ulted

Move manager scheduler task spec conversion to a shared func and reac…

e8844a8

…t to schema and validation changes

Update generated

a3fed6a

rzetelskik force-pushed the manager-validation branch from 144060d to a3fed6a Compare April 15, 2024 13:56

zimnx approved these changes Apr 15, 2024

View reviewed changes

scylla-operator-bot bot assigned zimnx Apr 15, 2024

scylla-operator-bot bot added the lgtm Indicates that a PR is ready to be merged. label Apr 15, 2024

scylla-operator-bot bot merged commit d978341 into scylladb:master Apr 15, 2024
12 checks passed

rzetelskik deleted the manager-validation branch April 15, 2024 18:36

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make manager tasks' interval and start date optional and non-defaulted #1868

Make manager tasks' interval and start date optional and non-defaulted #1868

rzetelskik commented Mar 28, 2024 •

edited

Loading

scylla-operator-bot bot commented Mar 28, 2024

zimnx left a comment

zimnx commented Apr 2, 2024

rzetelskik commented Apr 2, 2024

rzetelskik commented Apr 3, 2024

tnozicka left a comment

tnozicka Apr 15, 2024

rzetelskik Apr 15, 2024

zimnx left a comment

scylla-operator-bot bot commented Apr 15, 2024

rzetelskik commented Apr 15, 2024

rzetelskik commented Apr 15, 2024

Make manager tasks' interval and start date optional and non-defaulted #1868

Make manager tasks' interval and start date optional and non-defaulted #1868

Conversation

rzetelskik commented Mar 28, 2024 • edited Loading

scylla-operator-bot bot commented Mar 28, 2024

zimnx left a comment

Choose a reason for hiding this comment

zimnx commented Apr 2, 2024

rzetelskik commented Apr 2, 2024

rzetelskik commented Apr 3, 2024

tnozicka left a comment

Choose a reason for hiding this comment

tnozicka Apr 15, 2024

Choose a reason for hiding this comment

rzetelskik Apr 15, 2024

Choose a reason for hiding this comment

zimnx left a comment

Choose a reason for hiding this comment

scylla-operator-bot bot commented Apr 15, 2024

rzetelskik commented Apr 15, 2024

rzetelskik commented Apr 15, 2024

rzetelskik commented Mar 28, 2024 •

edited

Loading