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

[OSD] include Node 14.21.3 #3356

Merged
merged 1 commit into from
Apr 4, 2023

Conversation

kavilla
Copy link
Member

@kavilla kavilla commented Mar 30, 2023

Description

Including Node 14.21.3 to the env image since this is the version we call out in the nvm file.

We do a relaxed version check on main branch.

Issues Resolved

Potentially related to:
opensearch-project/OpenSearch-Dashboards#3282

I see the failures on fiber for arm deb which could be the case if the system but will need to verify it's not the diff in Node first.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Including Node 14.21.3 to the env image since this is the version
we call out in the nvm file.

We do a relaxed version check on main branch.

Signed-off-by: Kawika Avilla <[email protected]>
@@ -5,7 +5,7 @@ build:
version: 1.4.0
ci:
image:
name: opensearchstaging/ci-runner:centos7-x64-arm64-jdkmulti-node10.24.1-cypress6.9.1-20211028
name: opensearchstaging/ci-runner:ci-runner-centos7-opensearch-dashboards-build-v2
Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

Hi @kavilla isnt 1.x will lock to node 10.24.1?

Copy link
Member Author

@kavilla kavilla Mar 30, 2023

Choose a reason for hiding this comment

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

This image hasn't been updated for 1.4 but all the other builds on 1.x use this image. For example, https://github.com/opensearch-project/opensearch-build/blob/main/manifests/1.3.9/opensearch-dashboards-1.3.9.yml#L8

@codecov-commenter
Copy link

codecov-commenter commented Mar 30, 2023

Codecov Report

Merging #3356 (cb20cad) into main (59ab434) will not change coverage.
The diff coverage is n/a.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@           Coverage Diff           @@
##             main    #3356   +/-   ##
=======================================
  Coverage   91.74%   91.74%           
=======================================
  Files         172      172           
  Lines        4991     4991           
=======================================
  Hits         4579     4579           
  Misses        412      412           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

Copy link
Member

@peterzhuamazon peterzhuamazon left a comment

Choose a reason for hiding this comment

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

Hi @kavilla

  1. Please update all the existing dockerfile in current dir to this new version, if they install node as part of the building process.
  2. Need to update the code here to reflect Windows.
    https://github.com/opensearch-project/opensearch-ci/blob/main/packer/scripts/windows/scoop-install-commons.ps1#L130
  3. All images needs to be rebuild before consuming.

And could you please confirm what is this change for? Which version?
This needs to be coordinate with our team.

Thanks.

kavilla added a commit to kavilla/opensearch-ci that referenced this pull request Mar 30, 2023
As called here:
opensearch-project/opensearch-build#3356

Add the node version called out in our nvm file.

Signed-off-by: Kawika Avilla <[email protected]>
@kavilla
Copy link
Member Author

kavilla commented Mar 30, 2023

Hey @peterzhuamazon,

1. Please update all the existing dockerfile in `current` dir to this new version, if they install node as part of the building process.

I think I've updated the hardcoded refs of the node version to just have it in the path. Just as long as it's related to the build. Can you point to which one I should update?

2. Need to update the code here to reflect Windows.
   https://github.com/opensearch-project/opensearch-ci/blob/main/packer/scripts/windows/scoop-install-commons.ps1#L130

Sorry about that. Here it is: opensearch-project/opensearch-ci#264

3. All images needs to be rebuild before consuming.

Yeah, not a pressing priority, I wouldn't drop any other priorities. I would track this only for 3.0.0 and to help get the builds successful for OSD 3.0.0. Since we don't have a current successful release of 3.0.0 for a little I wanted to get the OSD project building on the .nvm version in our repo https://github.com/opensearch-project/OpenSearch-Dashboards/blob/main/.nvmrc. Which is causing some issues with our CI in the FTRepo.

The current build failure with DEB ARM seems to be related to the fibers not being available for the node version and platform:

Please make sure you are using a supported platform and node version. If you
would like to compile fibers on this machine please make sure you have setup your
build environment--
Windows + OS X instructions here: https://github.com/nodejs/node-gyp
Ubuntu users please run: `sudo apt-get install g++ build-essential`
RHEL users please run: `yum install gcc-c++` and `yum groupinstall 'Development Tools'` 
Alpine users please run: `sudo apk add python make g++`
/bin/sh: nodejs: command not found

Just wanted to get this one to get the repos aligned just in case it does help and should probably be done anyways. Right now the node version check is relaxed so we can build on other machines successfully even if the node version available is not the matching version.

But if this doesn't fix it I will fix something in our repo to not utilize fibers for DEB ARM. Fibers is used to speed up the build.

@peterzhuamazon
Copy link
Member

Thanks @kavilla for the responses I mistakenly identified another image having nodejs reference.
If this is ongoing for the 3.x I will just approve these and we can sync up next week on the builds.

Thanks.

@peterzhuamazon
Copy link
Member

Also please fix the check error, thanks.

- name: dashboards-reports
repository: https://github.com/opensearch-project/dashboards-reports.git
- name: opensearch-reports
repository: https://github.com/opensearch-project/reporting.git
Copy link
Member

Choose a reason for hiding this comment

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

Hi,
There is a repo split on the 2.5.0 version.
2.4.0 is untouched.
The tag exists for the dashboards-reports.
This should not be changed here.
Thanks.

@kavilla kavilla force-pushed the avillk/bump_node branch from cb20cad to a14b963 Compare April 3, 2023 23:16
@kavilla kavilla requested a review from peterzhuamazon April 3, 2023 23:53
@peterzhuamazon
Copy link
Member

Async is having issues on 3.x still.


* What went wrong:
A problem occurred evaluating root project 'asynchronous-search'.
> Can't get https://ci.opensearch.org/ci/dbc/distribution-build-opensearch/2.7.0/latest/linux/x64/tar/builds/opensearch/plugins/opensearch-asynchronous-search-2.7.0.0.zip to /tmp/tmpp1cctb6b/asynchronous-search/src/test/resources/org/opensearch/search/asynchronous/bwc/2.7.0.0/opensearch-asynchronous-search-2.7.0.0.zip

Not related, merged.

@peterzhuamazon peterzhuamazon merged commit 95cd94f into opensearch-project:main Apr 4, 2023
peterzhuamazon pushed a commit to opensearch-project/opensearch-ci that referenced this pull request Apr 4, 2023
As called here:
opensearch-project/opensearch-build#3356

Add the node version called out in our nvm file.

Signed-off-by: Kawika Avilla <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants