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

Only perform ref checks on components with stable refs. #1455

Merged
merged 1 commit into from
Jan 18, 2022

Conversation

dblock
Copy link
Member

@dblock dblock commented Jan 12, 2022

Signed-off-by: dblock dblock@amazon.com

Description

The manifest checks will continue taking longer and longer because we keep shipping releases. As of now, the 31 checks in https://github.com/opensearch-project/opensearch-build/actions/runs/1685288124 took 14m. It's not bad because the checks are parallelized.

We have suggested in #1335 to skip checking manifests for already released versions, but this PR proposes a better solution.

  • Only check that a ref exists without checking out the entire source if there are no actual checks to be performed.
  • Update any refs that weren't using tags in manifests for shipped releases.
  • Remove checks from components in released manifests to avoid re-running the same checks on stable targets.

Issues Resolved

Closes #1335.

Check List

  • 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.

@dblock dblock requested a review from a team as a code owner January 12, 2022 17:10
@dblock dblock force-pushed the faster-ci-checks branch 3 times, most recently from ca702ec to 0329939 Compare January 12, 2022 17:22
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.

What about ignoring checks on component references with tags/*? Less work than deleting the 'checks' on legacy manifests.

@dblock dblock requested a review from a team January 12, 2022 17:33
@dblock
Copy link
Member Author

dblock commented Jan 12, 2022

What about ignoring checks on component references with tags/*? Less work than deleting the 'checks' on legacy manifests.

I considered it, but it felt like hidden logic/functionality.

@peternied
Copy link
Member

Other considerations:

  • What about a top level, disable-checks-on-ref-tags: true that makes this more transparent?
  • What about a manifest promotion tool that effective does automatically what you are doing by hand?

@dblock
Copy link
Member Author

dblock commented Jan 12, 2022

Other considerations:

  • What about a top level, disable-checks-on-ref-tags: true that makes this more transparent?
  • What about a manifest promotion tool that effective does automatically what you are doing by hand?

These are good ideas, but why are they better than removing useless checks?

@peternied
Copy link
Member

Hmm, why keep around these manifests at all, why not delete them?

@dblock
Copy link
Member Author

dblock commented Jan 12, 2022

Hmm, why keep around these manifests at all, why not delete them?

So we can rebuild previous versions if needed/for reference. It's like keeping old source code.

@dblock
Copy link
Member Author

dblock commented Jan 18, 2022

Given that JCenter is now gone we also need to do this (or something like this) for manifests such as 1.1.1.

Signed-off-by: dblock <dblock@amazon.com>
@dblock dblock merged commit dda3255 into opensearch-project:main Jan 18, 2022
@dblock dblock deleted the faster-ci-checks branch January 18, 2022 20:07
peterzhuamazon pushed a commit to peterzhuamazon/opensearch-build that referenced this pull request Feb 16, 2022
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.

Skip old(er) manifests in CI
3 participants