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

Run remote Cloud Build triggers #100

Merged
merged 15 commits into from
Jan 29, 2020
Merged

Run remote Cloud Build triggers #100

merged 15 commits into from
Jan 29, 2020

Conversation

nkinkade
Copy link
Contributor

@nkinkade nkinkade commented Jan 29, 2020

This PR sets up a simple framework for being able to run remote repository Cloud Build triggers from this repository.

The first (and currently only) implementation of this in this PR is triggering epoxy-images stage1 builds from this repository. This will help to ensure that any sites which are added or removed from this repo will be properly reflected in the stage1 images which are available for those sites.

It is likely that we will add additional build triggers run_remote_build_triggers.sh for various other things which need setting up every time a site is added or removed from this repo (e.g., build switch configs, add ePoxy DS entities, add BMC entities, add SNMP DS entities, etc.).


This change is Reviewable

Copy link
Contributor

@stephen-soltesz stephen-soltesz left a comment

Choose a reason for hiding this comment

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

:lgtm: -- Thank you for taking the time to build this. Triggering dependent builds will become an increasingly common phenomenon I suspect, as we streamline more CI steps, e.g. triggering the automated node load testing, or running fresh docker images through their paces before manual changes to k8s-support.

I've added comments that outline what I think the longer term vision should be (without bash). Would you be willing to create an appropriate issue for the future plans and add TODOs that link to them? As you add more triggers to the bash script, I predict it will become unwieldy without preemptive steps.

Reviewable status: :shipit: complete! 1 of 1 approvals obtained (waiting on @nkinkade and @stephen-soltesz)


cloudbuild.yaml, line 51 at r1 (raw file):

- name: gcr.io/cloud-builders/gcloud
  entrypoint: '/bin/bash'
  args: ['./run_remote_build_triggers.sh']

Ultimately, I'd like to see the trigger names explicitly specified here in cloudbuild.yaml. That way the list of dependent builds is right here.


run_remote_build_triggers.sh, line 13 at r1 (raw file):

mlab-sandbox)
  TRIGGER_NAMES=(
    epoxy-images-push-to-sandbox-stage1

I believe the names of these triggers are user-specified. So, we can adopt better names that make per-project automation easier.


run_remote_build_triggers.sh, line 54 at r1 (raw file):

        --header "Content-Type: application/json" \
        --header "Authorization: Bearer $AUTH_TOKEN" \
        "${BASE_URL}/${PROJECT_ID}/triggers/${trigger_id}:run" \

It's amazing that so much of this can be done using bash -- "I'm not even mad".

My general rule of thumb is: when you're tempted to use bash arrays, the logic is too complex for bash.

Ultimately, I would prefer to see a simple tool in Go using the https://godoc.org/google.golang.org/api/cloudbuild/v1 package, i.e. https://godoc.org/google.golang.org/api/cloudbuild/v1#ProjectsTriggersService.Run. This small utility could be called directly from the siteinfo cloudbuild.yaml with flags that specify the correct trigger based on the current PROJECT_ID.

Copy link
Contributor Author

@nkinkade nkinkade left a comment

Choose a reason for hiding this comment

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

@stephen-soltesz: PTAL?

Reviewable status: :shipit: complete! 1 of 1 approvals obtained (waiting on @stephen-soltesz)


cloudbuild.yaml, line 51 at r1 (raw file):

Previously, stephen-soltesz (Stephen Soltesz) wrote…

Ultimately, I'd like to see the trigger names explicitly specified here in cloudbuild.yaml. That way the list of dependent builds is right here.

I had considered adding the trigger names as arguments to the script right here, but decided against it for the reason that this build step is run for all projects, and the trigger names are going to be different for every project. It wasn't immediately clear to me how to specify the trigger names for all three project here in the cloudbuild.yaml file in any sort of sensible way.


run_remote_build_triggers.sh, line 13 at r1 (raw file):

Previously, stephen-soltesz (Stephen Soltesz) wrote…

I believe the names of these triggers are user-specified. So, we can adopt better names that make per-project automation easier.

Yes, I actually renamed all the epoxy-images stage1 trigger names for this PR, since previously they were either not-helpful auto-created names, or were equivalent to the trigger ID (a long, random string).


run_remote_build_triggers.sh, line 54 at r1 (raw file):

Previously, stephen-soltesz (Stephen Soltesz) wrote…

It's amazing that so much of this can be done using bash -- "I'm not even mad".

My general rule of thumb is: when you're tempted to use bash arrays, the logic is too complex for bash.

Ultimately, I would prefer to see a simple tool in Go using the https://godoc.org/google.golang.org/api/cloudbuild/v1 package, i.e. https://godoc.org/google.golang.org/api/cloudbuild/v1#ProjectsTriggersService.Run. This small utility could be called directly from the siteinfo cloudbuild.yaml with flags that specify the correct trigger based on the current PROJECT_ID.

I didn't even know about this package, and it certainly never occurred to me write a utility in Go! Rather than kicking this down the line, maybe I should just scrap what I've done so far and do the thing over in Go?

Copy link
Contributor

@stephen-soltesz stephen-soltesz left a comment

Choose a reason for hiding this comment

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

Reviewable status: :shipit: complete! 1 of 1 approvals obtained (waiting on @nkinkade and @stephen-soltesz)


run_remote_build_triggers.sh, line 13 at r1 (raw file):

Previously, nkinkade wrote…

Yes, I actually renamed all the epoxy-images stage1 trigger names for this PR, since previously they were either not-helpful auto-created names, or were equivalent to the trigger ID (a long, random string).

Right, so my thinking is that since they all the epoxy triggers do the same thing (just based on different conditions) what if we name them the same thing? If there's one name for all projects, then specifying that name as a parameter in the cloudbuild.yaml would be possible.


run_remote_build_triggers.sh, line 54 at r1 (raw file):

Previously, nkinkade wrote…

I didn't even know about this package, and it certainly never occurred to me write a utility in Go! Rather than kicking this down the line, maybe I should just scrap what I've done so far and do the thing over in Go?

Up to you. This is a perfectly defensible checkpoint that provides new functionality now. I'm advising caution before adding additional triggers and logic in bash for additional heterogenously-named triggers across projects.

Copy link
Contributor Author

@nkinkade nkinkade left a comment

Choose a reason for hiding this comment

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

Reviewable status: :shipit: complete! 1 of 1 approvals obtained (waiting on @stephen-soltesz)


run_remote_build_triggers.sh, line 13 at r1 (raw file):

Previously, stephen-soltesz (Stephen Soltesz) wrote…

Right, so my thinking is that since they all the epoxy triggers do the same thing (just based on different conditions) what if we name them the same thing? If there's one name for all projects, then specifying that name as a parameter in the cloudbuild.yaml would be possible.

Done, at least for epoxy-images. In all three projects the epoxy-images stage1 build trigger is known as epoxy-images-stage1.


run_remote_build_triggers.sh, line 54 at r1 (raw file):

Previously, stephen-soltesz (Stephen Soltesz) wrote…

Up to you. This is a perfectly defensible checkpoint that provides new functionality now. I'm advising caution before adding additional triggers and logic in bash for additional heterogenously-named triggers across projects.

I started to look at it, and felt that likely the new go package would go in m-lab/go... then realized it would required a new submodule in siteinfo, then looked at the cloudbuild/v1 documentation and realized I had zero idea where to begin and how to integrate it properly.

For now I've just moved the build trigger names cloudbuild.yaml to genericize the script.

Copy link
Contributor

@stephen-soltesz stephen-soltesz left a comment

Choose a reason for hiding this comment

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

Reviewable status: :shipit: complete! 1 of 1 approvals obtained (waiting on @nkinkade)


run_remote_build_triggers.sh, line 13 at r1 (raw file):

Previously, nkinkade wrote…

Done, at least for epoxy-images. In all three projects the epoxy-images stage1 build trigger is known as epoxy-images-stage1.

👍


run_remote_build_triggers.sh, line 54 at r1 (raw file):

Previously, nkinkade wrote…

I started to look at it, and felt that likely the new go package would go in m-lab/go... then realized it would required a new submodule in siteinfo, then looked at the cloudbuild/v1 documentation and realized I had zero idea where to begin and how to integrate it properly.

For now I've just moved the build trigger names cloudbuild.yaml to genericize the script.

👍 -- sgtm I suggest that we create a new command, perhaps cbctl, with a subcommand "trigger" that accepts a trigger ID. This command should live in the gcp-config repo. My vision is to create a standard set of containers that will exist in our projects by default that we can just reference in the cloudbuild.yamls of other repos (like this one).

The other operations that cbctl would support is registering the trigger definitions for the standard workflow that we support between sandbox, staging, and prod using a uniform name, for a new (or existing) repo. Basically to remove the manual steps you had to do today.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants