Skip to content

Add feature flag to mounting volumes to services

Tomasz Maczukin requested to merge fix-volumes-mounting-regression into master

What does this MR do?

Restores the previous strategy of creating volumes in pair with services creation. The old strategy is hidden behind a new feature flag. I propose to deprecate and remove the old behavior in 12.6

Why was this MR needed?

With !1261 (merged), which was part of our road to #3909 (closed), we've changed the ordering of volumes creation and services creation.

Previously, the ordering was as follows:

  • create build volume,
  • create all services and mount volumes (so in reality - only the build volume) to them,
  • create other user-defined volumes.

With !1261 (merged) we've changed this to be:

  • create user-defined volumes,
  • create build volume,
  • create all services and mount volumes (so in reality - all of the created volumes) to them.

The reasoning behind such change was that we can allow users to mount the root builds directory in a place of their choice. If the mounting target of one of the user-defined volumes will be the same as the configured root builds directory, Runner will skip mounting the automatically created volume.

But for this we need to create the user-defined volumes before creating the build volume. And because we want to mount build volume to all services, services creation needs to be moved to the end of the list. Finally, this ends with a scenario where all services have all configured volumes + build volume mounted.

In some specific cases this may introduce a problem. For example, we've discovered this on our own machine, that is used by GitLab Runner project's pipeline. Because of an old, outdated configuration that was left on the Runner, after deploying 11.11.0-rc2 with volumes change from !1261 (merged) some of the jobs started failing. The reason was that an unexpected volume was mounted in one of the services and it was causing the service to fail.

We've removed the obsolete configuration from our Runner and jobs started working, but we've considered the problem as regression. And while we think that the current ordering is the proper one, we need to remember about users that may be not prepared for such change. That's why this MR adds a new feature flag, that allows anyone to switch to the old ordering, until the configuration will be adjusted.

Are there points in the code the reviewer needs to double check?

Does this MR meet the acceptance criteria?

  • Documentation created/updated
  • Added tests for this feature/bug
  • In case of conflicts with master - branch was rebased

What are the relevant issue numbers?

Related to #3909 (closed) Closes #4260 (closed)

Edited by Steve Xuereb

Merge request reports