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

control-service: Allow for jobs with no schedule to be deployed #835

Merged
merged 12 commits into from
May 17, 2022

Conversation

gabrielgeorgiev1
Copy link
Contributor

@gabrielgeorgiev1 gabrielgeorgiev1 commented May 13, 2022

Currently, if a job is deployed with no schedule_cron,
the deployment will fail. After this change, jobs with
no schedule will be deployed successfully, their cronjobs
will have a default schedule "0 0 30 2 *" - meaning Feb 30,
or never - but the job metadata will have an empty string
for a schedule, so that "vdk list" will not print a schedule
for those jobs (but jobs with their schedule explicitly set
to "0 0 30 2 *" will still have it show).

Testing done: tested locally by running control-service
connected to deployed K8s cluster, created and deployed
job successfully, verified that the created cronjob object
has its schedule set to "0 0 30 2 *", that executing the job
through the Execution API works and that getting the list of
deployed jobs returns an empty string for the schedule param

included 1 unit test covering the new addition

Signed-off-by: Gabriel Georgiev [email protected]

Currently, if a job is deployed with no schedule_cron,
the deployment will fail. After this change, jobs with
no schedule will be deployed successfully, their cronjobs
will have a default schedule "0 0 30 2 *" - meaning Feb 30,
or never - but the job metadata will have an empty string
for a schedule, so that "vdk list" will not print a schedule
for those jobs (but jobs with their schedule explicitly set
to "0 0 30 2 *" will still have it show).

Testing done: TBD

Signed-off-by: Gabriel Georgiev <[email protected]>
Signed-off-by: Gabriel Georgiev <[email protected]>
Signed-off-by: Gabriel Georgiev <[email protected]>
Signed-off-by: Gabriel Georgiev <[email protected]>
Signed-off-by: Gabriel Georgiev <[email protected]>
@mivanov1988
Copy link
Collaborator

LGTM! Please take a look at the failed tests.

Copy link
Contributor

@doks5 doks5 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall, LGTM

@gabrielgeorgiev1 gabrielgeorgiev1 enabled auto-merge (squash) May 17, 2022 07:33
@gabrielgeorgiev1 gabrielgeorgiev1 merged commit d5671ec into main May 17, 2022
@gabrielgeorgiev1 gabrielgeorgiev1 deleted the person/gageorgiev/tpcs-cron-schedule branch May 17, 2022 08:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants