scylla-operator
scylla-operator copied to clipboard
scylladb
[WIP] Support missing cron and related fields in manager task spec and integration
Description of your changes: This PR adds missing cron and related fields, currently missing from out API, to manager task specs and extends the unit tests to cover them.
Which issue is resolved by this Pull Request: Resolves #1739
/kind feature /priority important-longterm /cc
@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 #1739 Resolves #1796
/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.
![scylla-operator-bot[bot] avatar](https://avatars.githubusercontent.com/in/112385?v=4 "Published by a pub.dev verified publisher"){kind=small}
/priority critical-urgent since the bug fixes are a prerequisite for https://github.com/scylladb/scylla-operator/issues/1752
/cc @Michal-Leszczynski would you be able to verify if there's anything missing from manager task specs in our API?
Feel free to ignore my comments if they are not related to this PR. I just wanted to highlight some fields that perhaps require additional attention and give context for some of them.
The now syntax can cause a lot of pain since it is parsed on the operator or sctool side. Maybe it's worth to discourage users from using via appropriate field description?
Agreed. Is that mentioned in sctool/sm?
The interval field is (or should be) deprecated by SM. Is it also deprecated here?
There's a separate PR in queue with interval deprecation. Best we can do is a field description though.
Why is it string? SM supports float64 for backward compatibility, but all floats are converted to ints anyway, because they don't make sense anymore from Scylla POV.
To be honest - no idea. I don't think we can change this in this API version though.
split mapstructure fix into a separate PR: https://github.com/scylladb/scylla-operator/pull/1854 /hold for https://github.com/scylladb/scylla-operator/pull/1854
@zimnx @tnozicka I believe I have addressed all your comments - please have another go.
@tnozicka I added cron and timezone validation. TZ and CRON_TZ are not accepted as part of cron for now. I'll raise an issue with the manager team.
manager flake and upgrade flake - can't possibly be deteriorated by this PR https://github.com/scylladb/scylla-operator/issues/1694#issuecomment-2066105109 https://github.com/scylladb/scylla-operator/issues/1233#issuecomment-2066106629
/test images /retest
@rzetelskik: The following tests failed, say
/retestto rerun all failed tests or/retest-requiredto rerun all mandatory failed tests:Test name Commit Details Required Rerun command ci/prow/e2e-gke-release-script-latest 415c60b link true
/test e2e-gke-release-script-latestci/prow/e2e-gke-parallel-clusterip 7ea90ba link true/test e2e-gke-parallel-clusteripci/prow/e2e-gke-parallel 7ea90ba link true/test e2e-gke-parallelFull PR test history. Your PR dashboard.
manager flake /retest
@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-release-script-latest | 415c60b1b0dcf5fe7a85ec399e8ce47c4fd79bf2 | link | true | /test e2e-gke-release-script-latest |
Full PR test history. Your PR dashboard.
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. I understand the commands that are listed here.
[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
- ~~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