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

Add an ImageBuilder implementation for Conveyor #747

Merged
merged 1 commit into from
Feb 10, 2016
Merged

Conversation

ejholmes
Copy link
Contributor

This adds an implementation of the ImageBuilder interface backed by Conveyor. This means that if you're using Conveyor to build Docker images, it's no longer necessary to wait for the image to finish building before triggering a GitHub Deployment.

If a GitHub Deployment request is received by Empire, Empire will consult with Conveyor to obtain the Docker image for the given git sha, building it if necessary.

Not yet ready to merge, still needs some testing on both Conveyor's side and Empire's side.


a, err := c.client.Build(w, conveyor.BuildCreateOpts{
Repository: event.Repository.FullName,
Branch: "", // TODO,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is where things get a little weird, since we don't actually know if the ref that the deployment was triggered for was a branch, tag or sha.

This should probably be event.Deployment.Ref. Ultimately, Conveyor will need to support "branchless" builds, which should be pretty easy since the branch is only used to optimize the layer cache and isn't explicitly necessary to perform a build.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually, I think we should not set Branch at all since we really have no idea what the ref was. In most cases, Conveyor will have already started the build, using the branch cache and Empire will just wait for it to complete.

@ejholmes ejholmes force-pushed the conveyor branch 2 times, most recently from f7bd541 to fded317 Compare February 3, 2016 23:53
@ejholmes
Copy link
Contributor Author

ejholmes commented Feb 5, 2016

Note to self. We should definitely have some cancellations (context.Deadline) around deployment. Don't wanna have goroutines with pending deployments hanging around forever.

@ejholmes
Copy link
Contributor Author

ejholmes commented Feb 9, 2016

Ok. I think this is good to merge now. Been running this in staging for the last couple of days and no major issues aside from ejholmes/cloudwatch#4

Name: FlagGithubDeploymentsImageTemplate,
Value: github.DefaultTemplate,
Usage: "A Go text/template that will be used to determine the docker image to deploy.",
Usage: "A Go text/template that will be used to determine the docker image to deploy. This flag is only used when `--" + FlagGithubDeploymentsImageBuilder + "` is set to `template`.",
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this required if FlagGithubDeploymentsImageBuilder is set to template?

Copy link
Contributor

Choose a reason for hiding this comment

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

NM, I see the default

@phobologic
Copy link
Contributor

👍

ejholmes added a commit that referenced this pull request Feb 10, 2016
Add an ImageBuilder implementation for Conveyor
@ejholmes ejholmes merged commit 918a1bc into master Feb 10, 2016
@ejholmes ejholmes deleted the conveyor branch February 10, 2016 00:06
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