-
Notifications
You must be signed in to change notification settings - Fork 25
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
Improve Policy Recommendation e2e test #77
Conversation
/theia-test-e2e |
In this commit, we add these improvements for the Policy Recommendation e2e test: 1. Add test case for the failed policy recommendation job. 2. Add policy recommendation check for the Pod-to-Svc, and Pod-to-External flows. 3. Change Kind/Jenkins test script to build the latest policy recommendation image for testing, instead of pulling from Docker. Signed-off-by: Yongming Ding <[email protected]>
@@ -63,3 +85,19 @@ jobs: | |||
name: e2e-kind-fa.tar.gz | |||
path: log.tar.gz | |||
retention-days: 30 | |||
|
|||
# Runs after all other jobs in the workflow and deletes Theia Docker images uploaded as temporary | |||
# artifacts. It uses a third-party, MIT-licensed action (geekyeggo/delete-artifact). While Github |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not familiar with how temporary artifacts are managed, will there be possible conflicts if multi images are uploaded and deleted?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi Ziyou, if there are multiple jobs uploading a same image, the later jobs will overwrite what was previously uploaded. I think this won't be a problem for our testing usage?
t.Fatalf("Error when creating perftest-b Service: %v", err) | ||
} | ||
// Wait for the Service to be realized. | ||
time.Sleep(3 * time.Second) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a simple way to check if service is realized? A fixed waiting interval either wastes time or stops too early.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi Ziyou, this time.Sleep
was introduced in antrea-io/antrea#1815 first time, our tests inheriting from flowaggregator_test.go
keep it as well. I'm not sure if weiqiang found something while debugging so he added this wait statement. I tried removing it here and flowvisibility_test.go
, no errors are found so far.
Signed-off-by: Yongming Ding <[email protected]>
/theia-test-e2e |
|
||
svcB, err = data.CreateService("perftest-b", testNamespace, iperfPort, iperfPort, map[string]string{"antrea-e2e": "perftest-b"}, false, false, corev1.ServiceTypeClusterIP, &svcIPFamily) | ||
if err != nil { | ||
return nil, fmt.Errorf("Error when creating perftest-b Service: %v", err) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
return nil, fmt.Errorf("Error when creating perftest-b Service: %v", err) | |
return nil, fmt.Errorf("error when creating perftest-b Service: %v", err) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, addressed.
Signed-off-by: Yongming Ding <[email protected]>
Fix #72
In this commit, we add these improvements for the Policy Recommendation
e2e test:
recommendation image for testing, instead of pulling from Docker.
Signed-off-by: Yongming Ding [email protected]