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

Promote min and bundle artifacts to release-candidate after every bundle build #1378

Merged
merged 20 commits into from
Jan 12, 2022

Conversation

zylzulu
Copy link
Contributor

@zylzulu zylzulu commented Dec 17, 2021

Signed-off-by: Yilin Zhang yilizhan@amazon.com

Description

Uploads min and bundle artifacts for both opensearch and dashboards to the below location on every build:

https://artifacts.opensearch.org/release-candidates/core/opensearch/<version>/opensearch-min-<version>-linux-x64.tar.gz
https://artifacts.opensearch.org/release-candidates/bundle/opensearch/<version>/opensearch-<version>-linux-x64.tar.gz
https://artifacts.opensearch.org/release-candidates/core/opensearch-dashboards/<version>/opensearch-dashboards-min-<version>-linux-x64.tar.gz
https://artifacts.opensearch.org/release-candidates/bundle/opensearch-dashboards/<version>/opensearch-dashboards-<version>-linux-x64.tar.gz

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.

Signed-off-by: Yilin Zhang <yilizhan@amazon.com>
@codecov-commenter
Copy link

codecov-commenter commented Dec 17, 2021

Codecov Report

Merging #1378 (f683cc0) into main (09f65b3) will decrease coverage by 0.06%.
The diff coverage is 82.35%.

Impacted file tree graph

@@             Coverage Diff              @@
##               main    #1378      +/-   ##
============================================
- Coverage     94.45%   94.38%   -0.07%     
  Complexity       11       11              
============================================
  Files           140      140              
  Lines          3085     3101      +16     
  Branches         10       10              
============================================
+ Hits           2914     2927      +13     
- Misses          164      167       +3     
  Partials          7        7              
Impacted Files Coverage Δ
tests/jenkins/jobs/BuildManifest_Jenkinsfile 100.00% <ø> (ø)
src/jenkins/BuildManifest.groovy 88.00% <82.35%> (-12.00%) ⬇️

Continue to review full report at Codecov.

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

vars/uploadArtifacts.groovy Outdated Show resolved Hide resolved
vars/uploadArtifacts.groovy Outdated Show resolved Hide resolved
@peternied
Copy link
Member

Please make sure that the new output will handle these cases:

  • .../release-candidates/core/opensearch/1.2.2/opensearch-min-1.2.2-linux-x64.tar.gz
  • .../release-candidates/core/opensearch/1.2.2/opensearch-min-1.2.2-linux-arm64.tar.gz
  • .../snapshots/core/opensearch/1.2.2-SNAPSHOT/opensearch-min-1.2.2-SNAPSHOT-linux-x64-latest.tar.gz
  • .../snapshots/core/opensearch/1.2.2-SNAPSHOT/opensearch-min-1.2.2-SNAPSHOT-linux-arm64-latest.tar.gz

I think we've got the first case, but we need the snapshot ones too

vars/uploadArtifacts.groovy Outdated Show resolved Hide resolved
Comment on lines 53 to 61
public String getPackageName(String productName) {
String packagePrefix = [
productName,
this.build.version,
this.build.platform,
this.build.architecture,
].join('/')
return packagePrefix + '.tar.gz'
}
Copy link
Member

Choose a reason for hiding this comment

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

Can we use the name with version, platform, architecture and distribution from here?

@dblock
Copy link
Member

dblock commented Dec 29, 2021

Can we try to move this forward, please?

Signed-off-by: Yilin Zhang <yilizhan@amazon.com>
Copy link
Member

@dblock dblock left a comment

Choose a reason for hiding this comment

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

Add tests for the new functions at least, please.

src/jenkins/BuildManifest.groovy Show resolved Hide resolved
src/jenkins/BuildManifest.groovy Outdated Show resolved Hide resolved
…oduct name from manifest

Signed-off-by: Yilin Zhang <yilizhan@amazon.com>
@zylzulu zylzulu requested a review from dblock December 29, 2021 23:19
@dblock
Copy link
Member

dblock commented Dec 30, 2021

@zylzulu will review when the build is green and the CR is ready

Signed-off-by: Yilin Zhang <yilizhan@amazon.com>
Signed-off-by: Sayali Gaikawad <gaiksaya@amazon.com>
@gaiksaya gaiksaya removed their request for review January 5, 2022 03:03
Signed-off-by: Sayali Gaikawad <gaiksaya@amazon.com>
Signed-off-by: Sayali Gaikawad <gaiksaya@amazon.com>
Signed-off-by: Sayali Gaikawad <gaiksaya@amazon.com>
Signed-off-by: Sayali Gaikawad <gaiksaya@amazon.com>
@gaiksaya
Copy link
Member

gaiksaya commented Jan 7, 2022

Please make sure that the new output will handle these cases:

* .../release-candidates/core/opensearch/1.2.2/opensearch-min-1.2.2-linux-x64.tar.gz

* .../release-candidates/core/opensearch/1.2.2/opensearch-min-1.2.2-linux-arm64.tar.gz

* .../snapshots/core/opensearch/1.2.2-SNAPSHOT/opensearch-min-1.2.2-SNAPSHOT-linux-x64-latest.tar.gz

* .../snapshots/core/opensearch/1.2.2-SNAPSHOT/opensearch-min-1.2.2-SNAPSHOT-linux-arm64-latest.tar.gz

I think we've got the first case, but we need the snapshot ones too

Hi @peternied , @dblock,
currently the min and bundle are being uploaded to build/test-release-candidate folder (I'll change to release-candidates before I merge my code. Not making any code changes since I can use this for testing ).
Regarding snapshots from where do I retrieve them?
Also I believe snapshots only applies to min here

@gaiksaya gaiksaya self-assigned this Jan 7, 2022
@dblock
Copy link
Member

dblock commented Jan 7, 2022

Please make sure that the new output will handle these cases:

* .../release-candidates/core/opensearch/1.2.2/opensearch-min-1.2.2-linux-x64.tar.gz

* .../release-candidates/core/opensearch/1.2.2/opensearch-min-1.2.2-linux-arm64.tar.gz

* .../snapshots/core/opensearch/1.2.2-SNAPSHOT/opensearch-min-1.2.2-SNAPSHOT-linux-x64-latest.tar.gz

* .../snapshots/core/opensearch/1.2.2-SNAPSHOT/opensearch-min-1.2.2-SNAPSHOT-linux-arm64-latest.tar.gz

I think we've got the first case, but we need the snapshot ones too

Hi @peternied , @dblock, currently the min and bundle are being uploaded to build/test-release-candidate folder (I'll change to release-candidates before I merge my code. Not making any code changes since I can use this for testing ). Regarding snapshots from where do I retrieve them? Also I believe snapshots only applies to min here

Check out https://github.com/opensearch-project/opensearch-build/blob/main/jenkins/opensearch/distribution-build.jenkinsfile#L50, which calls publish-snapshot to Maven. You'll probably want to publish around there.

@gaiksaya
Copy link
Member

gaiksaya commented Jan 7, 2022

Please make sure that the new output will handle these cases:

* .../release-candidates/core/opensearch/1.2.2/opensearch-min-1.2.2-linux-x64.tar.gz

* .../release-candidates/core/opensearch/1.2.2/opensearch-min-1.2.2-linux-arm64.tar.gz

* .../snapshots/core/opensearch/1.2.2-SNAPSHOT/opensearch-min-1.2.2-SNAPSHOT-linux-x64-latest.tar.gz

* .../snapshots/core/opensearch/1.2.2-SNAPSHOT/opensearch-min-1.2.2-SNAPSHOT-linux-arm64-latest.tar.gz

I think we've got the first case, but we need the snapshot ones too

Hi @peternied , @dblock, currently the min and bundle are being uploaded to build/test-release-candidate folder (I'll change to release-candidates before I merge my code. Not making any code changes since I can use this for testing ). Regarding snapshots from where do I retrieve them? Also I believe snapshots only applies to min here

Check out https://github.com/opensearch-project/opensearch-build/blob/main/jenkins/opensearch/distribution-build.jenkinsfile#L50, which calls publish-snapshot to Maven. You'll probably want to publish around there.

Looking at the above code:
The buildManifest just builds the snapshot but does not upload it anywhere. Its the publish-snapshot that uploads the artifacts to maven.

https://github.com/opensearch-project/opensearch-build/blob/main/jenkins/opensearch/distribution-build.jenkinsfile#L45-L47

Should we directly upload from here (jenkins local) to snapshots (prod) s3? Also what about arm64? While I executed the jenkinfile to see the generated artifacts, I did not see anything generated for arm64 here.
OR Can we separate this problem from the release-candidates one?
Also talked to @mch2 seems like we have 2 possible solutions:

  1. Rename the above received artifact to snapshots as it guarantees that user is running tests against an artifact that will get released
  2. Build x64 (already doing) and arm64 snapshots and then upload to s3 (either just to prod or both staging and prod).

@dblock
Copy link
Member

dblock commented Jan 10, 2022

@gaiksaya I think it's perfectly OK to do the snapshot uploading as part of another PR, and we can discuss options there; (2) seems like the better thing to do

Signed-off-by: Sayali Gaikawad <gaiksaya@amazon.com>
@gaiksaya
Copy link
Member

@gaiksaya I think it's perfectly OK to do the snapshot uploading as part of another PR, and we can discuss options there; (2) seems like the better thing to do

Created the issue #1442 to address the min snapshots uploading part

@gaiksaya gaiksaya marked this pull request as ready for review January 10, 2022 18:38
@gaiksaya gaiksaya requested a review from a team as a code owner January 10, 2022 18:38
Signed-off-by: Sayali Gaikawad <gaiksaya@amazon.com>
src/jenkins/BuildManifest.groovy Outdated Show resolved Hide resolved
src/jenkins/BuildManifest.groovy Outdated Show resolved Hide resolved
src/jenkins/BuildManifest.groovy Outdated Show resolved Hide resolved
tests/jenkins/jobs/BuildManifest_Jenkinsfile Outdated Show resolved Hide resolved
vars/uploadArtifacts.groovy Outdated Show resolved Hide resolved
vars/uploadArtifacts.groovy Outdated Show resolved Hide resolved
vars/uploadArtifacts.groovy Outdated Show resolved Hide resolved
Signed-off-by: Sayali Gaikawad <gaiksaya@amazon.com>
@gaiksaya gaiksaya requested a review from dblock January 10, 2022 23:08
src/jenkins/BuildManifest.groovy Outdated Show resolved Hide resolved
Signed-off-by: Sayali Gaikawad <gaiksaya@amazon.com>
@gaiksaya gaiksaya changed the title Promote to release-candidate after every bundle build Promote min and bundle artifacts to release-candidate after every bundle build Jan 11, 2022
@gaiksaya gaiksaya requested a review from dblock January 11, 2022 23:19
@dblock dblock requested a review from a team January 11, 2022 23:34
@gaiksaya gaiksaya merged commit d0ac211 into opensearch-project:main Jan 12, 2022
peterzhuamazon pushed a commit to peterzhuamazon/opensearch-build that referenced this pull request Feb 16, 2022
…dle build (opensearch-project#1378)

* Promote to release candidate after every bundle build

Signed-off-by: Yilin Zhang <yilizhan@amazon.com>

Signed-off-by: Sayali Gaikawad <gaiksaya@amazon.com>

Co-authored-by: Sayali Gaikawad <gaiksaya@amazon.com>
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.

7 participants