Skip to content
This repository has been archived by the owner on Oct 24, 2023. It is now read-only.

chore: specify containerd version to install with moby-engine package #3723

Merged
merged 3 commits into from
Aug 26, 2020

Conversation

cpuguy83
Copy link
Member

Reason for Change:

Since moby packages now depend on a separate containerd package, we should specify which version of the containerd package should be picked up so that we don't accidentally introduce regressions for new installs/upgrades due to apt-get picking the latest containerd (which may include major version bumps).

This also enables testing aks-engine with different versions of containerd (such as the newly released 1.4) since the api model now allows specifying a containerd version when the moby version is >= 19.03.

Issue Fixed:

N/A

Requirements:

Notes:

@cpuguy83 cpuguy83 force-pushed the containerd_pinning branch 5 times, most recently from c2d2e71 to 60e77fa Compare August 17, 2020 23:40
@codecov
Copy link

codecov bot commented Aug 17, 2020

Codecov Report

Merging #3723 into master will increase coverage by 0.01%.
The diff coverage is 73.68%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3723      +/-   ##
==========================================
+ Coverage   73.16%   73.18%   +0.01%     
==========================================
  Files         147      148       +1     
  Lines       25318    25352      +34     
==========================================
+ Hits        18525    18553      +28     
- Misses       5655     5663       +8     
+ Partials     1138     1136       -2     
Impacted Files Coverage Δ
pkg/engine/templates_generated.go 53.42% <ø> (ø)
pkg/versions/compare.go 65.51% <65.51%> (ø)
pkg/api/defaults.go 92.91% <100.00%> (+0.06%) ⬆️
pkg/api/vlabs/validate.go 80.15% <100.00%> (+0.15%) ⬆️
pkg/api/common/versions.go 96.84% <0.00%> (ø)
pkg/engine/params_k8s.go 85.52% <0.00%> (+1.31%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 88add17...25c6753. Read the comment docs.

Moby needs containerd, we should lock in a default version for this.
Additionally this enables users to customize the containerd version to
use along with moby.
This gets aks-engine up to date with what's we currently have available
and updates the default to the latest.
This test cannot run in parallel due to the same  `c.components` being
re-used for different cases.
@@ -37,7 +38,7 @@ var (
"3.1.0", "3.1.1", "3.1.2", "3.1.2", "3.1.3", "3.1.4", "3.1.5", "3.1.6", "3.1.7", "3.1.8", "3.1.9", "3.1.10",
"3.2.0", "3.2.1", "3.2.2", "3.2.3", "3.2.4", "3.2.5", "3.2.6", "3.2.7", "3.2.8", "3.2.9", "3.2.11", "3.2.12",
"3.2.13", "3.2.14", "3.2.15", "3.2.16", "3.2.23", "3.2.24", "3.2.25", "3.2.26", "3.3.0", "3.3.1", "3.3.8", "3.3.9", "3.3.10", "3.3.13", "3.3.15", "3.3.18", "3.3.19", "3.3.22"}
containerdValidVersions = [...]string{"1.3.2"}
containerdValidVersions = [...]string{"1.3.2", "1.3.3", "1.3.4", "1.3.5", "1.3.6", "1.3.7"}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can 1.4.0 also be allowed? If I understand correctly, even with this change I won't be able to use containerRuntime as containerd and version 1.4.0

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Working on it, but not at this time.
I am holding back 1.4 packages due some issues around how apt manages packages.
Basically, as soon as we upload containerd 1.4, when someone installs our moby packages they will get containerd 1.4 (unless they do what is being done here), which is an untested scenario.

Copy link
Member

@mboersma mboersma left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

@acs-bot
Copy link

acs-bot commented Aug 24, 2020

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cpuguy83, mboersma

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@mboersma mboersma changed the title Specify containerd version to install with moby-engine package chore: specify containerd version to install with moby-engine package Aug 26, 2020
@mboersma mboersma merged commit ba57b8c into Azure:master Aug 26, 2020
@cpuguy83 cpuguy83 deleted the containerd_pinning branch August 26, 2020 18:30
penggu pushed a commit to penggu/aks-engine that referenced this pull request Oct 28, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants