Add plugin support #1519

twmb · 2021-06-02T03:25:43Z

Cover letter

This adds support for rpk to execute plugins, following the exact logic that kubernetes has, as documented here.

Followup PRs will be issued to add rpk plugin list, as well as plugin discovery. list will be easy and will again follow the same logic that kubernetes has, but discovery will be a good bit more difficult.

CLAassistant · 2021-06-02T03:25:54Z

All committers have signed the CLA.

src/go/rpk/pkg/cli/cmd/root.go

emaxerrno · 2021-06-02T03:39:12Z

groupcsaw
kverify

can really be deleted :) - we wrote those when we were testing things a while ago and don't need them any more.

src/go/rpk/pkg/cli/cmd/root.go

emaxerrno · 2021-06-02T03:41:02Z

Cover letter

This adds support for rpk to execute plugins, following the exact logic that kubernetes has, as documented here.

tip-top!

Followup PRs will be issued to add rpk plugin list, as well as plugin discovery. list will be easy and will again follow the same logic that kubernetes has, but discovery will be a good bit more difficult.

this is a beautiful start. minor nits on os.execlpe() rather than os.cmd()

emaxerrno · 2021-06-02T03:42:27Z

Cover letter

This adds support for rpk to execute plugins, following the exact logic that kubernetes has, as documented here.

tip-top!

Followup PRs will be issued to add rpk plugin list, as well as plugin discovery. list will be easy and will again follow the same logic that kubernetes has, but discovery will be a good bit more difficult.

this is a beautiful start. minor nits on os.execlpe() rather than os.cmd()

@twmb i wonder if we can create a plugin for kcl wholesale hmm.... just a random thought.

wow so many goodies here.

probably one for

rpk k8s like @RafalKorepta mentioned in the getting started.

very freeing this thing.

RafalKorepta

Good job. Comments inline.

src/go/rpk/go.mod

src/go/rpk/pkg/cli/cmd/root.go

emaxerrno · 2021-06-02T14:56:46Z

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@twmb - make sure you approve this cla

emaxerrno

lgtm

emaxerrno · 2021-06-04T04:27:08Z

@twmb can you rebase w/ some of @ivotron 's latest changes for CI - i think there was an issue he fixed. We can merge after. CI seems to be failing. let me investigate to see if benign.

emaxerrno · 2021-06-04T04:33:35Z

Step #2 - "run task": INFO:root:staging relocatable local dir at /workspace/vbuild/release/clang/dist/local/redpanda
Step #2 - "run task": DEBUG:root:Getting dependencies of /workspace/vbuild/release/clang/bin/redpanda
Step #2 - "run task": Traceback (most recent call last):
Step #2 - "run task":   File "/workspace/vtools/packaging/vpkg/vpkg.py", line 520, in <module>
Step #2 - "run task":     main()
Step #2 - "run task":   File "/workspace/vtools/packaging/vpkg/vpkg.py", line 516, in main
Step #2 - "run task":     create_packages(args, args.format)
Step #2 - "run task":   File "/workspace/vtools/packaging/vpkg/vpkg.py", line 453, in create_packages
Step #2 - "run task":     _relocatable_dir(local_dir, execs, configs, admin_api_swag,
Step #2 - "run task":   File "/workspace/vtools/packaging/vpkg/vpkg.py", line 166, in _relocatable_dir
Step #2 - "run task":     if is_newer(exe.path, dest_exe):
Step #2 - "run task":   File "/workspace/vtools/packaging/vpkg/vpkg.py", line 129, in is_newer
Step #2 - "run task":     src_mtime = os.stat(path_a).st_mtime
Step #2 - "run task": FileNotFoundError: [Errno 2] No such file or directory: '/workspace/vbuild/go/linux/bin/rpk'
Step #2 - "run task": task: Failed to run task "v-rp:run-ducktape-tests": task: Failed to run task "v-rp:build-pkg": exit status 1
Step #2 - "run task": task: Failed to run task "gcb:ci+pkg+gcs": task: Failed to run task "gcb:ci+pkg": task: Failed to run task "gcb:ci": task: Failed to run task "docker:task": exit status 1```

twmb · 2021-06-04T04:41:15Z

Ahhh that is likely from 367410a. I thought this may happen while making that change. I wonder if it's possible to support the commit and fix the build, otherwise I'm happy to delete this GOOS / GOPATH change.

emaxerrno · 2021-06-04T05:19:24Z

taskfiles/rpk.yml

@@ -6,8 +6,6 @@ tasks:
    vars:
      RPK_VERSION: '{{default "latest" .TAG_NAME}}'
    env:
-      GOOS: '{{OS | lower}}'


this change seems unrelated to adding plugin support IMO.

ok to have, but probably makes sense to have a separate PR for this. We'll need @ivotron 's thoughts on this. I think on the build machines you have multiple go packages installed (or we used to)

It was something I ran into while adding the taskfile mod tidy bit brought up in an earlier comment; I've dropped it from this PR. If possible, I do think this is worth revisiting.

we currently don't build for multiple OSs using taskfiles, so we could drop it. We just need to update the places that assume $GOOS is in the path, which is mainly our packaging tools

emaxerrno · 2021-06-04T05:19:45Z

taskfiles/rpk.yml

@@ -20,3 +20,10 @@ tasks:
           -X ${ver_pkg}.rev={{.SHORT_SHA}} \
           -X ${cont_pkg}.tag={{.RPK_VERSION}}" \
        -o "{{.BUILD_ROOT}}/go/$GOOS/bin" ./...
+  tidy:


emaxerrno · 2021-06-04T05:20:07Z

src/go/k8s/go.sum

@@ -511,6 +511,7 @@ github.com/mitchellh/mapstructure v1.4.1 h1:CpVNEelQCZBooIPDn+AR3NpivK/TIKU8bDxd
 github.com/mitchellh/mapstructure v1.4.1/go.mod h1:bFUtVrKA4DC2yAKiSyO/QUcy7e+RRV2QTWOzhPopBRo=
 github.com/mitchellh/reflectwalk v1.0.0/go.mod h1:mSTlrgnPZtwu0c4WaC2kGObEpuNDbx0jmZXqmk4esnw=
 github.com/moby/term v0.0.0-20200312100748-672ec06f55cd/go.mod h1:DdlQx2hp0Ss5/fLikoLlEeIYiATotOjgB//nb973jeo=
+github.com/moby/term v0.0.0-20201216013528-df9cb8a40635/go.mod h1:FBS0z0QWA44HXygs7VXDUOGoN/1TV3RuWkLO04am3wc=


thanks for making this a separate commit. makes it easy to read.

emaxerrno

this looks good to me. In the future, please keep a PR focused on one set of changes. ok to leave the go os removal for now on this one.

twmb · 2021-06-04T06:59:55Z

I've dropped the GOOS removal, should allow the build to pass in this PR and drops one unrelated aspect. I've kept the taskfile addition because it was brought up in an earlier comment, though I can move that to a separate PR as well.

RafalKorepta

LGTM

taskfiles/rpk.yml

For any dir under src/go, this will cd into it and run `go mod tidy`

0x5d

This is awesome! Left a few minor comments.

src/go/rpk/pkg/cli/cmd/root.go

0x5d · 2021-06-04T19:40:50Z

src/go/rpk/pkg/cli/cmd/root.go

+	args = append([]string{path}, args...)
+	env := os.Environ()
+	if runtime.GOOS == "windows" {
+		return (&exec.Cmd{


Is this for WSL2?

This is just for Windows in general. I don't think redpanda is supported on Windows, but a client should be able to connect from Windows to get the status, and an exec like this can support a module on Windows as well.

The reason I was asking is that we don't build for windows right now, so even though this is nice, it won't be used.

src/go/rpk/pkg/cli/cmd/root_test.go

`cmd` is usually next to `pkg`, to separate binaries from libraries.

Quote, """ * groupcsaw * kverify can really be deleted :) - we wrote those when we were testing things a while ago and don't need them any more. """

This follows the exact algorithm that is used in kubernetes for plugins; a followup PR will introduce a `plugin list` subcommand that will behavior similarly to kubernetes as well. Plugin support is fairly simple; the test is a bit overkill. Unlike kubernetes, I'd like to bundle in plugin search directly into rpk, rather than eschew responsibility to a separate command (krew). That can also be done as followup.

emaxerrno

@twmb - great work!

@graphcareful - the rpk wasm lsit should use this plugin approach. - though, we have to teach vtools to package them.

Add plugin support

twmb requested review from 0x5d and mmaslankaprv as code owners June 2, 2021 03:25

github-actions bot added the area/rpk label Jun 2, 2021

emaxerrno reviewed Jun 2, 2021

View reviewed changes

src/go/rpk/pkg/cli/cmd/root.go Outdated Show resolved Hide resolved

emaxerrno reviewed Jun 2, 2021

View reviewed changes

src/go/rpk/pkg/cli/cmd/root.go Show resolved Hide resolved

twmb requested a review from a team as a code owner June 2, 2021 05:18

github-actions bot added the area/k8s label Jun 2, 2021

RafalKorepta reviewed Jun 2, 2021

View reviewed changes

src/go/rpk/go.mod Show resolved Hide resolved

src/go/rpk/pkg/cli/cmd/root.go Outdated Show resolved Hide resolved

src/go/rpk/pkg/cli/cmd/root.go Outdated Show resolved Hide resolved

emaxerrno approved these changes Jun 4, 2021

View reviewed changes

emaxerrno reviewed Jun 4, 2021

View reviewed changes

emaxerrno previously approved these changes Jun 4, 2021

View reviewed changes

twmb dismissed emaxerrno’s stale review via b2da136 June 4, 2021 06:56

RafalKorepta previously approved these changes Jun 4, 2021

View reviewed changes

ivotron reviewed Jun 4, 2021

View reviewed changes

taskfiles/rpk.yml Outdated Show resolved Hide resolved

twmb dismissed RafalKorepta’s stale review via 8ffc366 June 4, 2021 17:28

rpk taskfile: add 'tidy'

04b5ef4

For any dir under src/go, this will cd into it and run `go mod tidy`

0x5d suggested changes Jun 4, 2021

View reviewed changes

emaxerrno previously approved these changes Jun 4, 2021

View reviewed changes

run 'task rpk:tidy'

30054d3

twmb added 3 commits June 4, 2021 16:46

rpk: move cmd binary package out of pkg directory

683898b

`cmd` is usually next to `pkg`, to separate binaries from libraries.

remove groupcsaw and kverify

b441a61

Quote, """ * groupcsaw * kverify can really be deleted :) - we wrote those when we were testing things a while ago and don't need them any more. """

twmb dismissed emaxerrno’s stale review via d97d494 June 4, 2021 22:48

emaxerrno approved these changes Jun 5, 2021

View reviewed changes

emaxerrno merged commit 3e46af5 into redpanda-data:dev Jun 5, 2021

twmb deleted the twmb/plugin branch June 8, 2021 20:37

joejulian pushed a commit to joejulian/redpanda that referenced this pull request Mar 10, 2023

(split) Merge pull request redpanda-data#1519 from twmb/twmb/plugin

6f3288f

Add plugin support

joejulian pushed a commit to joejulian/redpanda that referenced this pull request Mar 24, 2023

operator: Merge pull request redpanda-data#1519 from twmb/twmb/plugin

4208567

Add plugin support

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add plugin support #1519

Add plugin support #1519

twmb commented Jun 2, 2021

CLAassistant commented Jun 2, 2021 •

edited

Loading

emaxerrno commented Jun 2, 2021

emaxerrno commented Jun 2, 2021

Cover letter

emaxerrno commented Jun 2, 2021

Cover letter

RafalKorepta left a comment

emaxerrno commented Jun 2, 2021

emaxerrno left a comment

emaxerrno commented Jun 4, 2021

emaxerrno commented Jun 4, 2021

twmb commented Jun 4, 2021

emaxerrno Jun 4, 2021

twmb Jun 4, 2021

ivotron Jun 4, 2021 •

edited

Loading

emaxerrno Jun 4, 2021

emaxerrno Jun 4, 2021

emaxerrno left a comment

twmb commented Jun 4, 2021

RafalKorepta left a comment

0x5d left a comment

0x5d Jun 4, 2021

twmb Jun 4, 2021

0x5d Jun 6, 2021

emaxerrno left a comment

Add plugin support #1519

Add plugin support #1519

Conversation

twmb commented Jun 2, 2021

Cover letter

CLAassistant commented Jun 2, 2021 • edited Loading

emaxerrno commented Jun 2, 2021

emaxerrno commented Jun 2, 2021

Cover letter

emaxerrno commented Jun 2, 2021

Cover letter

RafalKorepta left a comment

Choose a reason for hiding this comment

emaxerrno commented Jun 2, 2021

emaxerrno left a comment

Choose a reason for hiding this comment

emaxerrno commented Jun 4, 2021

emaxerrno commented Jun 4, 2021

twmb commented Jun 4, 2021

emaxerrno Jun 4, 2021

Choose a reason for hiding this comment

twmb Jun 4, 2021

Choose a reason for hiding this comment

ivotron Jun 4, 2021 • edited Loading

Choose a reason for hiding this comment

emaxerrno Jun 4, 2021

Choose a reason for hiding this comment

emaxerrno Jun 4, 2021

Choose a reason for hiding this comment

emaxerrno left a comment

Choose a reason for hiding this comment

twmb commented Jun 4, 2021

RafalKorepta left a comment

Choose a reason for hiding this comment

0x5d left a comment

Choose a reason for hiding this comment

0x5d Jun 4, 2021

Choose a reason for hiding this comment

twmb Jun 4, 2021

Choose a reason for hiding this comment

0x5d Jun 6, 2021

Choose a reason for hiding this comment

emaxerrno left a comment

Choose a reason for hiding this comment

CLAassistant commented Jun 2, 2021 •

edited

Loading

ivotron Jun 4, 2021 •

edited

Loading