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

[CI] Distribute connect as a rpk managed plugin #2825

Merged
merged 1 commit into from
Sep 5, 2024

Conversation

jackietung-redpanda
Copy link
Contributor

@jackietung-redpanda jackietung-redpanda commented Aug 30, 2024

Project context is here.

This PR tackles the workflow of uploading connect to a designed S3 repo, which acts as the "rpk plugin" repo, used only by connect for now.

plugin_uploader.py
To be run after goreleaser has built the binaries. upload-archives creates and uploads the archives. upload-manifest creates and uploads the final manifest.json file. RPK uses manifest.json to know what plugins are available and how to download them (download URL, etc.). Comes with unit tests, basic linters, type checkers, setup in CI.

Additional release workflow
.github/workflows/upload_plugin.yml. High level:

  • goreleaser build
  • Run plugin_uploader.py upload-archives ...
  • Run plugin_uploader.py upload-manifest ...

Only uploads for real when triggered from a release tag (inc. RC). Otherwise, runs in --dry-run mode - no writes to S3.

Why not use goreleaser built in blob publish support?

Goreleaser has "blob" publish support, which kind of can do upload-archives in spirit. However, our use case requires additional flexibility in how we set S3 object tags. Our manifest.json must contain SHA256 checksums of the binary payload WITHIN the tar.gz (rpk requirement). It would not work gracefully with said blob publishing within Goreleaser.

manifest.json construction logic (upload-archives) would be needed in any case and most cleanly implemented separately from goreleaser.

Generally the design of goreleaser is not conducive to running, testing, debugging, retrying specific parts of the goreleaser steps. E.g. Publishing steps are all or nothing. To test blob publishing, one would also be publishing to Github!

Overall the monolithic design of goreleaser's runtime makes things tricky for more complex use cases.

Thankfully goreleaser provides a clean interface for extension scripts (artifacts.json, metadata.json)

Why not use goreleaser custom publisher support?

Similar reasoning to above.

Note the calculus may be different for other use cases. E.g. next task is to build Linux packages for redpanda-connect (deb / rpm). The functionality may fit much more closely such that we can accept the drawbacks we describe here. We do that for redpanda-console (see nfpm usage there).

Why not extend existing release.yml Github action workflow?

This could be an option in the long term. However this functionality is new and may introduce instability in the main release.yml pipeline in the meantime. It will be much cleaner to test, debug, run (and rerun!) as an independent workflow. Some observations:

  • The S3 upload flow does not need a goreleaser release, only goreleaser build!
  • release.yml pushes real Github releases and may not be idempotent!

Once we gain confidence that both .github/workflows/upload_plugin.yml AND release.yml are very reliable, we can consider merging into release.yml.

@jackietung-redpanda jackietung-redpanda changed the title [rpk plugin integration] tooling to allow automated distribution of connect as a rpk managed plugin [CI] Auto distribution of connect as a rpk managed plugin Aug 30, 2024
@jackietung-redpanda jackietung-redpanda force-pushed the jackie-plugin-uploader branch 2 times, most recently from ce82e8a to 8c56abc Compare August 30, 2024 20:26
@jackietung-redpanda jackietung-redpanda changed the title [CI] Auto distribution of connect as a rpk managed plugin [CI] Distribute connect as a rpk managed plugin Sep 3, 2024
@jackietung-redpanda jackietung-redpanda marked this pull request as ready for review September 3, 2024 20:49
Copy link
Member

@ivotron ivotron left a comment

Choose a reason for hiding this comment

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

🚀 lgtm, this is great!

Copy link
Member

@andrewhsu andrewhsu left a comment

Choose a reason for hiding this comment

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

needs guard in upload_plugin.yml

.github/workflows/upload_plugin.yml Show resolved Hide resolved
.github/workflows/upload_plugin.yml Outdated Show resolved Hide resolved
.github/workflows/test_plugin_uploader.yml Outdated Show resolved Hide resolved
Copy link
Collaborator

@Jeffail Jeffail left a comment

Choose a reason for hiding this comment

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

LGTM

@Jeffail
Copy link
Collaborator

Jeffail commented Sep 5, 2024

Hey @jackietung-redpanda, I'm aiming to tag a release later today, is this PR required before we do it or can it wait until the one afterwards?

@jackietung-redpanda jackietung-redpanda force-pushed the jackie-plugin-uploader branch 2 times, most recently from 6b1ee87 to 91fca6a Compare September 5, 2024 16:06
Copy link
Member

@andrewhsu andrewhsu left a comment

Choose a reason for hiding this comment

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

i think PRs from forks will still have errors in upload_plugin.yml, but if you only create PRs from branches on redpanda-data/connect you will be fine

.github/workflows/upload_plugin.yml Outdated Show resolved Hide resolved
@andrewhsu andrewhsu self-requested a review September 5, 2024 16:36
@andrewhsu andrewhsu dismissed their stale review September 5, 2024 16:37

dismissing my review to unblock merge, but left comment on suggested change

Copy link
Member

@andrewhsu andrewhsu left a comment

Choose a reason for hiding this comment

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

LGTM

@jackietung-redpanda jackietung-redpanda merged commit 3fb6464 into main Sep 5, 2024
7 checks passed
@mihaitodor mihaitodor deleted the jackie-plugin-uploader branch September 25, 2024 11:16
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.

4 participants