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

Ensure Github workflow runs on docker image used by Production Distribution Build #3868

Closed
wants to merge 1 commit into from

Conversation

reta
Copy link
Collaborator

@reta reta commented Dec 18, 2023

Description

Ensure Github workflow runs on docker image used by Production Distribution Build

Issues Resolved

Closes #3494

Is this a backport? If so, please add backport PR # and/or commits #

Testing

Tested as Github Action worflows

Check List

  • New functionality includes testing
  • New functionality has been documented
  • Commits are signed per the DCO using --signoff

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.

@reta reta force-pushed the issue-3494 branch 12 times, most recently from d9489f8 to 0313e6e Compare December 18, 2023 21:39
Copy link
Member

@peternied peternied left a comment

Choose a reason for hiding this comment

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

Thanks for getting a PR out here to review, can you do a runtime comparison between main and this change? I'm curious what the impact is to our PR checks time with this change.

.github/workflows/ci.yml Show resolved Hide resolved
@@ -13,6 +13,11 @@ env:
CI_ENVIRONMENT: normal

jobs:
Get-CI-Image-Tag:
uses: opensearch-project/opensearch-build/.github/workflows/get-ci-image-tag.yml@main
Copy link
Member

Choose a reason for hiding this comment

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

Lets use a published task not a direct repo reference

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Hm... may you share the example? (PS: this step is replicated to all repos)

Copy link
Member

Choose a reason for hiding this comment

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

I don't like tightly coupling these repositories together; security depends on opensearch-build, opensearch build depends on security - this is a formula to get into a broken state and have to start bypassing CI.

Copy link
Member

Choose a reason for hiding this comment

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

For example, here is a GitHub action that allows for multi-platform file download [1] it shouldn't be part of the security project even though it was originally written to support this project.

I have trouble adopting this level of coupling when we still have problems doing normal builds after version bumps [2]

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I am still unsure what you are suggesting, my apologies. If this is not the way we should share the workflows, no objections to change it, but to what model?

Copy link
Member

Choose a reason for hiding this comment

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

We should publish the workflows as github actions e.g. opensearch-project/opensearch-ci-image-by-tag@v1?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Oh I see, thanks @peternied , @peterzhuamazon fyi ^^

.github/workflows/ci.yml Outdated Show resolved Hide resolved
@reta
Copy link
Collaborator Author

reta commented Dec 18, 2023

Thanks for getting a PR out here to review, can you do a runtime comparison between main and this change? I'm curious what the impact is to our PR checks time with this change.

The only change should be an usage of our (OpenSearch) image for Linux runner (instead of ubuntu-latest), the only price to be paid is for pulling the image .

@reta
Copy link
Collaborator Author

reta commented Dec 19, 2023

@peternied opening it up for review, if you prefer to have Githib Action (instead of referencing the workflow directly), I have no objection - please feel to close this one (and preferably open an issue for opensearch-build repo to work on that), thank you!

@peternied
Copy link
Member

Thanks @reta I'm created an issue for OpenSearch-build to publish a GitHub action [1] that should be how we adopt that job. After this has been created, we'll be in a position to adopt this change in this repo.

@peternied peternied closed this Dec 19, 2023
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.

[Campaign] Ensure Github workflow runs on docker image used by Production Distribution Build
2 participants