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

Dockerfile has wrong parameters in final startup command #561

Open
p4jo opened this issue Jan 5, 2023 · 8 comments · May be fixed by #611
Open

Dockerfile has wrong parameters in final startup command #561

p4jo opened this issue Jan 5, 2023 · 8 comments · May be fixed by #611

Comments

@p4jo
Copy link

p4jo commented Jan 5, 2023

In the last line of the Dockerfile, it should be

CMD ["vzlogger", "--foreground", "-c", "/cfg/vzlogger.conf"]

instead of

CMD ["vzlogger", "--foreground"]

because, else it doesn't work when simply following the instructions in README.md.

@r00t-
Copy link
Contributor

r00t- commented Jan 5, 2023

[removed]

@r00t-
Copy link
Contributor

r00t- commented Jan 5, 2023

refering to this content of README.md:

docker run --restart=always -v /home/pi/projects/vzlogger-docker:/cfg \

added in e6c98fd

i wonder if it makes sense to hardcode the /cfg path in the dockerifle,
as it wiill also have to be provided by the user in the invocation.

@m-reuter: can you provide your reasoning on usage of the dockerfile?

@syntetic81
Copy link

i run in this issue!
i get it working with:
docker run --restart=always -v /home/pi/projects/vzlogger-docker:/etc

@r00t-
Copy link
Contributor

r00t- commented Nov 9, 2023

i have no opinion on this, as i don't use docker and don't see who would,
but overmounting /etc seems a litte more sensible than pointing vzlogger to /cfg.

@r00t- r00t- linked a pull request Nov 9, 2023 that will close this issue
@m-reuter
Copy link
Contributor

m-reuter commented Nov 9, 2023

Hi, either is fine. It seems that vzlogger now looks for the config in /etc and maybe earlier versions looked for it in /cfg ? That would at least explain why it may have worked originally.
Overwriting /etc may not be a great idea, because there are other files in /etc that would be missing and maybe there are mounted files that interfere with something.

So I would prefer the other solution, always forcing users to mount to /cfg and then hard coding that path in the docker image , as /cfg is not used for anything else. But again, either approach should work.

@r00t-
Copy link
Contributor

r00t- commented Nov 14, 2023

we are building an image intended to only run vzlogger,
i think the only relevant file in /etc would be resolv.conf (as vzlogger uses the network),
and that is being injected by docker anyway.
just wonder if having a mountpoint at /etc breaks that mechanism.

[~]$ docker run -it -v testtest:/etc:nocopy debian:bookworm
bash-5.2# ls /etc
hostname  hosts  resolv.conf

docker writes the injected configs to the mounted volume, no idea if this is an issue.

@r00t-
Copy link
Contributor

r00t- commented Nov 14, 2023

It seems that vzlogger now looks for the config in /etc and maybe earlier versions looked for it in /cfg ? That would at least explain why it may have worked originally.

even the plain C version >12 years ago had /etc/vzlogger.conf:
d125fd5#diff-db6d72b498f4a0a0754cf614c6141dc0380706687e9ed5549b2041a8864c446eL310

i`m waiting for a user of the dockerfile to explain if it ever worked for them.

@conmarti
Copy link

IMHO this could never have worked.
The solution I came up with is:
docker run -d --restart=always -v /home/pi/projects/vzlogger-docker/vzlogger.conf:/etc/vzlogger.conf vzlogger
which makes sure, that only the file vzlogger.conf is "bind mounted" into the container and everything else in /etc is left untouched.

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 a pull request may close this issue.

5 participants