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

[Bug]: 'latest' tag is pointing to "v1.46.1-jammy" docker image #32483

Closed
Th3S4mur41 opened this issue Sep 6, 2024 · 15 comments
Closed

[Bug]: 'latest' tag is pointing to "v1.46.1-jammy" docker image #32483

Th3S4mur41 opened this issue Sep 6, 2024 · 15 comments

Comments

@Th3S4mur41
Copy link
Contributor

Th3S4mur41 commented Sep 6, 2024

Version

1.47.0

Steps to reproduce

docker pull mcr.microsoft.com/playwright:latest

Expected behavior

Pulling "v1.47.0-noble" (sha256:3d41153494e2b12a5a5fa6e26cf1e854c3997d13758754d46e7bf902b5ba09b1)

Actual behavior

Pulling "v1.46.1-jammy" (sha256:02810c978d5396bf382ab6015c25ad6bed9e39f4a41c5b9c829e9fea439274e2)

Additional context

v1.47.0 Release notes => "The mcr.microsoft.com/playwright:v1.47.0 now serves a Playwright image based on Ubuntu 24.04 Noble.
To use the 22.04 jammy-based image, please use mcr.microsoft.com/playwright:v1.47.0-jammy instead."

Environment

System:
    OS: Linux 6.5 Ubuntu 20.04.6 LTS (Focal Fossa)
    CPU: (2) x64 AMD EPYC 7763 64-Core Processor
    Memory: 6.35 GB / 7.74 GB
    Container: Yes
  Binaries:
    Node: 20.17.0 - /usr/local/share/nvm/versions/node/v20.17.0/bin/node
    Yarn: 1.22.19 - /usr/bin/yarn
    npm: 10.5.1 - /workspaces/components/node_modules/.bin/npm
  IDEs:
    VSCode: 1.92.2 - /vscode/bin/linux-x64/fee1edb8d6d72a0ddff41e5f71a671c23ed924b9/bin/remote-cli/code
  Languages:
    Bash: 5.0.17 - /usr/bin/bash
@mxschmitt
Copy link
Member

We strongly recommend pinning to a versioned tag, e.g. in your case:

mcr.microsoft.com/playwright:v1.47.0-noble

using :latest won't guarantee a reproducible build environment and break over time. Would that work for you?

@Th3S4mur41
Copy link
Contributor Author

The Docker image is used to run the tests in GitHub CI.

I have Dependabot configured to update NPM packages including playwright automatically.
This works fine most of the time combined with using the :latest image.

While it would be ideal to have the image used by CI match the version from the NPM package, this would currently mean manually edit the image tag every time Dependabot opens a PR to update the playwright packages.
If there is a good way to do this automatically, this would be great otherwise it would require too much effort considering how rare :latest is actually causing an issue

@anastasiia-petrenko
Copy link

Having a hardcoded version on one hand means the stability of the executed code and on the other hand, users might miss to update once the new version is released and not being able to use new features w/o additional update of the tags in 2 places

@marshmn
Copy link
Contributor

marshmn commented Sep 6, 2024

I agree with the advice to use specific version tags for Docker images. It is true that it does lead to a bit of pain to keep the Docker image version in sync with the Playwright NPM package version - but worth the effort to automate that in some way.

FWIW, I posted in Discord a while ago regarding the rough process I follow for this:

  • In my package.json I have Playwright pinned to a specific version, e.g: "@playwright/test": "1.46.0", (note that there is no ^ or ~ in there)
  • In my Pipeline config, I use a specific Playwright image tag, e.g: mcr.microsoft.com/playwright:v1.46.0-jammy

I then have an update script which I run to update all of the dependencies in my project (usually on a weekly basis), which includes:

  • Queries the Microsoft container registry to find out what is the latest available Playwright container image tag
  • Based on the version detected, updates both package.json and the pipeline configuration to use the new version

This way, everything stays in sync.

I've found this approach to work well for me.

@Th3S4mur41
Copy link
Contributor Author

Th3S4mur41 commented Sep 6, 2024

Since Dependabot is handling the updates... That would mean writing a job in the CI that would read the playwright version from the package.json and update the Workflow YAML with the corresponding version...

I have to think about this a bit more and make sure this doesn't create some security issues or other kind of problems 🤔

The main question still remains though, shouldn't the latest tag point to the same version for the NPM package and the Docker Image?

@mxschmitt
Copy link
Member

Closing as per #32483 (comment). We recommend pinning for having reproducible builds. In your case configuring Dependabot to also update your Dockerfile might work. Alternatively without Docker, you can use our recommended approach which does not require bumping two versions.

We updated the release notes to reflect that change https://github.com/microsoft/playwright/releases/tag/v1.47.0.

The main question still remains though, shouldn't the latest tag point to the same version for the NPM package and the Docker Image?

:latest was not something we officially supported nor documented in the past, so we advise against it.

@marshmn
Copy link
Contributor

marshmn commented Sep 6, 2024

Whilst I'm an advocate that people should use a specific tag - especially in pipelines etc., it does seem very odd for the :latest tag to point to an old version. It goes against common Docker practice and will likely catch people out.

E.g. I'm sure it will be common for someone when first trying out Playwright, to just do something like docker pull mcr.microsoft.com/playwright without specifying any tag at all (and therefore using :latest by default). They wouldn't expect to get an old version.

I think this ought to be reconsidered. If you really don't want to support the :latest tag, then it should probably be removed entirely—but that will undoubtedly cause some chaos and will leave a slightly uncommon registry where no :latest tag exists...

@mxschmitt
Copy link
Member

While I agree with above, removing a tag is unfortunately not an option, since it will also break existing customers and is also against Docker best practises. If we would start again, we'd not publish :latest. In all our documentation we advocate for specific tags, so we don't think this is something where a lot of users will run into. We can collect feedback for now and re-iterate if needed.

@marshmn
Copy link
Contributor

marshmn commented Sep 7, 2024

OK, fair enough.

In that case, I'd suggest making the documentation a little clearer. Right now it states:

It is recommended to always pin your Docker image to a specific version if possible.

It should perhaps say that :latest is now deprecated and will give an old version of Playwright?

Sorry to keep pushing on this one, but I genuinely feel it's going to catch some people out, and hence updating the docs to make it as clear as possible would seem sensible.

@Th3S4mur41
Copy link
Contributor Author

Th3S4mur41 commented Sep 10, 2024

Unfortunately, using the tagged image turns out to be more complicated than expected.
We're using a shared workflow that is defined in a different repository than the ones actually having playwright installed. Moreover, it is used by various repos that potentially have different playwright version.

So, the ideal solution would require the calling workflow to output the playwright version installed and pass it as an argument to the shared workflow. The shared workflow should then set the container image based on this argument.
I'm not sure GitHub is allowing that though since image is the only container related field that is not using a variable in any of the examples in the docs.

Our current workaround is simply run npx playwright install in the container before running the test. Probably not the best solution, but working until we figure out if the way described above is feasible.

@marekdedic
Copy link

Hello, I'm also having trouble with this change - as probably more people in this thread, I used the docker container in GitHub actions with dependabot updating playwright automatically for me. That means there is no Dockerfile. If I understand the discussion correctly, I now have these options:

  1. Don't use the Docker container and install browsers on each CI run - in my experience, this means the CI takes twice as long.
  2. Keep the Docker container and manually update its version every time there is a Playwright update - creating more work for me perpetually
  3. Implement some script magic that overwrites my GitHub action pipeline config file every time playwright updates/periodically - this script doesn't seem to be implemented by anyone yet and since it would need to have write permissions for the repo, it comes with a potential security compromise.

Am I missing something here? Is there some "silver bullet"?

@mxschmitt
Copy link
Member

@marekdedic sorry to hear! I was looking into 3 and found an approach which seems to work quite nicely and which applies the best practises. Would that work for you?

name: Playwright Tests
on:
  push:
    branches: [ main, master ]
  pull_request:
    branches: [ main, master ]
jobs:
  read-playwright-version:
    name: 'Read Playwright Version'
    runs-on: ubuntu-latest
    outputs:
      playwright-version: ${{ steps.get_playwright_version.outputs.playwright-version }}
    steps:
      - uses: actions/checkout@v4
      - run: |
          playwright_version=$(node -e 'console.log(require("./package-lock.json")["packages"]["node_modules/@playwright/test"].version)')
          echo "playwright-version=$playwright_version" >> $GITHUB_OUTPUT
        id: get_playwright_version
  playwright:
    name: 'Run Playwright Tests'
    runs-on: ubuntu-latest
    needs: read-playwright-version
    container:
      image: mcr.microsoft.com/playwright:v${{ needs.read-playwright-version.outputs.playwright-version }}-noble
    steps:
      - uses: actions/checkout@v4
      - uses: actions/setup-node@v4
        with:
          node-version: 18
      - name: Install dependencies
        run: npm ci
      - name: Run your tests
        run: npx playwright test
        env:
          HOME: /root

(The reason why I read package-lock.json is to avoid doing "npm ci" which usually takes a few seconds. If you use a different package manager, this step might be different).

@marekdedic
Copy link

@mxschmitt That looks promising, thanks! It's still one more job than it used to be, but I think that compared to installing the browsers every time it would still be much better. :)

@marekdedic
Copy link

@mxschmitt I can confirm that your suggestion works nicely, thank you!

If anyone stumbles upon this in the future, see e.g. marekdedic/dedic.eu#351

@Th3S4mur41
Copy link
Contributor Author

Thanks for this example @mxschmitt.

I used it to create a GitHub Action to handle this that works with package-lock.json, yarn.lock and pnpm-lock.yaml

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants