[Windows] Update Windows instructions #2456

lzhecheng · 2021-07-23T09:41:46Z

* Mkdir kube-proxy log path before installation as a Service
  Otherwise kube-proxy Service may not work well.
* Updated asset links from kubernetes-sigs/sig-windows-tools repo
* Retry for Get-WebFileIfNotExist
* Have cleaner environment for docker (stop, remove docker.pid, start)
* Start Antrea Windows agent in Daemonset way for Windows NetworkPolicy test
  Currently the script way leads to many flaky failures
* Skip SCTP test for Windows NetworkPolicy tests since it requires privilege mode
  which is not available on Windows Node
* Force date sync after Windows VM is reverted to a snapshot

Signed-off-by: Zhecheng Li lzhecheng@vmware.com

codecov-commenter · 2021-07-23T09:47:19Z

Codecov Report

Merging #2456 (bd24d29) into main (9adf9c9) will decrease coverage by 17.61%.
The diff coverage is n/a.

@@             Coverage Diff             @@
##             main    #2456       +/-   ##
===========================================
- Coverage   59.79%   42.18%   -17.62%     
===========================================
  Files         284      148      -136     
  Lines       22171    18222     -3949     
===========================================
- Hits        13258     7687     -5571     
- Misses       7490     9840     +2350     
+ Partials     1423      695      -728

Flag	Coverage Δ
kind-e2e-tests	`?`
unit-tests	`42.18% <ø> (+0.16%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
pkg/agent/agent_linux.go	`0.00% <0.00%> (-100.00%)`	⬇️
pkg/ovs/openflow/default.go	`0.00% <0.00%> (-100.00%)`	⬇️
pkg/util/runtime/runtime.go	`0.00% <0.00%> (-100.00%)`	⬇️
pkg/agent/config/node_config.go	`0.00% <0.00%> (-100.00%)`	⬇️
pkg/agent/cniserver/pod_configuration_linux.go	`0.00% <0.00%> (-100.00%)`	⬇️
pkg/apis/controlplane/v1beta1/register.go	`0.00% <0.00%> (-92.86%)`	⬇️
pkg/ovs/openflow/logs.go	`9.52% <0.00%> (-90.48%)`	⬇️
pkg/apis/controlplane/register.go	`0.00% <0.00%> (-90.00%)`	⬇️
pkg/agent/nodeportlocal/k8s/annotations.go	`0.00% <0.00%> (-83.88%)`	⬇️
pkg/agent/client.go	`0.00% <0.00%> (-77.42%)`	⬇️
... and 231 more

docs/windows.md

lzhecheng · 2021-07-26T09:34:15Z

/test-windows-networkpolicy

lzhecheng · 2021-07-26T10:10:58Z

/test-windows-networkpolicy

wenyingd · 2021-07-26T10:15:19Z

docs/windows.md

@@ -130,7 +132,7 @@ kube-proxy version.

 ```bash
 # Example:
-curl -L https://github.com/kubernetes-sigs/sig-windows-tools/releases/latest/download/kube-proxy.yml | sed 's/VERSION/v1.18.0/g'  > kube-proxy.yml
+curl -L https://github.com/kubernetes-sigs/sig-windows-tools/releases/download/v0.1.5/kube-proxy.yml | sed 's/VERSION/v1.18.0/g' > kube-proxy.yml


Is it possible to use a varible for the version? You can use v0.1.5 as a default value if it is the version we wanted to pin.

hack/windows/Helper.psm1

wenyingd · 2021-07-26T10:17:12Z

hack/windows/Prepare-Node.ps1

@@ -4,7 +4,7 @@

 .DESCRIPTION
 This script is only used for test to assist with joining a Windows node to a cluster.
-For production environment please follow the antrea windows installation guide and use kubernetes official script: https://github.com/kubernetes-sigs/sig-windows-tools/blob/master/kubeadm/scripts/PrepareNode.ps1
+For production environment please follow the antrea windows installation guide and use kubernetes official script: https://github.com/kubernetes-sigs/sig-windows-tools/releases/download/v0.1.5/PrepareNode.ps1


This is the script description so I think we can keep it as it is? I have used variables for others.

maybe just point to the release page, without specifying a version, or we will probably forget to update it...

lzhecheng · 2021-07-26T10:50:39Z

/test-windows-networkpolicy

antoninbas · 2021-07-26T17:58:40Z

hack/windows/Prepare-Node.ps1

@@ -4,7 +4,7 @@

 .DESCRIPTION
 This script is only used for test to assist with joining a Windows node to a cluster.
-For production environment please follow the antrea windows installation guide and use kubernetes official script: https://github.com/kubernetes-sigs/sig-windows-tools/blob/master/kubeadm/scripts/PrepareNode.ps1
+For production environment please follow the antrea windows installation guide and use kubernetes official script: https://github.com/kubernetes-sigs/sig-windows-tools/releases/download/v0.1.5/PrepareNode.ps1


maybe just point to the release page, without specifying a version, or we will probably forget to update it...

antoninbas · 2021-07-26T18:01:13Z

docs/windows.md

+New-DirectoryIfNotExist "${AntreaHome}/logs"
+New-DirectoryIfNotExist "${KubeProxyLogPath}"
+nssm install kube-proxy "${KubernetesHome}/kube-proxy.exe" "--proxy-mode=userspace --kubeconfig=${KubeProxyKubeconfigPath} --log-dir=${KubeProxyLogPath} --logtostderr=false --alsologtostderr"
+nssm install antrea-agent "${AntreaHome}/bin/antrea-agent.exe" "--config=${AntreaHome}/etc/antrea-agent.conf --logtostderr=false --log_dir=${AntreaHome}/logs --alsologtostderr --log_file_max_size=100 --log_file_max_num=4"


I wonder if Antrea logs should go to c:/var/log/antrea instead, as this would be a less "unexpected" location and it may be easier for users to find the logs there. But there is no need to address this in this PR.

Good idea. I have created an issue to track this: #2472

lzhecheng · 2021-07-27T02:15:42Z

/skip-all

lzhecheng · 2021-07-27T07:16:35Z

/test-windows-networkpolicy

lzhecheng · 2021-07-27T11:04:58Z

/test-windows-networkpolicy

lzhecheng · 2021-07-28T02:01:15Z

/test-windows-networkpolicy

lzhecheng · 2021-07-28T02:58:15Z

/test-windows-networkpolicy

lzhecheng · 2021-07-28T04:02:44Z

/test-windows-networkpolicy

lzhecheng · 2021-07-28T05:25:30Z

/test-windows-networkpolicy

lzhecheng · 2021-07-28T07:20:10Z

/test-windows-networkpolicy

lzhecheng · 2021-07-28T09:07:34Z

/test-windows-networkpolicy

lzhecheng · 2021-07-29T02:04:07Z

/test-windows-networkpolicy

lzhecheng · 2021-07-29T03:11:12Z

/test-windows-networkpolicy

wenyingd · 2021-07-29T04:06:45Z

ci/jenkins/test.sh

@@ -232,7 +232,7 @@ function deliver_antrea_windows {
    chmod -R g-w build/images/ovs
    chmod -R g-w build/images/base
    DOCKER_REGISTRY="${DOCKER_REGISTRY}" ./hack/build-antrea-ubuntu-all.sh --pull
-    if [[ "$TESTCASE" =~ "networkpolicy" ]]; then
+    if [[ "$TESTCASE" =~ "networkpolicy-no" ]]; then


What is networkpolicy-no for?

This is used to let Antrea Windows agent start as Daemonset for Windows NetworkPolicy test.

wenyingd · 2021-07-29T04:09:00Z

ci/jenkins/test.sh

@@ -280,13 +282,15 @@ function deliver_antrea_windows {
        done

        # Use a script to run antrea agent in windows Network Policy cases
-        if [ "$TESTCASE" == "windows-networkpolicy" ]; then
+        if [ "$TESTCASE" == "windows-networkpolicy-temp-disabled" ]; then


what do you mean about "windows-networkpolicy-temp-disabled"? what about "windows-networkpolicy-process"?

wenyingd · 2021-07-29T04:11:24Z

ci/jenkins/test.sh

@@ -495,13 +499,13 @@ function run_conformance_windows {
    export PATH=$GOROOT/bin:$PATH
    export KUBECONFIG=$KUBECONFIG_PATH

-    if [[ "$TESTCASE" == "windows-conformance" ]]; then
+    if [[ "$TESTCASE" =~ "windows-networkpolicy-temp-disabled" ]]; then


You use "not" in the "if" statement, but says user wanna process in the content, is it as your design?

I think =~ means equal or include while != is no equal.

lzhecheng · 2021-07-29T05:31:06Z

/skip-e2e /skip-conformance /skip-networkpolicy /test-windows-conformance /test-windows-networkpolicy

wenyingd · 2021-07-29T05:49:41Z

docs/windows.md

@@ -250,7 +253,8 @@ container.

 ```powershell
 # Example:
-curl.exe -LO https://github.com/kubernetes-sigs/sig-windows-tools/releases/latest/download/PrepareNode.ps1
+$SigWindowsToolsVersion="v0.1.5"


wenyingd · 2021-07-29T05:51:16Z

docs/windows.md

@@ -130,7 +132,8 @@ kube-proxy version.

 ```bash
 # Example:
-curl -L https://github.com/kubernetes-sigs/sig-windows-tools/releases/latest/download/kube-proxy.yml | sed 's/VERSION/v1.18.0/g'  > kube-proxy.yml
+$SigWindowsToolsVersion="v0.1.5"


Since this is a guide document, I think you could have one generic introduction about the example version of variable $SigWindowsToolsVersion and $kubernetesVersion we used in the examples, then we can use the variable directly anywhere. And we can explain sig-windows v0.1.5 is tested if you feel necessary.

Used v0.1.5 for sig-windows-tool version. We think we can update kubernetes version in another PR after we verified a newer one than v1.18.0.

wenyingd

LGTM

lzhecheng · 2021-07-29T07:03:49Z

@antoninbas @tnqn PTAL. This PR wil unblock other PRs' Windows tests.

lzhecheng · 2021-07-29T07:04:07Z

/skip-e2e /skip-conformance /skip-networkpolicy /test-windows-conformance /test-windows-networkpolicy

lzhecheng · 2021-07-29T07:33:46Z

/test-windows-conformance

lzhecheng · 2021-07-29T07:50:23Z

/test-windows-networkpolicy

tnqn

How does this PR achieve "Start Antrea Windows agent in Daemonset way for Windows NetworkPolicy test"? Is it because we set "TESTCASE" to "windows-networkpolicy" so it won't mach "windows-networkpolicy-process"? And do we know why "Currently the script way leads to many flaky failures"? why we used different modes for conformance and networkpolicy tests in the beginning?

tnqn · 2021-07-29T17:33:56Z

docs/windows.md

@@ -58,6 +58,8 @@ and daemons are pre-installed on the Windows Nodes in the demo.
    Antrea on Windows, Antrea provides a test-signed OVS package for you.
    See details in [Join Windows worker Nodes](#Join-Windows-worker-nodes)
    section.
+* Some manifests are from [sig-windows-tool](https://github.com/kubernetes-sigs/sig-windows-tools) repo. Release version


could you wrap it like other lines?

tnqn · 2021-07-29T17:37:23Z

hack/windows/Prepare-Node.ps1

@@ -4,7 +4,8 @@

 .DESCRIPTION
 This script is only used for test to assist with joining a Windows node to a cluster.
-For production environment please follow the antrea windows installation guide and use kubernetes official script: https://github.com/kubernetes-sigs/sig-windows-tools/blob/master/kubeadm/scripts/PrepareNode.ps1
+For production environment please follow the antrea windows installation guide and use kubernetes official PrepareNode.ps1


Suggested change

For production environment please follow the antrea windows installation guide and use kubernetes official PrepareNode.ps1

For production environment please follow the antrea windows installation guide and use Kubernetes official PrepareNode.ps1

tnqn · 2021-07-29T17:45:12Z

ci/jenkins/test.sh

@@ -495,13 +499,13 @@ function run_conformance_windows {
    export PATH=$GOROOT/bin:$PATH
    export KUBECONFIG=$KUBECONFIG_PATH

-    if [[ "$TESTCASE" == "windows-conformance" ]]; then
+    if [[ "$TESTCASE" =~ "windows-networkpolicy-process" ]]; then


I notice some code use "==" while some use "=~". We don't need regular expression, right?

We need it.
e.g.

antrea/ci/jenkins/test.sh

Line 576 in 3bbb106

if [[ ${TESTCASE} =~ "windows" ]]; then

We use =~ here because windows testcases (windows-e2e, windows-conformance, windows-networpolicy) have windows in variable ${TESTCASE}

I mean even when it matchs "windows-networkpolicy-process", it sometimes uses "==" and sometimes uses "=~". I think the script should be accurate to express its intention, like what you explained here. We should use "==" when we want exact match.

Used == for all "windows-networkpolicy-process".

lzhecheng · 2021-07-30T00:10:06Z

How does this PR achieve "Start Antrea Windows agent in Daemonset way for Windows NetworkPolicy test"? Is it because we set "TESTCASE" to "windows-networkpolicy" so it won't mach "windows-networkpolicy-process"? And do we know why "Currently the script way leads to many flaky failures"? why we used different modes for conformance and networkpolicy tests in the beginning?

Yes.
No. I will investigate. #2500
So we can test both deployments (script and daemonset).

lzhecheng · 2021-07-30T03:07:34Z

/skip-e2e /skip-conformance /skip-networkpolicy

* Mkdir kube-proxy log path before installation as a Service Otherwise kube-proxy Service may not work well. * Updated asset links from kubernetes-sigs/sig-windows-tools repo * Retry for Get-WebFileIfNotExist * Have cleaner environment for docker (stop, remove docker.pid, start) * Start Antrea Windows agent in Daemonset way for Windows NetworkPolicy test Currently the script way leads to many flaky failures * Skip SCTP test for Windows NetworkPolicy tests since it requires privilege mode which is not available on Windows Node * Force date sync after Windows VM is reverted to a snapshot Signed-off-by: Zhecheng Li <lzhecheng@vmware.com>

tnqn

LGTM

lzhecheng · 2021-07-30T07:29:25Z

/test-windows-conformance /test-windows-networkpolicy

lzhecheng · 2021-07-30T08:25:41Z

/test-windows-networkpolicy

lzhecheng · 2021-07-30T09:14:30Z

/skip-e2e /skip-conformance /skip-networkpolicy /test-windows-networkpolicy

lzhecheng · 2021-07-30T09:54:05Z

@tnqn Please help merge.

lzhecheng requested review from wenyingd and tnqn July 23, 2021 09:41

lzhecheng force-pushed the win-script-mkdir branch from 0c9737e to 90cc54b Compare July 23, 2021 10:00

lzhecheng changed the title ~~[Windows] Mkdir kube-proxy log path before installation as a Service~~ [Windows] Update Windows instructions Jul 23, 2021

lzhecheng commented Jul 26, 2021

View reviewed changes

docs/windows.md Outdated Show resolved Hide resolved

lzhecheng force-pushed the win-script-mkdir branch from 90cc54b to fe45cb4 Compare July 26, 2021 04:29

lzhecheng requested a review from antoninbas July 26, 2021 09:23

wenyingd reviewed Jul 26, 2021

View reviewed changes

lzhecheng force-pushed the win-script-mkdir branch from eabbe03 to 29f472a Compare July 26, 2021 10:47

lzhecheng force-pushed the win-script-mkdir branch from 29f472a to f0a1d0f Compare July 26, 2021 11:08

antoninbas previously approved these changes Jul 26, 2021

View reviewed changes

lzhecheng dismissed antoninbas’s stale review via 410d767 July 27, 2021 02:10

lzhecheng force-pushed the win-script-mkdir branch from f0a1d0f to 410d767 Compare July 27, 2021 02:10

lzhecheng requested review from antoninbas and wenyingd July 27, 2021 02:15

lzhecheng force-pushed the win-script-mkdir branch from 762cf87 to fbd60d1 Compare July 28, 2021 05:25

lzhecheng force-pushed the win-script-mkdir branch from fbd60d1 to 6fed7df Compare July 28, 2021 07:19

lzhecheng force-pushed the win-script-mkdir branch from 33ed771 to c2e7305 Compare July 29, 2021 03:10

wenyingd reviewed Jul 29, 2021

View reviewed changes

lzhecheng force-pushed the win-script-mkdir branch from c2e7305 to 1d57a83 Compare July 29, 2021 05:25

lzhecheng force-pushed the win-script-mkdir branch from 1d57a83 to 5670a61 Compare July 29, 2021 05:40

wenyingd reviewed Jul 29, 2021

View reviewed changes

lzhecheng force-pushed the win-script-mkdir branch from 5670a61 to 6da1c60 Compare July 29, 2021 06:31

wenyingd previously approved these changes Jul 29, 2021

View reviewed changes

tnqn reviewed Jul 29, 2021

View reviewed changes

lzhecheng dismissed wenyingd’s stale review via 3bbb106 July 30, 2021 00:00

lzhecheng force-pushed the win-script-mkdir branch from 6da1c60 to 3bbb106 Compare July 30, 2021 00:00

lzhecheng force-pushed the win-script-mkdir branch from 3bbb106 to bd24d29 Compare July 30, 2021 06:45

tnqn approved these changes Jul 30, 2021

View reviewed changes

tnqn merged commit e575391 into antrea-io:main Jul 30, 2021

lzhecheng deleted the win-script-mkdir branch July 30, 2021 10:45

wenyingd mentioned this pull request Sep 22, 2021

Automated cherry pick of #2456: Update Windows instructions * Mkdir kube-proxy log #2810

Closed

tnqn mentioned this pull request Sep 22, 2021

windows-networkpolicy test kept failing #2816

Closed

	For production environment please follow the antrea windows installation guide and use kubernetes official PrepareNode.ps1
	For production environment please follow the antrea windows installation guide and use Kubernetes official PrepareNode.ps1

[Windows] Update Windows instructions #2456

[Windows] Update Windows instructions #2456

Conversation

lzhecheng commented Jul 23, 2021 • edited Loading

codecov-commenter commented Jul 23, 2021 • edited Loading

Codecov Report

lzhecheng commented Jul 26, 2021

lzhecheng commented Jul 26, 2021

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

lzhecheng commented Jul 26, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lzhecheng commented Jul 27, 2021

lzhecheng commented Jul 27, 2021

lzhecheng commented Jul 27, 2021

lzhecheng commented Jul 28, 2021

lzhecheng commented Jul 28, 2021

lzhecheng commented Jul 28, 2021

lzhecheng commented Jul 28, 2021

lzhecheng commented Jul 28, 2021

lzhecheng commented Jul 28, 2021

lzhecheng commented Jul 29, 2021

lzhecheng commented Jul 29, 2021

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

lzhecheng commented Jul 29, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wenyingd left a comment

Choose a reason for hiding this comment

lzhecheng commented Jul 29, 2021

lzhecheng commented Jul 29, 2021

lzhecheng commented Jul 29, 2021

lzhecheng commented Jul 29, 2021

tnqn 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

Choose a reason for hiding this comment

lzhecheng commented Jul 30, 2021

lzhecheng commented Jul 30, 2021

tnqn left a comment

Choose a reason for hiding this comment

lzhecheng commented Jul 30, 2021

lzhecheng commented Jul 30, 2021

lzhecheng commented Jul 30, 2021

lzhecheng commented Jul 30, 2021

lzhecheng commented Jul 23, 2021 •

edited

Loading

codecov-commenter commented Jul 23, 2021 •

edited

Loading