-
Notifications
You must be signed in to change notification settings - Fork 4.5k
connector-ci: Use Dagger in CAT #27858
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Comments
Well laid out @alafanechere 👏 Some great benefits here are
So far im a tentative thumbs up! One question, Is there any other known challenges to "dagger in dagger"? |
+1 from me too! The benefits are real, and the more airbyte code we can standardize, the better. I'd suggest time boxing this. If there are dagger-in-dagger problems, let's only spend a few days on this, as it is nice-to-have, not required |
Thank you for your inputs @bnchrch and @evantahler! |
UpdatesThe hypothesis we came up with the Dagger team was the following: I refactored the CAT code to use a single Dagger sessions.
Running locally, the |
I provided the Dagger team with access to |
Currently testing with nightly builds... |
Moving this back to the sprint backlog as I want to focus on other task force issue and #28656 fixed a lot of transient CAT failure. |
Context
Our Connector Acceptance Tests (CAT) project is a python program running tests on our connector docker images.
These tests execute commands on the container and perform assertions on the output.
We containerize CAT and ran it so far with airbyte-ci using a docker-in-docker pattern:
Problems
We'd love to not rely on the docker-in-docker pattern for the following reasons:
Solution
To get rid of the docker-in-docker pattern, we'd love to make CAT use Dagger.
Docker interactions with containers in CAT are located in a single class: ConnectorRunner.
Implementation
In CAT I refactored ConnectorRunner class to use Dagger instead of docker commands.
In airbyte-ci I refactored our
with_connector_acceptance_test
function to build the CAT container and mount the connector image tarball to it.How airbyte-ci shares the connector under test image / container with CAT:
We share the build connector image from airbyte-ci by mounting the tarball file to the CAT container and use
dagger_client.container().import_(<tarball_file>)
to create the connnector container.I originally tried to share the connector container by sharing the ContainerID to CAT and instantiate the container in CAT with
dagger_client.container(dagger.ContainerId(<container_id>))
but it didn't work I occassionally face error likefailed to load cache key: unable to get info about digest: NotFound: rpc error: code = NotFound desc = content sha256:7e9d2045e9891e8eef3c917af3a5246a292c2b2c8c7f8c0d0b76a357d5439ea8:
.Current blockers
It's not working for heavy images
The current implementation works for lightweight images (~50mb). But when I try to run CAT on a heavy images (~500mb) the test execution hangs...
My hypothesis is that the
import_
is a costly operation, and as we instantiate a new Dagger connection for each exec we want on to run on a container, we pay the cost of the import for each exec. We instantiate a new Dagger connection for each exec because the public methods ofConnectorRunner
function are synchronous, so we runanyio.run
inside them. I'd prefer to not change the public methods ofConnectorRunner
to be async as it would require to change the whole CAT codebase to be async.The text was updated successfully, but these errors were encountered: