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

Use custom built-in python with ch-backup #152

Closed
wants to merge 7 commits into from
Closed
Show file tree
Hide file tree
Changes from 5 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
52 changes: 52 additions & 0 deletions Dockerfile-deb-python-build
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
ARG PYTHON_BUILD_NAME
ARG BASE_IMAGE=ubuntu:22.04

FROM --platform=$TARGETPLATFORM $PYTHON_BUILD_NAME-build:latest as pybuild
Copy link
Contributor

Choose a reason for hiding this comment

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

How about passing full image name here without assumption *-build:latest


FROM --platform=$TARGETPLATFORM $BASE_IMAGE

ARG DEBIAN_FRONTEND=noninteractive
ARG TARGET_PYTHON_VERSION

RUN mkdir -p /opt/yandex/ch-backup/python
COPY --from=pybuild /opt/yandex/ch-backup/python /opt/yandex/ch-backup/python

RUN set -ex \
&& apt-get update \
&& apt-get install -y --no-install-recommends \
# Debian packaging tools
build-essential \
debhelper \
devscripts \
fakeroot \
# Managing keys for debian package signing
gpg \
gpg-agent \
# Python packaging tools
python3-pip \
python3-venv \
# Misc
curl \
locales \
# For building PyNacl library
libffi-dev libssl-dev libboost-all-dev libsodium-dev \
# Configure locales
&& locale-gen en_US.UTF-8 \
&& update-locale LANG=en_US.UTF-8 \
# Ensure that `python` refers to `python3` so that poetry works.
# It makes sense for ubuntu:18.04
Alex-Burmak marked this conversation as resolved.
Show resolved Hide resolved
&& ln -sf /opt/yandex/ch-backup/python/bin/python3.12 /usr/bin/python \
&& ln -sf /opt/yandex/ch-backup/python/bin/python3.12 /usr/bin/python3

# Project directory must be mounted here
VOLUME /src
WORKDIR /src

# For compiling PyNACL library, which is used by ch-backup
# See https://pynacl.readthedocs.io/en/latest/install/#linux-source-build
ENV SODIUM_INSTALL=system

ENV PYTHON=/opt/yandex/ch-backup/python/bin/python3.12
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's remove hard-coded versions where it is possible

ENV TARGET_PYTHON_VERSION=$TARGET_PYTHON_VERSION

CMD ["make", "build-deb-package-local"]
43 changes: 43 additions & 0 deletions Dockerfile-python-build
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
ARG BASE_IMAGE=ubuntu:18.04
FROM --platform=$TARGETPLATFORM $BASE_IMAGE

ENV DEBIAN_FRONTEND=noninteractive
ENV LANG=C.UTF-8

RUN set -ex \
&& apt-get update \
&& apt-get install -y --no-install-recommends \
build-essential \
libbz2-dev \
libffi-dev \
libgdbm-dev \
libncurses5-dev \
libnss3-dev \
libreadline-dev \
libsqlite3-dev \
libssl-dev \
wget \
xz-utils \
zlib1g-dev \
&& apt-get clean \
&& rm -rf /var/lib/apt/lists/*

ARG PYTHON_INSTALL_PREFIX=/src/python
Copy link
Contributor

Choose a reason for hiding this comment

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

I would propose to remove default value and add checking if it is empty. Like so

ARG PYTHON_INSTALL_PREFIX
RUN test -n "$PYTHON_INSTALL_PREFIX" || (echo "PYTHON_INSTALL_PREFIX must be provided" && false)

RUN mkdir -p $PYTHON_INSTALL_PREFIX

ARG PYTHON_VERSION=3.12.3
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's move all ARG at the beginning of dockerfile


RUN set -ex \
&& wget -O python.tar.xz --no-check-certificate "https://www.python.org/ftp/python/$PYTHON_VERSION/Python-$PYTHON_VERSION.tar.xz" \
&& tar -xf python.tar.xz \
&& rm python.tar.xz \
&& cd Python-$PYTHON_VERSION/ \
&& ./configure \
--build="$(dpkg-architecture --query DEB_BUILD_GNU_TYPE)" \
--prefix=$PYTHON_INSTALL_PREFIX \
--enable-optimizations \
&& make -j $(nproc) \
&& make altinstall \
&& rm -rf /Python-$PYTHON_VERSION

WORKDIR $PYTHON_INSTALL_PREFIX
18 changes: 16 additions & 2 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@ export DEB_TARGET_PLATFORM ?=
# If it is not provided, default value in Dockerfile is used
export DEB_BUILD_DISTRIBUTION ?=


.PHONY: build
build: install-deps ch_backup/version.txt

Expand Down Expand Up @@ -109,7 +108,7 @@ clean-pycache:


.PHONY: install
install:
install: .copy-target-python
@echo "Installing into $(INSTALL_DIR)"
$(PYTHON) -m venv $(INSTALL_DIR)
$(INSTALL_DIR)/bin/pip install -r requirements.txt
Expand Down Expand Up @@ -211,6 +210,21 @@ check-environment:
touch .install-deps


.PHONY: build-python
build-python:
Alex-Burmak marked this conversation as resolved.
Show resolved Hide resolved
PYTHON_INSTALL_PREFIX=${INSTALL_DIR}/python ./build_python_in_docker.sh

.PHONY: .copy-target-python
.copy-target-python:
@echo "Target python version: ${TARGET_PYTHON_VERSION}"
@if [[ -n "${TARGET_PYTHON_VERSION}" ]]; then \
echo "Copying custom python to ${INSTALL_DIR}/python"; \
mkdir -p ${INSTALL_DIR}/python; \
cp -R /opt/yandex/ch-backup/python ${INSTALL_DIR}; \
else \
echo "Custom target python version is not set."; \
fi

.PHONY: help
help:
@echo "Targets:"
Expand Down
2 changes: 1 addition & 1 deletion build_deb_in_docker.sh
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ BUILD_IMAGE=${PROJECT_NAME}-build
BUILD_ARGS=()

# Compose image name and build arguments
# Example of image name "clickhouse-tools-build-linux-amd64-linux-bionic"
# Example of image name "clickhouse-tools-build-linux-amd64-linux-bionic"
if [[ -n "${DEB_TARGET_PLATFORM}" ]]; then
BUILD_ARGS+=(--platform=${DEB_TARGET_PLATFORM})
BUILD_IMAGE="${BUILD_IMAGE}-${DEB_TARGET_PLATFORM}"
Expand Down
43 changes: 43 additions & 0 deletions build_deb_with_python_in_docker.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
#!/usr/bin/env bash

set -e

BUILD_IMAGE=${PROJECT_NAME}-build
BUILD_ARGS=()

# Compose image name and build arguments
# Example of image name "clickhouse-tools-build-linux-amd64-linux-bionic"
if [[ -n "${DEB_TARGET_PLATFORM}" ]]; then
BUILD_ARGS+=(--platform=${DEB_TARGET_PLATFORM})
BUILD_IMAGE="${BUILD_IMAGE}-${DEB_TARGET_PLATFORM}"
fi
if [[ -n "${DEB_BUILD_DISTRIBUTION}" ]]; then
BUILD_ARGS+=(--build-arg BASE_IMAGE=${DEB_BUILD_DISTRIBUTION})
BUILD_IMAGE="${BUILD_IMAGE}-${DEB_BUILD_DISTRIBUTION}"
fi
if [[ -n "${PYTHON_BUILD_NAME}" ]]; then
BUILD_ARGS+=(--build-arg PYTHON_BUILD_NAME=${PYTHON_BUILD_NAME})
fi
if [[ -n "${TARGET_PYTHON_VERSION}" ]]; then
BUILD_ARGS+=(--build-arg TARGET_PYTHON_VERSION=${TARGET_PYTHON_VERSION})
fi

# Normalize docker image name
BUILD_IMAGE=$(echo ${BUILD_IMAGE} | tr '[:upper:]' '[:lower:]' | sed 's/[^a-z0-9._-]/-/g')

RUN_ARGS=( \
-v ${PWD}:/src \
--env BUILD_DEB_OUTPUT_DIR="${BUILD_DEB_OUTPUT_DIR}" \
--env DEB_SIGN_KEY="${DEB_SIGN_KEY}" \
--env DEB_SIGN_KEY_ID="${DEB_SIGN_KEY_ID}" \
)
# Mount signing key file if its path is provided
if [[ -n "${DEB_SIGN_KEY_PATH}" ]]; then
RUN_ARGS+=( \
-v ${DEB_SIGN_KEY_PATH}:/signing_key \
--env DEB_SIGN_KEY_PATH=/signing_key \
)
fi

docker build "${BUILD_ARGS[@]}" -t "${BUILD_IMAGE}" -f Dockerfile-deb-python-build .
docker run "${RUN_ARGS[@]}" "${BUILD_IMAGE}"
26 changes: 26 additions & 0 deletions build_python_in_docker.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
#!/usr/bin/env bash

set -e

BUILD_IMAGE=${PYTHON_BUILD_NAME}-build
BUILD_ARGS=()

# Normalize docker image name
BUILD_IMAGE=$(echo ${BUILD_IMAGE} | tr '[:upper:]' '[:lower:]' | sed 's/[^a-z0-9._-]/-/g')

if [[ -n "${DEB_TARGET_PLATFORM}" ]]; then
BUILD_ARGS+=(--platform=${DEB_TARGET_PLATFORM})
fi
if [[ -n "${DEB_BUILD_DISTRIBUTION}" ]]; then
BUILD_ARGS+=(--build-arg BASE_IMAGE=${DEB_BUILD_DISTRIBUTION})
fi
if [[ -n "${TARGET_PYTHON_VERSION}" ]]; then
BUILD_ARGS+=(--build-arg PYTHON_VERSION=${TARGET_PYTHON_VERSION})
fi
if [[ -n "${PYTHON_INSTALL_PREFIX}" ]]; then
BUILD_ARGS+=(--build-arg PYTHON_INSTALL_PREFIX=${PYTHON_INSTALL_PREFIX})
fi

docker build "${BUILD_ARGS[@]}" -t "${BUILD_IMAGE}" -f Dockerfile-python-build .

echo "${BUILD_IMAGE} was built."
5 changes: 5 additions & 0 deletions debian/rules
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,11 @@ PYTHON_MINOR := $(shell $(PYTHON) -c 'import sys; print(sys.version_info[1])')
PYTHON_FROM := $(PYTHON_MAJOR).$(PYTHON_MINOR)
PYTHON_TO := $(PYTHON_MAJOR).$(shell echo $$(( $(PYTHON_MINOR) + 1 )))

# In case of using built-in python version (which is expected to have at least 3.12)
# we don't care about this dependency and it can be removed at all later.
ifeq ($(PYTHON_MINOR),12)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

not sure how to avoid this dependency more elegant than that, maybe we can just set SUBSTVARS to an empty string for example

with built-in python we don't need this (as well as Build-Depends) - but we can clean it up later somehow

Copy link
Member

Choose a reason for hiding this comment

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

I suppose we can workaround lack of numeric comparison in debin rules using shell directive.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

changed for using shell and proper comparison I guess

while this currently is needed for all builds with custom python, we can assume that we'll use it only for 3.12+, otherwise need to think how to remove this dependency properly

Copy link
Contributor

Choose a reason for hiding this comment

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

Nevertheless, I think that checking environment variable here if embedded python is used and using default value SUBSTVARS := -Vpython:Depends="" are more clear

PYTHON_FROM="3.6"
endif
# Use conditional python3 dependency because package for Bionic requires python3.6,
# but package for Jammy requires python3.10.
# All this is due to the fact that we put the entire virtual environment in a deb package
Expand Down
8 changes: 6 additions & 2 deletions requirements.txt
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
requests>2.19
boto3<1.19
botocore<1.22
boto3<1.19; python_version <"3.12"
botocore<1.22; python_version <"3.12"
psutil
PyYAML<5.4
PyNaCl==1.2.1
Expand All @@ -15,3 +15,7 @@ pypeln==0.4.9
dataclasses>=0.7,<0.8; python_version <"3.7" # required for pypeln==0.4.9
typing_extensions>=3.7.4,<4.0; python_version <"3.8" # required for pypeln==0.4.9
loguru
# for python3.12+
setuptools; python_version >="3.12" # instead of python3-setuptools package
boto3<1.21; python_version >="3.12"
botocore<1.24; python_version >="3.12"