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

fixup: Dockerfile: reinstate location of config.toml #271

Merged
merged 1 commit into from
Jan 2, 2022

Conversation

jtagcat
Copy link
Contributor

@jtagcat jtagcat commented Jan 2, 2022

readd WORKDIR /app
config.toml uses relative path, from the current directory, $PWD by default is /.

commit f337f6c
removed WORKDIR /app, and /config.toml was used instead of /app/config.toml.
The change was breaking, unexpected and undocumented.
As users already mount /app/config.toml, revert to using it.

Closes #270

@jtagcat
Copy link
Contributor Author

jtagcat commented Jan 2, 2022

Alternatively, PODSYNC_CONFIG_PATH env could be set inside the Dockerfile.

@mxpv
Copy link
Owner

mxpv commented Jan 2, 2022

COPY podsync /podsync needs to be changed as well, so podsync can locate the config file

@jtagcat
Copy link
Contributor Author

jtagcat commented Jan 2, 2022

It does not need to be changed, the binary's location does not affect working directory (from what config.toml is taken from).

I tried to test with:

  1. docker cp <running image of 2.4.1>:/podsync podsync/
  p.c7.ee:
#    image: mxpv/podsync:v2.4.1
#    build: github.com/jtagcat/podsync#patch-1
    build: podsync/ # git checked out at host $PWD/podsync
    volumes:
      - /barfoo:/app/data:rw
      - /foobar/config.toml:/app/config.toml:ro

it worked (though comments in Dockerfile need to be on newline)

Now, I have no idea how you get the binary in to the Dockerfile, make build builds a binary to bin/podsync, but copying that to the container, running the binary in the container results in standard_init_linux.go:228: exec user process caused: no such file or directory

@jtagcat
Copy link
Contributor Author

jtagcat commented Jan 2, 2022

anyw, ready to merge, tested

@mxpv
Copy link
Owner

mxpv commented Jan 2, 2022

Can you do (original version):

WORKDIR /app/
RUN wget ...
COPY podsync /app/podsync

ENTRYPOINT ["/app/podsync"]

data directory path needs to be compatible as well, as I guess it's better to just revert to the original version

`config.toml` uses relative path, from the current directory, `$PWD` by default is `/`.

commit f337f6c
removed `WORKDIR /app`, and `/config.toml` was used instead of `/app/config.toml`.
The change was breaking, unexpected and undocumented.
As users already mount `/app/config.toml`, revert to using it.

Revert 16315c0
(what was workaround for breaking change,
though there is no need to break anything here)

Closes mxpv#270

+ requested: podsync located again at /app/podsync, instead of /podsync
@jtagcat
Copy link
Contributor Author

jtagcat commented Jan 2, 2022

As requested.

@mxpv mxpv merged commit 656baa0 into mxpv:main Jan 2, 2022
@mxpv
Copy link
Owner

mxpv commented Jan 2, 2022

Thanks

@jtagcat jtagcat deleted the patch-1 branch January 2, 2022 15:45
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.

App broken updating just now
2 participants