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

Apply timeout to kubeclient and remove KubeTimeout #106

Merged
merged 16 commits into from
Jan 26, 2021

Conversation

s4nji
Copy link
Member

@s4nji s4nji commented Jan 22, 2021

This PR:

  • Removes all uses of KubeTimeout
  • Sets a default 5 seconds timeout to controller and proxy kubeClient
    • Adds KUBE_CLIENT_TIMEOUT environment variable to configure this
  • Adds 60 seconds timeout to controller syncHandler
    • This is configurable via SYNC_HANDLER_TIMEOUT environment variable
  • Sets a default 60 seconds timeout to kubeClients in integration tests
  • Retains 30 seconds timeout (from KubeTimeout) when waiting for kube resources to appear

s4nji added 4 commits January 22, 2021 13:26
Configurable with environment variable KUBE_CLIENT_TIMEOUT_SECONDS
Most of them are fast and local, with the only potentially slow part 
being kubeClient operations
- Apply 60 seconds time limit to controller syncHandler
- Retain 30 seconds time limit for waitfor resource
@s4nji s4nji self-assigned this Jan 22, 2021
@codecov
Copy link

codecov bot commented Jan 22, 2021

Codecov Report

Merging #106 (4c4c873) into master (ee23549) will decrease coverage by 0.33%.
The diff coverage is 48.64%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #106      +/-   ##
==========================================
- Coverage   57.42%   57.09%   -0.34%     
==========================================
  Files          35       35              
  Lines        2504     2487      -17     
==========================================
- Hits         1438     1420      -18     
- Misses        952      953       +1     
  Partials      114      114              
Impacted Files Coverage Δ
cmd/api.go 0.00% <0.00%> (ø)
cmd/controller.go 18.75% <0.00%> (ø)
cmd/proxy.go 0.00% <0.00%> (ø)
pkg/controller/loadtest.go 3.58% <0.00%> (ø)
pkg/kubernetes/client.go 89.36% <0.00%> (-5.03%) ⬇️
pkg/proxy/service.go 68.75% <ø> (-1.13%) ⬇️
pkg/controller/test_helper.go 54.00% <63.63%> (-0.13%) ⬇️
pkg/proxy/proxy.go 82.24% <91.66%> (-0.71%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ee23549...4c4c873. Read the comment docs.

@s4nji s4nji force-pushed the patch/EES-5032-timeouts branch from 2bf9bc6 to b85d857 Compare January 22, 2021 15:48
@s4nji s4nji force-pushed the patch/EES-5032-timeouts branch from f5dc28a to 68ee8a2 Compare January 25, 2021 11:33
@s4nji s4nji force-pushed the patch/EES-5032-timeouts branch from fd31a83 to d621ae0 Compare January 26, 2021 09:07
@s4nji s4nji force-pushed the patch/EES-5032-timeouts branch from 5e7ef06 to 4c4c873 Compare January 26, 2021 10:17
@boekkooi-fresh
Copy link
Contributor

boekkooi-fresh commented Jan 26, 2021

👍

Approved with Zappr Approved with Zappr Approved with Zappr Approved with Zappr

@s4nji s4nji merged commit d7c4859 into master Jan 26, 2021
@s4nji s4nji deleted the patch/EES-5032-timeouts branch January 26, 2021 10:39
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