Skip to content
This repository has been archived by the owner on Jan 19, 2023. It is now read-only.

plugins - automatically reload/remove Go plugins #2818

Merged
merged 3 commits into from
Sep 2, 2021

Conversation

wwitzel3
Copy link
Contributor

@wwitzel3 wwitzel3 commented Sep 1, 2021

Signed-off-by: Wayne Witzel III [email protected]

What this PR does / why we need it:
Currently Go plugins do not automatically reload or remove themselves based on changes happening to the binaries in the plugin folder and a full restart of Octant is required to view plugin changes. We want to enable the same behavior we have for the JavaScript plugins for the Go plugins.

  • If a binary is removed, this is treated as an unload operation.
  • If a binary is created, this is treated as a load operation.
  • If a binary is renamed, this is treated as an unload (old name), load (new name) operation.
  • If a binary is replaced, this is treated as an unload, load operation.

Which issue(s) this PR fixes

@GuessWhoSamFoo
Copy link
Contributor

Not necessarily something to be addressed in this PR: whenever a plugin panics, the content manager stops returning any data.

@xtreme-vikram-yadav
Copy link
Contributor

I faced some confusion while testing this by updating the plugin by repeatedly running go run build.go install-test-plugin. This only happens when I use the build.go because I am used to it. I see a log entry 2021-09-01T19:36:16.921-0400 INFO plugin/manager.go:448 removing plugin: C:\Users\vikram\AppData\Local\octant\plugins\octant-sample-plugin.exe 2021-09-01T19:36:17.991-0400 INFO plugin/manager.go:424 reloading: Go plugin: C:\Users\vikram\AppData\Local\octant\plugins\octant-sample-plugin.exe~.
But the original plugin binary was not deleted and it cannot be delete even as an admin because it is still running and interacting with octant. So, windows creates another binary by appending ~ (octant-sample-plugin.exe~). This causes octant to register the same plugin twice because the name of the plugin is technically different.
duplicate-plugin

Octant fails again if we try to install the plugins with more updates but since windows cannot create more file with a same name (I guess it's last attempt to resolve file name conflicts is to append ) and it fails to replace the existing binary(octant-sample-plugin.exe) and throws the following error:

2021/09/01 19:41:10 Running: C:\Program Files\Go\bin\go.exe build -o C:\Users\vikram\AppData\Local\octant\plugins\octant-sample-plugin.exe github.com/vmware-tanzu/octant/cmd/octant-sample-plugin
go build github.com/vmware-tanzu/octant/cmd/octant-sample-plugin: copying C:\Users\vikram\AppData\Local\Temp\go-build3339358845\b001\exe\a.out.exe: open C:\Users\vikram\AppData\Local\octant\plugins\octant-sample-plugin.exe: The process cannot access the file because it is being used by another process.
2021/09/01 19:41:13 exit status 1
exit status 1

All of this makes sense and this problem can be avoided by creating a new binary with a different name. May be it's worth documenting with this feature in case users run the same script to create a new binary with a same name. We should also update our build script to make install-test-plugin a bit more convenient. The build script update is a low priority issue and can be scheduled for later.

Signed-off-by: Wayne Witzel III <[email protected]>
@wwitzel3
Copy link
Contributor Author

wwitzel3 commented Sep 2, 2021

Not necessarily something to be addressed in this PR: whenever a plugin panics, the content manager stops returning any data.

Yeah, I think that would be out of scope for this. That is currently the case before this change. I'll create an issue for it.

@wwitzel3
Copy link
Contributor Author

wwitzel3 commented Sep 2, 2021

I faced some confusion while testing this by updating the plugin by repeatedly running go run build.go install-test-plugin. This only happens when I use the build.go because I am used to it. I see a log entry 2021-09-01T19:36:16.921-0400 INFO plugin/manager.go:448 removing plugin: C:\Users\vikram\AppData\Local\octant\plugins\octant-sample-plugin.exe 2021-09-01T19:36:17.991-0400 INFO plugin/manager.go:424 reloading: Go plugin: C:\Users\vikram\AppData\Local\octant\plugins\octant-sample-plugin.exe~.
But the original plugin binary was not deleted and it cannot be delete even as an admin because it is still running and interacting with octant. So, windows creates another binary by appending ~ (octant-sample-plugin.exe~). This causes octant to register the same plugin twice because the name of the plugin is technically different.
duplicate-plugin

Octant fails again if we try to install the plugins with more updates but since windows cannot create more file with a same name (I guess it's last attempt to resolve file name conflicts is to append ) and it fails to replace the existing binary(octant-sample-plugin.exe) and throws the following error:

2021/09/01 19:41:10 Running: C:\Program Files\Go\bin\go.exe build -o C:\Users\vikram\AppData\Local\octant\plugins\octant-sample-plugin.exe github.com/vmware-tanzu/octant/cmd/octant-sample-plugin
go build github.com/vmware-tanzu/octant/cmd/octant-sample-plugin: copying C:\Users\vikram\AppData\Local\Temp\go-build3339358845\b001\exe\a.out.exe: open C:\Users\vikram\AppData\Local\octant\plugins\octant-sample-plugin.exe: The process cannot access the file because it is being used by another process.
2021/09/01 19:41:13 exit status 1
exit status 1

All of this makes sense and this problem can be avoided by creating a new binary with a different name. May be it's worth documenting with this feature in case users run the same script to create a new binary with a same name. We should also update our build script to make install-test-plugin a bit more convenient. The build script update is a low priority issue and can be scheduled for later.

So I've been thinking about this and I'm not exactly sure what we can do as part of this set of changes. Windows filesystem provides some unique challenges with respect to processes in use and being able to overwrite files.

I faced some confusion while testing this by updating the plugin by repeatedly running go run build.go install-test-plugin. This only happens when I use the build.go because I am used to it. I see a log entry 2021-09-01T19:36:16.921-0400 INFO plugin/manager.go:448 removing plugin: C:\Users\vikram\AppData\Local\octant\plugins\octant-sample-plugin.exe 2021-09-01T19:36:17.991-0400 INFO plugin/manager.go:424 reloading: Go plugin: C:\Users\vikram\AppData\Local\octant\plugins\octant-sample-plugin.exe~.
But the original plugin binary was not deleted and it cannot be delete even as an admin because it is still running and interacting with octant. So, windows creates another binary by appending ~ (octant-sample-plugin.exe~). This causes octant to register the same plugin twice because the name of the plugin is technically different.
duplicate-plugin

Octant fails again if we try to install the plugins with more updates but since windows cannot create more file with a same name (I guess it's last attempt to resolve file name conflicts is to append ) and it fails to replace the existing binary(octant-sample-plugin.exe) and throws the following error:

2021/09/01 19:41:10 Running: C:\Program Files\Go\bin\go.exe build -o C:\Users\vikram\AppData\Local\octant\plugins\octant-sample-plugin.exe github.com/vmware-tanzu/octant/cmd/octant-sample-plugin
go build github.com/vmware-tanzu/octant/cmd/octant-sample-plugin: copying C:\Users\vikram\AppData\Local\Temp\go-build3339358845\b001\exe\a.out.exe: open C:\Users\vikram\AppData\Local\octant\plugins\octant-sample-plugin.exe: The process cannot access the file because it is being used by another process.
2021/09/01 19:41:13 exit status 1
exit status 1

All of this makes sense and this problem can be avoided by creating a new binary with a different name. May be it's worth documenting with this feature in case users run the same script to create a new binary with a same name. We should also update our build script to make install-test-plugin a bit more convenient. The build script update is a low priority issue and can be scheduled for later.

I think the solution to this is to create a Windows specific loader that makes a copy of the binary before executing it. Our process control would be around the copy of the binary, but our change detection would be around the original file. I think that this is out of scope for this work specifically.

I will create a new issue.

Copy link
Contributor

@GuessWhoSamFoo GuessWhoSamFoo left a comment

Choose a reason for hiding this comment

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

The plugin path redirect on delete has been addressed.

Remaining issues are windows specific or already existed so they are out of scope for 0.24

@GuessWhoSamFoo GuessWhoSamFoo merged commit 5a3a1f3 into vmware-archive:master Sep 2, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Installing a plugin requires restart of main octant process.
3 participants