fix e2e EnvoyShutdown

[shawnh2]

What type of PR is this?

What this PR does / why we need it:

• add test into codecov ignore path
• trying to fix flaky: e2e TestEGUpgrade/EnvoyShutdown #3262 by increasing the HTTPRoute Duration and move load generation after the deployment has been restarted
• fix the order of merge-gateways e2e test in makefile

Which issue(s) this PR fixes:

Fixes #3262

[shawnh2]

/retest

[guydc]

move load generation after the deployment has been restarted

Hi @shawnh2! The purpose of the test is to run load during the restart and assert that there's no loss of requests... Running it after restart means that we're not really testing for hitless shutdown anymore..

[shawnh2]

move load generation after the deployment has been restarted

Hi @shawnh2! The purpose of the test is to run load during the restart and assert that there's no loss of requests... Running it after restart means that we're not really testing for hitless shutdown anymore..

Thanks for the clarification, by root causing this e2e test case, sometimes after the restart process, the url that one request send to may be a 404 path, meaning there are some loss requests.

Maybe by sending another request to detect the url path right after the restart process can avoid this situation.

[alexwo]

move load generation after the deployment has been restarted

Hi @shawnh2! The purpose of the test is to run load during the restart and assert that there's no loss of requests... Running it after restart means that we're not really testing for hitless shutdown anymore..

Thanks for the clarification, by root causing this e2e test case, sometimes after the restart process, the url that one request send to may be a 404 path, meaning there are some loss requests.

Maybe by sending another request to detect the url path right after the restart process can avoid this situation.

Maybe the upgrade process isn't hitless anymore, I hadn't noticed the 404 issue before, so perhaps that's a new thing. Perhaps it's worth implementing a temporary workaround to prevent instability, but we should also address the underlying issue that causes some requests to result in a 404 error during an upgrade.

[guydc]

Hi @shawnh2 , @alexwo - that's interesting input. Yes, I assume that if failures are 404s, then we have a problem with new envoy proxies receiving traffic before they are programmed.

In that case, I think we can split this activity to two parts:

• Run load and log load stats, but only assert on success of converging single-user requests. Basically, only make sure that restarted proxies are eventually functional and stop this test from being flaky.
• Investigate how to fix the underlying issue causing 404s.

WDYT?

[alexwo]

Hi @shawnh2 , @alexwo - that's interesting input. Yes, I assume that if failures are 404s, then we have a problem with new envoy proxies receiving traffic before they are programmed.

In that case, I think we can split this activity to two parts:

• Run load and log load stats, but only assert on success of converging single-user requests. Basically, only make sure that restarted proxies are eventually functional and stop this test from being flaky.
• Investigate how to fix the underlying issue causing 404s.

WDYT?

It's also possible that existing proxies fail, as they may refresh their cache when attempting to obtain a new cache snapshot version from newly started EGs. Maybe something as initialDelaySeconds suggested here can help:
#2810 (comment)

Or potentially:
#2918

I'm not sure that EG infra controller, will re-deploy proxies in every EG upgrade test, perhaps only if there are changes that require re-deployment?

[zirain]

this test skiped in #3306, can you remove this SkipTests?

gateway/test/e2e/upgrade/eg_upgrade_test.go

Line 56 in d32256c

tests.EnvoyShutdownTest.ShortName, // https://github.com/envoyproxy/gateway/issues/3262

[arkodg]

hey @shawnh2 is this PR still needed ?

[shawnh2]

close this for now, will raise a fix if we have a clue.