Page MenuHomePhabricator

Make all CI images to use ci-src-setup-simple when running on CI
Open, Needs TriagePublic

Description

The ci-src-setup script takes care of initializing the source code based on CI parameters (from ZUUL_ prefixed environment variables). Some image do it by default (notably the tox ones) but most do not requiring using to invoke releng/ci-src-setup to provision the source code. I think the reason was to make it easy to use an image locally against an existing code base.

With T274347 , the ci-src-setup script only acts when either CI and/or JENKINS_URL environment variables are set. We can thus have all our images to invoke the script in their entry point.

For local use it just add a very thing overhead.

For CI that would let us:

  • skip invoking releng/ci-src-setup entirely
  • make the jobs definition a little bit more consistent

Images would need to be rebuild (the new script is already in ci-stretch, ci-buster) and the jobs that invoke them would need to drop the docker-ci-src-setup macro.

Event Timeline

My comment wasn't addressed on the previous task so here it is again:

Most new jobs have a different pattern:

  • Run ci-src-setup-simple docker image to clone repos
  • Run tox/composer/etc. image to run tests against cloned repos

I think we should just migrate whatever is still calling ci-src-setup in the entrypoint to the new format instead.

This will keep images self-contained and reusable for whatever purpose outside of CI too.

For a practical example, consider https://gerrit.wikimedia.org/r/plugins/gitiles/mediawiki/libs/Shellbox/+/refs/heads/master/.pipeline/blubber.yaml - it's using the releng/php72-composer image as part of CI where the magic env variables are probably set, but we really don't want it to start cloning something.

Context

The way I have originally envisioned the CI images was to have much of the logic embedded in the images in order to make the Jenkins job plain stupidalmost trivial.

Aka roughly:

job:
  name: build-Acme
  builders:
   - docker-run:
      image: docker-registry.wikimedia.org/releng/foo-lang

Zuul passing bunch of environment variables and the image takes care of fetching the patch by using the ci-src-setup script and that makes the Jenkins jobs super trivial.

One of my intuition is that would have made it easier to migrate the workload to Kubernetes (that plan got canceled) and I still feel it makes it easier to migrate to whatever new CI system (that got canceled / put on hold as well).

Last summer I have done a bit of refactoring to have the script included in all images ( T256462 ) with the idea to eventually switch all images to default to that logic.

Problems

We have discrepancy in how we retrieve patches, we either:

  • expect a call to the docker-src-dir and docker-ci-src-setup-simple JJB macro which invoke two images to create the ./src directory and fetch patch from CI.
  • or the image invokes the ci-src-setup script (such as the tox images)

That leads to some annoyances such as I and @Gehel have encountered today when making changes to maven images leading to some annoying follow up fixes (forgot to add docker-src-dir, we have two JJB macro one of mounting /src the other does not and they have meaningless names: docker-run-with-log-and-workspace-cache and docker-run-with-log-cache-src

@jbond is looking at adding an helper script to easily reproduce a CI build from a local machine using an existing code base. That was the rationale behind T274347: the tox images were always invoking ci-src-setup.sh which forces one to override the entrypoint, when they should probably just -v "$(pwd):/src and expect the right thing to happen.

Expectations

I would REALLY like to find a way to simplify the Jenkins job to a point where they just invoke a single image whenever possible. Then we also need to take in account reuse of those images as highlighted by @jbond project and what @Legoktm mentioned.

What I think might please both world (CI and local use) is to have the script to detect whether there is already a /src/.git. If there is, that means either:

  • CI already made the patch available by the use of docker-ci-src-setup
  • local use bound an existing git workspace to /src

And thus the ci-src-setup.sh script should early exit.

However if there is no /src/.git:

  • The CI job has not already prepared the repository and ci-src-setup.sh should be invoked
  • For local use, I guess bails out with an error messages giving instructions?

So a proposal would be to amend the following block:

dockerfiles/ci-common/content/ci-src-setup.sh
if [[ "${JENKINS_URL:-}" == "" && "${CI:-}" == "" ]]; then
    echo "Skipping git clone, not running on CI env (JENKINS_URL and CI vars empty/unset)."
    exit
fi

And use something such as:

dockerfiles/ci-common/content/ci-src-setup.sh
if [[ -e /src/.git ]]; then
  echo "Using existing code in /src"
  exit 0
fi

That would let me simplify the CI job, and would let local users entirely skip the ci-src-setup script whenever /src is bound in the container?

I think the proposal looks goot to me however i think there are potentially three use cases which would be nice to support

  1. Pull sources from git (used by CI)
  2. use /src mounted using -v /${PWD}://src for localy mounted jobs where CI dosn;t need to make modifications to the src directory
  3. mount $PWD to some local directory and then cp file to source

e.g.

if [[ -e /src/.git ]]; then
  echo "Using existing code in /src"
  exit 0
fi
RO_SRC=${RO_SRC:-/local}
if [[ -d ${RO_SRC} ]]; then
  cp -r ${RO_SRC} /src
  exit 0
fi

The third option would allow a user to run docker with `-v ${PWD}:/local' and not have to worry about hacking in rw mounts. See the comments in this CR for an example of what i mean by hacking in rw mounts