[CI] Add Legacy Integration Test Workflow #1122
Conversation
This expands on the existing hack which runs the controller for integration tests by making it capable of running the legacy (v1.x) version of the controller as well. This is needed in the interim between v1.x and v2.x so that all of the integration tests run (and must pass) on both versions to reduce the risk of regressions in controller functionality.
Codecov Report
@@ Coverage Diff @@
## next #1122 +/- ##
==========================================
+ Coverage 57.13% 57.23% +0.09%
==========================================
Files 34 34
Lines 3278 3278
==========================================
+ Hits 1873 1876 +3
+ Misses 1266 1264 -2
+ Partials 139 138 -1
Continue to review full report at Codecov.
|
We were running redundant test runs on every PR, so this change provides more targeting configuration which will reduce the excess.
9ff615a to
9b55079
Compare
| if resp.StatusCode == http.StatusOK { | ||
| // now that the ingress backend is routable, make sure the contents we're getting back are what we expect | ||
| // Expected: Welcome to nginx! | ||
| b := new(bytes.Buffer) | ||
| b.ReadFrom(resp.Body) | ||
| return strings.Contains(b.String(), "Welcome to nginx!") | ||
| } | ||
| return false | ||
| }, ingressTimeout, ingressTimeoutTick) | ||
|
|
||
| // now that the ingress backend is routable, make sure the contents we're getting back are what we expect | ||
| // Expected: Welcome to nginx! | ||
| resp, err := http.Get(fmt.Sprintf("%s/nginx", u.String())) | ||
| assert.NoError(t, err) | ||
| b := new(bytes.Buffer) | ||
| b.ReadFrom(resp.Body) | ||
| assert.Contains(t, b.String(), "Welcome to nginx!") | ||
|
|
There was a problem hiding this comment.
This change and the related one below add a fix for a race condition wherein it appears (this is just a theory, I have not dug deeply into this yet) "200 OK" responses from the Kong proxy can return with an empty body for some sub-second period when routes are updated due to ingress changes.
|
The codecov failure has to do with the extra functions added for deploying a controller manager for the tests, BUT we'll be removing that functionality in an upcoming iteration: So I would contend we should merge through the failure for now. |
CI picked up this data race where the *url.URL that the test suite waits to publish for the tests to poll. Was simple, the test suite was just reading from the pointer after sending it, fixed.
This adds our new integration tests to the existing
v1.xKIC controller so that every PR testsv1andv2until we fully transition to ensure compatibility and help protect against regressions.Resolves #1040
NOTE: you'll specifically see this line when legacy tests are triggered versus mainline:
ANOTHER NOTE: this PR co-opted the existing hack which runs the controller manager as an
exec.Commandin the test suite, this has been done intentionally for speed and the follow up to improve this before GA is #1128.