packer: pick protobuf/gob for serialisation #13025
Conversation
lbajolet-hashicorp
added
stage/waiting-on-upstream
|
Using this thread to capture finding and fixes we need to make. While testing using docker and shell-local I found a crash because of the protocol mixing. Internal plugin has gob, and external uses protobuf. Internal plugins don't rely on describe so we would need to set these up to use protobuf by default and gob if we can't use it for all plug-ins. protobuf
2024/06/10 14:53:57 [DEBUG] - common: receiving ConfigSpec as protobuf 2024/06/10 14:53:57 [INFO] Starting internal plugin packer-provisioner-shell-local
2024/06/10 14:53:57 Starting plugin: /Users/dev/Development/packer/bin/packer []string{"/Users/dev/Development/packer/bin/packer", "execute
", "packer-provisioner-shell-local"} 2024/06/10 14:53:57 Waiting for RPC address for: /Users/dev/Development/packer/bin/packer
2024/06/10 14:53:57 packer-provisioner-shell-local plugin: [INFO] Packer version: 1.12.0-dev [go1.21.8 darwin arm64]
2024/06/10 14:53:57 packer-provisioner-shell-local plugin: [INFO] PACKER_CONFIG env var not set; checking the default config file path 2024/06/10 14:53:57 packer-provisioner-shell-local plugin: [INFO] PACKER_CONFIG env var set; attempting to open config file: /Users/dev/.pa
ckerconfig
2024/06/10 14:53:57 packer-provisioner-shell-local plugin: [WARN] Config file doesn't exist: /Users/dev/.packerconfig
2024/06/10 14:53:57 packer-provisioner-shell-local plugin: [INFO] Setting cache directory: /Users/dev/.cache/packer
2024/06/10 14:53:57 packer-provisioner-shell-local plugin: Plugin address: unix /var/folders/jd/zhl6kpsn1156cmqdyttt22xm0000gp/T/packer-plu
gin439111653
2024/06/10 14:53:57 Received unix RPC address for /Users/dev/Development/packer/bin/packer: addr is /var/folders/jd/zhl6kpsn1156cmqdyttt22x
m0000gp/T/packer-plugin439111653
2024/06/10 14:53:57 packer-provisioner-shell-local plugin: Waiting for connection...
2024/06/10 14:53:57 packer-provisioner-shell-local plugin: Serving a plugin connection...
2024/06/10 14:53:57 packer-provisioner-shell-local plugin: [DEBUG] - common: sending ConfigSpec as gob
2024/06/10 14:53:57 [DEBUG] - common: receiving ConfigSpec as protobuf
2024/06/10 14:53:57 failed to unmarshal hclspec.Spec from raw protobuf: "proto:\u00a0cannot parse invalid wire-format data"
2024/06/10 14:53:57 waiting for all plugin processes to complete...
2024/06/10 14:53:57 /Users/dev/.config/packer/plugins/github.com/hashicorp/docker/packer-plugin-docker_v1.0.10-dev_x5.0_darwin_arm64: plugi
n process exited
2024/06/10 14:53:57 /Users/dev/Development/packer/bin/packer: plugin process exited
panic: failed to unmarshal hclspec.Spec from raw protobuf: "proto:\u00a0cannot parse invalid wire-format data" [recovered]
panic: failed to unmarshal hclspec.Spec from raw protobuf: "proto:\u00a0cannot parse invalid wire-format data"
goroutine 1 [running]:
log.Panic({0x14000a2ffc8?, 0x14000880000?, 0x14000184110?})
panic: failed to unmarshal hclspec.Spec from raw protobuf: "proto:\u00a0cannot parse invalid wire-format data" [recovered]
panic: failed to unmarshal hclspec.Spec from raw protobuf: "proto:\u00a0cannot parse invalid wire-format data"
goroutine 1 [running]:
log.Panic({0x14000a2ffc8?, 0x14000880000?, 0x14000184110?})
/usr/local/go/src/log/log.go:432 +0x60
github.com/hashicorp/packer/packer.(*cmdProvisioner).checkExit(0x14000904680?, {0x105f9e8c0?, 0x14000184110}, 0x0)
/Users/dev/Development/packer/packer/cmd_provisioner.go:50 +0x80
github.com/hashicorp/packer/packer.(*cmdProvisioner).ConfigSpec.func1()
/Users/dev/Development/packer/packer/cmd_provisioner.go:22 +0x40
panic({0x105f9e8c0?, 0x14000184110?})
/usr/local/go/src/runtime/panic.go:914 +0x218 |
lbajolet-hashicorp
force-pushed
the
poc_protobuf_and_gob
branch
4 times, most recently
from
40ebdbe to
72f47be
Compare
lbajolet-hashicorp
force-pushed
the
poc_protobuf_and_gob
branch
from
72f47be to
9707101
Compare
| @@ -100,6 +127,10 @@ func (c *ExecuteCommand) Run(args []string) int { | |||
| return 1 | |||
| } | |||
|
|
|||
| if args.UseProtobuf { | |||
| server.UseProto = true | |||
There was a problem hiding this comment.
Do the builtin plugins always use protobufs if supported? How do we toggle it off for built ins?
There was a problem hiding this comment.
So the main Packer process is responsible for that, internal components are still executed much like any binary through packer execute, so if plugins support protobuf, we'll add the flag to the command, and the internal components will run as a server with protobuf.
Also I believe we can do server.UseProto = args.UseProtobuf instead of the if here :)
There was a problem hiding this comment.
Okay understood. Yeah these plugins are also not plugin.Set right so we have to set server.UseProto = args.UseProtobuf got it now.
There was a problem hiding this comment.
Yeah exactly, plugin.Set is used only for plugins, as they register their components, and we can use the rest of the predefined commands for them.
Since this is internal, there's no concept of describe for example (not even sure the command's called start for an internal component), so Set is not used, but instead we directly create the server with the component requested by name.
| return c.Client(pluginPath, "start", "builder", builderName).Builder() | ||
| args := []string{"start", "builder"} | ||
|
|
||
| if PackerUseProto { |
There was a problem hiding this comment.
Should this be canProto? It looks like PackerUseProto is always true, which means we will always send --protobuf.
There was a problem hiding this comment.
Good question; nope :p
We use PackerUseProto as it's global, since this is a function that is lazily evaluated, we cannot access canProto as the value is lost once we leave this function (it's all local).
However if the value of PackerUseProto is changed to false while discovering plugins, when this is executed, it will use the last updated value for this, and correctly pick between protobuf/gob.
As we're trying to move away from gob for serialising data over the wire, this commit adds the capability for Packer to pick dynamically between gob or protobuf for the serialisation format to communicate with plugins. As it stands, if all the plugins discovered are compatible with protobuf, and we have not forced gob usage, protobuf will be the serialisation format picked. If any plugin is not compatible with protobuf, gob will be used for communicating with all the plugins that will be used over the course of a command.
nywilken
force-pushed
the
poc_protobuf_and_gob
branch
from
9707101 to
cab0eac
Compare
As we're trying to move away from gob for serialising data over the wire, this commit adds the capability for Packer to pick dynamically between gob or protobuf for the serialisation format to communicate with plugins. As it stands, if all the plugins discovered are compatible with protobuf, and we have not forced gob usage, protobuf will be the serialisation format picked. If any plugin is not compatible with protobuf, gob will be used for communicating with all the plugins that will be used over the course of a command.
As we're trying to move away from gob for serialising data over the wire, this commit adds the capability for Packer to pick dynamically between gob or protobuf for the serialisation format to communicate with plugins. As it stands, if all the plugins discovered are compatible with protobuf, and we have not forced gob usage, protobuf will be the serialisation format picked. If any plugin is not compatible with protobuf, gob will be used for communicating with all the plugins that will be used over the course of a command.
As we're trying to move away from gob for serialising data over the wire, this commit adds the capability for Packer to pick dynamically between gob or protobuf for the serialisation format to communicate with plugins. As it stands, if all the plugins discovered are compatible with protobuf, and we have not forced gob usage, protobuf will be the serialisation format picked. If any plugin is not compatible with protobuf, gob will be used for communicating with all the plugins that will be used over the course of a command.
|
I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. |
As we're trying to move away from gob for serialising data over the wire, this commit adds the capability for Packer to pick dynamically between gob or protobuf for the serialisation format to communicate with plugins.
As it stands, if all the plugins discovered are compatible with protobuf, and we have not forced gob usage, protobuf will be the serialisation format picked.
If any plugin is not compatible with protobuf, gob will eb used for communicating with all the plugins that will be used over the course of a command.
Note: this will require the SDK to support it, so we need to have the related PR committed, and released before we can pull the plug on this one.