From 00a2a8ea4df2c3b6686c6d486f5e1d0000d5eb17 Mon Sep 17 00:00:00 2001 From: Ash Berlin-Taylor Date: Tue, 19 Mar 2024 16:01:42 +0000 Subject: [PATCH] Correctly handle producing multiple Venvs for the same python version 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 --- buildkit/go.mod | 3 +- buildkit/go.sum | 11 +- buildkit/internal/transform/transforms.go | 18 +++- .../internal/transform/transforms_test.go | 101 ++++++++++++++---- 4 files changed, 108 insertions(+), 25 deletions(-) diff --git a/buildkit/go.mod b/buildkit/go.mod index 4f794e7..6c99149 100644 --- a/buildkit/go.mod +++ b/buildkit/go.mod @@ -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 @@ -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 diff --git a/buildkit/go.sum b/buildkit/go.sum index d2da6fe..d36deca 100644 --- a/buildkit/go.sum +++ b/buildkit/go.sum @@ -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= @@ -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= @@ -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= @@ -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= @@ -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= @@ -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= diff --git a/buildkit/internal/transform/transforms.go b/buildkit/internal/transform/transforms.go index 799b5c6..ac8fa97 100644 --- a/buildkit/internal/transform/transforms.go +++ b/buildkit/internal/transform/transforms.go @@ -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" @@ -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}} @@ -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 { @@ -60,6 +63,7 @@ type virtualEnv struct { PythonFlavour string PythonMajorMinor string RequirementsFile string + Py37OrOlder bool } func newTransformer(buildArgs map[string]string) *Transformer { @@ -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 { @@ -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 } @@ -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) @@ -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 } diff --git a/buildkit/internal/transform/transforms_test.go b/buildkit/internal/transform/transforms_test.go index 88ae827..22fa852 100644 --- a/buildkit/internal/transform/transforms_test.go +++ b/buildkit/internal/transform/transforms_test.go @@ -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 @@ -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 @@ -45,7 +57,6 @@ 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 @@ -53,14 +64,40 @@ 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) { @@ -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) { @@ -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) +}