Skip to content

Conversation

@jorgepiloto
Copy link
Member

@jorgepiloto jorgepiloto commented Jan 24, 2023

Fixes #62 by using a custom self-hosted runner for running the required jobs in the CI/CD pipeline.

  • The CI/CD pipeline links with the self-hosted runner
  • Docker images are present in the self-hosted runner:
    • ansys/stk:latest-centos7-pybase
    • ansys/stk:latest-centos7-python3.8
    • ansys/stk:latest-centos7-python3.9
    • ansys/stk:latest-centos7-python3.10
  • The CI/CD pipeline starts, runs and removes the container for the desired Python version
  • The actions-runner working directory is shared as a volume with the STK container

The reason why the container should have an image per Python version is that, right now, PySTK is not a remote Python API. It does not connect to a server living inside the container. Hence, we are forced to use the Python inside the container to execute a Python script.

Another possibility is to install multiple Python versions in the contianer. The official docker images for STK allow to do so.

Then, it should be a matter of starting the container, enabling the desired Python version and running the CI/CD commands. This also saves disk space in the machine.

This situation will solve as the project evolves.

@github-actions github-actions bot added the maintenance Generic maintenance related label Jan 24, 2023
@jorgepiloto jorgepiloto force-pushed the ci/self-hosted-runner branch 29 times, most recently from 09c7b40 to cd97069 Compare March 7, 2023 07:39
@jorgepiloto jorgepiloto requested a review from a team March 7, 2023 09:53
@duposyl
Copy link
Contributor

duposyl commented Mar 7, 2023

@jorgepiloto Looks good to me. I am fine with the current approach, and I thought about a small improvement that we could do now or later:

One small improvement would be to use a multi-stage build for the docker/linux/stk-engine-py3[8,9,10,11]/Dockerfile. Basically, this would mean installing the build environment and compiling Python in a first stage, and then the second stage would just copy the resulting Python install to the system and create the symlinks. This approach would avoid leaving gcc, openssl-devel, bzip2-devel, libffi-devel, tk-devel, etc. in the image, so the environment would be slightly cleaner when the image is used later on to run the PySTK tests.

What do you think?

@jorgepiloto
Copy link
Member Author

I think this is a great suggestion, @duposyl. It will help us to easily maintain all these images.

@duposyl
Copy link
Contributor

duposyl commented Mar 8, 2023

@jorgepiloto, thanks for the update. It is nice that installing the system dependencies and building openssl is now in one common place for all the python versions.

Another possible further improvement would be something similar to this for the Python specific version dockerfile (for instance for stk-engine-py38/Dockerfile):


##############################################
# 1) Build Python into its own builder stage
##############################################

ARG builderbaseImage=ansys/stk:latest-centos7-pybase
FROM ${builderbaseImage} as builder

# Install Python 3.10 and create a symbolic link named "python".
# This overrides the default Python 2.7 in CentOS7.
USER root
RUN cd /tmp && \
    wget https://www.python.org/ftp/python/3.10.10/Python-3.10.10.tgz && \
    tar xzf Python-3.10.10.tgz && \
    cd Python-3.10.10 && \
    ./configure --enable-optimizations && \
    make altinstall

#############################################################
# 2) Copy the Python build to the system in the second stage
#############################################################

# Switch to non-builder image
ARG baseImage=ansys/stk:latest-centos7
FROM ${baseImage}

COPY --from=builder ... copy python install ...

RUN ln -sfn /usr/local/bin/python3.10 /usr/bin/python && \
    ln -sfn /usr/local/bin/pip3.10 /usr/bin/pip

# Switch back to non-root user
USER stk

# Update the path to include Python executables
ENV PATH="${STK_USER_HOME}/.local/bin:${PATH}" \
    PIP_DEFAULT_TIMEOUT=600 \
    PIP_RETRY=50

ENTRYPOINT [ "python3.10" ]

That way the temporary build files and build dependencies do not get included in the final image used for testing the API.

Now if you think this is overkill, I am fine with what you currently have too.

Copy link
Contributor

@duposyl duposyl left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks for putting this together!

@jorgepiloto jorgepiloto force-pushed the ci/self-hosted-runner branch from c516dff to 951ba5e Compare March 16, 2023 06:45
@jorgepiloto jorgepiloto force-pushed the ci/self-hosted-runner branch from 951ba5e to 16703de Compare March 16, 2023 07:01
@MaxJPRey
Copy link
Contributor

Great work @jorgepiloto .

Co-authored-by: Maxime Rey <87315832+MaxJPRey@users.noreply.github.com>
@jorgepiloto jorgepiloto merged commit 8d19be7 into main Mar 16, 2023
@jorgepiloto jorgepiloto deleted the ci/self-hosted-runner branch March 16, 2023 14:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

maintenance Generic maintenance related

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Set up a self-hosted runner for CI/CD

4 participants