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

Registering cleanup Antrea interface #4393

Closed
wants to merge 2 commits into from
Closed

Registering cleanup Antrea interface #4393

wants to merge 2 commits into from

Conversation

knabben
Copy link
Member

@knabben knabben commented Feb 15, 2023 •
edited
Loading

What this PR does / why we need it

To ensure the persistence of HNS network Antrea provides a register script to restart the networks after reboot, this patches installs this script to run on each restart.

Which issue(s) this PR fixes

Fixes #4395

Describe testing done for PR

A custom clusterclass was created with this patch and tests were made to ensure the script is installed properly, sequential reboot confirms the script restart the interface.

Release note

Windows Antrea interfaces cleanup script is being installed in the node bootstrap.

@knabben knabben requested review from a team as code owners February 15, 2023 17:47
@github-actions
Copy link

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/4393/20230215175608/clustergen.diff.txt
Author/reviewers:
Please review to verify that the effects on the generated cluster configurations are exactly what the PR intended, and give a thumbs-up if so.

@github-actions
Copy link

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/4393/20230215211537/clustergen.diff.txt
Author/reviewers:
Please review to verify that the effects on the generated cluster configurations are exactly what the PR intended, and give a thumbs-up if so.

tenczar
tenczar approved these changes Feb 15, 2023
View reviewed changes
Copy link
Contributor

@tenczar tenczar left a comment

Choose a reason for hiding this comment

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

Powershell is not my forte, but I believe that this is adding a postKubeadmConfig command to modify some registries that will call the cleanup script when necessary. So this look reasonable to me.

@knabben
Copy link
Member Author

knabben commented Feb 15, 2023

We have been using this script on legacy and I tested it here again before opening the PR. The prekubeadm commands are only installing at boot time.

@codecov
Copy link

codecov bot commented Feb 15, 2023

Codecov Report

Merging #4393 (7b9c657) into main (ff96606) will decrease coverage by 0.90%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #4393      +/-   ##
==========================================
- Coverage   49.56%   48.67%   -0.90%     
==========================================
  Files         452      482      +30     
  Lines       44993    47112    +2119     
==========================================
+ Hits        22302    22933     +631     
- Misses      20572    22010    +1438     
- Partials     2119     2169      +50
Impacted Files Coverage Δ
tkg/clusterclient/clusterclient.go 49.14% <100.00%> (ø)
tkg/managementcomponents/helper.go 91.90% <100.00%> (-0.04%) ⬇️
addons/controllers/clusterbootstrap_controller.go 64.58% <0.00%> (-1.55%) ⬇️
...ons/controllers/packageinstallstatus_controller.go 79.15% <0.00%> (ø)
.../cli/plugin/cluster/set_machinehealthcheck_node.go 23.33% <0.00%> (ø)
.../isolated-cluster/imgpkginterface/client_imgpkg.go 0.00% <0.00%> (ø)
...olated-cluster/imagepushop/publishimagesfromtar.go 73.17% <0.00%> (ø)
cmd/cli/plugin/cluster/list.go 11.36% <0.00%> (ø)
.../cli/plugin/cluster/get_machinehealthcheck_node.go 9.30% <0.00%> (ø)
... and 29 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@knabben knabben added the cherry-pick/release-0.28 PRs to be cherry-picked into release-0.28 branch label Feb 15, 2023
@knabben
Copy link
Member Author

knabben commented Feb 15, 2023

/test install-vc7

@knabben knabben added the ok-to-merge PRs should be labelled with this before merging label Feb 15, 2023
@knabben
Copy link
Member Author

knabben commented Feb 16, 2023

//usebom:sandbox/v2.1.1-zshippable/447906785714507732/tkg-compatibility:v12

@alfredthenarwhal
Copy link
Collaborator

@knabben: /test install-vc7
Commit: 7b9c657

Tests passed! Build no: 4097

DanielXiao
DanielXiao suggested changes Feb 16, 2023
View reviewed changes
Copy link
Contributor

@DanielXiao DanielXiao left a comment

Choose a reason for hiding this comment

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

Could you hold on this PR to Halifax? It triggers rollout during cc package upgrade.

@knabben knabben added do-not-merge/hold Some fixes necessary, hold for merging and removed ok-to-merge PRs should be labelled with this before merging cherry-pick/release-0.28 PRs to be cherry-picked into release-0.28 branch labels Feb 16, 2023
@knabben
Copy link
Member Author

knabben commented Jun 19, 2023

already moved downstream

@knabben knabben closed this Jun 19, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@DanielXiao DanielXiao DanielXiao requested changes

@tenczar tenczar tenczar approved these changes

@lubronzhan lubronzhan lubronzhan approved these changes

Assignees
No one assigned
Labels
cla-not-required do-not-merge/hold Some fixes necessary, hold for merging
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Windows Antrea must have a Cleanup script set on node bootstrap
6 participants
@knabben @alfredthenarwhal @DanielXiao @tenczar @lubronzhan @vmwclabot