Skip to content

Add simple docker-compose file #2574

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

Closed
wants to merge 17 commits into from
Closed

Conversation

kaansk
Copy link
Contributor

@kaansk kaansk commented Sep 12, 2022

This PR adds a simple docker-compose file with VAST and integrates tests with Github CI.

📝 Reviewer Checklist

Review this pull request by ensuring the following items:

  • All user-facing changes have changelog entries
  • User-facing changes are reflected on vast.io

Also note that:

  • docker-compose version in CI is not explicitly provided.
  • docker-compose has no build instructions to keep it simple. Contributors/developers can use it with docker build and docker-compose
  • docker-compose up -d should bring a working VAST server up and user should be able to spawn VAST clients by docker-compose run ..
  • Tests under docker/*/tests.sh should be locally usable and should yield result in CI. Ex: https://github.com/tenzir/vast/actions/runs/3054458405/jobs/4926845094

@kaansk kaansk added the feature New functionality label Sep 12, 2022
Copy link
Member

@dominiklohmann dominiklohmann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is great. Just some minor things to address before we can merge this! 🚀

services:
vast:
container_name: tenzir-vast
image: tenzir/vast:${VAST_VERSION:-latest}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where did the build section go?

build:
  context: .

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I observed that even if we provide "image:..", docker-compose always starts building rather than pulling. This is not an expected behaviour, I wouldnt expect a docker-compose file in root folder to directly build when latest release is in DockerHub. We can put it back but it will increase the CI and local debugging time.

Let me know if there are alternatives we can use in docker-compose to fix this behaviour.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you put the pull_policy: build alongside the build into an override file for building you get the out-of-the-box experience that we were looking for: By default you pull, and you can opt-in to building by using the build file.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pull_policy also affects docker run and everytime tests try to docker compose run a VAST client, it tries to pull/build.

I removed the build instructions from docker-compose and included them in documentation if anyone needs to build. Behaviour seems to have side effects and complicates.

If you think we should lean towards this a bit more, I can check further. docker-compose should be easy. Building is an optional action most likely to be taken by developers/contributors.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you put the pull_policy: build alongside the build into an override file for building you get the out-of-the-box experience that we were looking for: By default you pull, and you can opt-in to building by using the build file.

Please read again—I actually got it to work as expected with this.

@kaansk kaansk self-assigned this Sep 15, 2022
@kaansk kaansk marked this pull request as ready for review September 15, 2022 07:16
@dominiklohmann
Copy link
Member

I'll be picking this work up in a new PR that builds upon the changes to the Docker workflow I made in #2585.

@dominiklohmann dominiklohmann deleted the story/sc-37375/vast-docker-compose branch September 21, 2022 12:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants