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.

Sign up for GitHub

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

Jump to bottom

[flexible-ipam] Add e2e for StatefulSet #3146

Merged
merged 2 commits into from
Jan 19, 2022
Merged

[flexible-ipam] Add e2e for StatefulSet #3146

merged 2 commits into from
Jan 19, 2022

Conversation

gran-vmv
Copy link
Contributor

@gran-vmv gran-vmv commented Dec 16, 2021 •
edited
Loading

This PR requires #3141 merged first.

@gran-vmv gran-vmv self-assigned this Dec 16, 2021
@gran-vmv gran-vmv marked this pull request as draft December 16, 2021 06:14
@gran-vmv gran-vmv added this to the Antrea v1.5 release milestone Dec 16, 2021
@codecov-commenter
Copy link

codecov-commenter commented Dec 16, 2021 •
edited
Loading

Codecov Report

Merging #3146 (5c4b001) into main (d9fb47f) will decrease coverage by 8.65%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #3146      +/-   ##
==========================================
- Coverage   59.45%   50.80%   -8.66%     
==========================================
  Files         298      495     +197     
  Lines       25589    54062   +28473     
==========================================
+ Hits        15215    27467   +12252     
- Misses       8738    24283   +15545     
- Partials     1636     2312     +676
Flag Coverage Δ
e2e-tests 52.39% <ø> (?)
integration-tests 29.53% <ø> (?)
kind-e2e-tests 44.62% <ø> (-0.30%) ⬇️
unit-tests 40.54% <ø> (-0.06%) ⬇️

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

Impacted Files Coverage Δ
pkg/agent/cniserver/pod_configuration_linux.go 26.31% <0.00%> (-40.36%) ⬇️
pkg/controller/ipam/antrea_ipam_controller.go 48.71% <0.00%> (-31.57%) ⬇️
pkg/controller/networkpolicy/endpoint_querier.go 61.46% <0.00%> (-29.97%) ⬇️
pkg/controller/egress/controller.go 61.11% <0.00%> (-27.34%) ⬇️
pkg/agent/multicast/mcast_controller.go 47.05% <0.00%> (-24.19%) ⬇️
.../registry/networkpolicy/clustergroupmember/rest.go 64.28% <0.00%> (-23.95%) ⬇️
pkg/agent/cniserver/ipam/antrea_ipam.go 55.55% <0.00%> (-23.62%) ⬇️
pkg/agent/cniserver/ipam/antrea_ipam_controller.go 56.30% <0.00%> (-23.46%) ⬇️
pkg/agent/util/ethtool/ethtool_linux.go 46.66% <0.00%> (-23.34%) ⬇️
pkg/controller/ipam/validate.go 57.14% <0.00%> (-22.86%) ⬇️
... and 479 more

@gran-vmv gran-vmv force-pushed the ipam-e2e branch 10 times, most recently from 4e98a77 to ab7ab60 Compare December 17, 2021 09:44
@gran-vmv gran-vmv marked this pull request as ready for review December 17, 2021 09:45
@gran-vmv gran-vmv changed the title [flexible-ipam] Add e2e for StatefulSet WIP: [flexible-ipam] Add e2e for StatefulSet Dec 17, 2021
@gran-vmv gran-vmv force-pushed the ipam-e2e branch 8 times, most recently from 4fb6fdd to 2bf8a9b Compare December 24, 2021 04:04
@gran-vmv gran-vmv changed the title WIP: [flexible-ipam] Add e2e for StatefulSet [flexible-ipam] Add e2e for StatefulSet Jan 6, 2022
@gran-vmv gran-vmv force-pushed the ipam-e2e branch 9 times, most recently from 3eb39be to d5f92dc Compare January 14, 2022 00:43
tnqn
tnqn reviewed Jan 17, 2022
View reviewed changes
test/e2e/antreaipam_test.go Outdated Show resolved Hide resolved
test/e2e/antreaipam_test.go
expectedIPAddressJson, _ := json.Marshal(expectedIPAddressMap)
tb.Logf("expectedIPAddressMap: %s", expectedIPAddressJson)

err = wait.Poll(time.Second*5, defaultTimeout, func() (bool, error) {
Copy link
Member

Choose a reason for hiding this comment

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

Isn't the pool status updated immediately in CNI Add? why would there be a delay to see it includes the IPs?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, but we cannot guarantee the StatefulSet update/delete process is finished when we call this function. This script will only send a StatefulSet create/update/delete request, and we need to wait a small period for the whole process.

test/e2e/antreaipam_test.go Outdated Show resolved Hide resolved
test/e2e/antreaipam_test.go Outdated Show resolved Hide resolved
test/e2e/antreaipam_test.go Outdated Show resolved Hide resolved
test/e2e/antreaipam_test.go Outdated Show resolved Hide resolved
test/e2e/antreaipam_test.go Show resolved Hide resolved
test/e2e/framework.go Outdated Show resolved Hide resolved
test/e2e/framework.go Outdated Show resolved Hide resolved
test/e2e/antreaipam_test.go Outdated Show resolved Hide resolved
@gran-vmv
[flexible-ipam] Add e2e for StatefulSet
c10ab17 
Signed-off-by: gran <gran@vmware.com>
@gran-vmv gran-vmv requested a review from tnqn January 17, 2022 07:44
tnqn
tnqn reviewed Jan 18, 2022
View reviewed changes
test/e2e/framework.go
@@ -887,7 +894,7 @@ func (data *TestData) restartCoreDNSPods(timeout time.Duration) error {
if err := data.clientset.CoreV1().Pods(antreaNamespace).DeleteCollection(context.TODO(), deleteOptions, listOptions); err != nil {
return fmt.Errorf("error when deleting all CoreDNS Pods: %v", err)
}
return data.waitForCoreDNSPods(timeout)
return retryOnConnectionLostError(retry.DefaultRetry, func() error { return data.waitForCoreDNSPods(timeout) })
Copy link
Member

Choose a reason for hiding this comment

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

Is this new? Please add a comment to explain why it would happen or move it to another PR if this is not related

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure. Added comments at the function retryOnConnectionLostError.
This issue happens intermittently after antrea-agent restarted and AntreaIPAM enabled, because the Node uplink network is changed during antrea-agent starting/stopping.

@gran-vmv
[flexible-ipam] Fix e2e connection lost error
5c4b001 
Signed-off-by: gran <gran@vmware.com>
@gran-vmv
Copy link
Contributor Author

/test-all
/test-windows-all
/test-ipv6-all
/test-ipv6-only-all
/test-flexible-ipam-e2e

@gran-vmv
Copy link
Contributor Author

/test-flexible-ipam-e2e

@gran-vmv
Copy link
Contributor Author

/test-integration
/test-all-features-conformance
/test-ipv6-conformance
/test-windows-e2e

@gran-vmv
Copy link
Contributor Author

/test-integration
/test-all-features-conformance
/test-windows-networkpolicy

@gran-vmv gran-vmv requested a review from tnqn January 19, 2022 01:22
tnqn
tnqn approved these changes Jan 19, 2022
View reviewed changes
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 3e548c8 into antrea-io:main Jan 19, 2022
yanjunz97 pushed a commit to yanjunz97/antrea that referenced this pull request Feb 14, 2022
@gran-vmv @yanjunz97
[flexible-ipam] Add e2e for StatefulSet (antrea-io#3146)
57d4c31 
* [flexible-ipam] Add e2e for StatefulSet

Signed-off-by: gran <gran@vmware.com>

* [flexible-ipam] Fix e2e connection lost error

Signed-off-by: gran <gran@vmware.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@annakhm annakhm annakhm left review comments

@tnqn tnqn tnqn approved these changes

@antoninbas antoninbas Awaiting requested review from antoninbas

@jianjuns jianjuns Awaiting requested review from jianjuns

@salv-orlando salv-orlando Awaiting requested review from salv-orlando

Assignees

@gran-vmv gran-vmv

Labels
None yet
Projects
None yet
Milestone
Antrea v1.5 release
Development

Successfully merging this pull request may close these issues.
4 participants
@gran-vmv @codecov-commenter @tnqn @annakhm