-
Notifications
You must be signed in to change notification settings - Fork 519
chore: install specific containerd version, make version configurable #516
chore: install specific containerd version, make version configurable #516
Conversation
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: 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 |
Codecov Report
@@ Coverage Diff @@
## master #516 +/- ##
==========================================
+ Coverage 54.39% 54.56% +0.16%
==========================================
Files 97 97
Lines 14570 14622 +52
==========================================
+ Hits 7925 7978 +53
Misses 5971 5971
+ Partials 674 673 -1 |
packer/install-dependencies.sh
Outdated
@@ -19,6 +19,7 @@ if [[ ${FEATURE_FLAGS} == *"docker-engine"* ]]; then | |||
DOCKER_ENGINE_REPO="https://apt.dockerproject.org/repo" | |||
installDockerEngine | |||
installGPUDrivers | |||
installContainerd |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just to be conservative/iterative, let's keep the installContainerd
invocation in the VHD for non-moby scenario.
(See below that it used to be here.)
I don't think we strictly need to be doing this installation, but the safest way is to make that change distinctly.
The reason we're removing it from moby is that the moby installation implementation currently installs containerd stuffs itself.
@@ -39,7 +40,10 @@ for CNI_PLUGIN_VERSION in $CNI_PLUGIN_VERSIONS; do | |||
downloadCNI | |||
done | |||
|
|||
installContainerd | |||
for CONTAINERD_VERSION in $CONTAINERD_VERSIONS; do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of actually doing the install, we're just downloading the artifacts in the VHD.
@@ -71,7 +71,9 @@ fi | |||
|
|||
installContainerRuntime | |||
installNetworkPlugin | |||
installContainerd | |||
if [[ "$CONTAINER_RUNTIME" == "clear-containers" ]] || [[ "$CONTAINER_RUNTIME" == "kata-containers" ]] || [[ "$CONTAINER_RUNTIME" == "containerd" ]]; then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We only actually install the containerd artifacts for required scenarios at provisioning time.
Reviewed |
@@ -706,9 +706,11 @@ func convertKubernetesConfigToVLabs(apiCfg *KubernetesConfig, vlabsCfg *vlabs.Ku | |||
vlabsCfg.ServiceCidr = apiCfg.ServiceCIDR | |||
vlabsCfg.NetworkPolicy = apiCfg.NetworkPolicy | |||
vlabsCfg.NetworkPlugin = apiCfg.NetworkPlugin | |||
vlabsCfg.ContainerRuntime = apiCfg.ContainerRuntime |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Whoops, found a bug while doing this work :(
Reason for Change:
This PR does specific version checking against any pre-installed versions of containerd and compares to the desired version. If a different version is present it cleans up the non-desired version and installs the desired version.
Also adds a
containerdVersion
configuration option tokuberntesConfig
to enable user-configurable containerd versions. Ships with the following versions support:No change made to the existing default 1.1.5 version, that change will happen in a follow-up PR.
WIP assumes moby provides any needed containerd deps, so skips install for normal docker flows.
Issue Fixed:
Fixes #512
Requirements:
Notes: