feat: make Moby version configurable #407
Conversation
Codecov Report
@@ Coverage Diff @@
## master #407 +/- ##
==========================================
+ Coverage 53.41% 53.42% +0.01%
==========================================
Files 95 95
Lines 14360 14366 +6
==========================================
+ Hits 7670 7675 +5
- Misses 6027 6028 +1
Partials 663 663 |
.pipelines/e2e-step-template.yaml
Outdated
| @@ -25,3 +25,4 @@ steps: | |||
| inputs: | |||
| artifactName: '${{ parameters.job }}_logs' | |||
| targetPath: '$(modulePath)/_logs' | |||
| condition: always() | |||
There was a problem hiding this comment.
this is just for testing, will move to a different PR
| @@ -11,7 +11,11 @@ CONTAINERD_DOWNLOAD_URL="${CONTAINERD_DOWNLOAD_URL_BASE}cri-containerd-${CRI_CON | |||
| CONTAINERD_TGZ_TMP=$(echo ${CONTAINERD_DOWNLOAD_URL} | cut -d "/" -f 5) | |||
|
|
|||
| removeEtcd() { | |||
| rm -rf /usr/bin/etcd & | |||
| rm -rf /usr/bin/etcd | |||
There was a problem hiding this comment.
removing the & here because I was about to do the same for Moby but realized this could lead to a race condition: we install etcd in /usr/bin/etcd right after calling removeEtcd
| "allowedValues": [ | ||
| "3.0.1", | ||
| "3.0.2", | ||
| "3.0.3" |
There was a problem hiding this comment.
not sure if we want to have this validation or just allow free form strings as versions to allow AKS to install a version AKS-Engine doesn't support yet but risk failing moby install later with a CSE error code
There was a problem hiding this comment.
I think let's include it. We can always spike on weird, experimental stuff on PRs by overriding the installMoby func to do an alternate installation, ignoring the value in mobyVersion
| @@ -20,6 +20,7 @@ if [[ ${FEATURE_FLAGS} == *"docker-engine"* ]]; then | |||
| installDockerEngine | |||
| installGPUDrivers | |||
| else | |||
| MOBY_VERSION="3.0.1" | |||
There was a problem hiding this comment.
for now, we can only install one version on the VHD at a time (that's because we actually install the packages as opposed to downloading a binary as we do for other things), same as for etcd
CecileRobertMichon
force-pushed
the
configurable-moby
branch
from
cac1dab
to
82da009
Compare
CecileRobertMichon
changed the title
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: CecileRobertMichon, jackfrancis The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
* feat: make Moby version configurable * remove extra comma * mobyVersion should be a parameter * removeMoby before install, specify version in packer script * Add default version in doc * convert api/vlabs types
|
Just a heads up that this pretty much broke the ability for shell scripts to determine what features the command line supports compared to other 'docker' commands. For example, if the client version is => 18, then scripts know that the docker command supports the --cache-from option. [Note that this is different than the Client API version.] |
|
@aw-was-here I'm not sure I understand how this particular PR (making the Moby version configurable) broke the scenario you described above. Can you please explain how this PR is breaking for that scenario? |
|
'docker version' is now returning docker client version 3.0.3 in azure pipelines. Prior it was 18.xxx. |
|
@aw-was-here this PR did not change that (it just makes the Moby version configurable). Switching to the Moby container runtime is what changed the version returned: Azure/acs-engine#3896. acs-engine (and now aks-engine) has been delivering moby as the default container runtime since v0.25.0, outlined in the release notes here: https://github.com/Azure/acs-engine/releases/tag/v0.25.0 Docker CE was never supported on acs-engine/aks-engine Linux nodes because of licensing issues (see Azure/acs-engine#4029 for context). The Moby version |
|
Ah, ok, thanks. I guess we'll likely just yank Azure support for Apache Yetus in Docker mode then since it's unlikely we'll get around to support moby any time soon. |
Reason for Change:
Moby version should be configurable in apimodel.
Issue Fixed:
Fixes #107
Requirements:
Notes: