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 FlowAggregator e2e test ANP deployment issue #42

Merged
merged 1 commit into from
Jun 10, 2022

Conversation

heanlan
Copy link
Contributor

@heanlan heanlan commented Jun 8, 2022

The original e2e test cases only check IngressRuleTable/EgressTable flow
count, and use the result to decide the if the NP rule is realized. But
these two tables are used to realize K8s NP rules. For a normal ANP
rule, AntreaPolicyIngressRuleTable/AntreaPolicyEgressRuleTable are used,
but never checked in the case. Besides, the original case always uses
number 2 to check the rule flow existence, but the fact is there always
no less than 2 flows in IngressRule/EgressRule table, which are used to
bypass the established and related packets in a valid connection. So the
case actually doesn't check if the rule is realized or not.

Signed-off-by: wenyingd wenyingd@vmware.com
Co-authored-by: heanlan hanlan@vmware.com

@heanlan
Copy link
Contributor Author

heanlan commented Jun 8, 2022

/theia-test-e2e

Copy link
Contributor

@salv-orlando salv-orlando left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@dreamtalen dreamtalen left a comment

Choose a reason for hiding this comment

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

a nit only, otherwise LGTM

}
t.Log("Network Policies are realized.")
return np1, np2
}

func deployAntreaNetworkPolicies(t *testing.T, data *TestData, srcPod, dstPod string) (anp1 *secv1alpha1.NetworkPolicy, anp2 *secv1alpha1.NetworkPolicy) {
func deployAntreaNetworkPolicies(t *testing.T, data *TestData, srcPod, dstPod string, srcNode, dstNode string) (anp1 *secv1alpha1.NetworkPolicy, anp2 *secv1alpha1.NetworkPolicy) {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: srcPod, dstPod string, srcNode, dstNode string -> srcPod, dstPod, srcNode, dstNode string

The original e2e test cases only check IngressRuleTable/EgressTable flow
count, and use the result to decide the if the NP rule is realized. But
these two tables are used to realize K8s NP rules. For a normal ANP
rule, AntreaPolicyIngressRuleTable/AntreaPolicyEgressRuleTable are used,
but never checked in the case. Besides, the original case always uses
number 2 to check the rule flow existence, but the fact is there always
no less than 2 flows in IngressRule/EgressRule table, which are used to
bypass the established and related packets in a valid connection. So the
case actually doesn't check if the rule is realized or not.

Signed-off-by: wenyingd <wenyingd@vmware.com>
Co-authored-by: heanlan <hanlan@vmware.com>
@heanlan heanlan merged commit a16e7ed into antrea-io:main Jun 10, 2022
@heanlan heanlan deleted the fix-fa-e2e branch June 10, 2022 19:16
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.

4 participants