-
Notifications
You must be signed in to change notification settings - Fork 173
Fix netpol e2e test #1244
Fix netpol e2e test #1244
Conversation
The test was failing incorrectly; it now passes if netpols are enabled and is skipped if they're not. Tested: on GKE with netpol enabled and on Kind with them disabled.
/assign @GinnyJI |
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.
/lgtm
/hold
Just 1 question and wait for @GinnyJI for another eye.
for i:=0; !netpolWorks && i<3; i++ { | ||
// This command will return a non-nil error if it works correctly | ||
stdout, _ := RunCommand(clientCmd, nsService1, alpineArgs, wgetArgs) | ||
if strings.Contains(stdout, "wget: download timed out") { |
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.
Question: If the netpol is not enabled in the cluster it will just get "Welcome to nginx!", right?
Exactly, yes.
…On Wed, Oct 28, 2020 at 12:12 PM Yiqi Gao ***@***.***> wrote:
***@***.**** commented on this pull request.
/lgtm
/hold
Just 1 question and wait for @GinnyJI <https://github.com/GinnyJI> for
another eye.
------------------------------
In incubator/hnc/test/e2e/quickstart_test.go
<#1244 (comment)>
:
> - netpolTestStdout := ""
- Eventually(func() error {
- stdout, err := RunCommand("kubectl run client -n", nsService1, clientArgs, cmdln)
- netpolTestStdout = stdout
- return err
- }).Should(Succeed())
- if !strings.Contains(netpolTestStdout, "wget: download timed out") {
+ //
+ // The standard matching functions won't work here because we're looking for a particular error
+ // string, but we don't want to fail if we've found it. So use the default timeout (2s) by
+ // trying up to three times with a 1s gap in between.
+ netpolWorks := false
+ for i:=0; !netpolWorks && i<3; i++ {
+ // This command will return a non-nil error if it works correctly
+ stdout, _ := RunCommand(clientCmd, nsService1, alpineArgs, wgetArgs)
+ if strings.Contains(stdout, "wget: download timed out") {
Question: If the netpol is not enabled in the cluster it will just get
"Welcome to nginx!", right?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#1244 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AE43PZGL4OCCB7PXU3ABUIDSNA7IRANCNFSM4TCQCV7A>
.
|
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: adrianludwin, GinnyJI The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/hold cancel |
The test was failing incorrectly; it now passes if netpols are enabled
and is skipped if they're not.
Tested: on GKE with netpol enabled and on Kind with them disabled.