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

TODO Eliminate busywork from hotfix process #25

Open
demerphq opened this issue Dec 29, 2012 · 7 comments
Open

TODO Eliminate busywork from hotfix process #25

demerphq opened this issue Dec 29, 2012 · 7 comments
Assignees

Comments

@demerphq
Copy link
Member

Currently the hotfix logic has a large manual process associated to it which is required to be done before a sync from the docs:

   Then do a:

       git pull --no-rebase

   Followed by:

       git push

   to push your hotfix to the Git server. But now you’re not at what you want to roll out, so do:

       git reset --hard NEW_SHA1
       git checkout -f

   This will ensure that you are on your hotfix commit, and that any git hooks are executed. You should then TEST the code. On a webserver this normally involves

       httpd restart

   followed by some manual testing of the relevant web site.

   When you are satisfied that things are OK, you can execute the sync:

       git-deploy sync

   TODO: The last 3 pull/push/reset steps are busywork that should be, and eventually will be merged into "git-deploy sync".

Note the last line of the docs.

@avar
Copy link
Member

avar commented Dec 29, 2012

After writing that I've since become very skeptical of running any git operations like this for the user.

I think a better solution if we do anything with this would be to allow rollouts where you don't have to pull/push at all, but that has other caveats.

@demerphq
Copy link
Member Author

On 29 December 2012 14:20, Ævar Arnfjörð Bjarmason <notifications@github.com

wrote:

After writing that I've since become very skeptical of running any git
operations like this for the user.

Heh. :-) I had the same evolution of thinking when I started working on
git-deploy.

In this case however I think there is a "clean" work around: add a new
command that rolls up the behavior sensibly.

Im thinking something like:

git-deploy push-hotfix

which would then do the magic. Basically automagical behavior without the
auto. :-)

I think a better solution if we do anything with this would be to allow
rollouts where you don't have to pull/push at all, but that has other
caveats.

I would like to hear your thoughts on this.

Yves

perl -Mre=debug -e "/just|another|perl|hacker/"

@avar
Copy link
Member

avar commented Jan 6, 2013

I had a chat with Hans on Friday about wanting to get rid of the whole
pull/push part of the hotfix process. We discussed this before but
there were some other impediments to doing that because it wouldn't
play well with other parts in git-deploy.

What I'd like to change about the git-deploy behavior are a few things
related to this:

  • Get rid of the error about not wanting to roll out a commit that
    isn't on your upstream rollout branch, this is what prevents you
    from just doing:

    git-deploy hotfix
    git cherry-pick
    git-deploy sync

  • Change "git-deploy start" so it doesn't do "git pull", but rather:

  1. Fetches the tags from upstream
  2. Finds which tags correspond to the current role.
  3. Does a "git reset --hard" to the latest rolled out tag
  4. Sets the the rollback marker to correspond to that tag, so "abort"
    will go back there and not to whatever we started with.
  5. Checks out the branch we're configured to roll out from.
  6. Does a "git reset --hard origin/$that_branch".

The point of doing this is that Hans wants to change the staging
servers so that there isn't just one per-role, but multiple servers
per-role (e.g. one in the fallback DC).

Currently if you switched from the main staging server to the fallback
one and did a "git-deploy start" the rollback marker would be the
state of your stale checkout and wouldn't correspond to the latest
released tag.

Thus if you did an "abort" after starting the sync we'd rollback to
something that's likely very old, and may never have been released.

If we always view the tags we get from the upstream repo as the
canonical tags for the latest release we did we can just start by
resetting to the latest rolled out tag, create a marker, and then go
from there to resetting to the latest version on the upstream branch
before creating another tag and rolling out.

And by jumping around with "git reset --hard" like this we don't do a
pull, and thus we could freely change the hotfix procedure to rollout
a branch that diverges from an old trunk without needing to pull trunk
&& push && reset --hard to the sha1 of the commit resulting from the
cherry-pick. Which is a very confusing procedure for most people, and
the only reason for doing it is to avoid the edge case where you
commit some original code as part of the hotfix which we'd like to
have in the upstream code.

We can still help people not to shoot themselves in the foot by
providing various checks when they rollout asking them if they're sure
the hotfix commits they're about to abandon should belong on trunk.

We could skip this if the commit looks cherry-picked, i.e. just get
the patch-id for the last N commits on the upstream branch and
comparing them before moving on.

What do you think?

demerphq added a commit that referenced this issue Jan 6, 2013
See Issue #25 - TODO Eliminate busywork from hotfix process.

This is an untested initial attempt at a recipe for this process. It is
interesting as we dont have an easy way currently to the lock a directory
without modifying the logfile. We solve this by holding a lock on the
logfile for the duration of the action.

IOW, this includes code to support a "logless lock". (Quite simple logic
actually).
@demerphq
Copy link
Member Author

demerphq commented Jan 6, 2013

On 6 January 2013 14:37, Ævar Arnfjörð Bjarmason
notifications@github.com wrote:

I had a chat with Hans on Friday about wanting to get rid of the whole
pull/push part of the hotfix process. We discussed this before but
there were some other impediments to doing that because it wouldn't
play well with other parts in git-deploy.

What I'd like to change about the git-deploy behavior are a few things
related to this:

Get rid of the error about not wanting to roll out a commit that
isn't on your upstream rollout branch,

As you know I am really against this.

IMO this is the only thing that guarantees that everyone can see
everything that was rolled out via a git log.

Also, I have prior experience that not insisting on this results in
bizarre situations. This check was added when we discovered that
people were regularly rolling out code that was not pushed upstream.
We really do not want that to ever happen. I even have a hazy
recollection that this problem was one of the original problems we
wanted to solve when I wrote the original version of git-deploytool.

this is what prevents you
from just doing:

git-deploy hotfix
git cherry-pick
git-deploy sync

Well, I am pretty happy with the recipe being (partially implemented
today actually)

git-deploy hotfix
git cherry-pick
git-deploy push-hotfix
git-deploy sync

And or (not implemented yet, basically alias sync-hotfix to "push-hotfix sync")

git-deploy hotfix
git cherry-pick
git-deploy sync-hotfix

Change "git-deploy start" so it doesn't do "git pull", but rather:

Fetches the tags from upstream

Finds which tags correspond to the current role.

Here I agree.

Does a "git reset --hard" to the latest rolled out tag

Sets the the rollback marker to correspond to that tag, so "abort"
will go back there and not to whatever we started with.

Checks out the branch we're configured to roll out from.

Does a "git reset --hard origin/$that_branch".

Here I dont. At least not without some prompting to the user. As far
as I can tell there are four cases:

A. The current commit is tagged for this role and the lastest

B. The current commit is tagged for this role but is behind

C. The current commit is untagged for this role, and has never been
tagged for the role (first commit)

D. The current commit is untagged, but has been tagged for this role
before (IOW its on a "strange commit").

All but D are easy to handle IMO, and more or less fit your plan.
However D is trickier, however I believe it is no problem to detect
this case and then prompt the user what to do. At the very least they
should be given a chance to do something useful with the changes in
the directory. So IMO for cases A and B your solution is reasonable.
It doesnt seem to cover C, and IMO it could be dangerous on D if not
protected by prompting.

Note that D used to be relatively common, these days AFAIK it is not,
but it used to come up a lot.

The point of doing this is that Hans wants to change the staging
servers so that there isn't just one per-role, but multiple servers
per-role (e.g. one in the fallback DC).

Currently if you switched from the main staging server to the fallback
one and did a "git-deploy start" the rollback marker would be the
state of your stale checkout and wouldn't correspond to the latest
released tag.

Well, if this is for a fallback server, shouldn't there just be a
cronjob running that syncs to the latest tag?

Anyway, IMO what I propose above should cover this. As long as it
matches a rollout tag then your logic to update would fire.

Thus if you did an "abort" after starting the sync we'd rollback to
something that's likely very old, and may never have been released.

Yep, I see the problem.

If we always view the tags we get from the upstream repo as the
canonical tags for the latest release we did we can just start by
resetting to the latest rolled out tag, create a marker, and then go
from there to resetting to the latest version on the upstream branch
before creating another tag and rolling out.

Like I said, so long as we do something intelligent for case D IMO we
should be fine.

Also note that your plan does not cover case C, where there are no
tags for the role, either upstream or on the local.

And by jumping around with "git reset --hard" like this we don't do a
pull, and thus we could freely change the hotfix procedure to rollout
a branch that diverges from an old trunk without needing to pull trunk
&& push && reset --hard to the sha1 of the commit resulting from the
cherry-pick. Which is a very confusing procedure for most people, and
the only reason for doing it is to avoid the edge case where you
commit some original code as part of the hotfix which we'd like to
have in the upstream code.

C'mon Avar you know that isn't the only reason. There are many reasons.

IMO the solution of providing some special commands for this case is
much easier.

We can still help people not to shoot themselves in the foot by
providing various checks when they rollout asking them if they're sure
the hotfix commits they're about to abandon should belong on trunk.

We could skip this if the commit looks cherry-picked, i.e. just get
the patch-id for the last N commits on the upstream branch and
comparing them before moving on.

As you know my opinion is that anything that has been rolled out
should be reachable from trunk. Ive seen this on p5p and I have seen
it at $work before we enforced this rule, where it caused a lot of
wasted time because nobody could find a given rollout commit in "git
log". That alone makes me extremely reluctant to agree to it not being
important.

So far I think adding a new action to automate some of the busywork is
preferable to relaxing this "system invariant".

cheers,
Yves

perl -Mre=debug -e "/just|another|perl|hacker/"

@avar
Copy link
Member

avar commented Jan 6, 2013

I just don't see anything wrong with rolling out a commit that's not on the main branch you always roll out if that commit is a hotfix, in fact I think it makes more sense to do that than to push a commit that duplicates something you just cherry-picked to the branch you cherry-picked it from.

In some cases you do want to pull/push/reset like we do now, but I think the tool shouldn't be enforcing it.

For showing all commits that have ever been rolled out on a given role foo:

git log $(git tag -l 'foo-*')

Shows you all commits that have ever been rolled out, including those that aren't on whatever branch you normally roll out from.

There's also all sorts of hairy edge cases with providing a wrapper that pushes the hotfix for you, since you can't lock the remote repository you're pulling/pushing from someone could push a commit after you do your initial "pull", so you'll have to "pull" again, I've had this happen to me.

Resolving these kind of things automatically is really tricky, and if you don't resolve them automatically you're going to leave the user really confused at the state you've left them in.

I think git-deploy should treat hotfixes exactly like you'd treat making a point-release on a branch you've branched off for a maintenance release. When we cherry-pick something for say the 5.10 branch from blead we don't go and pull/push that back to blead. That would just be absurd.

@demerphq
Copy link
Member Author

demerphq commented Jan 6, 2013

On 6 January 2013 18:59, Ævar Arnfjörð Bjarmason
notifications@github.comwrote:

I just don't see anything wrong with rolling out a commit that's not on
the main branch you always roll out if that commit is a hotfix, in fact I
think it makes more sense to do that than to push a commit that duplicates
something you just cherry-picked to the branch you cherry-picked it from.

I dont know what to say beyond repeating that it has been proved to me time
and again a time waster and problem causer when something that was live is
not reachable from trunk. Either literally at work, or implicitly on p5p
with tagged releases not being reachable from blead.

As far as I can tell the seed of this problem is that we are using git to
store two distinct commit transition graphs, one is the graph of commits
that leads to trunk, then other is a graph of commits that were live. We
assume that the two are the same in most circumstances, however they arent
necessarily so as you point out. Ensuring that anything live is merged
guarantees it. (And IMO in turn allows us to make other assumptions about
what we can and should do).

As far as I am concerned if I do something like:

git log --decorate

I expect to see every tag that is reported is with git-tag. I should not
find that what was live is unreachable from trunk. We had this happen a
couple of times and it caused a world of trouble.

In some cases you do want to pull/push/reset like we do now, but I
think the tool shouldn't be enforcing it.

I disagree for the reason above. I absolutely expect to see every commit
that was put live from the master branch.

For showing all commits that have ever been rolled out on a given role foo
:

git log $(git tag -l 'foo-*')

Shows you all commits that have ever been rolled out, including those that
aren't on whatever branch you normally roll out from.

IMO this is too much to expect devs to know. And it remains the case that
bisect wont find the commit either.

There's also all sorts of hairy edge cases with providing a wrapper that
pushes the hotfix for you, since you can't lock the remote repository
you're pulling/pushing from someone could push a commit after you do your
initial "pull", so you'll have to "pull" again, I've had this happen to me.

Yes, and you would just repeat the command. So what?

Resolving these kind of things automatically is really tricky, and if you
don't resolve them automatically you're going to leave the user really
confused at the state you've left them in.

I dont think so. "We failed to push, you should probably just try again" is
pretty simple.

I think git-deploy should treat hotfixes exactly like you'd treat making a
point-release on a branch you've branched off for a maintenance release.
When we cherry-pick something for say the 5.10 branch from blead we don't
go and pull/push that back to blead. That would just be absurd

The comparison is IMO invalid. The perl development process is that once a
major release is made we start a new permanent branch for that major
release. And we expect all patches to be applied to blead first and
backported. The expectation is that anything of interest should be in
blead. A better comparison is when the person doing a release manages to
fork blead, and then release an unpushed/unmerged commit into blead. This
actually happened recently and DID cause trouble (which was made worse by
renaming the tag).

Yves

perl -Mre=debug -e "/just|another|perl|hacker/"

@demerphq
Copy link
Member Author

demerphq commented Jan 6, 2013

On 6 January 2013 19:21, demerphq demerphq@gmail.com wrote:

On 6 January 2013 18:59, Ævar Arnfjörð Bjarmason notifications@github.com
wrote:

I just don't see anything wrong with rolling out a commit that's not on
the main branch you always roll out if that commit is a hotfix, in fact I
think it makes more sense to do that than to push a commit that duplicates
something you just cherry-picked to the branch you cherry-picked it from.

I dont know what to say beyond repeating that it has been proved to me time
and again a time waster and problem causer when something that was live is
not reachable from trunk. Either literally at work, or implicitly on p5p
with tagged releases not being reachable from blead.

For what its worth I always wanted a proper solution for this. For
instance imagine that we built a per-role rollout branch, which
contained a linear sequence of commits which pointed at the trees of
each rollout. The message could include the commit ids of the real
commits. The subject of the commits would be the tag name in master.

That way you would be able to view the rollout history distinct from
the commit history, with diffs easily visible between each rollout.

Then we also would have no need to ensure that every rolled out commit
was reachable from trunk.

Yves

perl -Mre=debug -e "/just|another|perl|hacker/"

@ghost ghost assigned bucciarati Feb 5, 2013
demerphq added a commit that referenced this issue Mar 18, 2013
See Issue #25 - TODO Eliminate busywork from hotfix process.

This is an untested initial attempt at a recipe for this process. It is
interesting as we dont have an easy way currently to the lock a directory
without modifying the logfile. We solve this by holding a lock on the
logfile for the duration of the action.

IOW, this includes code to support a "logless lock". (Quite simple logic
actually).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants