Repository navigation
feat: add option to run core integration tests on kind - #493
Conversation
|
The core image and the Helm chart version are still hardcoded. For now the goal is to run and test the integration test suite in the GHA CI. |
cf1d65f to
ce2b080
Compare
335b823 to
18e9c08
Compare
2660188 to
1e5b70a
Compare
297f18d to
29fb2ce
Compare
29fb2ce to
2579512
Compare
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 3 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit f17b65a. Configure here.
| marker = request.node.get_closest_marker("dedicated_core_cluster") | ||
| if marker and getattr(request, "param", "docker-compose") == "kind": | ||
| # Prefer marker args if present, else fallback to indirect param | ||
| params = marker.args[0] if marker.args else getattr(request, "param", {}) |
There was a problem hiding this comment.
Dedicated cluster backend selection is broken
High Severity
dedicated_helm_install and dedicated_compose_install read request.param on fixtures that are not parametrized, so getattr always falls back to docker-compose. Dedicated Kind releases never deploy, multi-node Kind tests run against the shared one-node session install, and --run-kind still spins up Compose for every @mark.dedicated_core_cluster test.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit f17b65a. Configure here.
| except (socket.error, ConnectionRefusedError): | ||
| if i == 9: | ||
| raise RuntimeError(f"Failed to connect to port-forward on {local_port}") | ||
| sleep(1) |
There was a problem hiding this comment.
Port-forward wait never reports failure
Medium Severity
Connection errors in the port-forward readiness loop are caught by the inner except, so the outer handler that sleeps and raises never runs. After a few immediate refusals the function still returns a URL even when no forward is listening, and Kind tests fail later with unrelated connection errors.
Reviewed by Cursor Bugbot for commit f17b65a. Configure here.
| ], | ||
| check=True, | ||
| ) | ||
| subprocess.run(["kubectl", "delete", "ns", setup_data["ns"]], check=True) |
There was a problem hiding this comment.
Namespace delete ignores kind context
Medium Severity
kubectl delete ns omits --context, unlike the other kubectl/helm calls. If the Kind cluster already exists, setup does not switch the current context, so teardown can delete a namespace on the wrong cluster or fail because the Kind namespace is not visible.
Reviewed by Cursor Bugbot for commit f17b65a. Configure here.
5 participants
Problem
Trade Off
Both issues could be resolved while sticking to compose, but we already have a firebolt-core Helm chart published in GHCR, which makes multi-node cluster setups straight forward and undergoes continuous testing. If there are more components released via Helm charts in the future, they can easily be integrated. Kind has the downside that we need to work with
kubectl port-forwardto reach the core containers, so it's another layer in the test setup.Implementation
--run-composeor--run-kindor both. This approach was also chosen to show that there are no breaking changes.kind_app_setup, as the CORE_URL is only known after we installed the Helm chart and ran kubectl port-forwardNote
Low Risk
Changes are limited to integration-test harness and CI workflows; production SDK code is untouched, though Core CI behavior shifts away from the reusable setup action.
Overview
Adds in-repo Firebolt Core provisioning for integration tests so CI and local runs no longer depend on the external
action-setup-coreworkflow. Core can be exercised via--run-compose(generated Docker Compose stack, including optional--run-httpsthrough nginx + self-signed certs) or--run-kind(sessionkindcluster + Firebolt Core Helm chart withkubectl port-forward).tests/integration/conftest.pyis reworked around anapp_setupfixture that parametrizes backends and injectsCORE_URL/ related env vars; shared session installs are complemented by@mark.dedicated_core_clusterfor per-test clusters (e.g. multi-node). CIintegration-tests-core.ymlruns Compose HTTP/HTTPS and a new Kind HTTP job; CONTRIBUTING documents the new pytest flags.Reviewed by Cursor Bugbot for commit f17b65a. Bugbot is set up for automated code reviews on this repo. Configure here.