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: Remove VOLUME instruction #20

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,10 @@ COPY --from=builder ${FROM_DIRECTORY}/consoles/ /usr

RUN ln -s /usr/share/prometheus/console_libraries /usr/share/prometheus/consoles/ /etc/prometheus/
RUN mkdir -p /prometheus && \
chown -R nobody:nobody etc/prometheus /prometheus
chgrp -R 0 /etc/prometheus /prometheus && \
chmod -R g=u /etc/prometheus /prometheus

Copy link

Choose a reason for hiding this comment

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

What's the reason for changing the ownership here? I'm not sure who is the default owner of the /prometheus directory, but wouldn't this change cause prometheus to no longer be able to write to the /prometheus dir?

Copy link
Author

Choose a reason for hiding this comment

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

These two commands are recommended by official OKD docs to be set on directories you'd need to write in runtime. https://docs.okd.io/3.10/creating_images/guidelines.html#use-uid

Probably since VOLUME was used before permissions where working OK but these commands explicitly will set root group and allows it to write to this directory, even if user ID is randomly set.

Copy link

Choose a reason for hiding this comment

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

Ok, makes sense.

USER nobody
EXPOSE 9090
VOLUME [ "/prometheus" ]
Copy link

Choose a reason for hiding this comment

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

I'd like to keep the configuration in sync with upstream prometheus as much as possible, so I'll probably wait for prometheus#5050 to be merged first.

In general it seems that this case is a bit different than kubernetes/kube-state-metrics#471, since the /prometheus directory contains data that should normally be persisted between container restarts.

Copy link
Author

@aramalipoor aramalipoor Jan 8, 2019

Choose a reason for hiding this comment

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

What this change will help is to avoid creating temporary volumes (i.e. within /var/lib/docker) while the user deliberately does not want to persist data and also helps with deploying this image on cloud providers who have restricted using VOLUME instruction / temporary volumes on host disk.

WORKDIR /etc/prometheus
ENTRYPOINT [ "/bin/prometheus" ]
4 changes: 2 additions & 2 deletions Dockerfile.rhel
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,11 @@ COPY --from=builder ${FROM_DIRECTORY}/consoles/ /usr

RUN ln -s /usr/share/prometheus/console_libraries /usr/share/prometheus/consoles/ /etc/prometheus/
RUN mkdir -p /prometheus && \
chown -R nobody:nobody etc/prometheus /prometheus
chgrp -R 0 /etc/prometheus /prometheus && \
chmod -R g=u /etc/prometheus /prometheus

USER nobody
EXPOSE 9090
VOLUME [ "/prometheus" ]
WORKDIR /prometheus
ENTRYPOINT [ "/bin/prometheus" ]
CMD [ "--config.file=/etc/prometheus/prometheus.yml", \
Expand Down
4 changes: 2 additions & 2 deletions Dockerfile.rhel7
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,11 @@ RUN yum install -y prometheus && \
yum clean all
RUN ln -s /usr/share/prometheus/console_libraries /usr/share/prometheus/consoles/ /etc/prometheus/
RUN mkdir -p /prometheus && \
chown -R nobody:nobody etc/prometheus /prometheus
chgrp -R 0 /etc/prometheus /prometheus && \
chmod -R g=u /etc/prometheus /prometheus

USER nobody
EXPOSE 9090
VOLUME [ "/prometheus" ]
WORKDIR /prometheus
ENTRYPOINT [ "/bin/prometheus" ]
CMD [ "--config.file=/etc/prometheus/prometheus.yml", \
Expand Down