search

rafecolton / docker-builder

Docker builder builds Docker images from a friendly config file.
MIT License
80 stars 11 forks source link

[DO NOT MERGE] Use Ref instead of CommitSHA #143

Closed wjzijderveld closed 9 years ago

wjzijderveld commented 9 years ago

Attempt to fix #142

I wanted to test that the correct field is used, but I don't have enough golang test knowledge to achieve that with the current test :)

rafecolton commented 9 years ago

No worries, thank you for taking a crack at it! I have been giving docker-builder some much-needed love over the past week or so, so I will take a look at this soon.

rafecolton commented 9 years ago

So the issue here is that when we clone down a commit sha, it is not on a branch, so we have to come up with something clever to determine what branch it likely came from. The reason we can't just clone down the branch is that we run the risk of cloning the branch at a different sha than what the user intended (i.e. if another commit has been made to that branch). I don't know off the top of my head what's in ref as passed by GitHub, but I assume we would have a similar consideration.

I don't have a good solution for this yet, but it is a problem I would like to solve. I'm looking at maybe parsing the output of git show-branch, but that seems less than ideal, so I'm definitely open to suggestions. Overall though, it is more important that we clone down the exact sha the user expects than that we have a branch name.

FWIW, this has been an ongoing issue at ModCloth as well. Our workaround has been to encourage people to deploy by sha or tag because deploying by branch name poses the same risk as cloning down by branch name.

rafecolton commented 9 years ago

I'm marking this as DO NOT MERGE, but I'll leave it open until we find another solution

wjzijderveld commented 9 years ago

Hmm, totally failed to see it uses the same ref for cloning.

In this case I'm only interested in the branch for tagging, so it could also just be added as meta data. We also still want to tag with commit hash.

Besides deployment, we also want to use the built-server for our isolated testing phase. Somebody will need to pull a branch and always wants the last one. So if that would lead to another commit then intended, it would actually be helpful.

rafecolton commented 9 years ago

I think I have a solution for this. Will submit a pull request tonight or tomorrow.

rafecolton commented 9 years ago

Superseded by https://github.com/rafecolton/docker-builder/pull/148