Skip to content

✨ Migrate to go-plugin system for provisioners#3166

Open
s3rj1k wants to merge 1 commit intometal3-io:mainfrom
s3rj1k:plugins
Open

✨ Migrate to go-plugin system for provisioners#3166
s3rj1k wants to merge 1 commit intometal3-io:mainfrom
s3rj1k:plugins

Conversation

@s3rj1k
Copy link
Copy Markdown
Member

@s3rj1k s3rj1k commented Apr 16, 2026

What this PR does / why we need it:

Migrate provisioners to go-plugin system for out-of-tree runtime BYO custom provisioner registration.

Checklist:

  • Documentation has been updated, if necessary.
  • Unit tests have been added, if necessary.
  • E2E tests have been added, if necessary.
  • Integration tests have been added, if necessary.

@metal3-io-bot
Copy link
Copy Markdown
Contributor

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@metal3-io-bot metal3-io-bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Apr 16, 2026
@metal3-io-bot
Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign elfosardo for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@metal3-io-bot metal3-io-bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Apr 16, 2026
@s3rj1k
Copy link
Copy Markdown
Member Author

s3rj1k commented Apr 16, 2026

@dtantsur This is how possible go plugin migration can look like

@dtantsur
Copy link
Copy Markdown
Member

This is not terrible, honestly. I'd like to hear from other folks, especially those more knowledgeable in Go, but this approach looks good to me.

@s3rj1k
Copy link
Copy Markdown
Member Author

s3rj1k commented Apr 20, 2026

This is not terrible, honestly. I'd like to hear from other folks, especially those more knowledgeable in Go, but this approach looks good to me.

If that lands, anyone could implement any (Lua, JS, Starlark, ...) interpreted runtime glue as go plugin or just directly use compiled Go code as plugin.

@s3rj1k s3rj1k force-pushed the plugins branch 2 times, most recently from 0d28199 to 623a641 Compare April 21, 2026 16:22
@s3rj1k s3rj1k marked this pull request as ready for review April 21, 2026 16:26
@metal3-io-bot metal3-io-bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Apr 21, 2026
@Rozzii
Copy link
Copy Markdown
Member

Rozzii commented Apr 22, 2026

/cc @Rozzii

@metal3-io-bot metal3-io-bot requested a review from Rozzii April 22, 2026 14:46
@s3rj1k s3rj1k force-pushed the plugins branch 2 times, most recently from 6fe05e8 to 031e5e1 Compare April 29, 2026 10:02
Comment thread main.go
Comment thread pkg/provisioner/plugin.go Outdated
Comment thread Makefile
Comment thread Makefile Outdated
Comment thread Makefile Outdated
Comment thread main.go Outdated
Comment thread main.go Outdated

```text
plugin was built with a different version of package <pkg>
```
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This may be a big problem in practice, and per https://pkg.go.dev/plugin it's virtually impossible to have plugins that are compiled separately from BMO itself. If so, isn't it easier to adjust the build process so that a different provisioner can be compiled in? And then add a flag to chose provisioner by name (not by its .so path)?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

You need to compile with same SDK env yes.

that a different provisioner can be compiled in

I want to ensure that we have ability to have out-of-tree plugins, not fork and add code into BMO repo

And then add a flag to chose provisioner by name (not by its .so path)?

That would mean hardcoding some specific path into BMO, I don't see any major issue with that, can be done

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Presumably someone could create an out-of-tree provisioner today and vendor the desired version of BMO to provide the controller (i.e. without having to fork BMO itself and add the provisioner code to it).
It seems like plugin authors would have to vendor BMO anyway in order to get the same versions of dependencies, so I guess it's not really clear to me what this is buying them.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Presumably someone could create an out-of-tree provisioner today and vendor the desired version of BMO to provide the controller

Have you tried to do that?

It seems like plugin authors would have to vendor BMO anyway

Vendoring go.mod is not a real vendoring

I guess it's not really clear to me what this is buying them

It gives consumer ability to use officially shipped BMO release binary with their plugin as opposed to shipping a rebuild of BMO and plugin.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

In recent rebase I've added .github/workflows/plugin-test.yml workflow and Dockerfile.plugin-test so we could verify OOT flow and how vendoring is supposed to work.

I would appreciate if someone could trigger Copilot on this PR, so we could catch other possible issues

@MahnoorAsghar
Copy link
Copy Markdown
Contributor

I meant to drop one comment, which failed (probably some browser issues), but 'Mahnoor started a review' is now showing up on the page for me atlease 😅

@s3rj1k s3rj1k force-pushed the plugins branch 4 times, most recently from ba0d6ba to 27d1290 Compare May 8, 2026 11:01
@metal3-io-bot metal3-io-bot added needs-rebase Indicates that a PR cannot be merged because it has merge conflicts with HEAD. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels May 8, 2026
@metal3-io-bot metal3-io-bot removed the needs-rebase Indicates that a PR cannot be merged because it has merge conflicts with HEAD. label May 8, 2026
Signed-off-by: s3rj1k <evasive.gyron@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants