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

Update build workflow to build based on distribution (OpenSearch) #1619

Conversation

peterzhuamazon
Copy link
Member

@peterzhuamazon peterzhuamazon commented Feb 12, 2022

Signed-off-by: Peter Zhu [email protected]

Description

Update build workflow to build based on distribution (OpenSearch).
You can now use --distribution to build tar/zip/rpm for OpenSearch-min.
Not completely bound to platform anymore.

This would also setup the stage for bundled RPM creation in assemble_workflow (#2073)

Issues Resolved

Part of #1543

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.

@peterzhuamazon peterzhuamazon self-assigned this Feb 12, 2022
@peterzhuamazon peterzhuamazon changed the title Update build workflow to build based on distribution [WIP] Update build workflow to build based on distribution Feb 12, 2022
@dblock
Copy link
Member

dblock commented Feb 14, 2022

This is going to require updating all build.sh's in all the active branches across all components to support -d. I think it's a lot of work, maybe only pass -d if specified, and default it to None?

@peterzhuamazon
Copy link
Member Author

This is going to require updating all build.sh's in all the active branches across all components to support -d. I think it's a lot of work, maybe only pass -d if specified, and default it to None?

My initial thinking process is to only do this for OpenSearch core component, as that is the only thing needed for this.
The idea is to build the core in deb or rpm form, then take the files either from deb/rpm-extracted folder, or decompress from the .deb / .rpm pkg during the assemble workflow, similar to untar, then install all the plugin zips then repack.

Will try to make it to none so it will only be consumed by OpenSearch core.

Thanks.

@peterzhuamazon
Copy link
Member Author

Hi @dblock I have implemented the changes you suggest, as well as tweaking the scripts to take None as default.
The problem right now is the build_recorder does not record distribution: something into the manifest.yml after builds.

Could you take a look at the code again?
Thanks.

@codecov-commenter
Copy link

codecov-commenter commented Feb 15, 2022

Codecov Report

Merging #1619 (4b10b30) into main (bddbc06) will increase coverage by 0.01%.
The diff coverage is 100.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##               main    #1619      +/-   ##
============================================
+ Coverage     94.67%   94.68%   +0.01%     
  Complexity       13       13              
============================================
  Files           163      163              
  Lines          3437     3446       +9     
  Branches         21       21              
============================================
+ Hits           3254     3263       +9     
  Misses          180      180              
  Partials          3        3              
Impacted Files Coverage Δ
src/run_build.py 91.11% <ø> (ø)
src/build_workflow/build_args.py 100.00% <100.00%> (ø)
src/build_workflow/build_recorder.py 100.00% <100.00%> (ø)
src/build_workflow/build_target.py 100.00% <100.00%> (ø)
src/build_workflow/builder_from_source.py 100.00% <100.00%> (ø)
src/manifests/build_manifest.py 100.00% <100.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 bddbc06...4b10b30. Read the comment docs.

@peterzhuamazon
Copy link
Member Author

I added the code to record to manifest.yml now. Thanks.

@peterzhuamazon peterzhuamazon marked this pull request as ready for review February 15, 2022 22:40
@peterzhuamazon peterzhuamazon requested a review from a team as a code owner February 15, 2022 22:40
@peterzhuamazon peterzhuamazon changed the title [WIP] Update build workflow to build based on distribution Update build workflow to build based on distribution Feb 15, 2022
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.

It's on the right track, but should be simpler and have fewer changes if you consider that you can never have distribution: None.

scripts/components/OpenSearch/build.sh Outdated Show resolved Hide resolved
scripts/components/OpenSearch/build.sh Outdated Show resolved Hide resolved
scripts/components/OpenSearch/build.sh Outdated Show resolved Hide resolved
scripts/components/OpenSearch/build.sh Outdated Show resolved Hide resolved
scripts/components/OpenSearch/build.sh Outdated Show resolved Hide resolved
@@ -60,6 +60,7 @@ def __init__(self, target: BuildTarget):
self.data["build"]["version"] = target.opensearch_version
self.data["build"]["platform"] = target.platform
self.data["build"]["architecture"] = target.architecture
self.data["build"]["distribution"] = target.distribution if target.distribution else "None"
Copy link
Member

Choose a reason for hiding this comment

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

Leave this as target.distribution, it should always have a value here coming from the caller.

Copy link
Member Author

Choose a reason for hiding this comment

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

@dblock but if I leave as is, if no one call --distribution then it will default to None, which then will error out during recording.

Copy link
Member Author

Choose a reason for hiding this comment

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

ValueError: Invalid manifest schema: {'build': [{'distribution': ['null value not allowed']}]}

Copy link
Member Author

Choose a reason for hiding this comment

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

Set to tar if None for now.

src/build_workflow/builder_from_source.py Show resolved Hide resolved
@@ -2,6 +2,7 @@
build:
platform: linux
architecture: x64
distribution: None
Copy link
Member

Choose a reason for hiding this comment

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

Version 1.1.0 never had a distribution field, remove.

@@ -1,6 +1,7 @@
---
build:
architecture: x64
distribution: None
Copy link
Member

Choose a reason for hiding this comment

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

Same as for 1.1.0, remove here and everywhere, except for 1.3.0 build output that should now write a value of distribution.

Copy link
Member Author

Choose a reason for hiding this comment

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

So to be clear this is to update a new test for 1.3.0 the release, not the schema?

@@ -2,6 +2,7 @@
build:
platform: linux
architecture: x64
distribution: None
Copy link
Member

Choose a reason for hiding this comment

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

Add a 1.3.0 manifest that has a value for distribution. Possibly a new test.

distribution: None is not a thing, I don't expect to ever find this in a build file, when we don't pass distribution it defaults to something (tar?) doesn't it?

Copy link
Member Author

Choose a reason for hiding this comment

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

I was following the suggestion to have None as default, while previously my commit I use tar as default.
I am bit confused here what exactly should I use?
Thanks.

@peterzhuamazon peterzhuamazon requested a review from a team February 15, 2022 23:16
@peterzhuamazon peterzhuamazon changed the title Update build workflow to build based on distribution Update build workflow to build based on distribution (OpenSearch-Core) Feb 15, 2022
@peterzhuamazon peterzhuamazon changed the title Update build workflow to build based on distribution (OpenSearch-Core) Update build workflow to build based on distribution (OpenSearch) Feb 15, 2022
dblock and others added 7 commits February 16, 2022 00:40
…dle build (opensearch-project#1378)

* Promote to release candidate after every bundle build

Signed-off-by: Yilin Zhang <[email protected]>

Signed-off-by: Sayali Gaikawad <[email protected]>

Co-authored-by: Sayali Gaikawad <[email protected]>
* Added a unit test for the Data Prepper build.

Signed-off-by: David Venable <[email protected]>
* Add more repos to offline scan

Signed-off-by: Zelin Hao <[email protected]>

* Simply the way we generate temp settings gradle

Signed-off-by: Zelin Hao <[email protected]>
peterzhuamazon and others added 24 commits February 16, 2022 00:40
…ect#1581)

* Adding checksum for latest snapshots artifacts

Signed-off-by: Peter Zhu <[email protected]>

* Change to snapshots dir name in path

Signed-off-by: Peter Zhu <[email protected]>

* Update test stack with new location

Signed-off-by: Peter Zhu <[email protected]>

* Simplify the naming in artifacts variables

Signed-off-by: Peter Zhu <[email protected]>

* Rename internal filename of sha512 so checksum pass

Signed-off-by: Peter Zhu <[email protected]>

* Update tests

Signed-off-by: Peter Zhu <[email protected]>

* Remove id number

Signed-off-by: Peter Zhu <[email protected]>

* Move sh block outside of withAWS block

Signed-off-by: Peter Zhu <[email protected]>
…i_workflow (opensearch-project#1582)

* Add python typechecking for ci-workflo

Signed-off-by: Zelin Hao <[email protected]>
…rch-project#1517)

* Hook up integ tests to distribution-build

Signed-off-by: Tyler Ohlsen <[email protected]>

* yamlfix test manifest file

Signed-off-by: Tyler Ohlsen <[email protected]>

* Remove unnecessary BuildManifest fns

Signed-off-by: Tyler Ohlsen <[email protected]>

* Remove runIntegTests(); refactor buildDockerImage vars as params

Signed-off-by: Tyler Ohlsen <[email protected]>

* Comment out notification block on dashboards dist build job

Signed-off-by: Tyler Ohlsen <[email protected]>

* Test out stabler integ test job

Signed-off-by: Tyler Ohlsen <[email protected]>

* Add test pipeline for pulling nested build info

Signed-off-by: Tyler Ohlsen <[email protected]>

* Remove test pipeline

Signed-off-by: Tyler Ohlsen <[email protected]>

* Add back notifications in OSD dist build

Signed-off-by: Tyler Ohlsen <[email protected]>

* Remove few extra test output lines

Signed-off-by: Tyler Ohlsen <[email protected]>

* Clean up buildDockerImage tests

Signed-off-by: Tyler Ohlsen <[email protected]>

* Update params in check-for-build cron; add icon in notifications

Signed-off-by: Tyler Ohlsen <[email protected]>

* Simplify notification

Signed-off-by: Tyler Ohlsen <[email protected]>

* Change references back to prod jenkins jobs

Signed-off-by: Tyler Ohlsen <[email protected]>
…nsearch-project#1590)

* Add invalid job case in check-for-build
* Remove catch block; pass TEST_MANIFEST for all jobs

Signed-off-by: Tyler Ohlsen <[email protected]>
* Align sign off with DCO expectation

Signed-off-by: Peter Nied <[email protected]>

* Allow manually running of the workflow and set the author

Signed-off-by: Peter Nied <[email protected]>

* Revert dbock signoff, back to ci-bot

Signed-off-by: Peter Nied <[email protected]>

* Updating documentation with how to manually trigger

Signed-off-by: Peter Nied <[email protected]>
…tests_manifests_workflow (opensearch-project#1599)

* Add python type checking for manifest workflow

Signed-off-by: Zelin Hao <[email protected]>
* BWC test workflow

Adding backwards compatability workflow for OpenSearch and OpenSearch
Dashboards.

Issue:
opensearch-project#705

Signed-off-by: Kawika Avilla <[email protected]>

* [BWC] Update docs and return cmd

Signed-off-by: Kawika Avilla <[email protected]>
Add additional documentation for the copyContainer function to make it clear what source/destinations can be used.

Signed-off-by: Peter Nied <[email protected]>
* Migrate the maven sign release script to build repo

Signed-off-by: Zelin Hao <[email protected]>
* Adding Release owner section to maintainers

Describing the intention behind being a release owner and approaches to different situations that occur during the a release.

Signed-off-by: Peter Nied <[email protected]>
…#1614)

* Adding dashboards-security-plugin to 1.3 manifest

Signed-off-by: Dave Lago <[email protected]>
…-project#1616)

* adding Anomaly Detection to 1.3 Dashboard manifest

Signed-off-by: Amit Galitzky <[email protected]>

* linting issue

Signed-off-by: Amit Galitzky <[email protected]>

* fixing linting issue

Signed-off-by: Amit Galitzky <[email protected]>

* linting issue fix after merging main

Signed-off-by: Amit Galitzky <[email protected]>

Co-authored-by: Peter Nied <[email protected]>
* Update maintainers list

Signed-off-by: Peter Nied <[email protected]>
Signed-off-by: Peter Zhu <[email protected]>
@peterzhuamazon peterzhuamazon force-pushed the opensearch-build-distribution-2 branch from 365068a to 923c161 Compare February 16, 2022 00:42
@peterzhuamazon
Copy link
Member Author

I somehow messed up this branch and could not recover during a rebase, will open a new PR soon.

@peterzhuamazon
Copy link
Member Author

Reopen PR: #1629

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.