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

chore: use master instead of latest to pull docker image #1100

Merged
merged 5 commits into from Jan 4, 2023

Conversation

sborrazas
Copy link
Contributor

@venimus is the intention to use this master tag instead of latest? Also, I wasn't able to find the ae_mdw_dev image on dockerhub, do we need to name it?

@sborrazas sborrazas self-assigned this Dec 27, 2022
@venimus
Copy link
Member

venimus commented Dec 27, 2022

Latest is the latest official version tagged image. “Master” is the latest push to master which is essentially the dev branch. There were pr branches but was dropped. I think it is the correct way to use master image for development (as it is here in the pr) and “latest” for public deployments (which is incorrect in the pr). There is also a “sha” tag to have a history of the master pushes so we can roll back in emergency. You don’t need ae_mdw_dev as it is the same as “master”. “Latest” and v* are supposed to be compiled with prod, master and “sha” tags are compiled with dev environment. But currently all are built with prod in the dockerhub workflow, so we need to think if we need to correct it.

@@ -6,7 +6,7 @@ services:
interval: 1m
timeout: 50s
retries: 3
image: aeternity/ae_mdw:latest
image: aeternity/ae_mdw:master
Copy link
Member

Choose a reason for hiding this comment

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

This should stay “latest”

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The latest tag doesn't exist on dockerhub, and it needs to be built locally every time. And if users want to run aeternity/ae_mdw they won't be able to. We should probably figure out a way to build on a per-version basis (not branch) and create latest tag as well. I'll revert it back to latest

@@ -6,7 +6,7 @@ services:
dockerfile: ./Dockerfile
args:
MIX_ENV: dev
image: aeternity/ae_mdw_dev:master
image: aeternity/ae_mdw:latest
Copy link
Member

Choose a reason for hiding this comment

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

I think that this should be master, so you can either pull the latest master or build it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good idea. Done

@@ -0,0 +1,2 @@
[safe]
Copy link
Collaborator

Choose a reason for hiding this comment

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

why is this needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ever since we made these docker updates, I'm starting to get these errors when developing locally:

fatal: detected dubious ownership in repository at '/app'
                                                         To add an exception for this directory, call:

                                                                                                      	git config --global --add safe.directory /app
                                                                                                                                                     ** (MatchError) no match of right hand side value: {"", 128}
    (stdlib 3.14.2.2) erl_eval.erl:453: :erl_eval.expr/5
    (stdlib 3.14.2.2) erl_eval.erl:126: :erl_eval.exprs/5
    (stdlib 3.14.2.2) erl_eval.erl:449: :erl_eval.expr/5
    (elixir 1.13.4) lib/code.ex:404: Code.validated_eval_string/3
    (elixir 1.13.4) lib/config.ex:260: Config.__eval__!/3
    (mix 1.13.4) lib/mix/tasks/loadconfig.ex:54: Mix.Tasks.Loadconfig.load_compile/1

this happens when git log in the config/config.exs file is executed

Copy link
Member

@jyeshe jyeshe Jan 3, 2023

Choose a reason for hiding this comment

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

This error was actually one of the motivations to get started working again on the Docker image.
The fatal: detected dubious ownership in repository at '/app' was happening here:
#1080

Locally what I am doing and working fine is running the dev image with docker-compose -f docker-compose-dev.yml up and then connect to it with a remote shell.

Copy link
Member

@jyeshe jyeshe left a comment

Choose a reason for hiding this comment

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

I would rather continue with the purpose we've discussed where the docker-compose-dev.yml is used for feature and bug fixing development (not for the master).

@venimus
Copy link
Member

venimus commented Jan 2, 2023

I would rather continue with the purpose we've discussed where the docker-compose-dev.yml is used for feature and bug fixing development (not for the master).

I don't really get the dev workflow, but IMHO those things are not really related to the image tag. My logic is that anyway that any code changes would go to the master branch eventually. This won't be pushed to dockerhub anyway, so you could tag it as you think is most appropriate, just being latest doesn't make much sense to me, as it refers to the stable release and development is never the stable.

@jyeshe
Copy link
Member

jyeshe commented Jan 2, 2023

I would rather continue with the purpose we've discussed where the docker-compose-dev.yml is used for feature and bug fixing development (not for the master).

I don't really get the dev workflow, but IMHO those things are not really related to the image tag. My logic is that anyway that any code changes would go to the master branch eventually. This won't be pushed to dockerhub anyway, so you could tag it as you think is most appropriate, just being latest doesn't make much sense to me, as it refers to the stable release and development is never the stable.

When both build and image is provided for an image that doesn't exist on dockerhub (ae_mdw_dev), the docker-compose creates the image locally with the name provided by the image field. It's helpful to separate what is ae_mdw_dev (for dev branches) from ae_mdw (for master and release tags).

@sborrazas sborrazas merged commit 9b06e72 into master Jan 4, 2023
@sborrazas sborrazas deleted the dev-compose branch January 4, 2023 11:11
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.

None yet

4 participants