Support multiple artifacts per bundled component.

[dblock]

Signed-off-by: dblock dblock@dblock.org

Description

Added support for multiple artifacts per distribution component.

With this change, the notifications plugin collects 2 ZIPs instead of 1, publishes and installs both.

Build Manifest

  - name: notifications
    repository: https://github.com/opensearch-project/notifications.git
    ref: main
    commit_id: 731ee820a36edc25bac07a5b6a624c35d1615ce5
    artifacts:
      plugins:
        - plugins/opensearch-notifications-core-2.0.0.0-alpha1-SNAPSHOT.zip
        - plugins/opensearch-notifications-2.0.0.0-alpha1-SNAPSHOT.zip

Bundle Manifest

  - name: notifications
    repository: https://github.com/opensearch-project/notifications.git
    ref: main
    commit_id: 731ee820a36edc25bac07a5b6a624c35d1615ce5
    locations:
      - /home/ubuntu/source/opensearch-project/opensearch-build/dblock-opensearch-build/tar/builds/opensearch/plugins/opensearch-notifications-core-2.0.0.0-alpha1-SNAPSHOT.zip
      - /home/ubuntu/source/opensearch-project/opensearch-build/dblock-opensearch-build/tar/builds/opensearch/plugins/opensearch-notifications-2.0.0.0-alpha1-SNAPSHOT.zip

It doesn't seem like the order of installation between notifications and notifications-core matters, but it could be that they are installed in the right order "accidentally". If there's an issue with this I can fix it in a future PR.

Issues Resolved

Closes #1950.

Will need to move the custom build.sh into notifications repo as script/build.sh after merging.

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]

@qreshi

[codecov-commenter]

Codecov Report

Merging #1956 (029f416) into main (8522a31) will decrease coverage by 0.05%.
The diff coverage is 93.15%.

@@             Coverage Diff              @@
##               main    #1956      +/-   ##
============================================
- Coverage     94.46%   94.40%   -0.06%     
  Complexity       22       22              
============================================
  Files           179      180       +1     
  Lines          3649     3702      +53     
  Branches         29       29              
============================================
+ Hits           3447     3495      +48     
- Misses          196      201       +5     
  Partials          6        6              

Impacted Files	Coverage Δ
src/assemble_workflow/bundle.py	94.04% <85.71%> (-1.08%)	⬇️
src/manifests/bundle/bundle_manifest_1_1.py	89.18% <89.18%> (ø)
src/assemble_workflow/bundle_opensearch.py	100.00% <100.00%> (ø)
.../assemble_workflow/bundle_opensearch_dashboards.py	100.00% <100.00%> (ø)
src/assemble_workflow/bundle_recorder.py	100.00% <100.00%> (ø)
src/manifests/bundle_manifest.py	97.50% <100.00%> (+0.06%)	⬆️

---

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 8522a31...029f416. Read the comment docs.

[qreshi]

The scripts have actually already been moved to Notifications, there are just two minor PRs open to make some changes to them (opensearch-project/notifications#400 and opensearch-project/notifications#401). So we can merge those in and remove the scripts in this PR if you like.

Also, the installation order does matter. I also brought this up in the tracking issue and you mentioned it here as well, it could be that the build manifest being outputted here happened to list the plugins in order:

    artifacts:
      plugins:
        - plugins/opensearch-notifications-core-2.0.0.0-alpha1-SNAPSHOT.zip
        - plugins/opensearch-notifications-2.0.0.0-alpha1-SNAPSHOT.zip

But is this order deterministic?

[qreshi]

Also @dblock I think @peterzhuamazon was looking into the option of listing separate entries in the manifest and supplying a different working_directory for each (since both of those plugins have their own subfolders). If that works, it would guarantee build and installation order so it's worth checking with him if we want to go with that solution.

[dblock]

The scripts have actually already been moved to Notifications, there are just two minor PRs open to make some changes to them (opensearch-project/notifications#400 and opensearch-project/notifications#401). So we can merge those in and remove the scripts in this PR if you like.

Ok, I see, deleting this from this PR.

Also, the installation order does matter. I also brought this up in the tracking issue and you mentioned it here as well, it could be that the build manifest being outputted here happened to list the plugins in order:

    artifacts:
      plugins:
        - plugins/opensearch-notifications-core-2.0.0.0-alpha1-SNAPSHOT.zip
        - plugins/opensearch-notifications-2.0.0.0-alpha1-SNAPSHOT.zip

But is this order deterministic?

I think it's repeatable in the order files are written, but I don't think we can rely on it.

[dblock]

Also @dblock I think @peterzhuamazon was looking into the option of listing separate entries in the manifest and supplying a different working_directory for each (since both of those plugins have their own subfolders). If that works, it would guarantee build and installation order so it's worth checking with him if we want to go with that solution.

I went down that rabbit hole first and it seemed like it's a smaller amount of changes. Then I thought that it would be great to support multiple artifacts produced by a single build anyway, so I did this change.

@peterzhuamazon Did you make it work with working_directory? Can we compare the two PRs?

[peterzhuamazon]

Also @dblock I think @peterzhuamazon was looking into the option of listing separate entries in the manifest and supplying a different working_directory for each (since both of those plugins have their own subfolders). If that works, it would guarantee build and installation order so it's worth checking with him if we want to go with that solution.

I went down that rabbit hole first and it seemed like it's a smaller amount of changes. Then I thought that it would be great to support multiple artifacts produced by a single build anyway, so I did this change.

@peterzhuamazon Did you make it work with working_directory? Can we compare the two PRs?

I was not sure if you are working on this issue.
If you are not I can take a look.
In theory two entries should run well with two different work directory.
Just need some tweak in the build.sh to take it.

[dblock]

@peterzhuamazon #1957

What do we like better @peterzhuamazon @qreshi?

The pros of this PR is that we can publish multiple ZIPs per component with it, so feels like a more generic fix.

The cons is that it's a bigger change, the order is still an issue, manifest checks may need work, and may need to be made more predictable, and the bundle manifest schema has changed.

[peterzhuamazon]

As this PR requires a lot more changes and probably will break a thing or two after the push, combined with the fact that notifications situation is a special case, I vote for #1957 as that is the minimum change for maximum outcome.

[peterzhuamazon]

The #1957 not only allow use to define the order, but also resolved the issue without adding existing code to further complicated the code base for a one time use case.

[dblock]

Closing in favor of #1957