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

Use new cron plugin for Werft cron jobs #8744

Merged
merged 8 commits into from
Mar 11, 2022
Merged

Conversation

mads-hartmann
Copy link
Contributor

@mads-hartmann mads-hartmann commented Mar 11, 2022

Description

This moves our existing Werft cron jobs to use the cron plugin which was released in Werft 0.2.0. This greatly simplifies the process for adding and updating Werft cron jobs as you no longer have to modify the Werft config and restart the service when adding or modifying the cron expressions for a cron job.

As Werft only detects jobs in .werft I had to move the existing cron jobs to .werft - previously this wasn't an issue as the cron definitions for each job in the Werft config pointed to a specific file. But now we're relying on Werft detecting the jobs in order to see the cron expression, so the job specifications need to live directly in .werft.

I introduced the same naming convention as we have in gitpod-io/ops, which is <team name>-<job-name> and updated CODEOWNERS.

I deleted the delete-preview-environments job and instead changed the delete-preview-environments-cron job to run every hour. Werft never triggered delete-preview-environments when branches were deleted so we have never seen the job work. Cleaning preview environments every hour is good enough for now.

Related Issue(s)

Fixes https://github.com/gitpod-io/ops/issues/1255

How to test

Once merged we have to verify that Werft does indeed schedule the cron jobs. As the delete preview environments cron job now runs every hour I will use that one to verify that it gets triggered.

Release Notes

NONE

Documentation

I have to updated our How-to guides on how to add cron jobs.

The job was never succesfully triggered and we can now have a cron
job that we can execute instead.

Fixes gitpod-io/ops#981
Also moves delete-preview-environments-cron to .werft as we now have
to rely on Werft detecting the job specification. Previously we had it
in the Werft config with a specific path, so that wasn't important
before, but as the cron expression is now part of the job spec we are
depending on Werft finding the job (and it only looks directly in the
.werft folder)

Fixes gitpod-io/ops#1255
Werft currently only detects jobs in the .werft folder (not subfolders)
so for it to pick up the job with the cron expression it has to be in
the .werft folder.
@codecov
Copy link

codecov bot commented Mar 11, 2022

Codecov Report

Merging #8744 (a701f73) into main (4580ba9) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #8744   +/-   ##
=======================================
  Coverage   11.17%   11.17%           
=======================================
  Files          18       18           
  Lines         993      993           
=======================================
  Hits          111      111           
  Misses        880      880           
  Partials        2        2           
Flag Coverage Δ
components-gitpod-cli-app 11.17% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

ArthurSens
ArthurSens previously approved these changes Mar 11, 2022
Copy link
Contributor

@ArthurSens ArthurSens left a comment

Choose a reason for hiding this comment

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

I'm adding the hold label just because it's not clear to me if you want this merged straight away or want to prepare something to test if right after merge 🤔

@mads-hartmann
Copy link
Contributor Author

mads-hartmann commented Mar 11, 2022

@ArthurSens No additional things to prepare, so just have to get it merged so I can see if Werft does indeed respect these new cron expressions ☺️

One of them were wrong :D
Copy link
Contributor

@felladrin felladrin left a comment

Choose a reason for hiding this comment

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

Thanks for separating it into atomic commits. Made it easy to follow the changes!
Good to go!

@roboquat roboquat merged commit 5e3897d into main Mar 11, 2022
@roboquat roboquat deleted the mads/move-cron-to-specs branch March 11, 2022 13:18
mads-hartmann added a commit that referenced this pull request Mar 14, 2022
In #8744 I moved files around
but had forgotten to update references to the files.

Fixes gitpod-io/ops#1255
roboquat pushed a commit that referenced this pull request Mar 14, 2022
In #8744 I moved files around
but had forgotten to update references to the files.

Fixes gitpod-io/ops#1255
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.

4 participants