deis / builder

Git server and application builder for Deis Workflow
https://deis.com
MIT License
40 stars 41 forks source link

fix(makefile): make the docker-build call the build #286

Closed kmala closed 8 years ago

kmala commented 8 years ago

Summary of Changes

make docker-build call build Jenkis use a common script to build an image out of the PR and run e2e tests https://github.com/deis/jenkins-jobs/blob/master/jobs/component_jobs.groovy#L101-L113

Issue(s) that this PR Closes

fixes #285

Testing Instructions

Please provide a detailed list for how to test the changes in this PR.

  1. Do make docker-build docker-push
  2. Use the above image deploy deis cluster
  3. deploy an app and it should work fine

    Pull Request Hygiene TODOs

    • [x] Your pull request is concise and to the point (make another PR for refactoring nearby code)
    • [x] Your commits are squashed into logical units of work
    • [x] Your commits follow the commit style guidelines
arschles commented 8 years ago

Why doesn't the existing deploy script, which runs make build docker-build ... already work? It seems like something else is wrong here.

vdice commented 8 years ago

@arschles I can't remember why the divergence with _scripts/deploy.sh occurred (perhaps each deploy.sh was different between components.)

Also, while that script does get run by Travis on merges to master, it doesn't for PRs (Travis won't push images from forked repo due to https://docs.travis-ci.com/user/pull-requests#Security-Restrictions-when-testing-Pull-Requests).

arschles commented 8 years ago

@vdice So if the goal here is to get new builder images for PRs, I'd rather not make the fix in the Makefile, because adding build as a dependency of docker-build will change the standard dev workflow. Can we make Jenkins just run make build docker-build?

vdice commented 8 years ago

@arschles I see. Yes, we could do that; we'd just have to ensure all component repos tracked by Jenkins (https://github.com/deis/jenkins-jobs/blob/master/common.groovy#L1-L15) have a build task (would be surprising if they didn't but just a double-check)

kmala commented 8 years ago

@arschles I see other places https://github.com/deis/router/blob/master/Makefile#L49 and https://github.com/deis/registry/blob/master/Makefile#L41 where we build a binary doing the same.So which method do you think is the better one...so that all the components can be on the same plate

arschles commented 8 years ago

I'd also be ok with adding a new build target, possibly called docker-build-complete...

arschles commented 8 years ago

@kmala Good point. However, since we don't seem to have a standard across all repos, I strongly prefer that we don't change this repo until we do settle on a standard.