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

Upgrade to Node 20 #3783

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

lucferbux
Copy link
Contributor

@lucferbux lucferbux commented Feb 20, 2025

Description

Upgrade dashboard and CI/CD to Node 22

How Has This Been Tested?

  • Running locally the dashboard with nvm
  • Installed quay.io/lferrnan/odh-dashboard:rhoaieng-7374

Test Impact

Request review criteria:

Self checklist (all need to be checked):

  • The developer has manually tested the changes and verified that the changes work
  • Testing instructions have been added in the PR body (for PRs involving changes that are not immediately obvious).
  • The developer has added tests or explained why testing cannot be added (unit or cypress tests for related changes)

If you have UI changes:

  • Included any necessary screenshots or gifs if it was a UI change.
  • Included tags to the UX team if it was a UI/UX change.

After the PR is posted & before it merges:

  • The developer has tested their solution on a cluster by using the image produced by the PR to main

Copy link
Contributor

openshift-ci bot commented Feb 20, 2025

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress This PR is in WIP state label Feb 20, 2025
Copy link
Contributor

openshift-ci bot commented Feb 20, 2025

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please ask for approval from lucferbux. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Copy link

codecov bot commented Feb 20, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 84.64%. Comparing base (9b6b7a6) to head (03a3753).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #3783      +/-   ##
==========================================
+ Coverage   84.63%   84.64%   +0.01%     
==========================================
  Files        1515     1515              
  Lines       35100    35100              
  Branches     9814     9814              
==========================================
+ Hits        29707    29712       +5     
+ Misses       5393     5388       -5     

see 4 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9b6b7a6...03a3753. Read the comment docs.

@lucferbux lucferbux changed the title Upgrade to Node 22 Upgrade to Node 20 Feb 25, 2025
@lucferbux lucferbux marked this pull request as ready for review February 25, 2025 11:01
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress This PR is in WIP state label Feb 25, 2025
@lucferbux
Copy link
Contributor Author

@christianvogt @andrewballantyne weird that there's still a Tests 18.x that's stuck I guess GA is not detecting the changes, either way, this is ready to go, I still need to test the image in a cluster, will do tomorrow. But devops is fine with the changes and we have red-hat-data-services#545 in place to change konflux too

@andrewballantyne
Copy link
Member

18.x is likely still here because it's required by branch protections... I could swap it to 20.x but it seems I might open up every other PR to not needing their tests 🤔

@@ -7,7 +7,7 @@ ARG CYPRESS_CACHE=$USER_HOME/.cypress-cache
ARG CHROME=https://dl.google.com/linux/direct/google-chrome-stable_current_x86_64.rpm
ARG OCP_CLI=https://mirror.openshift.com/pub/openshift-v4/clients/ocp/stable/openshift-client-linux.tar.gz
ARG NVM_INSTALLER=https://raw.githubusercontent.com/creationix/nvm/v0.39.7/install.sh
ARG NODE_VERSION=v20.15.0
ARG NODE_VERSION=v20.18.0
Copy link
Member

Choose a reason for hiding this comment

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

Have we always built our images with 20? lol... interesting...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, @manosnoam suggested this a long time ago, I cannot recall the conversation but we agreed on having ci tests on Node 20.

@lucferbux
Copy link
Contributor Author

Ok, tested in a current clsuter, everything seems to be working just fine, image is quay.io/lferrnan/odh-dashboard:rhoaieng-7374 this PR is ready to go cc @andrewballantyne @christianvogt

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