Brcampbe/windows dcos #1132

yakman2020 · 2017-07-28T15:31:02Z

What this PR does / why we need it: Support for windows agents for DCOS

Which issue this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close that issue when PR gets merged): fixes #1078

Special notes for your reviewer: Most changes in the form of new files.

Release note:

This change is

msftclas · 2017-07-28T15:31:07Z

@yakman2020,
Thanks for your contribution as a Microsoft full-time employee or intern. You do not need to sign a CLA.
Thanks,
Microsoft Pull Request Bot

seanknox · 2017-07-28T15:35:57Z

@acs-bot test this please

seanknox

Thanks @yakman2020! This generally looks good. Have a few change requests. My main question is whether this functionality should be in vlabs (not yet the supported by the ACS managed service).

seanknox · 2017-07-28T15:37:16Z

Makefile

@@ -57,6 +57,8 @@ checksum:
 .PHONY: clean
 clean:
 	@rm -rf $(BINDIR) ./_dist
+	go build -v -gcflags="-N -l" -ldflags="-X github.com/Azure/acs-engine/cmd.BuildSHA=${VERSION} -X github.com/Azure/acs-engine/cmd.BuildTime=${BUILD}"
+	cd test/acs-engine-test; go build -v


make clean should only do that—this logic is already handled in make build.

seanknox · 2017-07-28T15:38:19Z

parts/dcosWindowsAgentResourcesVmas.t.orig

@@ -0,0 +1,240 @@
+    {
+      "apiVersion": "[variables('apiVersionDefault')]",


This file is an artifact from a merge conflict, you can delete it.

seanknox · 2017-07-28T15:38:40Z

parts/dcosWindowsAgentResourcesVmss.t.orig

@@ -0,0 +1,204 @@
+    {
+      "apiVersion": "[variables('apiVersionDefault')]",


You can delete this file too.

seanknox · 2017-07-28T15:40:10Z

parts/dcoscustomdata190.t

@@ -1,6 +1,5 @@
 bootcmd:
- bash -c "if [ ! -f /var/lib/sdb-gpt ];then echo DCOS-5890;parted -s /dev/sdb mklabel
-  gpt;touch /var/lib/sdb-gpt;fi"
+- bash -c "if [ ! -f /var/lib/sdb-gpt ];then echo DCOS-5890;parted -s /dev/sdb mklabel gpt;touch /var/lib/sdb-gpt;fi"


This is a little harder to grok than the two line version.

done as requested.

seanknox · 2017-07-28T15:41:27Z

pkg/acsengine/templates.go

@@ -0,0 +1,1707 @@
+// Code generated by go-bindata.


You might be working off of an old branch as pkg/acsengine/templates.go is not checked into source control. Please delete it.

seanknox · 2017-07-28T15:42:25Z

pkg/api/apiloader.go

@@ -9,6 +9,7 @@ import (
 	"github.com/Azure/acs-engine/pkg/api/v20160930"
 	"github.com/Azure/acs-engine/pkg/api/v20170131"
 	"github.com/Azure/acs-engine/pkg/api/v20170701"
+	"github.com/Azure/acs-engine/pkg/api/v20170801"


@anhowe Should DCOS Windows support be in vlabs only for now?

acs-bot · 2017-07-28T16:15:55Z

Can one of the admins verify this patch? Say "@acs-bot test this please" to start tests.

yakman2020 · 2017-07-28T16:59:04Z

@acs-bot test this please

seanknox · 2017-07-28T17:20:52Z

@acs-bot test this please

seanknox · 2017-08-01T03:26:41Z

@acs-bot test this please

msftclas · 2017-08-01T03:30:29Z

@yakman2020,
Thanks for your contribution as a Microsoft full-time employee or intern. You do not need to sign a CLA.
Thanks,
Microsoft Pull Request Bot

seanknox · 2017-08-01T03:30:57Z

@acs-bot test this please

seanknox · 2017-08-01T03:33:45Z

@dmitsh believe you're on the docket to help @yakman2020 on this one...CI doesn't seem to be running for this branch, or at least the bot isn't kicking off jobs.

dmitsh · 2017-08-01T15:11:37Z

examples/windows/dcos-hybrid.json.env

@@ -0,0 +1 @@
+export EXPECTED_NODE_COUNT=2


Why do you need this?
By default we calculate the node count by parsing api model file.

removed the file.

dmitsh · 2017-08-01T15:13:18Z

pkg/acsengine/engine.go

 			attributeContents := getDCOSAgentCustomNodeLabels(profile)
+log.Info("GetDCOSAgentCustomData2\n");


not very descriptive log message

removed the log message

seanknox

Thanks for addressing. Please rebase off of master—we just merged in CircleCI support. Rebasing will kick off a job in CI (which you'll be able to see).

msftclas · 2017-08-02T00:45:50Z

@yakman2020,
Thanks for your contribution as a Microsoft full-time employee or intern. You do not need to sign a CLA.
Thanks,
Microsoft Pull Request Bot

Addressed

jackfrancis · 2017-08-16T21:13:00Z

parts/dcosmastervars.t

@@ -36,8 +46,15 @@
    "masterNSGName": "[concat(variables('orchestratorName'), '-master-nsg-', variables('nameSuffix'))]",
    "masterPublicIPAddressName": "[concat(variables('orchestratorName'), '-master-ip-', variables('masterEndpointDNSNamePrefix'), '-', variables('nameSuffix'))]",
    "apiVersionStorage": "2015-06-15",
-    "storageAccountBaseName": "[uniqueString(concat(variables('masterEndpointDNSNamePrefix'),variables('location'),variables('orchestratorName')))]",
-    "masterStorageAccountExhibitorName": "[concat(variables('storageAccountBaseName'), 'exhb0')]",
+{{if .HasWindows}}


These additional windows-specific vars should go inside the existing {{if .HasWindows}} block above. (see line 26)

jackfrancis · 2017-08-16T21:16:35Z

parts/dcosagentvars.t

+        "{{.Name}}WindowsRDPEndRangeStop": "[add(variables('{{.Name}}WindowsRDPNatRangeStart'), add(variables('{{.Name}}Count'),variables('{{.Name}}Count')))]",
+    {{end}}
+    "{{.Name}}windowsAgentCustomScriptArguments": "[concat('$arguments = ', variables('singleQuote'),'-subnet ', variables('{{.Name}}Subnet'), ' -MasterCount ', variables('masterCount'), ' -firstMasterIP ', parameters('firstConsecutiveStaticIP'), ' -bootstrapUri ', '\"', variables('dcosWindowsBootstrapURL'), '\"', ' -isAgent $true -isPublic:$true',  variables('singleQuote'),' ; ')]",


Shouldn't this be a windows-only change? (i.e., inside the {{if .IsWindows}} branch)?

jackfrancis · 2017-08-16T21:16:44Z

parts/dcosagentvars.t

+    "{{.Name}}windowsAgentCustomScriptArguments": "[concat('$arguments = ', variables('singleQuote'),'-subnet ', variables('{{.Name}}Subnet'), ' -MasterCount ', variables('masterCount'), ' -firstMasterIP ', parameters('firstConsecutiveStaticIP'), ' -bootstrapUri ', '\"', variables('dcosWindowsBootstrapURL'), '\"', ' -isAgent $true -isPublic:$true',  variables('singleQuote'),' ; ')]",
+{{else}}
+    "{{.Name}}windowsAgentCustomScriptArguments": "[concat('$arguments = ', variables('singleQuote'),'-subnet ', variables('{{.Name}}Subnet'), ' -MasterCount ', variables('masterCount'), ' -firstMasterIP ', parameters('firstConsecutiveStaticIP'), ' -bootstrapUri ', '\"', variables('dcosWindowsBootstrapURL'), '\"', ' -isAgent $true',  variables('singleQuote'),' ; ')]",


jackfrancis · 2017-08-16T21:16:54Z

parts/dcosagentvars.t

+    "{{.Name}}windowsAgentCustomScriptArguments": "[concat('$arguments = ', variables('singleQuote'),'-subnet ', variables('{{.Name}}Subnet'), ' -MasterCount ', variables('masterCount'), ' -firstMasterIP ', parameters('firstConsecutiveStaticIP'), ' -bootstrapUri ', '\"', variables('dcosWindowsBootstrapURL'), '\"', ' -isAgent $true',  variables('singleQuote'),' ; ')]",
+{{end}}
+    "{{.Name}}windowsAgentCustomScript": "[concat('powershell.exe -ExecutionPolicy Unrestricted -command \"', variables('{{.Name}}windowsAgentCustomScriptArguments'), variables('windowsCustomScriptSuffix'), '\" > %SYSTEMDRIVE%\\AzureData\\dcosWindowsProvision.log 2>&1')]",


yakman2020 · 2017-08-16T23:19:54Z

Jack's requested changes added

jackfrancis · 2017-08-17T18:55:40Z

Thanks for hanging with us, LGTM!

msftclas added the cla-not-required label Jul 28, 2017

seanknox suggested changes Jul 28, 2017

View reviewed changes

yakman2020 force-pushed the brcampbe/windows-dcos branch from bc4f900 to 5db9e86 Compare July 28, 2017 16:57

yakman2020 force-pushed the brcampbe/windows-dcos branch 3 times, most recently from d922c64 to 48f1183 Compare July 28, 2017 17:12

yakman2020 force-pushed the brcampbe/windows-dcos branch from 48f1183 to 0f1c833 Compare July 28, 2017 17:28

seanknox closed this Aug 1, 2017

seanknox reopened this Aug 1, 2017

ghost assigned seanknox Aug 1, 2017

ghost added the in progress label Aug 1, 2017

msftclas added the cla-not-required label Aug 1, 2017

seanknox requested a review from dmitsh August 1, 2017 03:32

seanknox removed their assignment Aug 1, 2017

dmitsh previously requested changes Aug 1, 2017

View reviewed changes

dmitsh requested a review from JiangtianLi August 1, 2017 15:14

seanknox previously requested changes Aug 2, 2017

View reviewed changes

seanknox closed this Aug 2, 2017

seanknox reopened this Aug 2, 2017

ghost removed the in progress label Aug 2, 2017

ghost assigned seanknox Aug 2, 2017

ghost added the in progress label Aug 2, 2017

msftclas added the cla-not-required label Aug 2, 2017

yakman2020 force-pushed the brcampbe/windows-dcos branch 10 times, most recently from 57506b1 to c9776eb Compare August 16, 2017 17:00

jackfrancis suggested changes Aug 16, 2017

View reviewed changes

DCOS with Windows Agents

909916e

yakman2020 force-pushed the brcampbe/windows-dcos branch from c9776eb to 909916e Compare August 16, 2017 23:18

jackfrancis approved these changes Aug 17, 2017

View reviewed changes

jackfrancis merged commit 61a2db0 into Azure:master Aug 17, 2017

ghost removed the in progress label Aug 17, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Brcampbe/windows dcos #1132

Brcampbe/windows dcos #1132

yakman2020 commented Jul 28, 2017 •

edited by acs-bot

Loading

msftclas commented Jul 28, 2017

seanknox commented Jul 28, 2017

seanknox left a comment

seanknox Jul 28, 2017

seanknox Jul 28, 2017

seanknox Jul 28, 2017

seanknox Jul 28, 2017

yakman2020 Jul 28, 2017

seanknox Jul 28, 2017

seanknox Jul 28, 2017

acs-bot commented Jul 28, 2017

yakman2020 commented Jul 28, 2017

seanknox commented Jul 28, 2017

seanknox commented Aug 1, 2017

msftclas commented Aug 1, 2017

seanknox commented Aug 1, 2017

seanknox commented Aug 1, 2017

dmitsh Aug 1, 2017

yakman2020 Aug 14, 2017

dmitsh Aug 1, 2017

yakman2020 Aug 14, 2017

seanknox left a comment

msftclas commented Aug 2, 2017

jackfrancis Aug 16, 2017

jackfrancis Aug 16, 2017

jackfrancis Aug 16, 2017

jackfrancis Aug 16, 2017

yakman2020 commented Aug 16, 2017

jackfrancis commented Aug 17, 2017

		@@ -0,0 +1,240 @@
		{
		"apiVersion": "[variables('apiVersionDefault')]",

		@@ -0,0 +1,204 @@
		{
		"apiVersion": "[variables('apiVersionDefault')]",

		attributeContents := getDCOSAgentCustomNodeLabels(profile)
		log.Info("GetDCOSAgentCustomData2\n");

Brcampbe/windows dcos #1132

Brcampbe/windows dcos #1132

Conversation

yakman2020 commented Jul 28, 2017 • edited by acs-bot Loading

msftclas commented Jul 28, 2017

seanknox commented Jul 28, 2017

seanknox left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

acs-bot commented Jul 28, 2017

yakman2020 commented Jul 28, 2017

seanknox commented Jul 28, 2017

seanknox commented Aug 1, 2017

msftclas commented Aug 1, 2017

seanknox commented Aug 1, 2017

seanknox commented Aug 1, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

seanknox left a comment

Choose a reason for hiding this comment

msftclas commented Aug 2, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yakman2020 commented Aug 16, 2017

jackfrancis commented Aug 17, 2017

yakman2020 commented Jul 28, 2017 •

edited by acs-bot

Loading