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

Build kubletwin/pause to match the node's OS version #2976

Merged
merged 1 commit into from
May 16, 2018

Conversation

PatrickLang
Copy link
Contributor

What this PR does / why we need it:
This container is built before each windows agent is added to the cluster. The OS of the container needs to match that of the windows agent. Without this change, it's hardcoded. With this change, it will match the host OS version (Windows Server 2016, Windows Server version 1709, Windows Server version 1803, or Windows Server Insider Preview)

Which issue this PR fixes

#2965
Special notes for your reviewer:
I could use some guidance testing this. Can I just build acs-engine as-is, or do I need to do some other steps to ensure this change is picked up?

If applicable:

  • documentation
  • unit tests
  • tested backward compatibility (ie. deploy with previous version, upgrade with this branch)

Will need to test on both Windows Server version 1709, and 1803 to make sure the kubletwin/pause image is build correctly and that pods can start successfully.

Release note:


JiangtianLi
JiangtianLi previously approved these changes May 15, 2018
Copy link
Contributor

@JiangtianLi JiangtianLi left a comment

Choose a reason for hiding this comment

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

/lgtm

@PatrickLang
Copy link
Contributor Author

I'm deploying a cluster with this, will report test results.

@codecov
Copy link

codecov bot commented May 16, 2018

Codecov Report

Merging #2976 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #2976   +/-   ##
=======================================
  Coverage   50.78%   50.78%           
=======================================
  Files          97       97           
  Lines       14692    14692           
=======================================
  Hits         7461     7461           
  Misses       6534     6534           
  Partials      697      697

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bfc9c7e...9152d6b. Read the comment docs.

@PatrickLang PatrickLang force-pushed the plang-pause-winver branch from be4cc96 to 9152d6b Compare May 16, 2018 01:09
@PatrickLang
Copy link
Contributor Author

Hmm, the single quote literals in my PowerShell scripts were breaking things. Switched to double quote, squashed, and rewrote history. Deployment is in progress

@PatrickLang
Copy link
Contributor Author

Alright - good news, it built kubletwin/pause correctly on Windows Server version 1803. I think this is ready to merge, but there may still be other things needed for 1803. If it doesn't work as expected, I think we should revert to using 1709 since we can manually override the offer to deploy 1803 as we fix bugs.

@PatrickLang
Copy link
Contributor Author

Deployed https://gist.githubusercontent.com/PatrickLang/2cf4b81c9518e531b828a71e4c430332/raw/f6a209a0282e0baf123084aa848965cd332985c9/whoami-1803.yaml
(although the hyperv annotation had no effect since I didn't run the kubelet with that experimental option turned on)

and it ran!

$ kubectl get pod -o wide
NAME                          READY     STATUS    RESTARTS   AGE       IP            NODE
whoami-1803-68fbdbcdc-sptg8   1/1       Running   0          4m        10.240.0.40   63924k8s9001

$ curl http://10.240.0.40:8080
I'm whoami-1803-68fbdbcdc-sptg8 running on windows/amd64

@PatrickLang
Copy link
Contributor Author

PatrickLang commented May 16, 2018

Marathon failed to install in DCOS, and I can't decypher the error in openshift. Neither of those paths can reach this code from my understanding. How should we proceed? @jackfrancis

@PatrickLang
Copy link
Contributor Author

Success on 1709

$ kubectl get node -o wide
NAME                    STATUS    ROLES     AGE       VERSION   EXTERNAL-IP   OS-IMAGE                    KERNEL-VERSION   CONTAINER-RUNTIME
36251k8s9000            Ready     <none>    19m       v1.10.1   <none>        Windows Server Datacenter   10.0.16299.371
                        docker://17.6.2
36251k8s9001            Ready     <none>    18m       v1.10.1   <none>    Windows Server Datacenter   10.0.16299.371
                        docker://17.6.2
k8s-master-36251029-0   Ready     master    22m       v1.10.1   <none>    Ubuntu 16.04.4 LTS   4.13.0-1016-azure   docker://1.13.1

$ kubectl apply -f https://gist.github.com/PatrickLang/2cf4b81c9518e531b828a71e4c430332/raw/32fd06442281c943f6bd0c3d3d3d8d2539a44bb0/whoami-1709.yaml
deployment.apps "whoami-1709" created
service "whoami-1709-svc" created

$ kubectl get pod -o wide -w
NAME                           READY     STATUS              RESTARTS   AGE       IP        NODE
whoami-1709-5cc9664744-ttlzw   0/1       ContainerCreating   0          6s        <none>    36251k8s9001
whoami-1709-5cc9664744-ttlzw   1/1       Running   0         2m        10.240.0.7   36251k8s9001

@JiangtianLi
Copy link
Contributor

@PatrickLang how did you deploy 1709 now since 1803 supercedes it?

Copy link
Member

@jackfrancis jackfrancis left a comment

Choose a reason for hiding this comment

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

lgtm

@jackfrancis jackfrancis merged commit 87408da into Azure:master May 16, 2018
@PatrickLang
Copy link
Contributor Author

@JiangtianLi - to make sure I was hitting 1709 or 1803, I set the windowsSku:

"windowsProfile": {
        "adminUsername": "azureuser",
        "adminPassword": "...",
        "windowsPublisher": "MicrosoftWindowsServer",
        "windowsOffer": "WindowsServerSemiAnnual",
        "windowsSku": "Datacenter-Core-1709-with-Containers-smalldisk"
    },

or

"windowsProfile": {
        "adminUsername": "azureuser",
        "adminPassword": "...",
        "windowsPublisher": "MicrosoftWindowsServer",
        "windowsOffer": "WindowsServerSemiAnnual",
        "windowsSku": "Datacenter-Core-1803-with-Containers-smalldisk"
    },

@JiangtianLi
Copy link
Contributor

@PatrickLang I see, windows profile has the option to choose windows sku and 1803 is the default.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants