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

Fix flaky e2e test TestL7NetworkPolicy #4723

Merged
merged 1 commit into from
Mar 22, 2023

Conversation

hongliangl
Copy link
Contributor

@hongliangl hongliangl commented Mar 17, 2023

Fixes #4718

Signed-off-by: Hongliang Liu lhongliang@vmware.com

@hongliangl hongliangl requested a review from tnqn March 17, 2023 12:47
@hongliangl hongliangl force-pushed the 20230317-fix-4718-2 branch from f7d9e23 to fd52765 Compare March 21, 2023 17:34
@hongliangl hongliangl changed the title Fix flaky e2e test TestL7NetworkPolicy by increasing wait time. Fix flaky e2e test TestL7NetworkPolicy Mar 21, 2023
@hongliangl hongliangl force-pushed the 20230317-fix-4718-2 branch 2 times, most recently from 502161f to 864b9f0 Compare March 21, 2023 17:37
@@ -398,9 +398,15 @@ func (c *ovsCtlClient) DumpPortsDesc() ([][]string, error) {

func (c *ovsCtlClient) SetPortNoFlood(ofport int) error {
// This command does not have standard output, and only has standard err when running with error.
_, err := c.ovsOfctlRunner.RunOfctlCmd("mod-port", strconv.FormatUint(uint64(ofport), 10), "no-flood")
// Note that, THIS CONFIGURATION MUST WORK WITH OpenFlow10.
Copy link
Contributor

Choose a reason for hiding this comment

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

🚀 just a question, looking at the changes in PR4517, could the bug also be caused by not capturing the standard err? because otherwise before the change we could use c.runOfctlCmd(false, "mod-port", ofport) to avoid Openflow15.

Copy link
Contributor Author

@hongliangl hongliangl Mar 22, 2023

Choose a reason for hiding this comment

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

just a question, looking at the changes in PR4517, could the bug also be caused by not capturing the standard err?

Impressive observation. However, I found that after executing the command 'ovs-ofctl mod-port br-int 6 no-flood -O Openflow15', there was no 'stout', and the 'no-flood' option was not configured.

because otherwise before the change we could use c.runOfctlCmd(false, "mod-port", ofport) to avoid Openflow15.

Thanks for reminding, we can use runOfctlCmd here.

@hongliangl hongliangl force-pushed the 20230317-fix-4718-2 branch from 864b9f0 to 858c5e8 Compare March 22, 2023 02:58
@hongliangl hongliangl requested review from tnqn and qiyueyao March 22, 2023 02:58
@hongliangl hongliangl added this to the Antrea v1.11 release milestone Mar 22, 2023
tnqn
tnqn previously approved these changes Mar 22, 2023
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
Copy link
Member

tnqn commented Mar 22, 2023

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

Fixes antrea-io#4718

Signed-off-by: Hongliang Liu <lhongliang@vmware.com>
@hongliangl
Copy link
Contributor Author

/test-e2e

@tnqn
Copy link
Member

tnqn commented Mar 22, 2023

/skip-conformance
/skip-networkpolicy

@tnqn tnqn merged commit 590323a into antrea-io:main Mar 22, 2023
@hongliangl hongliangl deleted the 20230317-fix-4718-2 branch March 22, 2023 08:48
jainpulkit22 pushed a commit to urharshitha/antrea that referenced this pull request Apr 28, 2023
Fixes antrea-io#4718

Signed-off-by: Hongliang Liu <lhongliang@vmware.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

e2e test TestL7NetworkPolicy failed frequently
3 participants