-
Notifications
You must be signed in to change notification settings - Fork 23
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
Matrix Python debian images #226
Conversation
127815e
to
952e638
Compare
952e638
to
26dc5d5
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
What about the sync-ecr.yml
and sync-ghcr.yml
workflows? Will those be updated in a subsequent change?
@@ -1,7 +1,7 @@ | |||
# syntax = docker/dockerfile:experimental | |||
# Interim container so we can copy pulumi binaries | |||
# Must be defined first | |||
ARG PYTHON_VERSION=3.9 | |||
ARG LANGUAGE_VERSION | |||
|
|||
FROM debian:buster-slim AS builder |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unrelated to this change, and not that it matters much (aside from staying up-to-date), but wondering if we should be using a more recent version for the interim builder container? Seems like we're sort of inconsistent across the Dockerfiles. Maybe something to consider improving in a follow-up change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Definitely, I'll add an issue to track this
@@ -16,7 +16,7 @@ RUN apt-get update -y && \ | |||
RUN curl -fsSL https://get.pulumi.com/ | bash -s -- --version $PULUMI_VERSION | |||
|
|||
# The runtime container | |||
FROM python:${PYTHON_VERSION}-slim | |||
FROM python:${LANGUAGE_VERSION}-slim |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not really related to this PR, but just something I wanted to discuss...
On the one hand, it's nice that we're using python:<version>-slim
here as it means we're already using bookworm, and any future debian upgrades would happen automatically as python's -slim
tags are updated.
I worry slightly that it may mean we'll end up using inconsistent debian versions across our language images. It would be nice if we were using the same version for all.
Should we be explicit here and have all our images explicitly use bookworm-slim, such that when the next debian release is available, we can control when we upgrade and do so easily across the board at the same time?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I was thinking the same, but I wanted to do this in a separate PR to avoid too many unrelated changes in this already large PR.
ENV GO_RUNTIME_386_SHA256 b93850666cdadbd696a986cf7b03111fe99db8c34a9aaa113d7c96d0081e1901 | ||
ENV GO_RUNTIME_AMD64_SHA256 b3075ae1ce5dab85f89bc7905d1632de23ca196bd8336afd93fa97434cfa55ae | ||
ENV GO_RUNTIME_ARM64_SHA256 7da1a3936a928fd0b2602ed4f3ef535b8cd1990f1503b8d3e1acc0fa0759c967 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm guessing you're already well aware, but in case you're not, we'll have to deal with these SHAs as part of the subsequent change to matrix the Go images.
python_default_version = "3.9" | ||
python_additional_versions = ["3.10", "3.11", "3.12"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The image without suffix
pulumi/pulumi-python
continues to use 3.9.
This makes sense to me. I assume at some point we'll want to make the default version use a more recent version, either the latest (or possibly latest-1). How would we go about doing that? Announcing the plan/timeline and then doing it? Would it look like the following (assuming we pick 3.12 as the default)?
python_default_version = "3.12"
python_additional_versions = ["3.9", "3.10", "3.11"]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I might revisit how we do the version list and default version setting when I add the matrix for nodejs, but for now this is how it works.
It's going to be a breaking change, but one I think we need to do. Users that require 3.9 can switch to the python-3.9
image. The announcement should also clearly set the expectation that the python
image will be updated regularly with a more recent python version.
Honestly I forgot about this, I'll create a ticket to track this. Also for the snyk scan. |
2cbecf1
to
bb1ef1f
Compare
Follow up to pulumi/pulumi-docker-containers#226 We need to sync the new suffixed python images to GHCR & ECR Fixes pulumi/pulumi-docker-containers#230
Create per language version images for python 3.9 to 3.12. The image without suffix
pulumi/pulumi-python
continues to use 3.9.Other languages or UBI based images are not matrixed, but this PR puts in place most of the infrastructure for this. To generate the matrix, we use a python script that creates a list of
include
entries for the matrix. We do this instead of using the normal way to specify a matrix using variables because each language has its own range of versions.For example for Python 3.12 we get the tags/manifests:
Example on my docker hub: https://hub.docker.com/repository/docker/jpoissonnier/pulumi-python-3.12/tags