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

feat(kuma-cp) e2e stabilization #939

Merged
merged 1 commit into from
Jul 29, 2020
Merged

feat(kuma-cp) e2e stabilization #939

merged 1 commit into from
Jul 29, 2020

Conversation

lobkovilya
Copy link
Contributor

@lobkovilya lobkovilya commented Jul 29, 2020

Signed-off-by: Ilya Lobkov ilya.lobkov@konghq.com

Summary

Policies might take some short period of time to be applied and propagated to Envoy. That's why all curl checks have to be done using Eventually

Signed-off-by: Ilya Lobkov <ilya.lobkov@konghq.com>
@lobkovilya lobkovilya requested a review from a team as a code owner July 29, 2020 13:50
_, stderr, err := c1.ExecWithRetries(TestNamespace, clientPod.GetName(), "demo-client",
"curl", "-v", "-m", "3", "echo-server")
return stderr, err
}, "10s", "1s").Should(ContainSubstring("HTTP/1.1 200 OK"))
Copy link
Contributor

Choose a reason for hiding this comment

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

shall we use the default for the timeouts/

Copy link
Contributor

Choose a reason for hiding this comment

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

Can we retry if not 200 OK returned?

Copy link
Contributor

Choose a reason for hiding this comment

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

Eventually + ExecWithRetries

Can we have single code for retries?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can we retry if not 200 OK returned?

@nickolaev I'm sorry, not sure that I'm following here. It will retry until a response contains HTTP/1.1 200 OK

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jakubdyszkiewicz I believe ExecWithRetries should be named just Exec. These retries are needed to exec command with status code equals 0. And eventually is needed to check that response itself is correct

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh actually it is OK. Please ignore.

Copy link
Contributor

Choose a reason for hiding this comment

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

I am tempted to have "CurlWithRetries" :)

@nickolaev
Copy link
Contributor

e2e passes - merging this one.

@nickolaev nickolaev merged commit 5ec0bc3 into master Jul 29, 2020
@lobkovilya lobkovilya deleted the fix/more-retries branch July 29, 2020 14:29
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.

None yet

3 participants