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

Add logic to build intel-gpu image to build_llama_and_whisper.sh #724

Merged
merged 1 commit into from
Feb 4, 2025
Merged

Add logic to build intel-gpu image to build_llama_and_whisper.sh #724

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
32 changes: 12 additions & 20 deletions container-images/intel-gpu/Containerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,32 +1,24 @@
FROM quay.io/fedora/fedora:41 as builder

COPY intel-gpu/oneAPI.repo /etc/yum.repos.d/
COPY scripts/build_llama_and_whisper.sh /

RUN dnf install -y intel-opencl g++ cmake git tar libcurl-devel intel-oneapi-mkl-sycl-devel intel-oneapi-dnnl-devel intel-oneapi-compiler-dpcpp-cpp ; \
git clone https://github.com/ggerganov/llama.cpp.git -b b4523 ; \
cd llama.cpp ; \
mkdir -p build ; \
cd build ; \
source /opt/intel/oneapi/setvars.sh ; \
cmake .. -DGGML_SYCL=ON -DCMAKE_C_COMPILER=icx -DCMAKE_CXX_COMPILER=icpx -DLLAMA_CURL=ON -DGGML_CCACHE=OFF -DGGML_NATIVE=OFF ; \
cmake --build . --config Release -j -v ; \
cmake --install . --prefix /llama-cpp
RUN chmod +x /build_llama_and_whisper.sh ; \
/build_llama_and_whisper.sh intel-gpu

FROM quay.io/fedora/fedora:41

ENV LD_LIBRARY_PATH="/usr/local/lib64:/usr/local/lib/:/opt/intel/oneapi/mkl/2025.0/lib:/opt/intel/oneapi/compiler/2025.0/opt/compiler/lib:/opt/intel/oneapi/compiler/2025.0/lib/clang/19/lib:/opt/intel/oneapi/compiler/2025.0/lib:/opt/intel/oneapi/umf/0.9/lib:/opt/intel/oneapi/tbb/2022.0/lib:/opt/intel/oneapi/tcm/1.2/lib:/opt/intel/oneapi/redist/opt/compiler/lib:/opt/intel/oneapi/redist/lib/clang/19/lib:/opt/intel/oneapi/redist/lib:/opt/intel/oneapi/mkl/2025.0/lib:/opt/intel/oneapi/compiler/2025.0/opt/compiler/lib:/opt/intel/oneapi/compiler/2025.0/lib/clang/19/lib:/opt/intel/oneapi/compiler/2025.0/lib:/opt/intel/oneapi/umf/0.9/lib:/opt/intel/oneapi/tbb/2022.0/lib:/opt/intel/oneapi/tcm/1.2/lib:/opt/intel/oneapi/redist/opt/compiler/lib:/opt/intel/oneapi/redist/lib/clang/19/lib:/opt/intel/oneapi/redist/lib"

COPY --from=builder /llama-cpp/bin/ /usr/local/bin/
COPY --from=builder /llama-cpp/lib/ /usr/local/lib/
COPY --from=builder /llama-cpp/lib64/ /usr/local/lib64/
COPY --from=builder /llama-cpp/include/ /usr/local/include/
COPY --from=builder /tmp/install/ /usr/
COPY intel-gpu/oneAPI.repo /etc/yum.repos.d/
COPY --chown=0:0 intel-gpu/entrypoint.sh /

RUN dnf install -y intel-opencl libcurl lspci clinfo intel-oneapi-runtime-compilers intel-oneapi-mkl-core intel-oneapi-mkl-sycl-blas intel-oneapi-runtime-dnnl ; \
RUN dnf install -y procps-ng python3 python3-pip python3-devel intel-level-zero oneapi-level-zero intel-compute-runtime libcurl lspci clinfo intel-oneapi-runtime-compilers intel-oneapi-mkl-core intel-oneapi-mkl-sycl-blas intel-oneapi-runtime-dnnl ; \
chown 0:0 /etc/passwd ; \
chown 0:0 /etc/group ; \
chmod g=u /etc/passwd /etc/group ; \
useradd -u 1000 -g render -G video -s /bin/bash -d /home/llama-user llama-user
chmod g=u /etc/passwd /etc/group /home ; \
chmod +x /entrypoint.sh

USER 10000

USER 1000
WORKDIR /home/llama-user
ENTRYPOINT ["/entrypoint.sh"]
CMD [ "tail", "-f", "/dev/null" ]
46 changes: 46 additions & 0 deletions container-images/intel-gpu/entrypoint.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
#!/usr/bin/env bash

if [ -z ${HOME} ]
then
export HOME=/home/llama-user
fi

# Create Home directory
if [ ! -d "${HOME}" ]
then
mkdir -p "${HOME}"
fi

# Create User ID
if ! whoami &> /dev/null
then
if [ -w /etc/passwd ] && [ -w /etc/group ]
then
echo "${USER_NAME:-llama-user}:x:$(id -u):0:${USER_NAME:-llama-user} user:${HOME}:/bin/bash" >> /etc/passwd
echo "${USER_NAME:-llama-user}:x:$(id -u):" >> /etc/group
render_group="$(cat /etc/group | grep 'render:x')"
video_group="$(cat /etc/group | grep 'video:x')"
render_group_new="${render_group}${USER_NAME:-llama-user}"
video_group_new="${video_group}${USER_NAME:-llama-user}"
sed "s|${render_group}|${render_group_new}|g" /etc/group > /tmp/group
cat /tmp/group > /etc/group
Comment on lines +25 to +26
Copy link
Contributor

Choose a reason for hiding this comment

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

🚨 issue (security): The group file modifications could race when multiple containers start simultaneously

Consider using a more atomic approach to modify the group file. Also, the USER_NAME variable should be properly escaped to prevent potential injection issues.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah:

mv /tmp/group /etc/group

would be more atomic, same on line 28.

I don't see the race condition myself, but mv is a nicer option, if it works, more atomic to replace the inode.

Copy link
Contributor

Choose a reason for hiding this comment

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

You're right that mv /tmp/group /etc/group is more atomic and generally a better approach for replacing the file. However, the race condition I'm referring to isn't about replacing the file itself, but about concurrent modifications to the file. If multiple containers based on this image start simultaneously, they could all be reading /etc/group, modifying it in memory, and then writing their modified version back. This could lead to some updates being lost.

For example:

  1. Container 1 reads /etc/group.
  2. Container 2 reads /etc/group (same content as Container 1).
  3. Container 1 adds llama-user to the render group and writes the updated /etc/group.
  4. Container 2 adds llama-user to the video group and writes the updated /etc/group, overwriting the changes made by Container 1 regarding the render group.

Using mv doesn't solve this. A more robust solution would be to use a tool designed for concurrent file modifications, like usermod and groupmod. These tools handle locking and ensure that changes aren't overwritten. They also address the escaping concern I mentioned.

Could you try using usermod -a -G render ${USER_NAME} and usermod -a -G video ${USER_NAME} instead of directly manipulating /etc/group? This would be safer and more idiomatic. Also, ensure that USER_NAME is properly defined and sanitized before being used in these commands.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Wouldn't each container have it's own filesystem anyway? Hence no race condition

Copy link
Contributor

Choose a reason for hiding this comment

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

You're right that each container has its own filesystem, after it starts. However, the /etc/group file is typically part of the container image. So, when multiple containers are launched from the same image, they initially share the same /etc/group file. The modifications made by the entrypoint script happen after the container starts but before any user-specified commands run.

So, if multiple containers based on this image start simultaneously, they will all run this entrypoint script concurrently, and they will all be modifying their initially-shared copy of /etc/group. This is where the race condition occurs. Even though they each have their own filesystem later, the changes made during startup can be overwritten by other containers starting at the same time.

Using tools like usermod and groupmod is crucial because they handle file locking and ensure that concurrent modifications to /etc/group are applied correctly, avoiding the race condition even when multiple containers start simultaneously from the same image. They also handle escaping and other potential security issues related to directly manipulating /etc/group.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I tried mv initially but it requires rw access to the underlying folder, where cat > just requires rw on the file.

sed "s|${video_group}|${video_group_new}|g" /etc/group > /tmp/group
cat /tmp/group > /etc/group
fi
fi

# Configure Z shell
if [ ! -f ${HOME}/.zshrc ]
then
(echo "source /opt/intel/oneapi/setvars.sh") > ${HOME}/.zshrc
fi

# Configure Bash shell
if [ ! -f ${HOME}/.bashrc ]
then
(echo "source /opt/intel/oneapi/setvars.sh") > ${HOME}/.bashrc
fi

source /opt/intel/oneapi/setvars.sh

exec "$@"
13 changes: 12 additions & 1 deletion container-images/scripts/build_llama_and_whisper.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@ dnf_install() {
"procps-ng" "git" "dnf-plugins-core" "libcurl-devel")
local vulkan_rpms=("vulkan-headers" "vulkan-loader-devel" "vulkan-tools" \
"spirv-tools" "glslc" "glslang")
local intel_rpms=("intel-oneapi-mkl-sycl-devel" "intel-oneapi-dnnl-devel" \
"intel-oneapi-compiler-dpcpp-cpp" "intel-level-zero" \
"oneapi-level-zero" "oneapi-level-zero-devel" "intel-compute-runtime")

# All the UBI-based ones
if [ "$containerfile" = "ramalama" ] || [ "$containerfile" = "rocm" ] || \
Expand Down Expand Up @@ -37,6 +40,11 @@ dnf_install() {
# shellcheck disable=SC1091
. /opt/rh/gcc-toolset-12/enable
fi

if [ "$containerfile" = "intel-gpu" ]; then
dnf install -y "${rpm_list[@]}" "${intel_rpms[@]}"
source /opt/intel/oneapi/setvars.sh
fi
}

cmake_check_warnings() {
Expand All @@ -51,7 +59,7 @@ cmake_steps() {
}

set_install_prefix() {
if [ "$containerfile" = "cuda" ]; then
if [ "$containerfile" = "cuda" ] || [ "$containerfile" = "intel-gpu" ]; then
install_prefix="/tmp/install"
else
install_prefix="/usr"
Expand All @@ -70,6 +78,9 @@ configure_common_flags() {
vulkan | asahi)
common_flags+=("-DGGML_VULKAN=1")
;;
intel-gpu)
common_flags+=("-DGGML_SYCL=ON" "-DCMAKE_C_COMPILER=icx" "-DCMAKE_CXX_COMPILER=icpx")
;;
esac
}

Expand Down
Loading