-
Notifications
You must be signed in to change notification settings - Fork 560
Docs - simplifying Windows walkthrough #3614
Docs - simplifying Windows walkthrough #3614
Conversation
@jsturtevant @jldeen @cwilhit @rpsqrd @sauryadas - any feedback on this doc update? |
/hold I'll squash this before final merge |
Codecov Report
@@ Coverage Diff @@
## master #3614 +/- ##
==========================================
- Coverage 55.77% 55.72% -0.05%
==========================================
Files 107 107
Lines 16238 16238
==========================================
- Hits 9057 9049 -8
- Misses 6408 6417 +9
+ Partials 773 772 -1 |
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.
Looks much simpler. Left a few comments inline
docs/kubernetes/windows.md
Outdated
|
||
Note, with the rollout of new Windows Server versions in acs-engine, the workload deployed on Windows cluster should match as documented here: https://docs.microsoft.com/en-us/virtualization/windowscontainers/deploy-containers/version-compatibility . Both the containers being deployed, | ||
as well as the `kubletwin/pause` container must match the version of the Windows host. Otherwise, [pods may get stuck at ContainerCreating](https://docs.microsoft.com/en-us/virtualization/windowscontainers/kubernetes/common-problems#my-kubernetes-pods-are-stuck-at-containercreating) state. | ||
- Getting the right tools |
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 find adding links to each section is help for folks to jump to a section they want to read about.
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.
Found a VS Code extension for that :)
docs/kubernetes/windows.md
Outdated
|
||
2. Find the latest version, and download the file ending in `-windows-amd64.zip`. | ||
|
||
3. Make a folder, and copy `acs-engine.exe` from the ZIP there |
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.
reword to something like "Extract the download to a folder in a working directory such as `c:\tools
acs-engine"
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.
Thanks! done
docs/kubernetes/windows.md
Outdated
|
||
3. Make a folder, and copy `acs-engine.exe` from the ZIP there | ||
|
||
4. Check that it runs with `.\acs-engine.exe version` |
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.
Add an additional/optional step to add to PATH
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.
Thanks! done
docs/kubernetes/windows.md
Outdated
|
||
Offer Publisher Sku Urn Version | ||
----------------------- ----------------------------- ---------------------------------------------- ------------------------------------------------------------------------------------------------------------- ----------------- | ||
If it does not exist, then run `ssh-keygen.exe`. Use the default file, and enter a passphrase if you wish to protect it. Be sure not to use a SSH key with blank passphrase in production. |
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 think they might need putty installed to run this command? Should point that out.
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 have this on a clean machine:
Directory: C:\Windows\system32\OpenSSH
Mode LastWriteTime Length Name
---- ------------- ------ ----
-a---- 3/10/2018 10:20 AM 343552 scp.exe
-a---- 3/10/2018 10:20 AM 408064 sftp.exe
-a---- 3/10/2018 10:20 AM 531968 ssh-add.exe
-a---- 3/10/2018 10:20 AM 495616 ssh-agent.exe
-a---- 3/10/2018 10:20 AM 657920 ssh-keygen.exe
-a---- 3/10/2018 10:20 AM 594944 ssh-keyscan.exe
-a---- 3/10/2018 10:20 AM 894464 ssh.exe
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.
must be included with the latest windows 10?
docs/kubernetes/windows.md
Outdated
|
||
2. a set of windows and linux nodes. The windows nodes can be accessed through an RDP SSH tunnel via the master node. To do this, follow these [instructions](../ssh.md#ssh-to-the-machine), replacing port 80 with 3389. Since your windows machine is already using port 3389, it is recommended to use 3390 to Windows Node 0, 10.240.0.4, 3391 to Windows Node 1, 10.240.0.5, and so on as shown in the following image: | ||
### Get an acs-engine apimodel |
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.
Would just call this "create an acs-engine apimodel"
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.
thanks
docs/kubernetes/windows.md
Outdated
#### 4. Enter the pod container to validate | ||
#### Get Kubectl for Windows | ||
|
||
The latest release of Kubernetes Control (kubectl) is available on the [Kubernetes release page](https://kubernetes.io/docs/imported/release/notes/). Look for `kubernetes-client-windows-amd64.tar.gz` and download it. |
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 would move this the first section "getting the right tools"
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.
:) done
docs/kubernetes/windows.md
Outdated
|
||
The latest release of Kubernetes Control (kubectl) is available on the [Kubernetes release page](https://kubernetes.io/docs/imported/release/notes/). Look for `kubernetes-client-windows-amd64.tar.gz` and download it. | ||
|
||
Windows 10 version 1803 already includes `tar`, so extract the archive and move `kubectl.exe` to your local path |
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.
what if they are not on 1803?
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.
Upgrade :) I also added a link to busybox which will work
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.
busybox like the linux image?
docs/kubernetes/windows.md
Outdated
Workaround: | ||
1. Use the FQDN in DNS lookups such as `kubernetes.kube-system.svc.cluster.local` | ||
2. Instead of DNS, use environment variables `* _SERVICE_HOST` and `*_SERVICE_PORT` to find service IPs and ports in the same namespace | ||
Once the pod is in `Running` state, get the IP from `kubectl get service` then visit `http://<external-ip>` to test your web server. |
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.
explicitly call this external IP from the output above
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.
ok clarified it
examples/windows/kubernetes.json
Outdated
"osDiskSizeGB": 127, | ||
"extensions": [ | ||
{ | ||
"name": "winrm" |
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.
create a link the getting started on how to use winrm extension and why it is added.
docs/kubernetes/windows-details.md
Outdated
|
||
### Changing the OS disk size | ||
|
||
The Windows Server deployments default to 30GB for the OS drive (C:), which may not be enough. You can change this size by adding `osDiskSizeGB` under the `agentPoolProfiles`, such as: |
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.
Add note on why this won't be enough. We have run into this in every engagement we have done.
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.
Should we consider setting larger VM as a default?
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.
Added more about disk sizes and updated agentPoolProfiles. I don't think I want to change the default VM size yet - with process isolation they will probably run out of pod IP addresses before containers fail to start.
Hi,
Apologies - i was OOO due to moving today. I glanced over it tonight, but will send in more thought-out comments tomorrow!
… On Aug 3, 2018, at 18:03, Patrick Lang ***@***.*** ***@***.***>> wrote:
@jsturtevant <https://github.com/jsturtevant> @jldeen <https://github.com/jldeen> @cwilhit <https://github.com/cwilhit> @rpsqrd <https://github.com/rpsqrd> - any feedback on this doc update?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub <#3614 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AHUhNtwKVap8QXnIUN29PJYhBYYXSwsTks5uNPLbgaJpZM4Vuxmn>.
|
docs/kubernetes/windows.md
Outdated
Windows 10 version 1803 already includes `tar`, so extract the archive and move `kubectl.exe` to your local path | ||
|
||
```powershell | ||
tar xvzf C:\Users\patrick\Downloads\kubernetes-client-windows-amd64.tar.tar |
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.
.tar.tar?
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.
That's not a typo. That's reflecting what Edge actually named the file. I'll go ahead and change it to .tar.gz to be less confusing and hope people can figure it out :)
docs/kubernetes/windows.md
Outdated
|
||
```powershell | ||
dir ~/.ssh/id_rsa.pub |
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.
Should this have backslashes?
With forward slashes, I get this if the file doesn't exist:
$ dir ~/.ssh/id_rsa.pub
Invalid switch - ".ssh".
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.
weird, both work for me. I'll change it though.
docs/kubernetes/windows.md
Outdated
|
||
In the image above, you can see the following parts: | ||
1. Set windowsProfile.adminUsername and adminPassword |
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.
Suggest: Link to https://docs.microsoft.com/en-us/azure/virtual-machines/windows/faq?toc=%2fazure%2fvirtual-machines%2fwindows%2ftoc.json#what-are-the-username-requirements-when-creating-a-vm so they don't fall into this trap. Same with password.
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.
Great! I didn't know that was documented
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 should fix this in acs-engine #3636
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.
just a few minor things to consider - those performing a Win k8s deployment may not be using a Windows platform to generate those templates. Include commands for both/all 3 - linux, windows, macOS
"availabilityProfile": "AvailabilitySet", | ||
"osType": "Windows", | ||
"osDiskSizeGB": 128 | ||
} |
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.
Perhaps we should update the ARM template with the appropriate osDiskSizeGB specification, rather than requiring the end user to do it.
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.
Yeah - the template from the quick start guide is updated too
"windowsOffer": "WindowsServerSemiAnnual", | ||
"windowsSku": "Datacenter-Core-1803-with-Containers-smalldisk" | ||
}, | ||
``` |
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.
Should we also have this set in the default kubernetes.json here: https://github.com/Azure/acs-engine/blob/master/examples/windows/kubernetes.json? That way we have a template pre-configured and the end user can change if he/she/they wishes.
0 File(s) 0 bytes | ||
2 Dir(s) 5,368,709,120 bytes free | ||
|
||
``` |
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.
This might sound silly, but should we include examples for both Windows and Linux commands/file paths? I can definitely see scenarios where those on macOS / linux still might want to deploy a Windows K8s cluster.
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.
done
docs/kubernetes/windows-details.md
Outdated
|
||
## Troubleshooting | ||
|
||
Windows support is still in active development with many changes each week. Read on for more info on known per-version issues and troubleshooting if you run into problems. |
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 recommend putting "active development" in bold to ensure those reading these docs are fully aware.
docs/kubernetes/windows.md
Outdated
|
||
#### Changing the OS disk size | ||
Click the [download](https://aka.ms/installazurecliwindows) link, and choose "Run". Click through the setup steps as needed. |
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.
Add links for linux and macOS az cli installs, too. Just in case they are a sysadmin tasked with managing both *nix and Windows applications and use an alternative platform for the Win K8s template generation / creation.
docs/kubernetes/windows.md
Outdated
- mode | ||
- dns.Nameservers | ||
- dns.Search | ||
```powershell |
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.
platform agnostic
docs/kubernetes/windows.md
Outdated
|
||
Affects: Azure CNI plugin <= 0.3.0 | ||
Now, you can check the status of the pod and service with `.\kubectl.exe get pod` and `.\kubectl.exe get service` respectively. |
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.
or just kubectl get pod
AND kubectl get service
to make more platform agnostic
docs/kubernetes/windows.md
Outdated
Workaround: | ||
1. Use the FQDN in DNS lookups such as `kubernetes.kube-system.svc.cluster.local` | ||
2. Instead of DNS, use environment variables `* _SERVICE_HOST` and `*_SERVICE_PORT` to find service IPs and ports in the same namespace | ||
```powershell |
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.
again make this and the below just platform independent
docs/kubernetes/windows.md
Outdated
|
||
PS C:\tools> .\kubectl.exe get service |
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.
same, platform independent
docs/kubernetes/windows.md
Outdated
|
||
Affects: All acs-engine deployed clusters | ||
The service will eventually show an EXTERNAL-IP as well: | ||
```powershell |
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.
same as platform agnostic comments above
Adding group, SP creation Change default to v1.10 Adding winrm extension and VM resize to default sample Adding more steps Fixing output Fixing keyData Finishing quickstart steps, still working on the rest More doc cleanup Windows kubectl steps Finishing first deployment section Removing old steps Splitting out quick start from long details Fixing anchor links Clarify disk size recommendation, and make the 2 most common examples consistent with docs Add TOC Clarify extract and add optional step of adding to PATH s/\//\\/ Change get an apimodel to create an apimodel Adding username/password requirements More script goodies Be more polite about JSON Use existing kubeconfig/..json Adding tar alternative Clarify service & external IP Adding active development caveat
69bed75
to
46645f8
Compare
Ok, just squashed my first version and all feedback into 1 commit. I'm adding a few more re: multiplatform now |
docs/kubernetes/windows-details.md
Outdated
|
||
It will run 2 containers: | ||
|
||
- iis-container: This is a basic static web serber |
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.
Typo: "serber" -> "server"
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.
needle in the haystack. fixed!
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.
Great stuff! I just had minor grammar touchups to point out.
docs/kubernetes/windows-details.md
Outdated
@@ -0,0 +1,335 @@ | |||
# More on Windows and Kubernetes | |||
|
|||
If you're trying to deploy Kubernetes with Windows the first time, be sure to check out the [quick start](windows.md) first. If you're looking for more details on deployments, examples or troubleshooting read on. |
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 would end this sentence using an em-dash:
...examples or troubleshooting--read on.
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.
Thanks! fixed in new changelist
docs/kubernetes/windows-details.md
Outdated
|
||
## Customizing Windows deployments | ||
|
||
ACS-Engine allows a lot more customizations available in the [docs](../), but here are a few important ones you should know for Windows deployments. Each of these are extra parameters you can add into the ACS-Engine apimodel file (such as kubernetes-windows.json from the quick start) before running `acs-engine generate` |
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.
Maybe put backticks around the filename kubernetes-windows.json
? Also the final sentence needs a period.
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.
Thanks! fixed in new changelist
docs/kubernetes/windows-details.md
Outdated
|
||
### Changing the OS disk size | ||
|
||
The Windows Server deployments default to 30GB for the OS drive (C:), which is not enough to pull multiple `microsoft/windowsservercore` based containers. It's easiest to start with 128GB, then see what your usage is over time before shrinking it down. You can change this size by adding `osDiskSizeGB` under the `agentPoolProfiles`, such as: |
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.
This phrase in the first sentence should be punctuated with a dash:
microsoft/windowsservercore
-based containers.
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.
Thanks! fixed in new changelist
docs/kubernetes/windows-details.md
Outdated
|
||
### Choosing the Windows Server version | ||
|
||
If you want to deploy a specific Windows Server version, you can. First, find available versions with `az vm image list --publisher MicrosoftWindowsServer --all -o table` |
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.
Rather than repeat the command twice, maybe this sentence could end this way:
First, find available versions with an az vm image list
command:
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.
Thanks! fixed in new changelist
docs/kubernetes/windows-details.md
Outdated
WindowsServerSemiAnnual MicrosoftWindowsServer Datacenter-Core-1803-with-Containers-smalldisk MicrosoftWindowsServer:WindowsServerSemiAnnual:Datacenter-Core-1803-with-Containers-smalldisk:1803.0.20180504 1803.0.20180504 | ||
``` | ||
|
||
You can use the Offer, Publisher and Sku to pick a specific version by adding `windowsOffer`, `windowsPublisher`, `windowsSku` and (optionally) `widndowsVersion` to the `windowsProfile` section. In this example, the latest Windows Server version 1803 image would be deployed. |
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.
Typo: widndowsVersion
kubectl get pvc/pvc-azurefile -o wide | ||
``` | ||
|
||
5. Create a pod with azure file pvc |
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.
Maybe link to pod documentation here?
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.
Thanks! fixed in new changelist
docs/kubernetes/windows-details.md
Outdated
|
||
6. Watch the status of pod until its `STATUS` is `Running` | ||
``` | ||
watch kubectl get po/iis-azurefile -o wide |
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 seem to have watch
installed on macOS High Sierra. Maybe this command could use kubectl -w
instead? I know its output gets muddled, but that would be more portable.
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.
Thanks! fixed in new changelist
docs/kubernetes/windows-details.md
Outdated
|
||
## Troubleshooting | ||
|
||
Windows support is still in **active development** with many changes each week. Read on for more info on known per-version issues and troubleshooting if you run into problems. |
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.
Maybe change the end of the final sentence to:
per-version issues and for help with troubleshooting if you run into problems.
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.
Thanks! fixed in new changelist
docs/kubernetes/windows-details.md
Outdated
There is a problem with the “L2Tunnel” networking mode not forwarding packets correctly specific to Windows Server version 1803. Windows Server version 1709 is not affected. | ||
|
||
Workarounds: | ||
**Fixes are still in development.** A Windows hotfix is needed, and willbe deployed by ACS-Engine once it's ready. The hotfix will be removed later when it's in a future cumulative rollup. |
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.
Typo: "willbe" -> "will be"
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.
Thanks! fixed in new changelist
docs/kubernetes/windows-details.md
Outdated
|
||
#### Pods cannot ping default route or internet IPs | ||
|
||
Affects: All acs-engine deployed clusters |
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.
This should be a hyphenated phrase ending with -deployed
, but that would read poorly. Maybe change to:
Affects: All clusters deployed by acs-engine
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.
Thanks! fixed in new changelist
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jackfrancis, 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 |
What this PR does / why we need it:
The Windows walkthrough had a lot of back & forth from other pages which made it hard to follow.
This is my attempt at making a simpler top to bottom, copy & paste workflow. I'm not a great writer, so any and all feedback (or PRs) are welcome!
Special notes for your reviewer:
If applicable: