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.

Sign up for GitHub

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

Smaller Docker image size #1

Merged
merged 1 commit into from
Nov 21, 2024
Merged

Smaller Docker image size #1

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
40 changes: 24 additions & 16 deletions Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
FROM debian:12.8-slim AS builder
FROM ubuntu:22.04
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Consider using a smaller base image to achieve the goal of reducing image size

Switching from debian:12-slim to ubuntu:22.04 may increase the Docker image size, as Ubuntu images are generally larger than Debian slim images. To meet the PR objective of reducing the image size, consider using debian:12-slim or another minimal base image.


# Some metadata
LABEL base_image="ubuntu:22.04"
LABEL version="2"
LABEL software="diann"
Expand All @@ -11,25 +12,32 @@ LABEL about.license_file="https://github.com/vdemichev/DiaNN/LICENSE.txt"
LABEL about.tags="Proteomics"
LABEL maintainer="Yasset Perez-Riverol <[email protected]>"

USER root
ENV DEBIAN_FRONTEND=noninteractive

RUN apt-get update -y --no-install-recommends && apt-get install -y \
g++ build-essential cmake zlib1g-dev libbz2-dev libboost-all-dev wget locales unzip && \
apt-get clean && rm -rf /var/lib/apt/lists/*
# Update package lists and ensure package versions are up to date
RUN apt-get update && apt-get upgrade -y
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Optimize package installation and avoid unnecessary apt-get upgrade.

Running apt-get upgrade in a Dockerfile can lead to larger image sizes and potential unpredictability. It's generally advisable to avoid it. Also, combining apt-get update and apt-get install in a single RUN command reduces the number of layers and prevents caching issues.

Consider applying the following changes:

-RUN apt-get update && apt-get upgrade -y

-RUN apt-get install wget unzip libgomp1 locales -y

+RUN apt-get update && apt-get install -y \
+    wget \
+    unzip \
+    libgomp1 \
+    locales && \
+    rm -rf /var/lib/apt/lists/*

This approach installs the necessary packages and cleans up the apt cache to minimize the image size.

Also applies to: 22-22


RUN sed -i '/en_US.UTF-8/s/^# //g' /etc/locale.gen && \
locale-gen && update-locale LANG=en_US.UTF-8
# Install necessary packages including locales
RUN apt-get install wget unzip libgomp1 locales -y
Comment on lines +19 to +22
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Optimize package installation to reduce image size and avoid unnecessary upgrades

Running apt-get upgrade is generally discouraged in Dockerfiles, as it can increase image size and may introduce unintended changes. Combining apt-get update and apt-get install in a single RUN command reduces image layers and avoids cache issues.

Apply this diff to optimize package installation:

-RUN apt-get update && apt-get upgrade -y

-# Install necessary packages including locales
-RUN apt-get install wget unzip libgomp1 locales -y
+RUN apt-get update && \
+    apt-get install -y --no-install-recommends wget unzip libgomp1 locales && \
+    rm -rf /var/lib/apt/lists/*
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
RUN apt-get update && apt-get upgrade -y
RUN sed -i '/en_US.UTF-8/s/^# //g' /etc/locale.gen && \
locale-gen && update-locale LANG=en_US.UTF-8
# Install necessary packages including locales
RUN apt-get install wget unzip libgomp1 locales -y
RUN apt-get update && \
apt-get install -y --no-install-recommends wget unzip libgomp1 locales && \
rm -rf /var/lib/apt/lists/*


RUN mkdir -p /opt/software && \
wget https://github.com/vdemichev/DiaNN/releases/download/1.9.2/diann-1.9.2.Linux_update_2024-10-31.zip -O /opt/software/diann-1.9.2.Linux_update_2024-10-31.zip && \
unzip /opt/software/diann-1.9.2.Linux_update_2024-10-31.zip -d /opt/software && \
mv /opt/software/diann-1.9.2 /usr/diann-1.9.2 && \
ln -s /usr/diann-1.9.2/diann-linux /usr/diann-1.9.2/diann && \
rm -rf /opt/software
# Configure locale to avoid runtime errors
RUN locale-gen en_US.UTF-8 && \
update-locale LANG=en_US.UTF-8

RUN chmod +x /usr/diann-1.9.2/diann-linux
# Set environment variables for locale
ENV LANG=en_US.UTF-8
ENV LANGUAGE=en_US:en
ENV LC_ALL=en_US.UTF-8

ENV PATH="$PATH:/usr/diann-1.9.2"
ENV DIA_NN_PATH="/usr/diann-1.9.2"
# Download DIA-NN version <version>
RUN wget https://github.com/vdemichev/DiaNN/releases/download/1.9.2/diann-1.9.2.Linux.zip -O diann-1.9.2.Linux.zip

WORKDIR /data/
# Unzip the DIA-NN package
RUN unzip diann-1.9.2.Linux.zip

# Set appropriate permissions for the DIA-NN folder
RUN chmod -R 775 /diann-1.9.2

# NOTE: It is entirely the user's responsibility to ensure compliance with DIA-NN license terms.
# Please review the licensing terms for DIA-NN before using or distributing this Docker image.
Comment on lines +42 to +43
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Ensure compliance with DIA-NN licensing terms to avoid legal issues.

DIA-NN version 1.9.2 may have licensing restrictions that prohibit the distribution of Docker images containing the software. Building and sharing this Docker image could violate the license agreement. It's crucial to review the license terms and obtain the necessary permissions before proceeding.

Would you like assistance in reviewing the licensing terms or exploring alternative methods to comply with the license, such as instructing users to download DIA-NN during the build process?

2 changes: 1 addition & 1 deletion README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,5 +39,5 @@ docker build -t diann:1.9.2 .
### **Running the Container**

```bash
docker run -it diann:1.9.2 diann
docker run -it diann:1.9.2 ./diann-1.9.2/diann-linux
```