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

Report an error if proxy does not receive any requests #320

Merged
merged 2 commits into from
Aug 25, 2023

Conversation

roobre
Copy link
Collaborator

@roobre roobre commented Aug 24, 2023

Description

Fixes #223

Checklist:

  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works.
  • I have run linter locally (make lint) and all checks pass.
  • I have run tests locally (make test) and all tests pass.
  • I have run relevant e2e test locally (make e2e-xxx for agent, disruptors, kubernetes or cluster related changes)
  • Any dependent changes have been merged and published in downstream modules

Extra:

  • Naive manual test

@roobre
Copy link
Collaborator Author

roobre commented Aug 24, 2023

Ran a manual test of this as well, by pointing the disruptor to a service unrelated to SUT. Tt seems to be working fine. New error looks like the following:

roobre@Archiroo  ±main ●● ☸ minikube
15:49:19 ~/Devel/quickpizza $> ~/Devel/xk6-disruptor/build/k6 run k6/disruptor/01.basic.js

          /\      |‾‾| /‾‾/   /‾‾/
     /\  /  \     |  |/  /   /  /
    /  \/    \    |     (   /   ‾‾\
   /          \   |  |\  \ |  (‾)  |
  / __________ \  |__| \__\ \_____/ .io

  execution: local
     script: k6/disruptor/01.basic.js
     output: -

  scenarios: (100.00%) 2 scenarios, 6 max VUs, 10m30s max duration (incl. graceful stop):
           * disrupt: 1 iterations shared among 1 VUs (maxDuration: 10m0s, exec: disrupt, gracefulStop: 30s)
           * load: 5 looping VUs for 30s (startTime: 5s, gracefulStop: 30s)

ERRO[0040] GoError: error injecting fault: failed command execution for pod "ingress-nginx-controller-68fb8cf9cc-2gcx9": command terminated with exit code 1
disruptor did not receive any request

	at reflect.methodValueCall (native)
	at disrupt (file:///home/roobre/Devel/quickpizza/k6/disruptor/01.basic.js:37:31(30))  executor=shared-iterations scenario=disrupt source=stacktrace

     ✗ status is 200
      ↳  90% — ✓ 127 / ✗ 14

     checks.........................: 90.07% ✓ 127      ✗ 14
     data_received..................: 84 kB  2.1 kB/s
     data_sent......................: 46 kB  1.1 kB/s
     http_req_blocked...............: avg=29.54µs min=2.01µs  med=8.19µs  max=739.7µs  p(90)=10.33µs  p(95)=11µs
     http_req_connecting............: avg=8.7µs   min=0s      med=0s      max=264.16µs p(90)=0s       p(95)=0s
     http_req_duration..............: avg=93.27ms min=6.39ms  med=84.28ms max=566.62ms p(90)=161.34ms p(95)=181.88ms
       { expected_response:true }...: avg=83.91ms min=6.39ms  med=78.98ms max=422.07ms p(90)=133.86ms p(95)=170.03ms
     http_req_failed................: 9.92%  ✓ 14       ✗ 127
     http_req_receiving.............: avg=94.25µs min=28.25µs med=99.16µs max=267.29µs p(90)=132.76µs p(95)=137.19µs
     http_req_sending...............: avg=50.13µs min=11.7µs  med=48.41µs max=176.73µs p(90)=69.59µs  p(95)=83.29µs
     http_req_tls_handshaking.......: avg=0s      min=0s      med=0s      max=0s       p(90)=0s       p(95)=0s
     http_req_waiting...............: avg=93.13ms min=6.34ms  med=84.14ms max=566.51ms p(90)=161.22ms p(95)=181.84ms
     http_reqs......................: 141    3.489252/s
     iteration_duration.............: avg=1.37s   min=1s      med=1.08s   max=40.4s    p(90)=1.16s    p(95)=1.19s
     iterations.....................: 142    3.513998/s
     vus............................: 1      min=1      max=6
     vus_max........................: 6      min=6      max=6


running (00m40.4s), 0/6 VUs, 142 complete and 0 interrupted iterations
disrupt ✓ [======================================] 1 VUs  00m40.4s/10m0s  1/1 shared iters
load    ✓ [======================================] 5 VUs  30s

Manual test for a correctly configured scenario reports no error, as expected.

@roobre roobre marked this pull request as ready for review August 24, 2023 13:53
Copy link
Collaborator

@pablochacin pablochacin left a comment

Choose a reason for hiding this comment

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

LGTM.

@pablochacin
Copy link
Collaborator

I only have a concern regarding the UX. Seeing the test, which basically does not send any traffic and yet generates an error, maybe this behavior is too strict. I can image other cases such as a test with many target pods but that generates a few requests. There's a chance some pods do not receive any traffic and make the test fail

maybe we should add an option to control this behavior and make it optional. The option should be added to the fault injection options and passed all the way down to the agent via a parameter in the fault injection.

@roobre
Copy link
Collaborator Author

roobre commented Aug 25, 2023

The option should be added to the fault injection options and passed all the way down to the agent via a parameter in the fault injection.

@pablochacin I agree with the UX concern you mention: This error can be helpful in some cases but an annoyance in others. Would you consider this a blocker for this PR, or something to be discussed after it?

@pablochacin
Copy link
Collaborator

@roobre I approved the PR because I don't think this issue should be blocking. I will add an issue about the UX problem.

@roobre roobre merged commit 0ca1f0e into main Aug 25, 2023
7 checks passed
@roobre roobre deleted the error-no-requests branch August 25, 2023 11:07
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.

Fail fault injection if no traffic has been intercepted
2 participants