Skip to content
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.

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

Add containerd runtime support for antrea agent on windows #4279

Merged
merged 2 commits into from
Dec 20, 2022

Conversation

XinShuYang
Copy link
Contributor

Add new containerd yaml file for windows agent because of the different cni workflow.

Signed-off-by: Shuyang Xin [email protected]

@codecov
Copy link

codecov bot commented Oct 11, 2022

Codecov Report

Merging #4279 (3d08fa2) into main (330a7de) will increase coverage by 2.80%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #4279      +/-   ##
==========================================
+ Coverage   64.01%   66.81%   +2.80%     
==========================================
  Files         400      403       +3     
  Lines       56888    57690     +802     
==========================================
+ Hits        36418    38547    +2129     
+ Misses      17783    16401    -1382     
- Partials     2687     2742      +55     
Flag Coverage Δ
integration-tests 34.63% <ø> (+0.04%) ⬆️
kind-e2e-tests 41.07% <ø> (-0.30%) ⬇️
unit-tests 56.71% <ø> (+6.84%) ⬆️
Impacted Files Coverage Δ
...g/agent/apiserver/handlers/addressgroup/handler.go 5.00% <0.00%> (-35.00%) ⬇️
...agent/apiserver/handlers/appliedtogroup/handler.go 5.00% <0.00%> (-35.00%) ⬇️
...nt/apiserver/handlers/serviceexternalip/handler.go 3.70% <0.00%> (-25.93%) ⬇️
...uster/commonarea/labelidentityimport_controller.go 54.32% <0.00%> (-20.99%) ⬇️
...agent/flowexporter/connections/deny_connections.go 66.66% <0.00%> (-18.28%) ⬇️
pkg/agent/apiserver/handlers/multicast/handler.go 69.76% <0.00%> (-9.31%) ⬇️
pkg/agent/flowexporter/connections/connections.go 82.22% <0.00%> (-5.56%) ⬇️
.../flowexporter/connections/conntrack_connections.go 72.85% <0.00%> (-4.29%) ⬇️
pkg/controller/ipam/antrea_ipam_controller.go 75.25% <0.00%> (-4.02%) ⬇️
pkg/agent/util/iptables/iptables.go 45.07% <0.00%> (-3.63%) ⬇️
... and 78 more

@XinShuYang XinShuYang changed the title Support containerd runtime support for antrea agent on windows Add containerd runtime support for antrea agent on windows Oct 13, 2022
@XinShuYang XinShuYang marked this pull request as ready for review October 17, 2022 15:34
@XinShuYang
Copy link
Contributor Author

/test-windows-containerd-e2e

1 similar comment
@XinShuYang
Copy link
Contributor Author

/test-windows-containerd-e2e

@XinShuYang XinShuYang force-pushed the wincontainerd branch 2 times, most recently from 365645b to e7bddbe Compare October 18, 2022 05:19
@XinShuYang
Copy link
Contributor Author

/test-windows-containerd-e2e

2 similar comments
@XinShuYang
Copy link
Contributor Author

/test-windows-containerd-e2e

@XinShuYang
Copy link
Contributor Author

/test-windows-containerd-e2e

@XinShuYang
Copy link
Contributor Author

/test-windows-containerd-e2e

@XinShuYang XinShuYang force-pushed the wincontainerd branch 2 times, most recently from 80bc54b to 30330bb Compare October 18, 2022 11:10
@XinShuYang
Copy link
Contributor Author

/test-windows-containerd-e2e

@XinShuYang
Copy link
Contributor Author

/test-windows-containerd-e2e

1 similar comment
@XinShuYang
Copy link
Contributor Author

/test-windows-containerd-e2e

@XinShuYang
Copy link
Contributor Author

/test-windows-containerd-e2e

@XinShuYang
Copy link
Contributor Author

/test-windows-containerd-e2e

@XinShuYang XinShuYang force-pushed the wincontainerd branch 2 times, most recently from e8813f6 to 2e8b776 Compare October 18, 2022 15:25
@XinShuYang
Copy link
Contributor Author

/test-windows-containerd-e2e

@XinShuYang XinShuYang force-pushed the wincontainerd branch 2 times, most recently from 6a6fbd2 to b949f0d Compare October 18, 2022 23:33
@XinShuYang
Copy link
Contributor Author

/test-windows-containerd-e2e

@wenyingd
Copy link
Contributor

wenyingd commented Dec 9, 2022

@XinShuYang @wenyingd should this be included in milestone 1.10?

Yes, it is better if we can include it.

@luolanzone luolanzone added this to the Antrea v1.10 release milestone Dec 9, 2022
luolanzone
luolanzone previously approved these changes Dec 9, 2022
Copy link
Contributor

@luolanzone luolanzone left a comment

Choose a reason for hiding this comment

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

LGTM

@XinShuYang XinShuYang dismissed stale reviews from luolanzone and wenyingd via 4f17371 December 14, 2022 08:43
@XinShuYang XinShuYang force-pushed the wincontainerd branch 3 times, most recently from 34e8716 to cdf7952 Compare December 14, 2022 10:14
@XinShuYang
Copy link
Contributor Author

/test-windows-containerd-e2e
/test-windows-containerd-networkpolicy
/test-windows-containerd-conformance
/test-windows-all

Copy link
Member

@tnqn tnqn left a comment

Choose a reason for hiding this comment

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

LGTM overall, some minor comments

ci/jenkins/test.sh Outdated Show resolved Hide resolved
ci/jenkins/test.sh Outdated Show resolved Hide resolved
Comment on lines 538 to 559
govc snapshot.revert -vm ${WORKER_NAME} win-initial
# If Windows VM fails to power on correctly in time, retry several times.
winVMIPs=""
for i in `seq 10`; do
winVMIPs=$(govc vm.ip -wait=2m -a ${WORKER_NAME})
if [[ $winVMIPs != "" ]]; then
echo "Windows VM ${WORKER_NAME} powered on"
break
fi
echo "Windows VM ${WORKER_NAME} failed to power on"
govc vm.power -on ${WORKER_NAME} || true
done
if [[ $winVMIPs == "" ]]; then
echo "Windows VM ${WORKER_NAME} didn't power on after 3 tries, exiting"
exit 1
fi
IP=$(kubectl get node "${WORKER_NAME}" -o jsonpath='{.status.addresses[0].address}')
# Windows VM is reverted to an old snapshot so computer date needs updating.
for i in `seq 24`; do
sleep 5
ssh -o StrictHostKeyChecking=no -n Administrator@${IP} "W32tm /resync /force" | grep successfully && break
done
Copy link
Member

Choose a reason for hiding this comment

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

this seems same as the code for jumper VM, perhaps extract a function to reduce redundancy?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

exacted a function revert_snapshot_windows

Comment on lines 576 to 544
# Compress antrea repo and copy it to a Windows node
mkdir -p jenkins
tar --exclude='./jenkins' -czf jenkins/antrea_repo.tar.gz -C "$(pwd)" .
for i in `seq 2`; do
timeout 2m scp -o StrictHostKeyChecking=no -T jenkins/antrea_repo.tar.gz Administrator@${IP}: && break
done
echo "=== Build Windows on Windows Node==="
ssh -o StrictHostKeyChecking=no -n Administrator@${IP} "docker pull ${DOCKER_REGISTRY}/antrea/golang:${GO_VERSION}-nanoserver && docker tag ${DOCKER_REGISTRY}/antrea/golang:${GO_VERSION}-nanoserver golang:${GO_VERSION}-nanoserver"
ssh -o StrictHostKeyChecking=no -n Administrator@${IP} "rm -rf antrea && mkdir antrea && cd antrea && tar -xzf ../antrea_repo.tar.gz > /dev/null && NO_PULL=${NO_PULL}; DOCKER_NETWORK=host make build-windows && docker save -o antrea-windows.tar antrea/antrea-windows:latest" || true
for i in `seq 2`; do
timeout 2m scp -o StrictHostKeyChecking=no -T Administrator@${IP}:antrea/antrea-windows.tar.gz . && break
done
else
for i in `seq 2`; do
timeout 2m scp -o StrictHostKeyChecking=no -T antrea-windows.tar.gz Administrator@${IP}: && break
done
ssh -o StrictHostKeyChecking=no -n Administrator@${IP} "gzip -d antrea-windows.tar.gz && ctr -n k8s.io images import antrea-windows.tar"
fi
Copy link
Member

Choose a reason for hiding this comment

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

Haven't it been executed once after L522?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed. If building fails on jumper node, the test will exit.

Comment on lines +857 to +812
ginkgo -timeout=2h --no-color $E2ETEST_PATH -- --provider=skeleton --ginkgo.focus="$WINDOWS_NETWORKPOLICY_FOCUS" --ginkgo.skip="$WINDOWS_NETWORKPOLICY_CONTAINERD_SKIP" > windows_conformance_result_no_color.txt || true
else
ginkgo --no-color $E2ETEST_PATH -- --provider=skeleton --node-os-distro=windows --ginkgo.focus="$WINDOWS_CONFORMANCE_FOCUS" --ginkgo.skip="$WINDOWS_CONFORMANCE_SKIP" > windows_conformance_result_no_color.txt || true
Copy link
Member

Choose a reason for hiding this comment

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

Doesn't windows-containerd-networkpolicy need to specify --node-os-distro=windows?

Copy link
Contributor

Choose a reason for hiding this comment

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

We can not specify --node-os-distro=windows in windows-containerd-networkpolicy or windows-networkpolicy, it is because most of the network policy test cases we expect to run in kubernetes test suite have the mark "[LinuxOnly]", and the parameter will lead the cases are passed. Although kubernetes mark these case to run on Linux only, they are supposed to work on Windows since Antrea Windows supports NP functions.

Copy link
Member

Choose a reason for hiding this comment

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

better to add a comment for above explanation.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added comments.

@tnqn tnqn added the action/release-note Indicates a PR that should be included in release notes. label Dec 15, 2022
@XinShuYang
Copy link
Contributor Author

/test-windows-containerd-e2e
/test-windows-containerd-networkpolicy
/test-windows-containerd-conformance
/test-windows-all

@XinShuYang
Copy link
Contributor Author

/test-windows-e2e
/test-windows-conformance
/test-windows-containerd-networkpolicy

@XinShuYang
Copy link
Contributor Author

/test-windows-proxyall-e2e

@XinShuYang
Copy link
Contributor Author

/test-all

@XinShuYang
Copy link
Contributor Author

/test-windows-conformance
/test-windows-containerd-networkpolicy
/test-windows-proxyall-e2e

Comment on lines +857 to +812
ginkgo -timeout=2h --no-color $E2ETEST_PATH -- --provider=skeleton --ginkgo.focus="$WINDOWS_NETWORKPOLICY_FOCUS" --ginkgo.skip="$WINDOWS_NETWORKPOLICY_CONTAINERD_SKIP" > windows_conformance_result_no_color.txt || true
else
ginkgo --no-color $E2ETEST_PATH -- --provider=skeleton --node-os-distro=windows --ginkgo.focus="$WINDOWS_CONFORMANCE_FOCUS" --ginkgo.skip="$WINDOWS_CONFORMANCE_SKIP" > windows_conformance_result_no_color.txt || true
Copy link
Member

Choose a reason for hiding this comment

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

better to add a comment for above explanation.

ci/jenkins/test.sh Outdated Show resolved Hide resolved
ci/jenkins/test.sh Outdated Show resolved Hide resolved
ci/jenkins/test.sh Outdated Show resolved Hide resolved
ci/jenkins/test.sh Outdated Show resolved Hide resolved
ci/jenkins/test.sh Outdated Show resolved Hide resolved
@XinShuYang
Copy link
Contributor Author

/test-windows-containerd-e2e
/test-windows-containerd-networkpolicy
/test-windows-containerd-conformance
/test-windows-all
/test-all

tnqn
tnqn previously approved these changes Dec 19, 2022
Copy link
Member

@tnqn tnqn left a comment

Choose a reason for hiding this comment

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

LGTM

@XinShuYang
Copy link
Contributor Author

/test-windows-containerd-e2e
/test-windows-containerd-networkpolicy
/test-windows-containerd-conformance
/test-windows-all
/test-all

Add e2e,conformance and networkpolicy tests for windows containerd testbed.

ginkgo:v2.1.6
kubernetes:v1.25
containerd:v1.6.6

Signed-off-by: Shuyang Xin <[email protected]>
@XinShuYang
Copy link
Contributor Author

/test-windows-containerd-e2e
/test-windows-containerd-networkpolicy
/test-windows-containerd-conformance
/test-windows-all
/test-all

Copy link
Member

@tnqn tnqn left a comment

Choose a reason for hiding this comment

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

LGTM

@tnqn tnqn merged commit 721fc9e into antrea-io:main Dec 20, 2022
GraysonWu pushed a commit to GraysonWu/antrea that referenced this pull request Jan 27, 2023
…#4279)

Add new containerd yaml file for windows agent because of the different cni workflow.

Add e2e,conformance and networkpolicy tests for windows containerd testbed.

Signed-off-by: Shuyang Xin <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
action/release-note Indicates a PR that should be included in release notes.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants