scap deploy-cache: Fetch refs/tags/* and refs/remotes/*
ClosedPublic

Authored by mmodell on Dec 16 2017, 8:29 AM.

Details

Maniphest Tasks
T182865: Source revision is in Phabricator, but can't be found by deployment tools
Reviewers
awight
thcipriani
demon
Group Reviewers
Release-Engineering-Team
Commits
rMSCA5acb672bea5b: scap deploy-cache: Fetch refs/tags/* and refs/remotes/*
Patch without arc
git checkout -b D918 && curl -L https://phabricator.wikimedia.org/D918?download=true | git apply
Summary

I outlined the situation which this addresses in T182865#3837272:

This is what we (@mmodell and @awight) learned through quite a lot of manual poking at the ores1001 target to form a hypothesis and then attempting a workaround based on that, and finally re-running a deploy which succeeded.

  1. On tin, 15d5283b7422919d85203b5ba907027f9356e421 only existed at HEAD and was not on any local branch of the submodule's deployment repo.
  2. On the target, after scap has already updated .gitmodules and ran git submodule sync we see the following refspec in the submodule's .git/config:
[remote "origin"]       
        url = http://tin.eqiad.wmnet/ores/deploy/.git/modules/submodules/editquality
        fetch = +refs/heads/*:refs/remotes/origin/*

So fetching from origin is only going to fetch local branches from tin, it will not see the remote tracking branches.

git for-each-ref of tin:

f1c517c35a1a6bfc852b74cd6f4c6c6eb8f367ed commit	refs/heads/master
d4bd3e922a4141ae82d21caef35d37d4464336b7 commit	refs/remotes/origin/CoC
15d5283b7422919d85203b5ba907027f9356e421 commit	refs/remotes/origin/HEAD
2203c6ae71fdec1a0b919ff8ebb21f5b41fa2ad6 commit	refs/remotes/origin/Pix1234-patch-2
fb2c5a3e815a61141257757ffd4cd5d71f55a7d3 commit	refs/remotes/origin/adds_urdu

Note that the commit in question (15d5283b) only exists in refs/remotes/origin/HEAD from the perspective of the repo on tin. origin is ssh://vcs@git-ssh.wikimedia.org/source/editquality.git

I advised @awight of the above and suggested as follows (irc log slightly edited for clarity)

<twentyafterfour> awight: I think what needs to happen is that you check out the branch in the submodule on tin so that there is a local ref that can be fetched"
I can probably fix scap's refspec to also fetch the remotes but that'll take an update to the scap package which will take time to build and deploy

<awight> I’m pretty sure that detached head is the default for submodules tho
So it’s really surprising that this didn’t hit us until today

<twentyafterfour> yeah it is ok if the submodule is in a detached head but the commit that it's pointing to needs to also exist on a branch

<awight> Awesome debugging, that gives me a simple workaround at least, of going into the submodules, checking out master, then back to the root and updating to the correct snapshot.

<twentyafterfour> right that _should_ work

<awight> yep lemme try that

<awight> :D works like a charm.

<twentyafterfour> this may be a change in behavior in scap due to the --reference majic
I think that the old way that we cloned submodules might have ended up with a different refspec (treating the repo as bare vs non-bare, is the only thing I can think of )

<awight> When was this change deployed to production scap?
\o/ parallel deployment to ores* just completed perfectly, and in 58 seconds
That’s gonads to the wall.

<twentyafterfour> monday the 11th is when it got published

<awight> That’s gotta be it, then.

Test Plan

tested locally: deployed phabricator locally with scap deploy, looked at scap deploy-log and saw reasonable output. Then examined deploy-cache repos and saw the expected refs.

Diff Detail

Repository
rMSCA Scap
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
mmodell created this revision.Dec 16 2017, 8:29 AM
Restricted Application added a reviewer: Release-Engineering-Team. · View Herald TranscriptDec 16 2017, 8:29 AM
Restricted Application added a project: Release-Engineering-Team. · View Herald Transcript
mmodell requested review of this revision.Dec 16 2017, 8:30 AM
mmodell edited the summary of this revision. (Show Details)Dec 16 2017, 8:33 AM
demon added a comment.Dec 19 2017, 4:28 AM

Couple of inline nits, but looks good otherwise.

scap/deploy.py
319

Forgot something here?

scap/git.py
340

Let's also append --prune around here somewhere.

mmodell updated this revision to Diff 2442.Dec 20 2017, 9:10 PM

Address @demon's comments.

mmodell marked 2 inline comments as done.Dec 20 2017, 9:11 PM

@demon Done.

This looks nice, from what I understand of it. Curious why you're fetching --tags, though? Most releases are being done using branches without any rigorous tagging, and scap itself seems to operate on SHA revision hashes only? Do we support scap deploy <TAG>?

In D918#18575, @awight wrote:

This looks nice, from what I understand of it. Curious why you're fetching --tags, though? Most releases are being done using branches without any rigorous tagging, and scap itself seems to operate on SHA revision hashes only? Do we support scap deploy <TAG>?

This is for the clients doing fetching, not the master. And we tag each deploy, so yeah we'll want the tags :)

(I've got some similar but not identical plans in motion for the masters)

demon accepted this revision.Dec 20 2017, 11:41 PM
This revision is now accepted and ready to land.Dec 20 2017, 11:41 PM
This revision was automatically updated to reflect the committed changes.