karapace icon indicating copy to clipboard operation
karapace copied to clipboard

Add a dev target to Dockerfile, add a dev compose file

Open gpkc opened this issue 11 months ago • 7 comments

About this change - What it does

  • Add a dev target to Dockerfile which will install dev and static-check dependencies
  • Set up a volume that syncs local changes into the running containers
  • Create a new compose service for barebones development and running tests

What this enables:

  • Running tests: for example docker-compose -f compose.yml -f compose-dev.yml run karapace-dev pytest -s -vvv tests/unit
  • Update containers with new code without having to rebuild everything (docker-compose down followed by docker-compose up is enough)

Why this way

See https://github.com/Aiven-Open/karapace/issues/585.

  • We add a couple new build targets: dev-builder and dev.
    • The new builder target is needed for installing the dev dependencies, since it's necessary to build wheels, and the non-slim docker base image has the necessary python-dev dependency among others.
    • The new dev target is needed for isolation. It will have the development deps installed and also changes PYTHONPATH to point to the synced folder.
  • We are syncing the sources folder from the local machine into the container via volumes in the dev compose file.
    • This way, there is no need to rebuild everything every time the source changes.
    • In conjunction with PYTHONPATH being set to this folder, every instance of e.g. python3 -m karapace ... or import karapace will be seeing the synced folder.
  • We add a new compose service: karapace-dev
    • Theoretically it would be possible to run tests using the existing services karapace-registry and karapace-rest, but that would require overriding their configurations in a manner that would mix things up too much. Instead, I've opted to create a new service that has the clear purpose of being used directly in the test-develop cycle.

In the future, one can also add a watcher to the running servers, so that not even restarting the container is necessary.

gpkc avatar Mar 11 '24 17:03 gpkc

Funny logs from the smoke test ... 🤪

container container-karapace-dev-1 exited (0)
Error: Process completed with exit code 1.

aiven-anton avatar Mar 12 '24 16:03 aiven-anton

Hmm why did that happen. Is the test checking if the services run indefinitely or something like that? So if they end, it fails

gpkc avatar Mar 12 '24 19:03 gpkc

Hmm why did that happen. Is the test checking if the services run indefinitely or something like that? So if they end, it fails

No, the test just checks a single basic API call returns successfully, but the CI flow did not reach the test stage, it fails at the build stage. I think the logs just looks funny because of concurrency.

I think because we introduce the special dev service in the compose file, let's leave karapace-registry and karapace-rest intact and running the prod target? I think we should also update the smoke test to only start karapace-registry and karapace-rest.

It could perhpas also make sense to add something like this to the smoke test to give confidence test dependencies aren't installed in the default target:

if ! docker compose exec karapace-registry which pytest >/dev/null; then
    echo 'Error: test dependencies found in production image'
    exit 1
fi

aiven-anton avatar Mar 13 '24 08:03 aiven-anton

I think because we introduce the special dev service in the compose file, let's leave karapace-registry and karapace-rest intact and running the prod target? I think we should also update the smoke test to only start karapace-registry and karapace-rest.

The main issue with that for me is the development cycle, as you will be required to rebuild the image every time you make a change on the code. I guess theoretically I could just move the PYTHONPATH setting to the compose file, I'll see if that works fine

gpkc avatar Mar 13 '24 08:03 gpkc

The main issue with that for me is the development cycle, as you will be required to rebuild the image every time you make a change on the code. I guess theoretically I could just move the PYTHONPATH setting to the compose file, I'll see if that works fine

But wouldn't you always develop locally against the new dev service?

The smoke test must run the production target. If we introduce some separate means to achieve that, it's completely fine either way IMO (separate service, or separate compose file).

aiven-anton avatar Mar 13 '24 08:03 aiven-anton

But wouldn't you always develop locally against the new dev service?

I'm not sure, perhaps you want to make changes and quickly check them in the running REST or registry services? The dev service doesn't contain any of the env vars, etc which are present in the other services

That's why I'd say it could be useful to have the -dev.yml compose, so that the normal compose.yml can be used if you want to have an environment that is closer to prod (e.g. for smoke tests), whereas the compose-dev.yml would modify the target to be dev for fast local development

gpkc avatar Mar 13 '24 08:03 gpkc

@gpkc I think your reasoning is sound, my knee-jerk reaction against multiple compose files was just for attempting to keeping things simpler.

aiven-anton avatar Mar 21 '24 14:03 aiven-anton