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

automate deployment #1312

Open
MylesBorins opened this issue Aug 12, 2020 · 30 comments
Open

automate deployment #1312

MylesBorins opened this issue Aug 12, 2020 · 30 comments

Comments

@MylesBorins
Copy link
Contributor

How can we automate deployment of the docker images?

What steps are we currently doing manually?

Could we do this with actions?

/cc @mmarchini for her actions expertise

@mmarchini
Copy link

I think some of it is automated already? Yesterday I logged into the bot account and saw some activity where the bot sends PRs to the official Docker image library. Seems like we could also automate bumping the Node.js versions when there's a new release.

Funny enough I wrote a script a couple weeks ago to fetch the latest versions of all maintained release lines:

'use strict'

import { default as nv} from '@pkgjs/nv';
import { default as semver } from "semver";
import { writeFile } from 'fs/promises';

const versions = (await nv('maintained')).map(obj => obj.version)
  .sort(semver.compare)

console.log(versions.join('\n'))

Could be adapted to whatever we need for this repository.

@mmarchini
Copy link

Actually, looks like the repository already has a script to update all images? https://github.com/nodejs/docker-node/blob/master/update.sh

If so, we could have a daily Action that runs the script, and if the repository has any changes pushes it and creates a PR (or just pushes to the default branch, whichever works best).

@SimenB
Copy link
Member

SimenB commented Aug 12, 2020

Yeah, an action running the update script should be fine. One snag is that we need to wait for the musl builds to complete so we can get them from https://unofficial-builds.nodejs.org/download/release/. Usually they are available not long after the official builds (I think about an hour?). We should probably just make the update script fail if it's not available while official builds are.

At that point, the bot could auto merge green PRs and the usual build from master would open a PR (the last part is implemented today)

@SimenB
Copy link
Member

SimenB commented Aug 12, 2020

Semi-related is #1194 which converts much of our CI from Travis to GH Actions

@SimenB
Copy link
Member

SimenB commented Aug 12, 2020

Oh, and /cc @nodejs/docker

@mmarchini
Copy link

Just pointing out that adding a scheduler Action should be possible without #1194, especially since that's a long PR and the scheduler Action would probably be short.

@SimenB
Copy link
Member

SimenB commented Aug 12, 2020

For sure, I just assume the action (or whatever) for auto-merging PRs would work better if it just checked that all other workflows passed successfully rather than pinging travis. I guess we can just check the status tho, so maybe there's no difference 🙂

@nschonni
Copy link
Member

Took a quick kick at the cron action in #1314

@tianon
Copy link
Contributor

tianon commented Sep 21, 2020

Very much related to this, the current downstream automation periodically makes very poor PRs, and we'd like to see it either fixed or disabled before it makes any more.

See docker-library/official-images#8281 (comment), docker-library/official-images#8403 (comment), and docker-library/official-images#8727 (comment) for notes, and docker-library/official-images#7947 + docker-library/official-images#7898 for further examples.

@SimenB
Copy link
Member

SimenB commented Sep 21, 2020

We should probably go for squash merges, then we can make sure the commit message and commit body makes during merge and just re-use that as is for the upstream PR. Currently squash merges are disabled in this repo since they mess up the script (it looks for merge commits IIRC)

@nschonni
Copy link
Member

@tianon is it just the PR titles, or is there something else about the PRs as well?

Was sketching out an action to use the pull_request closed event instead of push to master, so it can use the PR title

name: Create official images PR

on:
  pull_request_target:
    types:
      - closed
    paths:
      - "generate-stackbrew-*.sh"
      - "**/Dockerfile"
      - "**/architectures"

jobs:
  on_merged:
    if: github.event.pull_request.merged_by != ''
    runs-on: ubuntu-latest

    steps:
      - name: Checkout the docker-node repo
        uses: actions/checkout@v2
        with:
          path: docker-node

      - name: Checkout the official-images repo
        uses: actions/checkout@v2
        with:
          path: official-images
          repository: docker-library/official-images

      - name: Generate Stackbrew for diff
        run: |
          cd docker-node
          ./generate-stackbrew-library.sh > ../official-images/library/node
          
      - name: Generate Stackbrew
        run: echo ${{ github.event.pull_request.title}}

@tianon
Copy link
Contributor

tianon commented Sep 23, 2020

It's just the PR/commit titles -- otherwise the PRs are fine.

@ttshivers
Copy link
Member

I might also investigate writing an cronjob action that auto creates PRs for new nodejs, yarn, and alpine versions.

@nschonni
Copy link
Member

nschonni commented Sep 24, 2020

There are some pending PRs for that #1317 #1314, but for Yarn, it's only bumped during a non-security Node release https://github.com/nodejs/docker-node/blob/master/CONTRIBUTING.md#version-updates

A nice thing for syncing would be to parse the official images upstream to match the supported architectures

@ttshivers
Copy link
Member

There are some pending PRs for that #1317 #1314, but for Yarn, it's only bumped during a non-security Node release https://github.com/nodejs/docker-node/blob/master/CONTRIBUTING.md#version-updates

I nice syncing with be to parse the official images upstream to match the supported architectures

Understood about the versioning of the yarn and npm and see the existing PR for alpine.

I don't quite understand what you mean by the last sentence.

@nschonni
Copy link
Member

Sorry, didn't proof that obviously. Fixed it up, but I mean grabbing the architectures from the base image values like https://github.com/docker-library/official-images/blob/master/library/debian#L48

@ttshivers
Copy link
Member

Sorry, didn't proof that obviously. Fixed it up, but I mean grabbing the architectures from the base image values like https://github.com/docker-library/official-images/blob/master/library/debian#L48

Is that what the architecture files are sourced from now? Are any arches filtered out based on node support?

@rhbecker
Copy link

If this issue were resolved, would that mean that docker hub would have image versions available on the same day that corresponding versions are announced as available?

As someone developing downstream pipeline automations, the lag is frustrating, as is the ambiguity around how long the lag will persist.

@SimenB
Copy link
Member

SimenB commented Sep 28, 2021

Main issue now (from our (docker team) side) is the wait for the musl build. It'd be trivial to build automation for the GH release of the prebuilt binaries on nodejs.org, but there's still at least an hour wait for the musl build.

Anyways, for further automation downstream you'd still be blocked by Docker merging the PR and then building and publishing it. So this issue is just a smaller part of the entire picture.

@Pehesi97
Copy link
Contributor

Me and some folks at NearForm want to help on this.

Could we get an quick overview of the current situation just so we're on the same page and maybe some steps we would need to take in order to automate docker-node images and push them to DockerHub when new node versions are released?

@SimenB
Copy link
Member

SimenB commented Feb 14, 2022

Awesome!

AFAIK my last comment is still true. A great first step is automatically opening a PR once the musl builds are complete, then merge it once CI is green. After that I think we still need a manual process on Docker's side, but at least things are as automated as possible on Node's side.

Security releases should run the update script with -s as well, so might need something to lookup of a particular release is a security release?

(a somewhat lazy version would be a cron job just running ./update.sh until it succeeds and there's a diff, but something more precise would be preferable.)

@Pehesi97
Copy link
Contributor

Pehesi97 commented Feb 15, 2022

So, the ideal GH Action would be checking the official node builds for new versions, right? Maybe every hour?

Then if it found a new version, it would check the unofficial node builds for new musl versions.

If it finds a new musl build and the new version is not a security release, run update.sh without args in a new branch and create a PR with main as the base branch.
If the new version is a security release, run update.sh -s and create a PR with main as the base branch.
If it doesn't find a musl build for the new version, exit gracefully.

Then we would have another action which would run when CI is green for the recently created PR, and automatically merge it.

Is that correct? I just want to make sure I'm getting every part of the process right.

@SimenB
Copy link
Member

SimenB commented Feb 15, 2022

Yeah, that sounds correct 👍

Ideally instead of a cron checking every hour there'd be some sort of webhook or something like it to trigger the actions when a new version is available (including musl), but that can be v2 I guess 🙂

@SimenB
Copy link
Member

SimenB commented Mar 10, 2022

First run of the action: https://github.com/nodejs/docker-node/actions/runs/1965626254. Failed since the unofficial build for musl isn't done yet. It just finished 2 minutes ago though, so a PR might be created when it next runs in 15 minutes.

EDIT: or not, seems the json file isn't updated until all versions are built, which takes a few more hours

@SimenB
Copy link
Member

SimenB commented Mar 10, 2022

Ok, one thing we need to change is just skip and not fail when waiting for musl:
image
very annoying notifications

@nschonni
Copy link
Member

This is moving in the right direction!
From #1662 I think there are some other minor tweaks:

  • shouldn't stage key-only changes. EX there are a few files changed because there is a new releaser, but those files shouldn't get pushed till an actual node release, like we do for Yarn-only changes
  • should swap the committer to the github bot Update to 17.7.0 #1662 (comment)

@nschonni
Copy link
Member

Figured out that git diff -G"ENV" --name-only will list only the files that have a version bump. Now I just need to see if I can integrate that into the workflow 😄

@SimenB
Copy link
Member

SimenB commented Mar 11, 2022

Wrong author: #1664

It also fails to actually request a review
image

Both of these might actually be fixed by using the bot token we have instead of the default token

@SimenB
Copy link
Member

SimenB commented Mar 11, 2022

Since we merged another PR updating, we will have to wait for a new node release to test it, though.

@SimenB
Copy link
Member

SimenB commented Mar 11, 2022

@nschonni getting rid of the extra diff is probably easiest by just doing git checkout DIR_OF_MAJOR_THAT_DIDN'T_CHANGE

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

No branches or pull requests

10 participants