[CI] A better way to rebuild docker image

[zhanglx13]

Problem

The current logic to rebuild the docker image is when a new branch is pushed. This has the following two drawbacks:

1. If one of the dependencies in requirements.txt is changed during the development of the new branch, the docker image is not rebuilt.
2. If a new branch does not change anything in the dependencies, the docker image is rebuilt twice (one for /usr/local and another one for /opt/rocm), which is a wasted of resources (about 40 mins and some disk space).

Proposal

The docker image should be rebuild if and only if some dependencies or the Dockerfile itself are changed. Here is the general idea of the proposal:

1. Include not only the branch name, but also the commit id (git rev-parse --short HEAD) in the tag of the image. Therefore, the name of each image should has the form: ${MIOPEN_IMAGE_URL}:miopen_ci_${branchname}_${commitID}_[usr|opt]
2. Every time Jenkins is notified of a change to a branch (either new commits are added or new branch pushed), it tries to find the latest image built for the branch.
   1. If there is one, obtain the commit id from the image name and compare with HEAD to see if there is a change in the dependencies or Dockerfile.
      1. If so, build a new image with HEAD as the commit id in the tag.
      2. If not, use the last built image. Note that the develop branch can only fall into this case since Jenkins should always have images built for the develop branch.
   2. If an image is not found, it means this is a new branch or this branch (cannot be the develop branch) has not changed any dependencies or the Dockerfile before. Then Jenkins compares HEAD with the fork point between this branch and the develop branch to see if there is a change in the dependencies of the Dockerfile.
      1. If so, build a new image with HEAD as the commit id in the tag.
      2. If not changed, use the latest built image for the develop branch. Note that the commit id of this image is the same or later than the fork point. In this way, the new branch will always be tested with the latest dependencies installed in the image.

Benefits

The effect of the proposed mechanism is a great drop of the number of rebuilds. The docker image is rebuilt if and only if some dependency requirements are changes with one exception mentioned in Case 5.1.

Demos

Here are some scenarios to illustrate the proposed image building mechanism.

                     A           F           B
branch develop O --- O --- O --- O --- O --- O
                                  \
mybranch                           X --- X --- X --- X --- X
                                               P           Q

In the above diagram, O stands for a commit on the develop branch and X for commits on a new branch mybranch. F is the fork point between the two branches.
Assume I first pushed mybranch when P is the HEAD. Then I added two commits and pushed again when Q is the HEAD.

Case 1: no dependency change in mybranch

Since I am not changing anything about the dependencies, there is no need to build a new image for mybranch. And since no images are built for mybranch, both pushes fall into the scenario of 2.ii.b, i.e. neither git diff P..F nor git diff Q..F contains dependency changes. And in this case, the image used for mybranch is the latest built image for the develop branch. Then we have two sub-cases:

1. the latest image is ${MIOPEN_IMAGE_URL}:miopen_ci_develop_[A|F]_[usr|opt]
   In this case, the dependency requirements have not changed since A or F. Therefore, it makes sense for P and Q to used to same image as A or F.
2. the latest image is ${MIOPEN_IMAGE_URL}:miopen_ci_develop_B_[usr|opt]
   In this case, some dependency requirements have changed by another PR, which have been merged into the develop branch before my push of mybranch. It makes sense for mybranch to be tested with the latest dependency requirements to catch bugs as early as possible (at least before mybranch is merged).

Case 2: dependency changes in my first push but no more changes in my second push

When I first push mybranch with head P, Jenkins finds that the difference between F and P contains dependency changes. Therefore, it rebuilds the image with tag miopen_ci_mybranch_P_[usr|opt] as in 2.ii.a.
At this moment, the image tag for the develop branch can point to A, F, or B. In the case of A or F, mybranch has a newer version of the dependency requirements. However, in the case of B, the newer version of the dependency requirements could be on the develop branch. In this case, mybranch is tested with its local change to the dependency requirements. The local change could be tested again when mybranch is merged into the develop branch.

The second push of mybranch with head Q does not have more changes to the dependency requirements compared to P. Therefore, Jenkins will use the previously built image with tag miopen_ci_mybranch_P_[usr|opt] as indicated in 2.i.b.

Case 3: no dependency change in my first push but some are in my second push

The first push (with head P) falls into 2.ii.b, in which case the latest built image for the develop branch is used.
For the second push with head Q, Jenkins still cannot find an image with a tag that include mybranch. However, since the second push includes changes to the dependency requirements, the image is rebuilt with a new tag miopen_ci_mybranch_Q_[usr|opt] as indicated in 2.ii.a. Note that Jenkins compares Q with F to make the decision since P and F are equivalent in terms of dependency requirements. And since mybranch has local changes to the dependency requirements, those on the develop branch are ignored when testing mybranch.

Case 4: dependency changes in my first push and more dependency changes in my second push

The first push falls into 2.ii.a since this is the first time mybranch is pushed and the image is rebuilt with tag miopen_ci_mybranch_P_[usr|opt].
The second push falls into 2.i.a, in which case Jenkins rebuild the image again with a new tag miopen_ci_mybranch_Q_[usr|opt].

Case 5: What happens when mybranch is merged

                     A           F           B               M
branch develop O --- O --- O --- O --- O --- O ------------- O
                                  \                         /
mybranch                           X --- X --- X --- X --- X
                                               P           Q

In the above diagram, M is the merge point. The image is rebuilt for the develop branch in the following cases:

1. mybranch has changes but no changes after F on the develop branch
   This case implies that the last built image for the develop branch has a tag pointing to A or F, but cannot to B. For the develop branch, M contains the dependency requirements changes that is not found in F or A. Therefore, the image is rebuilt with tag miopen_ci_develop_M_[usr|opt] as indicated in 2.i.a.
   Note that this image is duplicate to the latest image built for mybranch. The proposed mechanism cannot eliminate such duplicates.
2. mybranch has changes and some commits (say B) after F on the develop branch have changes
   In this case, the merge must have resolved any conflicts of dependency requirements. And Jenkins will trigger a rebuild if and only if mybranch has changes that are not covered in B, i.e. M is different from B in terms of dependency requirements. And the image is rebuilt according to 2.i.a.

When mybranch does not have dependency requirements changes, no rebuild will happen when mybranch is merged into the develop branch since M must be the same as B or F or A in terms of dependency requirements. And this falls into 2.i.b.

[JehandadKhan]

My solution to this problem was to provide an override parameter to the pipeline which can be manually set forcing the generation of a new Docker image ? See here for code

[zhanglx13]

@JehandadKhan Thank you for the code. That is indeed a simple workaround to build docker image during the branch development. I'll borrow it to get my PR #1673 done first.

[atamazov]

@junliume Please add @pfultz2 to this discussion. Also I recommend labeling with request_for_comments.

[atamazov]

@zhanglx13 I highly recommend removing all the mentions of develop from the description. Technically, this branch is equal to all other branches except some details unrelated to this topic (e.g. it's special role in the promotion process etc.)

[atamazov]

The idea of reusing docker images from the "parent" branches is good but it's generic implementation seems too complex.

Why docker build caching is not effective anymore? I remember it worked well some time ago.

[jerryyin]

I believe build cache should still work. I think the benefit of @zhanglx13 's proposal is that it reduced the chance of invoking docker build to the minimal, and scooped to the cases to when it is strictly necessary. Also, it is an automated process so it doesn't require developer to do any manual override should there be an update in the dependencies. The inclusion of commit id adds the traceability of docker images. We use a somewhat similar idea in MLIR in the build process and this turns out to be very effective in reducing overhead.

"Strictly necessary" is defined as a change in either:

• Dockerfile, or
• requirements.txt

The idea behind this is to use git diff to find out whether there's change between those two files between the latest docker and the current branch. I think this is more effective heuristic compared to existing implementation. And the result will be more frequent docker pull instead of docker build.

[pfultz2]

"Strictly necessary" is defined as a change in either:

Those aren't the only files that affect the dockerfiles. You need to check all files that added with the ADD directive.

[jerryyin]

Okay to summarize the complete list (in addition to dockerfile) should be:

• rbuild.ini
• requirements.txt
• dev-requirements.txt
• doc/requirements.txt

This does sounds like an excessive (but still doable) list to check from.

[atamazov]

[Informative] @zhanglx13 Thank you very much for the detailed analysis 👍 !

[zhanglx13]

@atamazov It seems to me that other branches are merging into the develop branch, which is considered to be the only relevant difference. This is only for demonstration purpose and the complex proposal handles all branches the same way.
I am new to docker and I'll take a look at docker build caching. Or if you know someone who is more familiar with the topic, could you ask him/her to take a look?

[pfultz2]

I dont see the reason to reinvent the wheel. Why not just use --cache-from to get centralized docker caches?

Alternatively, you could always build the docker files on the same machine so you can reuse the local build cache. Then tag the docker image with the commit hash and push the image to dockerhub/artifactory. Then each job would pull the docker image tag with its commit hash. The local docker layer caches will help speed up the push and pull of docker images.

There is no need to track branch names or changes against HEAD of the current branch.

[jerryyin]

Thanks for the idea, Paul! Clarification first, when saying reinvent the wheel do you mean the "wheel" that MIOpen already has or what docker is capable of?

It could be an orthogonal topic on what the most desired form of docker build is for MIOpen, but that's not what we are trying to address here. I see a fair amount of work to force build happen on a single machine (and yet to tolerate the single point of failure on that machine). Your proposal of building for every commit is a different approach. Compared to that, I'd argue MLIR's way of triggering build is better: to decouple the build process from testing completely, and that can be in a different discussion.

On the other hand, what existing MIOpen has for the build triggering doesn't quite work when there's a dependency bump. If we leave it as-is, it require manual intervention both at the dev branch and at develop branch to keep make image built and CI to pass. I see the proposal as a step further trying to address that problem, making the step "automatic". In other words, the requirement is targeting towards fixing existing procedure while reducing the times of build if necessary.

[atamazov]

Proposal of the process of updating the CI docker image + discussion: #1897 (comment)

[jerryyin]

Note, I still think the discussion happened in #1897 is a temporary manner because of a number of reasons:

• If multiple PR that requires to update the dependency at the same time may very likely ended up missing out one or another based on the merge sequence. Since this is a manual process, the PR owner may not be aware of other changes.
• MIOpen doesn't have enough control over how image is built: What if developer used a slightly different dockerfile from the one posted in the review? Or what if the developer made a typo in the image name update process?

I think a slight improvement that discussion is just to post a jenkins job using branch + image name as input (that under the hood would kick off a new image build based on latest dockerfile in develop branch, and update the dockerhub image name pointer accordingly). Then it'd be easier to update the miopen/ci:latest from develop branch when there's new PR that updated dependency merged.

[atamazov]

Please feel free to develop & publish the improved process. Thanks.