Code0x58 commented on a change in pull request #3669:
URL: https://github.com/apache/incubator-heron/pull/3669#discussion_r563189399
##########
File path: docker/dist/Dockerfile.dist.centos7
##########
@@ -34,14 +34,15 @@ RUN yum -y install epel-release \
ENV JAVA_HOME /usr/
-ADD artifacts /heron
+# run Heron installer
+RUN --mount=type=bind,source=artifacts,target=/tmp/heron
/tmp/heron/heron-install.sh && \
+ rm /usr/local/heron/dist/heron-core.tar.gz && \
+ rm -rf /opt/heron/heron-core/lib/scheduler/heron-local-scheduler.jar && \
+ rm -rf /opt/heron/heron-core/lib/scheduler/heron-mesos-scheduler.jar && \
+ rm -rf /opt/heron/heron-core/lib/scheduler/heron-slurm-scheduler.jar
Review comment:
Turns out this pattern which is seen in all the Dockerfiles since ~2017
is now junk, the `-rf` is unnecissary, and the `-f` has hidden the fact that
these files do not exist. Even if this worked it would spare ~40M but
presumably with the cost of causing less than ideal user experiences, so I
think it's fair to remove these at some point (maybe in a PR to get rid of all
`rm -f` in Dockerfiles).
##########
File path: docker/dist/Dockerfile.dist.centos7
##########
@@ -34,14 +34,15 @@ RUN yum -y install epel-release \
ENV JAVA_HOME /usr/
-ADD artifacts /heron
+# run Heron installer
+RUN --mount=type=bind,source=artifacts,target=/tmp/heron
/tmp/heron/heron-install.sh && \
+ rm /usr/local/heron/dist/heron-core.tar.gz && \
+ rm -rf /opt/heron/heron-core/lib/scheduler/heron-local-scheduler.jar && \
+ rm -rf /opt/heron/heron-core/lib/scheduler/heron-mesos-scheduler.jar && \
+ rm -rf /opt/heron/heron-core/lib/scheduler/heron-slurm-scheduler.jar
Review comment:
Turns out this pattern which is seen in all the Dockerfiles since ~2017
is now junk, the `-rf` is unnecissary, and the `-f` has hidden the fact that
these files do not exist. Even if this worked it would spare ~40M but
presumably with the cost of causing less than ideal user experiences (or just
an indicator of redundant files for removal in #3670), so I think it's fair to
remove these `rm`s at some point (maybe in a PR to get rid of all `rm -f` in
Dockerfiles).
##########
File path: docker/dist/Dockerfile.dist.centos7
##########
@@ -34,14 +34,15 @@ RUN yum -y install epel-release \
ENV JAVA_HOME /usr/
-ADD artifacts /heron
+# run Heron installer
+RUN --mount=type=bind,source=artifacts,target=/tmp/heron
/tmp/heron/heron-install.sh && \
+ rm /usr/local/heron/dist/heron-core.tar.gz && \
+ rm -rf /opt/heron/heron-core/lib/scheduler/heron-local-scheduler.jar && \
+ rm -rf /opt/heron/heron-core/lib/scheduler/heron-mesos-scheduler.jar && \
+ rm -rf /opt/heron/heron-core/lib/scheduler/heron-slurm-scheduler.jar
Review comment:
I imagine as a `.tar.gz` it isn't of use at runtime, so probably fair to
strip. I wouldn't be surprised if it's just an artefact that was packed into
`heron-install.sh` and it was just unpacked but not cleaned up.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
