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

Add Docker image #140

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Add Docker image #140

wants to merge 6 commits into from

Conversation

LuiggiTenorioK
Copy link
Member

In GitLab by @LuiggiTenorioK on Jun 3, 2024, 15:25

MR to add a docker image spec of the app

@LuiggiTenorioK LuiggiTenorioK self-assigned this Nov 12, 2024
@LuiggiTenorioK
Copy link
Member Author

In GitLab by @LuiggiTenorioK on Jun 3, 2024, 16:32

added 1 commit

Compare with previous version

@LuiggiTenorioK
Copy link
Member Author

In GitLab by @LuiggiTenorioK on Jun 4, 2024, 11:27

added 1 commit

Compare with previous version

@LuiggiTenorioK
Copy link
Member Author

In GitLab by @kinow on Jun 6, 2024, 10:46

Commented on docker/Dockerfile line 2

Is there a bookworm too?

@LuiggiTenorioK
Copy link
Member Author

In GitLab by @kinow on Jun 6, 2024, 11:13

I think I can test this one and approve the merge request now.

@LuiggiTenorioK
Copy link
Member Author

In GitLab by @kinow on Jun 6, 2024, 11:18

Commented on docker/build.sh line 5

Oh, now I see the &>build.log. Could we leave the logs in the terminal? I had to open another terminal and tail -f (it's slow on my machine as it's my first time downloading the files...).

@LuiggiTenorioK
Copy link
Member Author

In GitLab by @kinow on Jun 6, 2024, 11:20

Commented on docker/README.md line 25

This will build :latest, IIRC. Probably better to use the git branch as tag too.

@LuiggiTenorioK
Copy link
Member Author

In GitLab by @kinow on Jun 6, 2024, 11:27

Commented on docker/README.md line 25

image

Yup, it's latest by default.

@LuiggiTenorioK
Copy link
Member Author

In GitLab by @kinow on Jun 6, 2024, 11:29

image

It works! Left a few comments about tag & logs redirection. But everything works, so +1. Thanks @LuiggiTenorioK

@LuiggiTenorioK
Copy link
Member Author

In GitLab by @kinow on Jun 6, 2024, 11:29

approved this merge request

@LuiggiTenorioK
Copy link
Member Author

In GitLab by @kinow on Jun 6, 2024, 11:31

Commented on docker/Dockerfile line 10

BTW, are you going to pin on a commit/version here too?

@LuiggiTenorioK
Copy link
Member Author

In GitLab by @LuiggiTenorioK on Jun 6, 2024, 11:32

Commented on docker/build.sh line 5

It could be, but that's the example script 😅

@LuiggiTenorioK
Copy link
Member Author

In GitLab by @LuiggiTenorioK on Jun 6, 2024, 11:34

Commented on docker/Dockerfile line 10

Yup, that change is going to be on cascade. First, we deal with Autosubmit, then API, then GUI.

@LuiggiTenorioK
Copy link
Member Author

In GitLab by @kinow on Jun 6, 2024, 11:41

Commented on docker/Dockerfile line 10

Sounds good! Sorry that I started the other way around 😆

@LuiggiTenorioK
Copy link
Member Author

In GitLab by @LuiggiTenorioK on Jun 12, 2024, 12:05

added 1 commit

Compare with previous version

@LuiggiTenorioK
Copy link
Member Author

In GitLab by @LuiggiTenorioK on Jun 12, 2024, 12:16

mentioned in merge request autosubmit!446

@LuiggiTenorioK
Copy link
Member Author

In GitLab by @LuiggiTenorioK on Jun 13, 2024, 16:10

An idea for the future:

We want to change API URL dynamically when doing docker run like this:

docker run -e "API_URL=https://..." --rm -d -p 8080:8080 autosubmit-gui

This is a limitation since this value is set on the build of the static files and cannot be changed easily from there.

An alternative is to set the AUTOSUBMIT_API_SOURCE=/api (as a relative path from the same host) and then proxy the desired API_URL to this subpath /api using nginx.

To do it, we can set an entrypoint.sh in the image which:

  1. Add the proxy rule if the API_URL env variable exists
  2. Starts the server
#!/bin/sh

# Replace %%_PROXY_RULE%% if API_URL exists else remove it
if [ -n "$API_URL" ]; then
    sed -i "s|%%_PROXY_RULE%%|location /api/ { proxy_pass $API_URL; }|g" /etc/nginx/nginx.conf;
else
    sed -i "s|%%_PROXY_RULE%%||g" /etc/nginx/nginx.conf;
fi

# Start nginx
exec nginx -g 'daemon off;'

@LuiggiTenorioK
Copy link
Member Author

In GitLab by @kinow on Jun 14, 2024, 10:13

That is a good idea! No objections :slight_smile:

Another idea/question, @LuiggiTenorioK; we spoke some time ago about API+GUI together (a version of GUI copied under an API folder, and point the static files of Flask to that folder).

I think that could simplify it a bit as well. That's how Cylc UI is deployed, inside the Cylc UI Service, their API (I believe we spoke about it).

Either way is fine for me :slight_smile:

@LuiggiTenorioK
Copy link
Member Author

In GitLab by @kinow on Jun 18, 2024, 19:03

Commented on docker/Dockerfile line 5

We can delete cached files.

@LuiggiTenorioK
Copy link
Member Author

In GitLab by @LuiggiTenorioK on Jun 19, 2024, 10:14

Commented on docker/Dockerfile line 5

Actually, in this case, Phase 1 is deleted and the second base is only left resulting in a final docker image of 28MB. So, no need to do it in this case.

https://docs.docker.com/build/building/multi-stage/

@LuiggiTenorioK
Copy link
Member Author

In GitLab by @kinow on Jun 19, 2024, 10:40

Commented on docker/Dockerfile line 5

Argh, missed that, sorry 😅

@LuiggiTenorioK
Copy link
Member Author

In GitLab by @LuiggiTenorioK on Jun 21, 2024, 15:18

added 1 commit

  • d984f69 - add nginx proxy by changing env variable

Compare with previous version

@LuiggiTenorioK
Copy link
Member Author

In GitLab by @LuiggiTenorioK on Jun 25, 2024, 16:05

Commented on docker/build.sh line 5

changed this line in version 6 of the diff

@LuiggiTenorioK
Copy link
Member Author

In GitLab by @LuiggiTenorioK on Jun 25, 2024, 16:05

added 1 commit

Compare with previous version

@codecov-commenter
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 70.74%. Comparing base (2310b27) to head (0484fd4).

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #140   +/-   ##
=======================================
  Coverage   70.74%   70.74%           
=======================================
  Files          52       52           
  Lines        1528     1528           
  Branches      192      192           
=======================================
  Hits         1081     1081           
  Misses        337      337           
  Partials      110      110           
Flag Coverage Δ
fast-tests 70.74% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants