-
Notifications
You must be signed in to change notification settings - Fork 528
Conversation
This is going to be open a few days or more. My goal is to get the PR open early with the changes that could break Linux or fail unit tests and let the CI system find those issues. Meanwhile, I still have a lot of work to do in the Windows setup steps that aren't covered in those tests. |
Codecov Report
@@ Coverage Diff @@
## master #1322 +/- ##
==========================================
+ Coverage 72.47% 72.47% +<.01%
==========================================
Files 140 140
Lines 25589 25612 +23
==========================================
+ Hits 18545 18562 +17
- Misses 5976 5981 +5
- Partials 1068 1069 +1 |
3b9b03c
to
471486a
Compare
This may not even build right now - did a few hours of work on a plane and then --force-with-lease 🤠 |
64a2b94
to
16e3beb
Compare
16e3beb
to
b73bebe
Compare
e09c1dc
to
7da421d
Compare
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: PatrickLang 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 |
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
abb9a97
to
5590f23
Compare
db6a8f2
to
3eb5b29
Compare
ok, another day, another rebase + squash with a commit amend to appease Semantic Pull Request |
/hold cancel Ready for review. It's not done, but it builds clusters. next steps (not in any order):
|
"networkPlugin": "kubenet", | ||
"containerRuntime": "containerd", | ||
"windowsContainerdURL": "https://acsenginedev.blob.core.windows.net/cri-containerd/windows-cri-containerd-05072019.zip", | ||
"windowsSdnPluginURL": "https://acsenginedev.blob.core.windows.net/cri-containerd/windows-cni-containerd-05072019.zip" |
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.
Right now it looks like we already have a mix of windows specific settings in both kubernetesConfig (azureCNIURLWindows) and windowsProfile (windowsDockerVersion)...
I don't think we should add this to agentPoolProfile but need to think about kubernetesConfig or windowsProfile
"networkPlugin": "kubenet", | ||
"containerRuntime": "containerd", | ||
"windowsContainerdURL": "https://acsenginedev.blob.core.windows.net/cri-containerd/windows-cri-containerd-05072019.zip", | ||
"windowsSdnPluginURL": "https://acsenginedev.blob.core.windows.net/cri-containerd/windows-cni-containerd-05072019.zip" |
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.
Lets keep it in the kubernetesConfig since it logically fits in with the other config values in there.
If we need more flexibility I think it make more sense to add a per node pool override in the agentPoolProfile but that can come later.
"networkPlugin": "kubenet", | ||
"containerRuntime": "containerd", | ||
"windowsContainerdURL": "https://acsenginedev.blob.core.windows.net/cri-containerd/windows-cri-containerd-05072019.zip", | ||
"windowsSdnPluginURL": "https://acsenginedev.blob.core.windows.net/cri-containerd/windows-cni-containerd-05072019.zip" |
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.
And we should maybe consider moving the WindowsDockerVersion out of windowsProfile to kubernetesConfig too (and document that flag in clusterdefintions.md)
| controllerManagerConfig | no | Configure various runtime configuration for controller-manager. See `controllerManagerConfig` [below](#feat-controller-manager-config) | | ||
| customWindowsPackageURL | no | Configure custom windows Kubernetes release package URL for deployment on Windows. The format of this file is a zip file with multiple items (binaries, cni, infra container) in it. This setting will be deprecated in a future release of aks-engine where the binaries will be pulled in the format of Kubernetes releases that only contain the kubernetes binaries. | | ||
| WindowsNodeBinariesURL | no | Windows Kubernetes Node binaries can be provided in the format of Kubernetes release (example: https://github.com/kubernetes/kubernetes/blob/master/CHANGELOG-1.11.md#node-binaries-1). This setting allows overriding the binaries for custom builds. | | ||
| WindowsContainerdURL | no (for development only) | **Experimental** - see [Windows ContainerD](features.md#windows-containerd) | |
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.
I don't think we should change this now but i'm curious if you think these belong here or under the WindowsProfile long-term?
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.
a bit of history - this codebase originally supported mesos and docker swarm too, so the container runtime version existed before kubernetesConfig
.
I do think these may should have a cluster-wide default, but with a per-agentpool override. All of them happen to be in kubernetesConfig
containerRuntime
- runtime version (
containerdVersion
/mobyVersion
) kubeletConfig
So getting back to an answer - I'd consider moving them from kubernetesConfig
to an agent pool setting as a group in the future for consistency, not just a single setting
3d73463
to
c2c400f
Compare
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.
/lgtm
Looks good for the state we are currently targeting :)
(cherry picked from commit b068aa7)
* feat: add support for Kubernetes 1.18.0-beta.1 (#2791) * feat: add support for Kubernetes 1.18.0-beta.1 See https://github.com/kubernetes/kubernetes/blob/master/CHANGELOG/CHANGELOG-1.18.md/#v1180-beta1 * fix: windows support .zip URL (cherry picked from commit 34706cf) * feat: add support for single stack IPv6 (#2781) (cherry picked from commit 1b9beb4) * feat: allow iptables mode for dualstack 1.18+ (#2882) (cherry picked from commit ff5362e) * chore: update cluster-autoscaler for k8s 1.18 (#2901) See https://github.com/kubernetes/autoscaler/releases/tag/cluster-autoscaler-1.18.0 (cherry picked from commit 0cc985d) * feat: add support for Kubernetes 1.18.0 (#2957) See https://github.com/kubernetes/kubernetes/blob/master/CHANGELOG-1.18.md (cherry picked from commit 6dadaf0) * test: fix test TestGetProvisionScriptParametersCommon and TestGetK8sVersionComponents * feat: Adding WindowsNodeReset.ps1 script to reset/cleanup state for nodes (#2457) * Adding WindowsNodeReset.ps1 script to reset/cleanup state for windox^C nodes * fix linting errors * fixing comments per CR feedback (cherry picked from commit 8be7000) * feat: Experimental support for Windows+ContainerD (#1322) (cherry picked from commit b068aa7) * feat: installing csi-proxy for windows at node deployment time (#2930) (cherry picked from commit 13e72f2) Co-authored-by: Matt Boersma <Matt.Boersma@microsoft.com> Co-authored-by: Anish Ramasekar <anish.ramasekar@gmail.com> Co-authored-by: Mark Rossetti <marosset@microsoft.com>
(cherry picked from commit b068aa7)
…3076) * test: fix test TestGetK8sVersionComponents and TestGetProvisionScriptParametersCommon * feat: Adding WindowsNodeReset.ps1 script to reset/cleanup state for nodes (#2457) * Adding WindowsNodeReset.ps1 script to reset/cleanup state for windox^C nodes * fix linting errors * fixing comments per CR feedback (cherry picked from commit 8be7000) * feat: add support for Kubernetes 1.18.0-beta.1 (#2791) * feat: add support for Kubernetes 1.18.0-beta.1 See https://github.com/kubernetes/kubernetes/blob/master/CHANGELOG/CHANGELOG-1.18.md/#v1180-beta1 * fix: windows support .zip URL (cherry picked from commit 34706cf) * feat: Experimental support for Windows+ContainerD (#1322) (cherry picked from commit b068aa7) * feat: add support for single stack IPv6 (#2781) (cherry picked from commit 1b9beb4) * feat: allow iptables mode for dualstack 1.18+ (#2882) (cherry picked from commit ff5362e) * chore: update cluster-autoscaler for k8s 1.18 (#2901) See https://github.com/kubernetes/autoscaler/releases/tag/cluster-autoscaler-1.18.0 (cherry picked from commit 0cc985d) * feat: installing csi-proxy for windows at node deployment time (#2930) (cherry picked from commit 13e72f2) * feat: add support for Kubernetes 1.18.0 (#2957) See https://github.com/kubernetes/kubernetes/blob/master/CHANGELOG-1.18.md (cherry picked from commit 6dadaf0) * fix: make build with go 1.14 (#3005) (cherry picked from commit 509bc9c) * feat: add support for Kubernetes 1.18.1 (#3045) * feat: add support for Kubernetes 1.18.1 See https://github.com/kubernetes/kubernetes/blob/master/CHANGELOG/CHANGELOG-1.18.md#changelog-since-v1180 * ci: test PRs with k8s 1.18 (cherry picked from commit 66ff61c) Co-authored-by: Mark Rossetti <marosset@microsoft.com> Co-authored-by: Matt Boersma <Matt.Boersma@microsoft.com> Co-authored-by: Anish Ramasekar <anish.ramasekar@gmail.com> Co-authored-by: Sertaç Özercan <852750+sozercan@users.noreply.github.com>
(cherry picked from commit b068aa7)
This reverts commit d2024cc.
…onfig agent support for Windows (#3630) * Add staging Windows scripts * Update parts folder for Windows * feat: Consume signed powershell scripts * update the signed package version (#3584) * fix: use mcr.azk8s.cn for Azure CNI in China cloud * feat: add hosts config agent support for Windows nodes * Generate templates_generated.go * feat: Experimental support for Windows+ContainerD (#1322) * chore: update Windows default VHD for July (#3619) * fix Co-authored-by: James Sturtevant <jstur@microsoft.com>
Reason for Change:
Once completed, this will install ContainerD and configure Kubernetes to use it on Windows
Requirements:
Notes:
/hold do-not-merge