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

Update GitHub Actions to use Node 20 #4679

Open
wants to merge 2 commits into
base: dev
Choose a base branch
from

Conversation

@jdblischak
Copy link
Contributor Author

Well that failed. I didn't see the warning in the YAML file:

# v4 uses node 20 which is incompatible with the libc version of the manylinux image

Do we have a plan for how to proceed with this? The manylinux Docker image is purposefully as old as possible and rarely updated. As long as GitHub doesn't escalate the current warning into an error, I think it should be fine to continue running Node 16 actions in a Docker container that supports Node 16.

@jdblischak
Copy link
Contributor Author

As long as GitHub doesn't escalate the current warning into an error, I think it should be fine to continue running Node 16 actions in a Docker container that supports Node 16.

No, I was wrong. Because the manylinux builds are included in the same matrix as non-manylinux builds, we need to run the Node 16 actions in the manylinux containers and the Node 20 actions in the GitHub runners. This requires a lot of duplicate steps. The alternative would be to separate the manylinux builds out into their own workflows. That would also require a decent amount of duplication, but I think it could result in a more readable workflow (especially since there are already existing steps that only run either inside or outside of a manylinux build).

@teo-tsirpanis
Copy link
Member

CentOS 7 goes out of support in June. Maybe we can wait until then and bump our manylinux images?

As long as GitHub doesn't escalate the current warning into an error

I don't think it will, they will force these actions to use the new Node, which I guess will fail on the old manylinux.

@jdblischak
Copy link
Contributor Author

CentOS 7 goes out of support in June. Maybe we can wait until then and bump our manylinux images?

Hard to say. I don't know how serious GitHub is about their Spring 2024 deadline.

I don't think it will, they will force these actions to use the new Node, which I guess will fail on the old manylinux.

What do you think of the strategy I've started in this PR? (there are more actions that need updated) I'm trying to run the Node 20 version on the GitHub runners and only use the deprecated Node 16 versions when they are executed inside the manylinux image.

@@ -132,7 +161,7 @@ jobs:
runs-on: ubuntu-latest
steps:
- name: Download release artifacts
uses: actions/download-artifact@v2
uses: actions/download-artifact@v4
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Artifacts uploaded by v3 cannot be downloaded by v4. The release workflow should always use v3 for the artifact actions.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good find. This seems like a problem to me. IIUC the deprecation notice correctly, at some point Node 16 actions will not be able to run on the standard GitHub Actions runners. Thus we'd have to use v4, and could no longer download the manylinux artifacts uploaded by v3.

One workaround would be to run this job that downloads and tests the release artifacts inside of the manylinux image for the manylinux artifacts. But at some point it might be easier to simply migrate the manylinux builds to a separate CI pipeline (either its own dedicated GitHub Actions pipeline(s) or switch to another service like Azure).

@teo-tsirpanis
Copy link
Member

What do you think of the strategy I've started in this PR?

Unless someone else objects to some duplication in our workflows, looks fine by me.

@jdblischak
Copy link
Contributor Author

Relevant for this PR. Just saw this in the TileDB 2.21.0-rc0 release notes:

Support for GLIBC 2.17 (manylinux2014, based on CentOS 7) will be removed in TileDB version 2.31, to be released in Q4 2024. It will be replaced by support for GLIBC 2.28 (manylinux_2_28; RHEL8; Rocky Linux 8).

If we need to use the CentOS 7 manylinux image (which doesn't have Node 20) in our CI until Q4 2024, then we'll have to decide how to address GitHub's deprecation of the Node 16 actions in Spring 2024. We can either use lots of if statements (like this PR attempts) to only run the Node 16 actions inside the manylinux container, or alternatively split the manylinux CI into its own pipeline (after working on this PR, I think that approach might be easier and more readable).

@teo-tsirpanis
Copy link
Member

only run the Node 16 actions inside the manylinux container

If I understand GitHub's announcement correctly, it will always run all actions with Node 20, even if specified to run with Node 16, and even if running inside the CentOS 7 manylinux container. Therefore if we want to run CI with CentOS 7 manylinux beyond GitHub's deprecation's date, we will have to resort to means outside standard GitHub Actions (ranging from doing the build/tests in a Dockerfile, to self-hosted runners).

@jdblischak
Copy link
Contributor Author

If I understand GitHub's announcement correctly, it will always run all actions with Node 20, even if specified to run with Node 16, and even if running inside the CentOS 7 manylinux container.

That's not my understanding. My understanding is that they are going to drop Node 16 from their runners, so any Node 16 actions will fail.

Therefore if we want to run CI with CentOS 7 manylinux beyond GitHub's deprecation's date, we will have to resort to means outside standard GitHub Actions (ranging from doing the build/tests in a Dockerfile, to self-hosted runners).

Isn't that what we are already doing? The manylinux CI steps run in the manylinux Docker container, which has Node 16 installed. As long was we keep the actions versions to the Node 16 ones, they should continue to execute fine inside the Docker container

@teo-tsirpanis
Copy link
Member

Hmm maybe you are right and I did not understand correctly. Let's see by then…

@jdblischak
Copy link
Contributor Author

Updated timeline from GitHub for migrating from Node 16 to Node 20 actions:

  • June 30th, 2024: Node 20 becomes the default. Have to set ACTIONS_ALLOW_USE_UNSECURE_NODE_VERSION=true to continue using Node 16
  • October 2024: Must complete migration to Node 20

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

Successfully merging this pull request may close these issues.

2 participants