Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/19006 )
Change subject: IMPALA-8770: Support building Docker images on Redhat-based distributions ...................................................................... Patch Set 13: (4 comments) http://gerrit.cloudera.org:8080/#/c/19006/13/docker/CMakeLists.txt File docker/CMakeLists.txt: http://gerrit.cloudera.org:8080/#/c/19006/13/docker/CMakeLists.txt@60 PS13, Line 60: set(DISTRO_BASE_IMAGE "$ENV{IMPALA_REDHAT7_DOCKER_BASE}") > Maybe add a comment here stating that leaving QUICKSTART_BASE_IMAGE unset h Done http://gerrit.cloudera.org:8080/#/c/19006/13/docker/daemon_entrypoint.sh File docker/daemon_entrypoint.sh: http://gerrit.cloudera.org:8080/#/c/19006/13/docker/daemon_entrypoint.sh@32 PS13, Line 32: This can get more detailed if there are specific steps : # for specific versions, but at the moment the distribution : # is all we need. : DISTRIBUTION=Unknown : if [[ -f /etc/redhat-release ]]; then : echo "Identified Redhat image." : DISTRIBUTION=Redhat : else : source /etc/lsb-release : if [[ $DISTRIB_ID == Ubuntu ]]; then : echo "Identified Ubuntu image." : DISTRIBUTION=Ubuntu : fi : fi : : if [[ $DISTRIBUTION == Unknown ]]; then : echo "ERROR: Did not detect supported distribution." : echo "Only Ubuntu and Redhat-based distributions are supported." : exit 1 : fi > At the risk of overgeneralizing: could be worth extracting such snippets in That is definitely an option. One complication is that as we split things apart, we need to keep adding the various dependencies to the Docker build. I think I'm going to skip that in this change, but I will file a JIRA. http://gerrit.cloudera.org:8080/#/c/19006/13/docker/install_os_packages.sh File docker/install_os_packages.sh: http://gerrit.cloudera.org:8080/#/c/19006/13/docker/install_os_packages.sh@91 PS13, Line 91: vim > Maybe add 'which' and the dnsutils package for dig and nslookup? Added dnsutils. 'which' is preinstalled on Ubuntu. There doesn't seem to be a package. http://gerrit.cloudera.org:8080/#/c/19006/13/docker/install_os_packages.sh@110 PS13, Line 110: vim > Similarly, maybe add 'which' and bind-utils? Added which and bind-utils -- To view, visit http://gerrit.cloudera.org:8080/19006 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ibaff2560ef971ac2c2231a8e43921164ea1d2f4d Gerrit-Change-Number: 19006 Gerrit-PatchSet: 13 Gerrit-Owner: Joe McDonnell <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Joe McDonnell <[email protected]> Gerrit-Reviewer: Laszlo Gaal <[email protected]> Gerrit-Comment-Date: Wed, 05 Oct 2022 22:34:29 +0000 Gerrit-HasComments: Yes
