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

[New builder] Introduce Cancelot #271

Merged
merged 4 commits into from
Sep 11, 2019
Merged

[New builder] Introduce Cancelot #271

merged 4 commits into from
Sep 11, 2019

Conversation

pavlospt
Copy link
Contributor

Background

After trying out CloudBuild we needed a way to cancel any running builds for the same branch when we were pushing a new commit, in order to avoid billing extra not needed minutes. Since CloudBuild does not offer an out of the box solution we built Cancelot.

How it works

Cancelot will be invoked when the build starts and will try to find any running jobs that match the following filter:

build_id != "[CURRENT_BUILD_ID]" AND 
source.repo_source.branch_name = "[BRANCH_NAME]" AND 
status = "WORKING" AND 
start_time<"[CURRENT_BUILD_START_TIME]"

Usage

Add the builder as the first step in your project's cloudbuild.yaml:

steps:
- name: 'gcr.io/$PROJECT_ID/cancelot'
  args: [ 
    '--current_build_id', '$BUILD_ID',
    '--branch_name', '$BRANCH_NAME'
  ]

@googlebot googlebot added the cla: yes CLA looks good label Jul 23, 2019
Copy link
Contributor

@rharter rharter left a comment

Choose a reason for hiding this comment

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

This looks awesome, I'm installing from your fork today!

.gitignore Outdated
@@ -0,0 +1 @@
.idea
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be in here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm not 100% sure, just added it cause I was using Goland for it!

Copy link
Contributor

Choose a reason for hiding this comment

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

Although I can't imagine we would want to check in intellij config directories, there's no extant gitignore so I'd leave it out of this commit.

@pavlospt
Copy link
Contributor Author

pavlospt commented Aug 2, 2019

@rharter Hey, sry for pinging you, but do you maybe know if there is anyone from Google checking this repo? 😞 It seems like there has been some inactivity.

@rharter
Copy link
Contributor

rharter commented Aug 2, 2019

Hey @nof20, I've tested this out and it works great (and fixes a pain point I've had for some time, causing me to manually cancel builds when I discover 🤦‍♂️ errors when reviewing the diff while posting a PR). Any chance we can push it along? I'm happy to help how I can.

@viktorvoltaire
Copy link

A feature request for cancelot, could you add the option to look for a specific tag instead of branch?

It seems like source.repo_source.branch_name is not getting populated when using the GCB github app to trigger builds. My workaround for this would be to specify the branch name as a tag

@pavlospt
Copy link
Contributor Author

@viktorvoltaire sure, this is something that could be done! How would you expect it to work through? Have either Branch name or tag expected and terminate in case none is declared or both are declared?

@viktorvoltaire
Copy link

Well as long as branch_name is not mandatory, since it can't be filtered on when using the github app, im happy :)
These are the flags i would run it with --current_build_id $BUILD_ID --tag $BRANCH_NAME

@pavlospt
Copy link
Contributor Author

Will see what I can have ready in the end of the week !

@viktorvoltaire
Copy link

@pavlospt unfortunately my feature request might not be valid due to limitations in cloudbuild. One cant have special characters like / in tags so if you have a branch like feature/add_character cloudbuild will fail with invalid build tag "feature/add_character": must match format "^([\\w][\\w.-]{0,127})$"

@pavlospt
Copy link
Contributor Author

@viktorvoltaire yes, this is indeed a limitation which caused some trouble for us as well.

@jthegedus
Copy link

Is it an issue to run a regex replace over the BRANCH_NAMEs before using them?

@viktorvoltaire
Copy link

I haven't been able to solve tags are specified and evaluated before running build steps and seems to be static. @jthegedus do you have a solution that works?

@pavlospt
Copy link
Contributor Author

@viktorvoltaire there is source.repo_source.tag_name as well. Isn't that populated either?

@viktorvoltaire
Copy link

No that is empty.

@pavlospt
Copy link
Contributor Author

@viktorvoltaire just to be sure I am up-to-date which Github app are you referring to?

@viktorvoltaire
Copy link

@pavlospt
Copy link
Contributor Author

@sanastos would it be possible to take a look at this one as well?

@sanastos sanastos self-assigned this Sep 10, 2019
Copy link
Contributor

@sanastos sanastos left a comment

Choose a reason for hiding this comment

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

This looks good to me, although I recommend adding an example that exercises this behavior (I guess two builds, one that hangs, one that cancels). This will reduce the likelihood that a future maintainer will accidentally break this.

.gitignore Outdated
@@ -0,0 +1 @@
.idea
Copy link
Contributor

Choose a reason for hiding this comment

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

Although I can't imagine we would want to check in intellij config directories, there's no extant gitignore so I'd leave it out of this commit.

@pavlospt
Copy link
Contributor Author

@sanastos I do not think that this is something doable, because the work done by Cancelot is based on different builds triggered with different UUIDs, which means there will be no way to check if things will break unfortunately :/

@pavlospt
Copy link
Contributor Author

@sanastos I removed the .gitignore

@sanastos
Copy link
Contributor

Ah, I see what you're saying. This only cancels triggered builds, and there's no easy way to fake that.

Well, as a bare minimum, how about a test like:

steps:
- name: 'gcr.io/$PROJECT_ID/cancelot'
  args: [
    '--current_build_id', '$BUILD_ID',
    '--branch_name', "$BRANCH_NAME"
  ]
- name: 'ubuntu'
  args: ['echo', 'done']

And then a README.md that instructs you to launch as:
gcloud builds submit . --config=cloudbuild.yaml --substitutions=BRANCH_NAME="test"

Alternatively just hard code a fake branch name in the cloudbuild.yaml.

This at least exercises the code a bit, better than nothing.

@pavlospt
Copy link
Contributor Author

@sanastos sure will add your suggestions tomorrow then!

@sanastos
Copy link
Contributor

Looks good, thanks for your contribution. And kudos on the cool name 💯

@sanastos sanastos merged commit 507a352 into GoogleCloudPlatform:master Sep 11, 2019
@pavlospt pavlospt deleted the introduce_cancelot branch September 11, 2019 14:34
@Kampe
Copy link

Kampe commented Sep 14, 2019

Love the concept, but is there any way to specify which repository this should actually apply to? Right now it just cancels all running builds if a new one comes in no matter the source.

@pavlospt
Copy link
Contributor Author

@Kampe this takes into account branch name. Do you happen to have duplicate branch names between different repos? If so please file it as an issue and i will prepare a PR to handle this case as well!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes CLA looks good
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants