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

feat: Windows containerd + azure-cni #2864

Merged
merged 1 commit into from Mar 30, 2020
Merged

Conversation

ksubrmnn
Copy link
Contributor

@ksubrmnn ksubrmnn commented Mar 9, 2020

Reason for Change:

Once completed, this will install ContainerD and configure Kubernetes to use it on Windows with Azure CNI

Issue Fixed:

Requirements:

Notes:
/hold do-not-merge

@welcome
Copy link

welcome bot commented Mar 9, 2020

💖 Thanks for opening your first pull request! 💖 We use semantic commit messages to streamline the release process. Before your pull request can be merged, you should make sure your first commit and PR title start with a semantic prefix. Examples of commit messages with semantic prefixes: - fix: change azure disk cachingMode to ReadOnly - feat: make maximumLoadBalancerRuleCount configurable - docs: add note on AKS Engine and AKS relationship
Make sure to check out the developer guide for guidance on testing your change.

@codecov
Copy link

codecov bot commented Mar 25, 2020

Codecov Report

Merging #2864 into master will increase coverage by <.01%.
The diff coverage is 85.71%.

@@            Coverage Diff             @@
##           master    #2864      +/-   ##
==========================================
+ Coverage   72.57%   72.58%   +<.01%     
==========================================
  Files         141      141              
  Lines       25889    25891       +2     
==========================================
+ Hits        18790    18794       +4     
+ Misses       6005     6003       -2     
  Partials     1094     1094

@ksubrmnn ksubrmnn changed the title wip: Windows containerd + azure-cni Windows containerd + azure-cni Mar 27, 2020
@@ -1677,9 +1677,6 @@ func (a *Properties) validateContainerRuntime() error {

// TODO: These validations should be relaxed once ContainerD and CNI plugins are more readily available
if containerRuntime == Containerd && a.HasWindows() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we add any validation around k8s version here?
Will containerd work with versions older than 1.18?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure. Who could I ask?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we'll need to try it out 🤣

@marosset
Copy link
Contributor

@ksubrmnn I think you need to remake the generated files after formatting the files with containerd functions

@ksubrmnn
Copy link
Contributor Author

@ksubrmnn I think you need to remake the generated files after formatting the files with containerd functions

Yep, realized that then got on a call mid make bootstrap. Should be fixed!

@marosset
Copy link
Contributor

/lgtm

@acs-bot acs-bot added the lgtm label Mar 30, 2020
@marosset marosset changed the title Windows containerd + azure-cni feat: Windows containerd + azure-cni Mar 30, 2020
@marosset marosset merged commit 8453a6a into Azure:master Mar 30, 2020
@welcome
Copy link

welcome bot commented Mar 30, 2020

Congrats on merging your first pull request! 🎉🎉🎉

@acs-bot
Copy link

acs-bot commented Mar 30, 2020

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ksubrmnn, marosset

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

@ksubrmnn ksubrmnn deleted the containerd branch May 12, 2020 22:16
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

3 participants