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

[Campaign] Ensure Github workflow runs on docker image used by Production Distribution Build #614

Closed
peterzhuamazon opened this issue Oct 6, 2023 · 6 comments · Fixed by #620
Assignees
Labels
enhancement New feature or request v2.12.0

Comments

@peterzhuamazon
Copy link
Member

Hi All,

This is coming from the campaign here:

Overview

We would like your CI check (specifically plugin build) in GitHub Repo to run on top of the Build Docker Images from production distribution pipeline.

This is to ensure every plugin repo will use the exact docker images we used in Jenkins build, to check their PRs and run tests before merging the code, so that issues can be detected earlier, and environment can be identical across teams.

Solutions

The Build Team has created a simple script to dynamically retrieve the current docker image name/tag, so everyone can easily pull the images for their CI checks.

We have a trial run of the above with k-NN team. The script retrieves the docker image dynamically, save output, and use it as the docker image to pull for the upcoming run:

Note that GitHub Actions only support LINUX docker container at the time of this writing, so we will add Windows containers later on as well as macOS.

Implementation Notes

We would like you to review above PR and implement similar changes. Note on line 33 of the above k-NN PR, -u and -p parameters needs to assign values accordingly.

  • OpenSearch Plugin:
          CI_IMAGE_VERSION=`opensearch-build/docker/ci/get-ci-images.sh -p centos7 -u opensearch -t build | head -1`
  • OpenSearch-Dashboards Plugin:
          CI_IMAGE_VERSION=`opensearch-build/docker/ci/get-ci-images.sh -p rockylinux8 -u opensearch-dashboards -t build | head -1`

Note that in the above k-NN PR, despite it being OpenSearch plugin, it still uses rockylinux8, as we initially plan to upgrade to rockylinux. We have since revert back to centos7 to support older versions of systems running k-NN lib. As a result, all OpenSearch plugins still uses centos7 for the time being, and all OpenSearch-Dashboards plugins can go to rockylinux8.

Completion Date

The above should be implemented by Nov. 1, 2023 (2023-11-01) by Plugin Owners to their repository.
And backport the changes to 2.x branch after merging in main branch.

Contacts

Please contact @peterzhuamazon for any questions on this campaign.

cc: @bbarani

Thanks.

@peterzhuamazon peterzhuamazon added the enhancement New feature or request label Oct 6, 2023
@minalsha
Copy link

minalsha commented Oct 6, 2023

@peterzhuamazon, @bbarani completing this by 11/1 is a short notice with all the ongoing work. What OpenSearch release are we targeting this for? cc @sean-zheng-amazon

@ohltyler
Copy link
Member

ohltyler commented Oct 10, 2023

@peterzhuamazon the OSD CI image version doesn't seem to exist - see https://github.com/opensearch-project/anomaly-detection-dashboards-plugin/actions/runs/6472097461/job/17572014571?pr=620#step:4:10 as the echo statements are blank.

@peterzhuamazon
Copy link
Member Author

@peterzhuamazon the OSD CI image version doesn't seem to exist - see https://github.com/opensearch-project/anomaly-detection-dashboards-plugin/actions/runs/6472097461/job/17572014571?pr=620#step:4:10 as the echo statements are blank.

That echo is to save value to github env vars, so you for sure will not see it.

          echo "ci-image-version-linux=$CI_IMAGE_VERSION" >> $GITHUB_OUTPUT

@ohltyler
Copy link
Member

@peterzhuamazon oh, copying over it looks like I missed that part, just updated, thanks. It looks like I will need to add container/image keywords for running the linux tests as well.

@ohltyler ohltyler self-assigned this Oct 10, 2023
@peterzhuamazon
Copy link
Member Author

We have a new approach established here.
Please see this sample workflow file from Alerting for example:
https://github.com/opensearch-project/alerting/blob/main/.github/workflows/multi-node-test-workflow.yml

Thanks.

@ohltyler
Copy link
Member

@peterzhuamazon changing to this approach has caused some weird behavior where we are getting failures trying to install yarn with the new approach. I confirmed by reverting the change and seeing it is passing with the previous approach.

Ref (passing with old approach): link
Ref (failing with new approach): link

Any idea? Thanks.

@peterzhuamazon peterzhuamazon self-assigned this Oct 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request v2.12.0
Projects
None yet
3 participants