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

feat: allow custom containerd package for Linux nodes #3878

Merged
merged 2 commits into from
Oct 2, 2020
Merged

feat: allow custom containerd package for Linux nodes #3878

merged 2 commits into from
Oct 2, 2020

Conversation

chewong
Copy link

@chewong chewong commented Sep 28, 2020

Signed-off-by: Ernest Wong [email protected]

Reason for Change:

Allow installation of custom containerd package on Linux nodes to increase e2e test coverage for ContainerD pipeline.

Issue Fixed:

Credit Where Due:

Does this change contain code from or inspired by another project?

  • No
  • Yes

If "Yes," did you notify that project's maintainers and provide attribution?

  • No
  • Yes

Requirements:

Notes:

@codecov
Copy link

codecov bot commented Sep 28, 2020

Codecov Report

Merging #3878 into master will increase coverage by 0.01%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3878      +/-   ##
==========================================
+ Coverage   72.83%   72.84%   +0.01%     
==========================================
  Files         149      149              
  Lines       23171    23211      +40     
==========================================
+ Hits        16876    16908      +32     
- Misses       5178     5182       +4     
- Partials     1117     1121       +4     
Impacted Files Coverage Δ
pkg/api/types.go 93.83% <ø> (ø)
pkg/api/vlabs/types.go 71.81% <ø> (ø)
pkg/engine/cse.go 100.00% <ø> (ø)
pkg/engine/templates_generated.go 48.65% <ø> (ø)
pkg/api/converterfromapi.go 94.80% <100.00%> (+<0.01%) ⬆️
pkg/api/convertertoapi.go 94.53% <100.00%> (+<0.01%) ⬆️
pkg/engine/template_generator.go 82.01% <100.00%> (+0.14%) ⬆️
pkg/engine/systemroleassignments.go 97.95% <0.00%> (-2.05%) ⬇️
pkg/engine/armtype.go 85.00% <0.00%> (ø)
... and 2 more

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 1a3556e...f02c41b. Read the comment docs.

@jackfrancis
Copy link
Member

Do we have a stable URL we can add to test/e2e/test_cluster_configs/containerd.json and regularly test?

@@ -8,6 +8,7 @@
"kubernetesConfig": {
"networkPlugin": "azure",
"containerRuntime": "containerd",
"linuxContainerdURL": "https://moby.blob.core.windows.net/moby-containerd/moby-containerd/1.4.0+azure/bionic/linux_amd64/moby-containerd_1.4.0+azure-1_amd64.deb",
Copy link
Member

Choose a reason for hiding this comment

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

❤️

@chewong
Copy link
Author

chewong commented Sep 29, 2020

/hold

jackfrancis
jackfrancis previously approved these changes Sep 29, 2020
Copy link
Member

@jackfrancis jackfrancis left a comment

Choose a reason for hiding this comment

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

/lgtm

@cpuguy83
Copy link
Member

Can you also update installMoby to take the custom containerd URL?

@acs-bot acs-bot added size/L and removed size/M labels Sep 29, 2020
@chewong
Copy link
Author

chewong commented Sep 29, 2020

Fixed. Also squashed all commits.

@acs-bot acs-bot added size/M and removed size/L labels Sep 29, 2020
sozercan
sozercan previously approved these changes Sep 29, 2020
Copy link
Member

@sozercan sozercan left a comment

Choose a reason for hiding this comment

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

/lgtm

cpuguy83
cpuguy83 previously approved these changes Sep 29, 2020
Copy link
Member

@cpuguy83 cpuguy83 left a comment

Choose a reason for hiding this comment

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

LGTM

@devigned
Copy link
Member

@chewong looks like the packer build is failing due to a var not getting replaced in the provisioning scripts.

==> azure-arm: /home/packer/provision_installs.sh: line 120: {{-: command not found
==> azure-arm: + LINUX_CONTAINERD_URL='{{GetLinuxContainerdURL}}'
==> azure-arm: + [[ -n {{GetLinuxContainerdURL}} ]]
==> azure-arm: + DEB='{{GetLinuxContainerdURL}}'
==> azure-arm: + retrycmd_no_stats 120 5 25 curl -fsSL '{{GetLinuxContainerdURL}}'

@chewong chewong dismissed stale reviews from cpuguy83 and sozercan via e582233 September 30, 2020 15:39
@acs-bot acs-bot removed the lgtm label Sep 30, 2020
pkg/api/types.go Outdated Show resolved Hide resolved
@cpuguy83
Copy link
Member

cpuguy83 commented Oct 1, 2020

This looks great! One thing I'd add is an env var passthrough for LINUX_CONTAINERD_URL and WINDOWS_CONTAINERD_URL in test/e2e/engine/template.go and test/e2e/cluster.sh

@cpuguy83
Copy link
Member

cpuguy83 commented Oct 1, 2020

Looks like cluster provisioning failed on 1.19 for some reason.

Copy link
Member

@jackfrancis jackfrancis 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 acs-bot added the lgtm label Oct 2, 2020
@jackfrancis jackfrancis merged commit 7dc01ab into Azure:master Oct 2, 2020
@jackfrancis
Copy link
Member

Thanks @chewong!

@acs-bot
Copy link

acs-bot commented Oct 2, 2020

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: chewong, jackfrancis, sozercan

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

@chewong chewong deleted the custom-containerd branch October 5, 2020 05:33
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.

6 participants