Skip to content

ci: add windows build pool #1073

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 13 commits into from
Nov 5, 2021
Merged

ci: add windows build pool #1073

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
088fc39
build pool windows
matmerr Nov 3, 2021
0da6591
Merge branch 'master' into windowspipeline
matmerr Nov 3, 2021
343a5c5
add npm dockerfile
matmerr Nov 3, 2021
96ba472
script
matmerr Nov 3, 2021
3291fcc
tag
matmerr Nov 4, 2021
af45201
tag
matmerr Nov 4, 2021
d4ba7c7
docker build
matmerr Nov 4, 2021
71fc5f8
image pipeline
matmerr Nov 4, 2021
9eea7d0
nanoserver
matmerr Nov 4, 2021
36f2d9b
save
matmerr Nov 4, 2021
f54a0ea
servercore
matmerr Nov 4, 2021
b9d003e
logger
matmerr Nov 4, 2021
11c6a0f
ai
matmerr Nov 4, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 14 additions & 12 deletions .pipelines/pipeline.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,10 @@ trigger:
- honeycomb-dev

stages:
- stage: build_and_test
- stage: setup
displayName: ACN
jobs:
- job: setup
- job: env
displayName: Setup
pool:
name: "$(BUILD_POOL_NAME_DEFAULT)"
Expand All @@ -32,13 +32,19 @@ stages:
displayName: "Set environmental variables"
condition: always()


- template: windows-image.yaml

- stage: build_and_test
displayName: ACN
dependsOn:
- "setup"
jobs:
- job: build
displayName: Build Binaries
dependsOn:
- "setup"
variables:
TAG: $[ dependencies.setup.outputs['EnvironmentalVariables.Tag'] ]
STORAGE_ID: $[ dependencies.setup.outputs['EnvironmentalVariables.StorageID'] ]
TAG: $[ stagedependencies.setup.env.outputs['EnvironmentalVariables.Tag'] ]
STORAGE_ID: $[ stagedependencies.setup.env.outputs['EnvironmentalVariables.StorageID'] ]
pool:
name: "$(BUILD_POOL_NAME_DEFAULT)"
steps:
Expand Down Expand Up @@ -85,10 +91,8 @@ stages:

- job: build_images
displayName: Build Images
dependsOn:
- "setup"
variables:
TAG: $[ dependencies.setup.outputs['EnvironmentalVariables.Tag'] ]
TAG: $[ stagedependencies.setup.env.outputs['EnvironmentalVariables.Tag'] ]
pool:
name: "$(BUILD_POOL_NAME_DEFAULT)"
steps:
Expand Down Expand Up @@ -158,10 +162,8 @@ stages:

- job: test
displayName: Run Tests
dependsOn:
- "setup"
variables:
STORAGE_ID: $[ dependencies.setup.outputs['EnvironmentalVariables.StorageID'] ]
STORAGE_ID: $[ stagedependencies.setup.env.outputs['EnvironmentalVariables.StorageID'] ]
pool:
name: "$(BUILD_POOL_NAME_DEFAULT)"
steps:
Expand Down
10 changes: 6 additions & 4 deletions .pipelines/singletenancy/aks-engine/e2e-job-template.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,9 @@ parameters:
stages:
- stage: ${{ parameters.name }}
displayName: E2E - ${{ parameters.displayName }}
dependsOn: build_and_test
dependsOn:
- setup
- build_and_test
jobs:
- job: ${{ parameters.name }}
displayName: Singletenancy AKS Engine Suite - (${{ parameters.name }})
Expand All @@ -27,9 +29,9 @@ stages:
GOBIN: "$(GOPATH)/bin" # Go binaries path
modulePath: "$(GOPATH)/src/github.com/Azure/aks-engine"
acnPath: "$(GOPATH)/src/github.com/Azure/azure-container-networking"
Tag: $[ stagedependencies.build_and_test.setup.outputs['EnvironmentalVariables.Tag'] ]
CommitHash: $[ stagedependencies.build_and_test.setup.outputs['EnvironmentalVariables.CommitHash'] ]
StorageID: $[ stagedependencies.build_and_test.setup.outputs['EnvironmentalVariables.StorageID'] ]
Tag: $[ stagedependencies.setup.env.outputs['EnvironmentalVariables.Tag'] ]
CommitHash: $[ stagedependencies.setup.env.outputs['EnvironmentalVariables.CommitHash'] ]
StorageID: $[ stagedependencies.setup.env.outputs['EnvironmentalVariables.StorageID'] ]
steps:
- template: e2e-step-template.yaml
parameters:
Expand Down
6 changes: 4 additions & 2 deletions .pipelines/singletenancy/aks-swift/e2e-job-template.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@ parameters:
stages:
- stage: ${{ parameters.name }}
displayName: E2E - ${{ parameters.displayName }}
dependsOn: build_and_test
dependsOn:
- setup
- build_and_test
jobs:
- job: ${{ parameters.name }}
displayName: Singletenancy AKS Swift Suite - (${{ parameters.name }})
Expand All @@ -19,7 +21,7 @@ stages:
variables:
GOPATH: "$(Agent.TempDirectory)/go" # Go workspace path
GOBIN: "$(GOPATH)/bin" # Go binaries path
Tag: $[ stagedependencies.build_and_test.setup.outputs['EnvironmentalVariables.Tag'] ]
Tag: $[ stagedependencies.setup.env.outputs['EnvironmentalVariables.Tag'] ]
modulePath: "$(GOPATH)/src/github.com/Azure/azure-container-networking"
steps:
- template: e2e-step-template.yaml
Expand Down
55 changes: 55 additions & 0 deletions .pipelines/windows-image.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
stages:
- stage: build_windows_images
displayName: Build Windows Images
dependsOn:
- "setup"
jobs:
- job: NPM
displayName: Build NPM
variables:
tag: $[ stagedependencies.setup.env.outputs['EnvironmentalVariables.Tag'] ]
pool:
name: "$(BUILD_POOL_NAME_DEFAULT_WINDOWS)"
steps:
- powershell: |
powershell.exe -command "& { . .\windows.ps1; azure-npm-image $(tag)-windows-amd64 }"
Copy link
Contributor

@JungukCho JungukCho Nov 5, 2021

Choose a reason for hiding this comment

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

Minor: naming to clarify what they are?
windows.ps1 -> npm-image-builder.ps1
azure-npm-image -> build-azure-npm-image

name: "build_npm"
displayName: "Build"
- task: Docker@2
displayName: Docker Login
inputs:
containerRegistry: $(ACR_SERVICE_CONNECTION)
command: 'login'
addPipelineData: false

- powershell: |
docker tag acnpublic.azurecr.io/azure-npm:$(tag)-windows-amd64 acnpublic.azurecr.io/azure-npm:$(tag)-windows-amd64-test
Copy link
Contributor

@JungukCho JungukCho Nov 5, 2021

Choose a reason for hiding this comment

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

Is it possible to declare acnpublic.azurecr.io/azure-npm:$(tag)-windows-amd64 and acnpublic.azurecr.io/azure-npm:$(tag)-windows-amd64-test in Variables fields?
If so, better use variables to avoid multiple changes.

Copy link
Member Author

@matmerr matmerr Nov 5, 2021

Choose a reason for hiding this comment

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

I agree, but I'd like to leave this as is for the moment, and take that in #1079 since that's the theme for that pr, and this will be refactored again there

docker push acnpublic.azurecr.io/azure-npm:$(tag)-windows-amd64-test
name: "push_npm"
displayName: "Push"
- powershell: |
mkdir .\output\images\windows\
Copy link
Contributor

@JungukCho JungukCho Nov 5, 2021

Choose a reason for hiding this comment

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

I am wondering why we store this image as file?
I think in linux, we also store the image as file.

Copy link
Contributor

@vakalapa vakalapa Nov 5, 2021

Choose a reason for hiding this comment

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

I think to release it, when we cut a release on this particular pipeline build these files are what get artifact'ed into binaries you see in github release.

docker save acnpublic.azurecr.io/azure-npm:$(tag)-windows-amd64-test -o .\output\images\windows\azure-npm-$(tag)-windows-amd64.tar
name: "save_npm"
displayName: "Save"
- task: Docker@2
displayName: Docker Logout
inputs:
containerRegistry: $(ACR_SERVICE_CONNECTION)
command: 'logout'
addPipelineData: false

- task: CopyFiles@2
inputs:
sourceFolder: "output"
targetFolder: $(Build.ArtifactStagingDirectory)
condition: succeeded()

- task: PublishBuildArtifacts@1
inputs:
artifactName: "output"
pathtoPublish: "$(Build.ArtifactStagingDirectory)"
condition: succeeded()
2 changes: 1 addition & 1 deletion Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ ifeq ($(GOOS),linux)
all-binaries: azure-cnm-plugin azure-cni-plugin azure-cns azure-npm
all-images: azure-npm-image azure-cns-image
else
all-binaries: azure-cnm-plugin azure-cni-plugin azure-cns
all-binaries: azure-cnm-plugin azure-cni-plugin azure-cns azure-npm
Copy link
Contributor

@JungukCho JungukCho Nov 5, 2021

Choose a reason for hiding this comment

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

Seems possible to extract it above ifeq ($(GOOS),linux) since now linux and windows build same one.

all-images:
@echo "Nothing to build. Skip."
endif
Expand Down
3 changes: 3 additions & 0 deletions log/logger_windows.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@ func (logger *Logger) SetTarget(target int) error {
var err error

switch target {
case TargetStdout:
logger.out = os.Stdout

case TargetStderr:
logger.out = os.Stderr

Expand Down
20 changes: 20 additions & 0 deletions npm/Dockerfile.windows
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
FROM golang:windowsservercore-ltsc2022 AS builder
Copy link
Contributor

@JungukCho JungukCho Nov 5, 2021

Choose a reason for hiding this comment

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

Using golang:windowsservercore-ltsc2022 instead of golang:1.17 is for copying netapi32.dll?
Curious what netapi32.dll does? I guess it is related to HNS.

Copy link
Member Author

@matmerr matmerr Nov 5, 2021

Choose a reason for hiding this comment

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

it's due to this issue: docker-library/golang#348

klog has a dependency on os/user, so we need to take that dll from servercore to be able to use nanoserver

# Build args
ARG VERSION
ARG NPM_AI_PATH
ARG NPM_AI_ID

WORKDIR /usr/src/npm
RUN mkdir /usr/bin/
# Copy the source
COPY . .

RUN $Env:CGO_ENABLED=0; go build -v -o /usr/bin/npm.exe -ldflags """-X main.version=${env:VERSION} -X ${env:NPM_AI_PATH}=${env:NPM_AI_ID}""" -gcflags="-dwarflocationlists=true" ./npm/cmd/

# Copy into final image
FROM mcr.microsoft.com/windows/nanoserver:ltsc2022
COPY --from=builder /Windows/System32/netapi32.dll /Windows/System32/netapi32.dll
COPY --from=builder /usr/bin/npm.exe \
/usr/bin/npm.exe

ENTRYPOINT ["/usr/bin/npm.exe", "start"]
15 changes: 15 additions & 0 deletions windows.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@

function azure-npm-image {
$env:ACN_PACKAGE_PATH = "github.com/Azure/azure-container-networking"
$env:NPM_AI_ID = "014c22bd-4107-459e-8475-67909e96edcb"
$env:NPM_AI_PATH="$env:ACN_PACKAGE_PATH/npm.aiMetadata"

if ($null -eq $env:VERSION) { $env:VERSION = $args[0] }
Copy link
Contributor

@JungukCho JungukCho Nov 5, 2021

Choose a reason for hiding this comment

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

Is $env:VERSION always null? If not, it seems like that something is wrong.
I am not sure, is it better to return error when it is not null?

Is it slightly better to get above three as argument as well for maintenance?

Copy link
Member Author

@matmerr matmerr Nov 5, 2021

Choose a reason for hiding this comment

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

version is tuneable via argument so the script can be used when building locally, and tbh arguments are much easier to work with in powershell than env's imo

Copy link
Member Author

@matmerr matmerr Nov 5, 2021

Choose a reason for hiding this comment

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

the other three are very static and should not be changed

docker build `
-f npm/Dockerfile.windows `
-t acnpublic.azurecr.io/azure-npm:$env:VERSION `
--build-arg VERSION=$env:VERSION `
--build-arg NPM_AI_PATH=$env:NPM_AI_PATH `
--build-arg NPM_AI_ID=$env:NPM_AI_ID `
.
}