Description
| Status | Subtype | Assigned | Task | ||
|---|---|---|---|---|---|
| Declined | None | T310592 [Tracking] Resolve the API Dependabot PRs | |||
| Resolved | None | T310702 Resolve CI Failing Dependabot PRs | |||
| Resolved | Evelien_WMDE | T311205 🔷 Upgrade kubernetes from 1.21 to 1.22 | |||
| Resolved | Evelien_WMDE | T322542 🔷 Remove all instances of removed 1.22 k8s APIs | |||
| Resolved | Evelien_WMDE | T311505 🔵 Resolve Kubernetes client bump from dependabot PR |
Event Timeline
this draft has some changes I made while working on fixing this PR https://github.com/wbstack/api/pull/479
After having a look at this with Tobias, we had the feeling this will better be worked on after or during kubernetes bump task.
If you apply changes from the draft PR, you get this errors.
1) Tests\Jobs\KubernetesIngressCreateTest::testCreateIngressJobDoesNotFail TypeError: Return value of Maclof\Kubernetes\Repositories\Repository::create() must be of the type array, string returned /var/www/html/vendor/maclof/kubernetes-client/src/Repositories/Repository.php:108 /var/www/html/app/Jobs/KubernetesIngressCreate.php:112 /var/www/html/tests/Jobs/KubernetesIngressCreateTest.php:44 phpvfscomposer:///var/www/html/vendor/phpunit/phpunit/phpunit:97
The fact that the eeror comes from vendor/, give the impression it could be a version dependency thing
here is a documentation I used as a guide when making the draft https://github.com/maclof/kubernetes-client
Before we gave up on this for now we said it might make sense to pick this up again when we look at resolving T311205: 🔷 Upgrade kubernetes from 1.21 to 1.22
Looks like this is important to us now because this library is calling the old beta k8s ingress api. We should confirm that whatever version of this library we upgrade to does indeed start using the generally available version of the ingress api
From the release notes it looks like the package moved from the beta APIs from v0.25.0 on
1) Tests\Jobs\KubernetesIngressCreateTest::testCreateIngressJobDoesNotFail TypeError: Argument 3 passed to Maclof\Kubernetes\Client::__construct() must be an instance of Psr\Http\Client\ClientInterface or null, instance of GuzzleHttp\Client given, called in /var/www/html/tests/Jobs/KubernetesIngressCreateTest.php on line 44
To me it seems like there is some issue with Guzzle used because it's using a GuzzleHttp\ClientInterface instead of Psr\Http\Client\ClientInterface, which is expected by the k8s library we are using. Not sure how to resolve this at the moment, I'm missing some overview of the situation. Do we need another guzzle version? Or just using it differently is enough?
I tried to do the update on a new branch and got a different error, not sure what to make out of it yet:
$ make test FILTER=tests/Jobs/KubernetesIngressCreateTest.php docker-compose exec api vendor/bin/phpunit tests/Jobs/KubernetesIngressCreateTest.php Xdebug: [Step Debug] Could not connect to debugging client. Tried: host.docker.internal:9003 (through xdebug.client_host/xdebug.client_port) :-( PHPUnit 9.5.26 by Sebastian Bergmann and contributors. E 1 / 1 (100%) Time: 00:00.211, Memory: 28.00 MB There was 1 error: 1) Tests\Jobs\KubernetesIngressCreateTest::testCreateIngressJobDoesNotFail TypeError: Return value of Maclof\Kubernetes\Repositories\Repository::create() must be of the type array, string returned /var/www/html/vendor/maclof/kubernetes-client/src/Repositories/Repository.php:108 /var/www/html/app/Jobs/KubernetesIngressCreate.php:112 /var/www/html/tests/Jobs/KubernetesIngressCreateTest.php:44 phpvfscomposer:///var/www/html/vendor/phpunit/phpunit/phpunit:97 ERRORS! Tests: 1, Assertions: 0, Errors: 1. make: *** [Makefile:6: test] Error 2
Today I tried using php-http/guzzle7-adapter to fill a dependency requirement which I thought might work and wouldn't force us to downgrade to guzzle v6 - but that didn't work out. Used xdebug to gain some insights on what the actual problem is but I don't really know more than before just yet.
Apparently the code to call the k8s client lib has changed a bit, so after I changed it according to the current documentation it seems to work? I still don't see the ingress locally, but maybe thats just a local issue.
api-queue-65dbcf79bf-hsbtn api-queue [2022-11-11 12:02:05][DjlrFyXhlWQrWHDLfncM4ChFGXxZCJf7] Processing: App\Jobs\KubernetesIngressCreate api-queue-65dbcf79bf-hsbtn api-queue [2022-11-11 12:02:05] production.INFO: Creating k8s client api-queue-65dbcf79bf-hsbtn api-queue [2022-11-11 12:02:05] production.INFO: Getting ingress resources api-queue-65dbcf79bf-hsbtn api-queue [2022-11-11 12:02:05] production.INFO: Checking if resource exists: mediawiki-site-17 api-queue-65dbcf79bf-hsbtn api-queue [2022-11-11 12:02:05] production.INFO: Creating ingress resource api-queue-65dbcf79bf-hsbtn api-queue [2022-11-11 12:02:05][DjlrFyXhlWQrWHDLfncM4ChFGXxZCJf7] Processed: App\Jobs\KubernetesIngressCreate
Adjusted the existing test case to the format of v0.26, but it still fails https://github.com/wbstack/api/pull/479/commits/7b3aa4022111af706a958fb97fcb2952bc578fea
Today we found that the k8s manifest metadata part in our code still used the format from the old beta ingress and adapting it makes it work again: https://github.com/wbstack/api/pull/479/commits/55779725840cbb831e7f989eebb6a30bd3589e96
The k8s client php lib failed silently, which made it pretty hard to spot, therefore I created T323211.
Now it's hopefully just the test case failing.
Fixed the test! Earlier the mocked API response was just an empty 200, now it's a 201 with an empty json array: https://github.com/wbstack/api/pull/479/files#diff-466c67cf20c031afb88ea3afcffa5486a32b1e10c430b42adbc9441f74228273R37
The updated package seems to be happy with that - but we should note for the future that this doesn't actually test the creation of an ingress, but "just" the communication with the ingress creation part of the k8s API.
Deployed it to staging and created a test wiki with a custom domain - ingress got created and it is reachable
https://github.com/wmde/wbaas-deploy/pull/594
https://221121-dev-test-wbaas-custom-deer.duckdns.org/wiki/Main_Page
Sounds like an integration tests for this part of the platform API could be beneficial rather than just a unit test.