Skip to content

Commit

Permalink
Correctly handle producing multiple Venvs for the same python version
Browse files Browse the repository at this point in the history 
I had about 80% of the approach implemented from day one - we already
checked a map to see if we'd already seen that version. The problem was
we never set anything in it so it was always empty.

This also tidies up the tests a bit, and removes the needless link to
`/usr/local/include/python3.7m` for >=Py3.8 (which doesn't need it)

I used `coreos/go-semver` for Semver parsing as that was already in our
dep tree
  • Loading branch information
@ashb
ashb committed Mar 19, 2024
1 parent 731b730 commit 00a2a8e
Show file tree
Hide file tree
Showing 4 changed files with 108 additions and 25 deletions.
3 changes: 2 additions & 1 deletion buildkit/go.mod
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ go 1.21.6

require (
github.com/EricHripko/buildkit-fdk v0.2.0
github.com/coreos/go-semver v0.3.1
github.com/docker/distribution v2.8.1+incompatible
github.com/moby/buildkit v0.10.6
github.com/pkg/errors v0.9.1
Expand Down Expand Up @@ -42,7 +43,7 @@ require (
google.golang.org/genproto v0.0.0-20211208223120-3a66f561d7aa // indirect
google.golang.org/grpc v1.45.0 // indirect
google.golang.org/protobuf v1.27.1 // indirect
gopkg.in/yaml.v3 v3.0.0 // indirect
gopkg.in/yaml.v3 v3.0.1 // indirect
)

replace github.com/Sirupsen/logrus => ./deps/logrus
11 changes: 9 additions & 2 deletions buildkit/go.sum
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ github.com/containerd/typeurl v1.0.2/go.mod h1:9trJWW2sRlGub4wZJRTW83VtbOLS6hwcD
github.com/coreos/bbolt v1.3.2/go.mod h1:iRUV2dpdMOn7Bo10OQBFzIJO9kkE559Wcmn+qkEiiKk=
github.com/coreos/etcd v3.3.10+incompatible/go.mod h1:uF7uidLiAD3TWHmW31ZFd/JWoc32PjwdhPthX9715RE=
github.com/coreos/go-semver v0.2.0/go.mod h1:nnelYz7RCh+5ahJtPPxZlU+153eP4D4r3EedlOD2RNk=
github.com/coreos/go-semver v0.3.1 h1:yi21YpKnrx1gt5R+la8n5WgS0kCrsPp33dmEyHReZr4=
github.com/coreos/go-semver v0.3.1/go.mod h1:irMmmIw/7yzSRPWryHsK7EYSg09caPQL03VsM8rvUec=
github.com/coreos/go-systemd v0.0.0-20190321100706-95778dfbb74e/go.mod h1:F5haX7vjVVG0kc13fIWeqUViNPyEJxv/OmvnBo0Yme4=
github.com/coreos/pkg v0.0.0-20180928190104-399ea9e2e55f/go.mod h1:E3G3o1h8I7cfcXa63jLwjI0eiQQMgzzUDFVpN/nH/eA=
github.com/cpuguy83/go-md2man/v2 v2.0.0/go.mod h1:maD7wRr/U5Z6m/iR4s+kqSMx2CaBsrgA7czyZG/E6dU=
Expand Down Expand Up @@ -79,6 +81,7 @@ github.com/golang/glog v0.0.0-20160126235308-23def4e6c14b/go.mod h1:SBH7ygxi8pfU
github.com/golang/groupcache v0.0.0-20190129154638-5b532d6fd5ef/go.mod h1:cIg4eruTrX1D+g88fzRXU5OdNfaM+9IcxsU14FzY7Hc=
github.com/golang/mock v1.1.1/go.mod h1:oTYuIxOrZwtPieC+H1uAHpcLFnEyAGVDL/k47Jfbm0A=
github.com/golang/mock v1.6.0 h1:ErTB+efbowRARo13NNdxyJji2egdxLGQhRaY+DUumQc=
github.com/golang/mock v1.6.0/go.mod h1:p6yTPP+5HYm5mzsMV8JkE6ZKdX+/wYM6Hr+LicevLPs=
github.com/golang/protobuf v1.2.0/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U=
github.com/golang/protobuf v1.3.1/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U=
github.com/golang/protobuf v1.3.2/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U=
Expand Down Expand Up @@ -165,6 +168,7 @@ github.com/prometheus/procfs v0.0.0-20181005140218-185b4288413d/go.mod h1:c3At6R
github.com/prometheus/procfs v0.0.0-20190507164030-5867b95ac084/go.mod h1:TjEm7ze935MbeOT/UhFTIMYKhuLP4wbCsTZCD3I8kEA=
github.com/prometheus/procfs v0.6.0/go.mod h1:cz+aTbrPOrUb4q7XlbU9ygM+/jj0fzG6c1xBZuNvfVA=
github.com/prometheus/procfs v0.7.3 h1:4jVXhlkAyzOScmCkXBTOLRLTz8EeU+eyjrwB/EPq0VU=
github.com/prometheus/procfs v0.7.3/go.mod h1:cz+aTbrPOrUb4q7XlbU9ygM+/jj0fzG6c1xBZuNvfVA=
github.com/prometheus/tsdb v0.7.1/go.mod h1:qhTCs0VvXwvX/y3TZrWD7rabWM+ijKTux40TwIPHuXU=
github.com/rogpeppe/fastuuid v0.0.0-20150106093220-6724a57986af/go.mod h1:XWv6SoW27p1b0cqNHllgS5HIMJraePCO15w5zCzIWYg=
github.com/rogpeppe/fastuuid v1.2.0/go.mod h1:jVj6XXZzXRy/MSR5jhDC/2q6DgLz+nrA6LYCDYWNEvQ=
Expand Down Expand Up @@ -193,7 +197,9 @@ github.com/tonistiigi/fsutil v0.0.0-20220315205639-9ed612626da3 h1:T1pEe+WB3SCPV
github.com/tonistiigi/fsutil v0.0.0-20220315205639-9ed612626da3/go.mod h1:oPAfvw32vlUJSjyDcQ3Bu0nb2ON2B+G0dtVN/SZNJiA=
github.com/ugorji/go v1.1.4/go.mod h1:uQMGLiO92mf5W77hV/PUCpI3pbzQx3CRekS0kk+RGrc=
github.com/vishvananda/netlink v1.1.1-0.20210330154013-f5de75959ad5 h1:+UB2BJA852UkGH42H+Oee69djmxS3ANzl2b/JtT1YiA=
github.com/vishvananda/netlink v1.1.1-0.20210330154013-f5de75959ad5/go.mod h1:twkDnbuQxJYemMlGd4JFIcuhgX83tXhKS2B/PRMpOho=
github.com/vishvananda/netns v0.0.0-20210104183010-2eb08e3e575f h1:p4VB7kIXpOQvVn1ZaTIVp+3vuYAXFe3OJEvjbUYJLaA=
github.com/vishvananda/netns v0.0.0-20210104183010-2eb08e3e575f/go.mod h1:DD4vA1DwXk04H54A1oHXtwZmA0grkVMdPxx/VGLCah0=
github.com/xiang90/probing v0.0.0-20190116061207-43a291ad63a2/go.mod h1:UETIi67q53MR2AWcXfiuqkDkRtnGDLqkBTpCHuJHxtU=
github.com/xordataexchange/crypt v0.0.3-0.20170626215501-b2862e3d0a77/go.mod h1:aYKd//L2LvnjZzWKhF00oedf4jCCReLcmhLdhm1A27Q=
github.com/yuin/goldmark v1.1.27/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74=
Expand Down Expand Up @@ -265,6 +271,7 @@ golang.org/x/sys v0.0.0-20220715151400-c0bba94af5f8 h1:0A+M6Uqn+Eje4kHMK80dtF3JC
golang.org/x/sys v0.0.0-20220715151400-c0bba94af5f8/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo=
golang.org/x/term v0.0.0-20210615171337-6886f2dfbf5b h1:9zKuko04nR4gjZ4+DNjHqRlAJqbJETHwiNKDqTfOjfE=
golang.org/x/term v0.0.0-20210615171337-6886f2dfbf5b/go.mod h1:jbD1KX2456YbFQfuXm/mYQcufACuNUgVhRMnK/tPxf8=
golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ=
golang.org/x/text v0.3.5/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ=
Expand Down Expand Up @@ -330,8 +337,8 @@ gopkg.in/yaml.v2 v2.2.1/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI=
gopkg.in/yaml.v2 v2.2.2/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI=
gopkg.in/yaml.v2 v2.2.3/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI=
gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
gopkg.in/yaml.v3 v3.0.0 h1:hjy8E9ON/egN1tAYqKb61G10WtihqetD4sz2H+8nIeA=
gopkg.in/yaml.v3 v3.0.0/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA=
gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
gotest.tools/v3 v3.0.3 h1:4AuOwCGf4lLR9u3YOe2awrHygurzhO/HeQ6laiA6Sx0=
gotest.tools/v3 v3.0.3/go.mod h1:Z7Lb0S5l+klDB31fvDQX8ss/FlKDxtlFlw3Oa8Ymbl8=
honnef.co/go/tools v0.0.0-20190102054323-c2f93a96b099/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4=
Expand Down
18 changes: 14 additions & 4 deletions buildkit/internal/transform/transforms.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"text/template"

"github.com/astronomer/astro-runtime-frontend/internal/dockerfile"
"github.com/coreos/go-semver/semver"
"github.com/docker/distribution/reference"
"github.com/moby/buildkit/frontend/dockerfile/instructions"
"github.com/moby/buildkit/frontend/dockerfile/parser"
Expand All @@ -23,8 +24,9 @@ COPY --link --from=python:{{.PythonVersion}}-{{.PythonFlavour}} /usr/local/lib/p
COPY --link --from=python:{{.PythonVersion}}-{{.PythonFlavour}} /usr/local/lib/*{{.PythonMajorMinor}}*.so* /usr/local/lib/
COPY --link --from=python:{{.PythonVersion}}-{{.PythonFlavour}} /usr/local/lib/python{{.PythonMajorMinor}} /usr/local/lib/python{{.PythonMajorMinor}}
RUN /sbin/ldconfig /usr/local/lib
# hack for python <= 3.7
{{ if .Py37OrOlder -}}
RUN ln -s /usr/local/include/python{{.PythonMajorMinor}} /usr/local/include/python{{.PythonMajorMinor}}m
{{ end }}
USER astro
`
virtualEnvTemplate = `RUN mkdir -p /home/astro/.cache/pip /home/astro/.venv/{{.Name}}
Expand All @@ -46,6 +48,7 @@ ENV ASTRO_PYENV_{{.Name}} /home/astro/.venv/{{.Name}}/bin/python
var (
venvNamePattern = regexp.MustCompile(`[a-zA-Z0-9_-]+`)
pythonVersionPattern = regexp.MustCompile(`[0-9]+\.[0-9]+(\.[0-9]+)?(-.*)?`)
v3_8 = *semver.New("3.8.0")
)

type Transformer struct {
Expand All @@ -60,6 +63,7 @@ type virtualEnv struct {
PythonFlavour string
PythonMajorMinor string
RequirementsFile string
Py37OrOlder bool
}

func newTransformer(buildArgs map[string]string) *Transformer {
Expand Down Expand Up @@ -191,9 +195,11 @@ func (t *Transformer) processPyenv(pyenv *parser.Node) (*parser.Node, error) {
return nil, err
}
lineOffset := 0
for _, n := range pythonVersionNode.Children {
newNode.AddChild(n, n.StartLine, n.EndLine)
lineOffset = n.EndLine
if pythonVersionNode != nil {
for _, n := range pythonVersionNode.Children {
newNode.AddChild(n, n.StartLine, n.EndLine)
lineOffset = n.EndLine
}
}
venvNode, err := t.addVirtualEnvironment(venv)
if err != nil {
Expand Down Expand Up @@ -223,6 +229,8 @@ func parsePyenvDirective(s string) (*virtualEnv, error) {
return nil, err
}
env.PythonMajorMinor = extractPythonMajorMinor(env.PythonVersion)

env.Py37OrOlder = semver.New(env.PythonMajorMinor + ".0").LessThan(v3_8)
return env, nil
}

Expand All @@ -231,6 +239,7 @@ func validateVirtualEnv(env *virtualEnv) error {
if !pythonVersionPattern.MatchString(env.PythonVersion) {
return fmt.Errorf("invalid python version %s, should match pattern %v", env.PythonVersion, pythonVersionPattern)
}

// validate venv name
if !venvNamePattern.MatchString(env.Name) {
return fmt.Errorf("invalid virtual env name %s, should match pattern %v", env.Name, venvNamePattern)
Expand Down Expand Up @@ -259,6 +268,7 @@ func (t *Transformer) addPythonVersion(venv *virtualEnv) (*parser.Node, error) {
if err != nil {
return nil, err
}
t.pythonVersions[venv.PythonMajorMinor] = struct{}{}
return parsedNodes.AST, nil
}

Expand Down
101 changes: 83 additions & 18 deletions buildkit/internal/transform/transforms_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,19 @@ import (
"github.com/stretchr/testify/require"
)

func AssertDockerfileTransform(t *testing.T, input, expectedPreamble, expectedDockerfile string) {
t.Helper()

preamble, body, err := Transform([]byte(input), map[string]string{})
require.NoError(t, err)
assert.NotNil(t, preamble)
assert.NotNil(t, body)
bodyText, err := dockerfile.Print(body)
assert.Equal(t, expectedDockerfile, bodyText)
preambleText, err := dockerfile.Print(preamble)
assert.Equal(t, expectedPreamble, preambleText)
}

func TestTransformPyenvs(t *testing.T) {
testDockerfile := `# syntax=astronomer/astro-runtime
ARG baseimage
Expand All @@ -29,7 +42,6 @@ COPY --link --from=python:3.8-slim-bullseye /usr/local/lib/*3.8*.so* /usr/local/
COPY --link --from=python:3.8-slim-bullseye /usr/local/lib/python3.8 /usr/local/lib/python3.8
RUN /sbin/ldconfig /usr/local/lib
RUN ln -s /usr/local/include/python3.8 /usr/local/include/python3.8m
USER astro
RUN mkdir -p /home/astro/.cache/pip /home/astro/.venv/venv1
COPY --chown=50000:0 reqs/venv1.txt /home/astro/.venv/venv1/requirements.txt
Expand All @@ -45,22 +57,47 @@ COPY --link --from=python:3.10-slim-bullseye /usr/local/lib/*3.10*.so* /usr/loca
COPY --link --from=python:3.10-slim-bullseye /usr/local/lib/python3.10 /usr/local/lib/python3.10
RUN /sbin/ldconfig /usr/local/lib
RUN ln -s /usr/local/include/python3.10 /usr/local/include/python3.10m
USER astro
RUN mkdir -p /home/astro/.cache/pip /home/astro/.venv/venv2
RUN /usr/local/bin/python3.10 -m venv /home/astro/.venv/venv2
ENV ASTRO_PYENV_venv2 /home/astro/.venv/venv2/bin/python
RUN mkdir /tmp/bar
`
preamble, body, err := Transform([]byte(testDockerfile), map[string]string{})
require.NoError(t, err)
assert.NotNil(t, preamble)
assert.NotNil(t, body)
bodyText, err := dockerfile.Print(body)
assert.Equal(t, expectedDockerfile, bodyText)
preambleText, err := dockerfile.Print(preamble)
assert.Equal(t, expectedPreamble, preambleText)

AssertDockerfileTransform(t, testDockerfile, expectedPreamble, expectedDockerfile)
}

func TestPython3_8(t *testing.T) {
testDockerfile := `# syntax=astronomer/astro-runtime
ARG baseimage
FROM ${baseimage}
PYENV 3.7 venv1 reqs/venv1.txt
COPY foo bar
`
expectedPreamble := `
ARG baseimage
FROM ${baseimage}
`
expectedDockerfile := `USER root
COPY --link --from=python:3.7-slim-bullseye /usr/local/bin/*3.7* /usr/local/bin/
COPY --link --from=python:3.7-slim-bullseye /usr/local/include/python3.7* /usr/local/include/python3.7
COPY --link --from=python:3.7-slim-bullseye /usr/local/lib/pkgconfig/*3.7* /usr/local/lib/pkgconfig/
COPY --link --from=python:3.7-slim-bullseye /usr/local/lib/*3.7*.so* /usr/local/lib/
COPY --link --from=python:3.7-slim-bullseye /usr/local/lib/python3.7 /usr/local/lib/python3.7
RUN /sbin/ldconfig /usr/local/lib
RUN ln -s /usr/local/include/python3.7 /usr/local/include/python3.7m
USER astro
RUN mkdir -p /home/astro/.cache/pip /home/astro/.venv/venv1
COPY --chown=50000:0 reqs/venv1.txt /home/astro/.venv/venv1/requirements.txt
RUN /usr/local/bin/python3.7 -m venv /home/astro/.venv/venv1
ENV ASTRO_PYENV_venv1 /home/astro/.venv/venv1/bin/python
RUN --mount=type=cache,uid=50000,gid=0,target=/home/astro/.cache/pip /home/astro/.venv/venv1/bin/pip --cache-dir=/home/astro/.cache/pip install -r /home/astro/.venv/venv1/requirements.txt
COPY foo bar
`

AssertDockerfileTransform(t, testDockerfile, expectedPreamble, expectedDockerfile)
}

func TestTransformsBuildArgsFromDockerfile(t *testing.T) {
Expand All @@ -73,14 +110,7 @@ ARG ver=7.0.0
FROM quay.io/astronomer/astro-runtime:7.0.0-base
`

preamble, body, err := Transform([]byte(testDockerfile), map[string]string{})
require.NoError(t, err)
assert.NotNil(t, preamble)
assert.NotNil(t, body)
bodyText, err := dockerfile.Print(body)
assert.Equal(t, "", bodyText)
preambleText, err := dockerfile.Print(preamble)
assert.Equal(t, expectedPreamble, preambleText)
AssertDockerfileTransform(t, testDockerfile, expectedPreamble, "")
}

func TestTransformsBuildArgsProvided(t *testing.T) {
Expand Down Expand Up @@ -137,3 +167,38 @@ ONBUILD RUN echo "hi!"
bodyText, err := dockerfile.Print(body)
assert.Equal(t, expectedDockerfile, bodyText)
}

func TestMultipleVenvOfSameVersion(t *testing.T) {
testDockerfile := `# syntax=astronomer/astro-runtime
FROM quay.io/astronomer/astro-runtime:9.11.0
PYENV 3.11 one
PYENV 3.11 two
`

expectedPreamble := `
FROM quay.io/astronomer/astro-runtime:9.11.0-base
`

// Since this is two versions of the same python, we only need to copy it once
expectedDockerfile := `USER root
COPY --link --from=python:3.11-slim-bullseye /usr/local/bin/*3.11* /usr/local/bin/
COPY --link --from=python:3.11-slim-bullseye /usr/local/include/python3.11* /usr/local/include/python3.11
COPY --link --from=python:3.11-slim-bullseye /usr/local/lib/pkgconfig/*3.11* /usr/local/lib/pkgconfig/
COPY --link --from=python:3.11-slim-bullseye /usr/local/lib/*3.11*.so* /usr/local/lib/
COPY --link --from=python:3.11-slim-bullseye /usr/local/lib/python3.11 /usr/local/lib/python3.11
RUN /sbin/ldconfig /usr/local/lib
USER astro
RUN mkdir -p /home/astro/.cache/pip /home/astro/.venv/one
RUN /usr/local/bin/python3.11 -m venv /home/astro/.venv/one
ENV ASTRO_PYENV_one /home/astro/.venv/one/bin/python
RUN mkdir -p /home/astro/.cache/pip /home/astro/.venv/two
RUN /usr/local/bin/python3.11 -m venv /home/astro/.venv/two
ENV ASTRO_PYENV_two /home/astro/.venv/two/bin/python
`

AssertDockerfileTransform(t, testDockerfile, expectedPreamble, expectedDockerfile)
}

0 comments on commit 00a2a8e

Please sign in to comment.