Runner ability to fetch refs for merge request pipelines

gitlab-org/gitlab-ee#7380 will introduce building combined refs for merge request pipelines.

GitLab will need to schedule jobs for refs/merge-requests/:iid:/merge ref that is outside of the default cloning ref-spec.

Today users can control how they retrieve source code (clone, fetch, or none). Given there's no way to clone outside of the default ref-spec, we will force a fetch mode in case of merge request pipeline.

Links

• gitlab-org/gitlab-ee#7380
• gitlab-org/gitlab-ce#47110
• PoC: !1092 (closed)

/cc @jlenny @darbyfrey @dosuken123

changed milestone to %11.8

added Deliverable direction + 1 deleted label

changed milestone to %11.9

changed title from prospective merge pipeline code fetching to Runner handling of prospective merge pipelines

added devopsrelease [DEPRECATED] label

Thanks for creating this issue @nolith !

changed title from Runner handling of prospective merge pipelines to Runner ability to fetch prospective merge pipelines

I wonder about this, and the notion of fetch vs clone.

The git clone is effectively the:

rm -rf repo
git init repo
cd repo
git remote add origin address
git fetch origin
git checkout SHA

Or git fetch the differs only in lack of rm -rf repo.

I don't think that we need this distinction, as our git clone strategy effectively says: wipe-out git repository every time.

Also, the GIT_DEPTH just becomes git fetch --depth $GIT_DEPTH :)

If we could effectively detect the type of refs to fetch (or not detect and be dumb?) (but rather pass full ref spec to runner? this would be cool) we could fix that problem in slightly different way and very consistient too:

1. remove if wipe is set (our todays git clone),
2. git init,
3. git fetch origin <use-refspec-if-specified-by-gitlab|otherwise do not pass>,
4. git checkout SHA.

I will not take the risk of breaking the default cloning method, I'll prefer ship the new one only for perspective merge pipeline and then extend it to the clone strategy.

I'm quite sure it will work on docker with our helper image, but what about non-disposable environments like shell, and ssh? We have no control over the git version there.

Always, we need feature flag for that, so the risk is minimal.

It will work with very old versions of Git. Once we figure a default clonespec I will run a series of tests against all older versions of Git.

Can we extend Runner with?

git: {
  refspec: [array-of-strings]
  depth: null | integer
}

Then prefer to use refspec and depth (where depth is taken from GIT_DEPTH variable):

And refspec looks like follows:

if depth>0:
  - for tag:
    +refs/tags/v1.0:refs/tags/v1.0
  - for branch or detached MR:
    +refs/heads/master:refs/origin/heads/master
  - for merge request merge:
    +refs/merge_requests/iid/merge:refs/origin/merge_requests/iid/merge

else:
  - for tag or branch or detached MR:
    +refs/heads/*:refs/origin/heads/*
    +refs/tags/*:refs/tags/*

  - for merge request merge:
    +refs/merge_requests/iid/merge:refs/origin/merge_requests/iid/merge

This makes us remove some of the logic from runner and prefer refspec to be GitLab controlled and fully flexible.

I like this, this will also solve my concerns about changing the way we fetch/clone as we can implement a new method and apply it only when the new API is available. That way we can control this with a feature flag at gitlab level but we still retain backward compatibility with old gitlab instances.

Yes, we can drop ref_type for the goodness.

BTW. git clone also supports refspec so effectively we can make a way less changes than in this MR, thus reduce potential impact of breaking.

git clone support only refspec inside the default refspec. Or at least this was the last time i checked it.

Ah. You are right, so we are still left with git fetch.

@ayufan regarding depth. I think that today we can override it with an environment variable defined at gitlab-runner level.

If we move it to the API, we'll lose this feature.

I don't think it's a big loss, I can hardly find a use case for this. Conversely, this will simplify runner code.

created merge request !1203 (merged) to address this issue

mentioned in merge request !1203 (merged)

@nolith

I wish we could set default GIT_DEPTH: set for all clones, and disable if not needed. I prefer if we could set it in Runner API and push default from GitLab, with like depth=100.

changed title from Runner ability to fetch prospective merge pipelines to Runner ability to fetch merge request pipeline refs

changed the description

changed title from Runner ability to fetch merge request pipeline refs to Runner ability to fetch refs for merge request pipeline

changed title from Runner ability to fetch refs for merge request pipeline to Runner ability to fetch refs for merge request pipelines

@nolith Here is a sample response.

{
   "id":6591,
   "token":"qjEvjQVZzJtsyY_2xgqx",
   "allow_git_fetch":true,
   "job_info":{
      "name":"build",
      "stage":"test",
      "project_id":89,
      "project_name":"prospective-merge-pipelines"
   },
   "git_info":{
      "repo_url":"http://gitlab-ci-token:qjEvjQVZzJtsyY_2xgqx@localhost:8181/root/prospective-merge-pipelines.git",
      "sha":"5b9b19cd1b6ee9fdce2f7ddb852f7878b8ce6d4a",
      "before_sha":"0000000000000000000000000000000000000000",
      "ref":"refs/merge-requests/5/head",
      "ref_type":"merge_request",
      "ref_spec":[
         "+refs/merge-requests/5/head:refs/merge-requests/5/head"
      ],
      "depth":3
   },
   "runner_info":{
      "timeout":null,
      "runner_session_url":null
   },
...
}

ref_type will still be presented in order to support the legacy runners. But this is to be dropped.

A sample response when depth is not specified.

{
   "id":6594,
   "token":"FNWRozxyDZ4Q1HHWyW-1",
   "allow_git_fetch":true,
   "job_info":{
      "name":"deploy",
      "stage":"test",
      "project_id":89,
      "project_name":"prospective-merge-pipelines"
   },
   "git_info":{
      "repo_url":"http://gitlab-ci-token:FNWRozxyDZ4Q1HHWyW-1@localhost:8181/root/prospective-merge-pipelines.git",
      "sha":"abb6076c965eec51b92667388be8e7ef559f9ca3",
      "before_sha":"0000000000000000000000000000000000000000",
      "ref":"refs/heads/patch-7",
      "ref_type":"branch",
      "ref_spec":[
         "+refs/heads/*:refs/remotes/origin/*",
         "+refs/tags/*:refs/tags/*"
      ],
      "depth":0
   },
   ...
}

Rails-side currnetly enforcing depth to zero when GIT_DEPTH is not specified. https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/25233/diffs#c01214b277b69971fe5cc7555b37b2e8fe167585_655_664. Let me know if you need it to be absent or something.

Actually we should also drop ref along side with ref-type. It's just used for composing refspec in runner.

Rails still keep sending though, it's subject to change.

removed direction label

added to epic &848 (closed)

Rails-side currnetly enforcing depth to zero when GIT_DEPTH is not specified. https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/25233/diffs#c01214b277b69971fe5cc7555b37b2e8fe167585_655_664. Let me know if you need it to be absent or something.

@dosuken123 on the runner it will be 0 in both cases, feel free to act as it looks better on rails side

Rails side is same https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/25233/diffs#14f02b2235c4d81b632e491d1bd1267f4b63e9b4_17_39. I think it's consistent.

added workflowin review label

closed via merge request !1203 (merged)

mentioned in commit 61542f2b

mentioned in issue gl-retrospectives/release#6 (closed)

mentioned in issue #4080 (closed)

mentioned in issue #4111 (closed)

mentioned in merge request !2218 (closed)

mentioned in issue #37947 (closed)

mentioned in merge request !5010