Skip to content
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

Create Dockerfile and docker workflow #2029

Closed
wants to merge 21 commits into from

Conversation

epassaro
Copy link
Member

@epassaro epassaro commented May 18, 2022

Description

  • Adds a Dockerfile with a fully functioning TARDIS.
  • Adds a workflow to build and push Docker images of TARDIS repo with the labels: master (latest changes, gets overwritten on every push to master), <tag-name> (on release), and latest (points to latest tag, on release).

Motivation and context

Run TARDIS everywhere.

How has this been tested?

  • Testing pipeline
  • Other

Locally build and run the Docker image:

docker build . -t tardis-rt/tardis:latest
docker run -p 8888:8888 tardis-rt/tardis:latest

To re-start an exited container, first check the NAMES field of your container with docker ps -a and then

docker start -ia <container_name>

Examples

Type of change

  • Bug fix
  • New feature
  • Breaking change
  • None of the above

Checklist

  • I have updated the documentation according to my changes
  • I have built the documentation by applying the build_docs label to this pull request (if you don't have enough privileges a reviewer will do it for you)
  • I have requested two reviewers for this pull request

@codecov
Copy link

codecov bot commented May 18, 2022

Codecov Report

Merging #2029 (c10f2ee) into master (958664e) will not change coverage.
The diff coverage is n/a.

❗ Current head c10f2ee differs from pull request most recent head 26c03be. Consider uploading reports for the commit 26c03be to get more accurate results

@@           Coverage Diff           @@
##           master    #2029   +/-   ##
=======================================
  Coverage   71.96%   71.96%           
=======================================
  Files         137      137           
  Lines       12514    12514           
=======================================
  Hits         9006     9006           
  Misses       3508     3508           

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@epassaro epassaro changed the title Create Dockerfile Create Dockerfile and push to Docker Hub workflow May 20, 2022
@epassaro epassaro marked this pull request as ready for review May 23, 2022 16:38
- name: Extract metadata
uses: docker/metadata-action@v4
with:
images: tardis-rt/tardis
Copy link
Member Author

Choose a reason for hiding this comment

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

I assumed this new account is going to be named tardis-rt.

@andrewfullard
Copy link
Contributor

I don't know very much about Docker, I'll try this out when I get the time

@epassaro
Copy link
Member Author

I don't know very much about Docker, I'll try this out when I get the time

Great, tell me if you have any doubt.

Do you know someone in TARDIS that knows about Docker and should be a reviewer of this PR?

@epassaro epassaro requested a review from jaladh-singhal May 27, 2022 16:54
@andrewfullard
Copy link
Contributor

I don't know very much about Docker, I'll try this out when I get the time

Great, tell me if you have any doubt.

Do you know someone in TARDIS that knows about Docker and should be a reviewer of this PR?

I do not. You should ask on Slack.

@epassaro epassaro requested review from DhruvSondhi and removed request for andrewfullard and jaladh-singhal May 31, 2022 15:02
@epassaro epassaro force-pushed the ci/create-dockerfile branch from 172d6c6 to 7576689 Compare May 31, 2022 15:03
@epassaro epassaro removed the request for review from wkerzendorf June 2, 2022 15:11
@epassaro epassaro changed the title Create Dockerfile and push to Docker Hub workflow Create Dockerfile and push-to-DockerHub workflow Jun 3, 2022
@epassaro epassaro changed the title Create Dockerfile and push-to-DockerHub workflow Create Dockerfile and docker workflow Jun 3, 2022
@epassaro epassaro added the CI/CD label Jun 5, 2022
@epassaro epassaro force-pushed the ci/create-dockerfile branch 2 times, most recently from 5d3cda8 to f27301f Compare June 8, 2022 15:23
@tardis-bot
Copy link
Contributor

tardis-bot commented Jun 8, 2022

*beep* *bop*

Hi, human.

The docs workflow has failed

Click here to see the build log.

jaladh-singhal
jaladh-singhal previously approved these changes Jun 9, 2022
Copy link
Member

@jaladh-singhal jaladh-singhal 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 a very great addition @epassaro 🎉

I tried running it locally - it works perfectly except I had to install curl otherwise quickstart notebook wasn't able to download tardis_example.yml. Perhaps, we can add following step in dockerfile:

RUN apt-get update && apt-get install -y \
curl

But I understand if we don't since quickstart notebook isn't the only thing container users are gonna use. As you know better, it's your call whether we should add this or not, hence I'm approving it!

@epassaro epassaro marked this pull request as draft June 9, 2022 13:06
@jaladh-singhal
Copy link
Member

This is a very great addition @epassaro tada
I tried running it locally - it works perfectly except I had to install curl otherwise quickstart notebook wasn't able to download tardis_example.yml. Perhaps, we can add following step in dockerfile:

RUN apt-get update && apt-get install -y \
curl

But I understand if we don't since quickstart notebook isn't the only thing container users are gonna use. As you know better, it's your call whether we should add this or not, hence I'm approving it!

That's why I'm changing the curl command to wget in #2042. wget is always present in minimal Docker images and VMs while curl is usually not. Installing curl will bloat the image with ~500MB, and since the Quickstart is there just to show TARDIS is fully functional I don't think it's worth it.

Sounds good @epassaro!

@epassaro epassaro force-pushed the ci/create-dockerfile branch from aaea3c1 to dd3c96a Compare June 2, 2023 20:40
@Knights-Templars
Copy link
Member

Closed in favor of PR #2569.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

6 participants