add integration testing#163
Conversation
ted-ross
left a comment
ted-ross
left a comment
There was a problem hiding this comment.
There is a typo in the second bullet in the PR description.
This PR looks very good. I would like Fernando to also provide a review.
Thanks! I will wait for @fgiorgetti to review before merging. One question. In the skupper-install script, I used the multi-van router image from the tedlross quay repo. It looks like we swiched to using the main image from the skupper repo? I also pulled all the CRDs from Fernando's repo (I had a recent CRD regression that caused all sorts of issues, so I may have overcompensated by pulling all of them). If either or you wouldn't mind taking a look at those and letting me know what the correct images and CRDs are, that would be great. It may be worth discussing having a more centralized location for these. The getting-started doc is currently out of sync, so there doesn't seem to really be a great source of truth for what we need to be running. |
End-to-end Vitest specs against a local Kind cluster with Keycloak, cert-manager, Postgres, Skupper, and backbone site bootstrap coverage. Co-authored-by: Cursor <cursoragent@cursor.com>
6cafe1d to
619a00b
Compare
fgiorgetti
left a comment
fgiorgetti
left a comment
There was a problem hiding this comment.
A few minor comments only, but overall it seems to work well.
I noticed that once I start the kind cluster and immeditally run the integration tests, it is failing 1 test only once, but if I run it again, it passes:
❯ integration tests/integration/kind/specs/backbone-bootstrap.test.js (8 tests | 1 failed) 10203ms
✓ Postgres has backbone site seeded and bootstrapped 414ms
× site-controller deployment is available in site-a 57ms
✓ skupper-router pod is running in site-a 49ms
✓ site-controller startup log markers are present 114ms
✓ GET /api/v1alpha1/hostnames returns manage access point ingress bundle 3031ms
✓ manage access point ingress appears in Postgres after state sync 906ms
✓ manage access-point TLS secret exists in site namespace 112ms
✓ management-controller logs AMQP connection to site manage access point 163ms
✓ integration tests/integration/kind/specs/mgmt-health.test.js (7 tests) 3956ms
✓ Postgres schema tables exist 2260ms
✓ Configuration seed row is present 489ms
✓ ManagementControllers row created on startup 521ms
✓ integration tests/integration/kind/specs/mgmt-auth-api.test.js (6 tests) 1597ms
✓ POST /backbones creates a backbone and DELETE removes it 601ms
✓ integration tests/integration/kind/specs/mgmt-certs.test.js (5 tests) 632ms
Another comment that is not appropriate for this PR, but something for an upcoming enhancement just to keep in mind (like when we enable integration tests to run through the CI), would be to add a test reporter, to write results in JSON/JUnit format. As those resources could be stored so we can compare results against separate runs.
| * @param {string} [namespace] | ||
| * @param {number} [timeoutMs] | ||
| */ | ||
| export async function waitForRunningPod( |
There was a problem hiding this comment.
Maybe move this to kubectl.js
| } | ||
| try { | ||
| const logs = kubectl( | ||
| ["logs", pod.metadata.name, "-c", "config-init", "--tail=20"], |
There was a problem hiding this comment.
should it be static to config-init?
There was a problem hiding this comment.
This can probably made more general, especially if moved to kube.js. Would the suggestion here be to find any init containers that didn't complete and grab logs from those, rather than hardcoding the config-init container?
There was a problem hiding this comment.
Correct. Get the list of initContainer names and eventually iterate through them.
| # shellcheck source=../config.sh | ||
| source "${ROOT}/config.sh" | ||
|
|
||
| echo "Ensuring namespace ${SITE_NAMESPACE}..." |
There was a problem hiding this comment.
What is the purpose of ensuring SITE_NAMESPACE ?
There was a problem hiding this comment.
Only necessary here in conjunction with the router delete below. This can be moved to the bootstrapping function if the router deletion is removed.
There was a problem hiding this comment.
Alright, I will leave it up to you.
| kubectl --context "${KUBECTL_CONTEXT}" -n "${SKUPPER_NAMESPACE}" rollout status deployment/skupper-controller --timeout=300s | ||
|
|
||
| echo "Removing router deployments created with stock kube-adaptor (controller will recreate them)..." | ||
| for ns in "${SITE_NAMESPACE}" "${NAMESPACE}"; do |
There was a problem hiding this comment.
Is this clean up really needed here?
There was a problem hiding this comment.
No this can be removed. I think the only situation where it would do anything would be if you were manually playing around with the cluster and deployed a router with different images. Rerunning cluster:up would ensure they all had the same images afterwards. Probably not a likely edge case though...
Hmm, I think I see why this might be happening, though I haven't been able to replicate the issue. This is a good suggestion. I'll make note of it in #157 |
3 participants
Adds Kind-based end-to-end integration tests for the management stack and backbone site bootstrap. Tests run via Vitest against a local Kind cluster with PostgreSQL, cert-manager, Keycloak, management-controller, Skupper (multi-van), and site-controller. Test coverage is outlined in the README in /tests/integration/kind.
Testing:
pnpm installpnpm test:integration:localto spin up a Kind cluster, run integration tests, and tear down the cluster; ORpnpm cluster:up,pnpm test:integration, andpnpm cluster:downto tear down the clusterDepends on #161
Part of #126