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

feat: enable WindowsProfile in defaults enforcement code flow #1103

Merged
merged 1 commit into from Apr 22, 2019

Conversation

jackfrancis
Copy link
Member

@jackfrancis jackfrancis commented Apr 19, 2019

Reason for Change:

Adds WindowsProfile into the standard defaults enforcement code flow. We retain the existing ARM parameters defaults to ensure back-compat defaults enforcement for pre-existing clusters.

This PR delivers the following opinionated defaults, which are subject to review:

// DefaultWindowsPublisher sets the default WindowsPublisher value in WindowsProfile
DefaultWindowsPublisher = "MicrosoftWindowsServer"
// DefaultWindowsOffer sets the default WindowsOffer value in WindowsProfile
DefaultWindowsOffer = "WindowsServerSemiAnnual"
// DefaultWindowsSku sets the default WindowsSku value in WindowsProfile
DefaultWindowsSku = "Datacenter-Core-1809-with-Containers-smalldisk"
// DefaultImageVersion sets the default ImageVersion value in WindowsProfile
DefaultImageVersion = "1809.0.20190314"

Previous defaults were:

WindowsPublisher = "MicrosoftWindowsServer" (unchanged in this PR)
WindowsOffer = "WindowsServerSemiAnnual" (unchanged in this PR)
WindowsSku = "Datacenter-Core-1809-with-Containers-smalldisk" (unchanged in this PR)
WindowsVersion = "latest" (this PR changes this value to an opinonated, dated version)

Issue Fixed:

Requirements:

Notes:

@acs-bot acs-bot added the size/L label Apr 19, 2019
// setWindowsProfileDefaults sets default WindowsProfile values
func (p *Properties) setWindowsProfileDefaults(isUpgrade, isScale bool) {
windowsProfile := p.WindowsProfile
if !isUpgrade && !isScale {
Copy link
Member Author

Choose a reason for hiding this comment

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

We will not apply any defaults enforcement for pre-existing clusters.

@xizha162
Copy link
Contributor

/LGTM

@acs-bot
Copy link

acs-bot commented Apr 19, 2019

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jackfrancis, xizhamsft

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

@codecov
Copy link

codecov bot commented Apr 19, 2019

Codecov Report

Merging #1103 into master will increase coverage by 0.04%.
The diff coverage is 84.61%.

@@            Coverage Diff             @@
##           master    #1103      +/-   ##
==========================================
+ Coverage   74.35%   74.39%   +0.04%     
==========================================
  Files         131      131              
  Lines       18274    18287      +13     
==========================================
+ Hits        13588    13605      +17     
+ Misses       3905     3903       -2     
+ Partials      781      779       -2

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 pending Windows E2E tests

@jackfrancis
Copy link
Member Author

@PatrickLang and @craiglpeters should sign off on this change, as it includes a change to the default Windows image version string we use to build Windows node vms.

@jackfrancis
Copy link
Member Author

Welp. So much for this improving Windows 1.14 IIS flakiness.

@PatrickLang is this the image we expect from the 1809.0.20190314 reference?:

  • Windows Server Datacenter 10.0.17763.379

@jackfrancis jackfrancis merged commit 3798db0 into Azure:master Apr 22, 2019
@jackfrancis jackfrancis deleted the windows-profile-defaults branch April 22, 2019 19:15
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