e2e: reload test #1683
e2e: reload test #1683
Conversation
Co-authored-by: tony-2023 <138949958+tony-2023@users.noreply.github.com> Signed-off-by: Ronnie Personal <76408835+Ronnie-personal@users.noreply.github.com>
Signed-off-by: Ronnie Personal <76408835+Ronnie-personal@users.noreply.github.com>
Codecov Report
@@ Coverage Diff @@
## main #1683 +/- ##
==========================================
+ Coverage 60.40% 60.54% +0.13%
==========================================
Files 86 86
Lines 12737 12737
==========================================
+ Hits 7694 7711 +17
+ Misses 4545 4533 -12
+ Partials 498 493 -5 |
Co-authored-by: tony-2023 <138949958+tony-2023@users.noreply.github.com> Signed-off-by: Ronnie Personal <76408835+Ronnie-personal@users.noreply.github.com>
test/e2e/tests/reload.go
Outdated
| fmt.Printf("Port forwarding started. Access the service locally at: localhost:%d\n", localPort) | ||
|
|
||
| // Perform an HTTP GET request to the forwarded port | ||
| resp, err := http.Get(fmt.Sprintf("http://localhost:%d/config_dump", localPort)) |
There was a problem hiding this comment.
can we also include eds ?
https://www.envoyproxy.io/docs/envoy/latest/operations/admin#get--config_dump?include_eds
test/e2e/tests/reload.go
Outdated
| var numReloads = 5 | ||
|
|
||
| for i := 0; i < numReloads; i++ { | ||
| // Step 2: Restart or reload the Envoy Gateway |
There was a problem hiding this comment.
can we run step2 and (step3 and step4) in an async way (in a go routine) to ensure that state while reloading doesnt change versus state after reloading doesnt change ?
|
hey @Ronnie-personal the PR is looking good, left some comments |
Signed-off-by: Ronnie Personal <76408835+Ronnie-personal@users.noreply.github.com>
Thank you for the comments, I will check them out. Thanks. |
Signed-off-by: Ronnie Personal <76408835+Ronnie-personal@users.noreply.github.com>
Signed-off-by: Ronnie Personal <76408835+Ronnie-personal@users.noreply.github.com>
Co-authored-by: tony-2023 <138949958+tony-2023@users.noreply.github.com> Signed-off-by: Ronnie Personal <76408835+Ronnie-personal@users.noreply.github.com>
Signed-off-by: Ronnie Personal <76408835+Ronnie-personal@users.noreply.github.com>
Signed-off-by: Ronnie Personal <76408835+Ronnie-personal@users.noreply.github.com>
Signed-off-by: Ronnie Personal <76408835+Ronnie-personal@users.noreply.github.com>
Signed-off-by: Ronnie Personal <76408835+Ronnie-personal@users.noreply.github.com>
Signed-off-by: Ronnie Personal <76408835+Ronnie-personal@users.noreply.github.com>
Co-authored-by: tony-2023 <138949958+tony-2023@users.noreply.github.com> Signed-off-by: Ronnie Personal <76408835+Ronnie-personal@users.noreply.github.com>
|
@arkodg But one existing e2e test TestE2E/RateLimitBasedJwtClaims failed, here is the log I'm not sure it's an intermittent issue or not. Could you help to assess? Thanks. |
|
/retest |
|
|
||
| // Step 4: Compare the obtained `/config_dump` output with the initial configuration | ||
| // Define options for comparison | ||
| sortOpts := cmpopts.SortSlices(func(a, b interface{}) bool { |
There was a problem hiding this comment.
why are we not comparing everything and why are we sorting ?
There was a problem hiding this comment.
It fails if we compare without sorting.
Cluster 'accesslog' and cluster 'tracing' are the two values in 'synamic_endpoint_configs' array. The contents are same, but the order is different.
Please find more detailed log at https://github.com/envoyproxy/gateway/actions/runs/5638125714/job/15272037648#step:6:4166
There was a problem hiding this comment.
this might be because we are marshaling into a map, is there a need to do that, can't we just compare the o/p of /config_dump as a string ?
There was a problem hiding this comment.
Sure I will double check on that and run one more test.
There was a problem hiding this comment.
@arkodg
Please refer to the log from another draft PR, the test failed when comparing either bytes or string.
pr: https://github.com/envoyproxy/gateway/pull/1722/files#diff-7ceb4e766433e95b4b40c8b6cd6996a2c2146242d471d0eddee208b366b74aa3R82
log: https://github.com/envoyproxy/gateway/actions/runs/5685198922/job/15409803355?pr=1722#step:6:2685
There was a problem hiding this comment.
@arkodg
I would like to follow up on this comment if you don't mind. Please let me know if you have any other questions.
Thanks.
There was a problem hiding this comment.
@arkodg
I would like to follow up once again. Please let me know if there is anything else I can do for this PR.
Thanks.
There was a problem hiding this comment.
hi @Ronnie-personal, we need to investigate to see why these values are changing
There was a problem hiding this comment.
hi @Ronnie-personal, we need to investigate to see why these values are changing
Thanks for the comment, I will think about it.
I guess this PR #1798 will fix the issue of values changing?
|
This pull request has been automatically marked as stale because it has not had activity in the last 30 days. Please feel free to give a status update now, ping for review, when it's ready. Thank you for your contributions! |
|
This pull request has been automatically marked as stale because it has not had activity in the last 30 days. Please feel free to give a status update now, ping for review, when it's ready. Thank you for your contributions! |
What type of PR is this? chore: add E2E reload test
What this PR does / why we need it: Define multiple HTTPRoute, TLSRoute. Restart Envoy gateway and check if Envoy proxy configurations change or not
Which issue(s) this PR fixes: #1503
Fixes #