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

Implement a new RetryClient that wraps the Kubernetes Client with retries #547

Merged

Conversation

jbarrick-mesosphere
Copy link
Member

What type of PR is this?

/kind bug

What this PR does / why we need it:

Implement a new RetryClient that wraps the Kubernetes Client with retries

This ensures that we properly retry on network failures in our integration tests to prevent an flaky tests.

Which issue(s) this PR fixes:

Fixes #526

Does this PR introduce a user-facing change?:

NONE

…ries.

Fixes kudobuilder#526

This ensures that we properly retry on network failures in our integration tests to prevent an flaky tests.
@gerred
Copy link
Member

gerred commented Jul 10, 2019

This looks good! Do you think you'd get any value in swapping the underlying implementation to https://github.com/hashicorp/go-retryablehttp? I've had success with it in the past, but it looks like the way your doing it is a lot more Kubernetes client specific (with the status retrier). I have no preferences but wanted to make sure you were aware of it. :)

@jbarrick-mesosphere
Copy link
Member Author

jbarrick-mesosphere commented Jul 10, 2019

This looks good! Do you think you'd get any value in swapping the underlying implementation to https://github.com/hashicorp/go-retryablehttp? I've had success with it in the past, but it looks like the way your doing it is a lot more Kubernetes client specific (with the status retrier). I have no preferences but wanted to make sure you were aware of it. :)

Yeah, that looks HTTP-specific, whereas this is focused on Kubernetes. We could swap the existing Retry method for something from another library, but I implemented my own as I wasn't sure if we wanted to pull in a new dependency just for that (and the ones that I found in the Kubernetes library didn't work the way I needed / had confusing interfaces).

@gerred
Copy link
Member

gerred commented Jul 10, 2019

Nope, sgtm. I'd rather have a new function than a new dependency.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Go integration tests can return "unexpected end of JSON input"
2 participants