From f4d3f38921878582363e0017b02c99f8c4600ccf Mon Sep 17 00:00:00 2001 From: sophia Date: Wed, 15 Jun 2022 10:35:55 -0500 Subject: [PATCH 1/4] Fix type in communicator#ready funcspec --- plugins/commands/serve/service/communicator_service.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/commands/serve/service/communicator_service.rb b/plugins/commands/serve/service/communicator_service.rb index 4f3b618ff..3ea407bd4 100644 --- a/plugins/commands/serve/service/communicator_service.rb +++ b/plugins/commands/serve/service/communicator_service.rb @@ -8,7 +8,7 @@ module VagrantPlugins logger.debug("generating ready spec") funcspec( args: [ - SDK::Target::Machine, + SDK::Args::Target::Machine, ], result: SDK::Communicator::ReadyResp, ) From e0a30f9ba704c18241898748f217da0f2773559a Mon Sep 17 00:00:00 2001 From: sophia Date: Wed, 15 Jun 2022 10:36:19 -0500 Subject: [PATCH 2/4] Raise error if communicator is not ready when getting guest from machine --- lib/vagrant/machine/remote.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/vagrant/machine/remote.rb b/lib/vagrant/machine/remote.rb index dfd8433a6..60014db88 100644 --- a/lib/vagrant/machine/remote.rb +++ b/lib/vagrant/machine/remote.rb @@ -85,6 +85,8 @@ module Vagrant end def guest + # require "pry-remote"; binding.pry_remote + raise Errors::MachineGuestNotReady if !communicate.ready? if !@guest @guest = Guest.new(self, nil, nil) end From adadf26c9c9e30d63fb324941e8f3c65cbda3406 Mon Sep 17 00:00:00 2001 From: sophia Date: Wed, 15 Jun 2022 11:08:45 -0500 Subject: [PATCH 3/4] Check if communicator is available before detecting guests --- internal/core/machine.go | 11 +++++++++++ lib/vagrant/machine/remote.rb | 1 - plugins/commands/serve/client/target/machine.rb | 1 - 3 files changed, 11 insertions(+), 2 deletions(-) diff --git a/internal/core/machine.go b/internal/core/machine.go index 9bac5d04a..f4f4bba30 100644 --- a/internal/core/machine.go +++ b/internal/core/machine.go @@ -98,6 +98,17 @@ func (m *Machine) Box() (b core.Box, err error) { // Guest implements core.Machine func (m *Machine) Guest() (g core.Guest, err error) { + comm, err := m.Communicate() + if err != nil { + return nil, err + } + isReady, err := comm.Ready(m) + if err != nil { + return nil, err + } + if !isReady { + return nil, fmt.Errorf("unable to communicate with guest") + } defer func() { if g != nil { err = seedPlugin(g, m) diff --git a/lib/vagrant/machine/remote.rb b/lib/vagrant/machine/remote.rb index 60014db88..225c96589 100644 --- a/lib/vagrant/machine/remote.rb +++ b/lib/vagrant/machine/remote.rb @@ -85,7 +85,6 @@ module Vagrant end def guest - # require "pry-remote"; binding.pry_remote raise Errors::MachineGuestNotReady if !communicate.ready? if !@guest @guest = Guest.new(self, nil, nil) diff --git a/plugins/commands/serve/client/target/machine.rb b/plugins/commands/serve/client/target/machine.rb index 3551835f9..a64f7ec0f 100644 --- a/plugins/commands/serve/client/target/machine.rb +++ b/plugins/commands/serve/client/target/machine.rb @@ -37,7 +37,6 @@ module VagrantPlugins end # @return [Guest] machine guest - # TODO: This needs to be loaded properly def guest g = client.guest(Empty.new) Guest.load(g, broker: broker) From 8859e2e03b7bef7e2c54bfa50584e3e7978b7451 Mon Sep 17 00:00:00 2001 From: sophia Date: Wed, 15 Jun 2022 14:03:20 -0500 Subject: [PATCH 4/4] Add test communicator plugin to guest detection test --- internal/core/machine_test.go | 35 +++++++++++++++++++++++++--------- internal/core/testing_basis.go | 13 +++++++++++++ 2 files changed, 39 insertions(+), 9 deletions(-) diff --git a/internal/core/machine_test.go b/internal/core/machine_test.go index 165a75c64..548022ae8 100644 --- a/internal/core/machine_test.go +++ b/internal/core/machine_test.go @@ -138,6 +138,14 @@ func TestMachineGetExistentBox(t *testing.T) { } func TestMachineConfigedGuest(t *testing.T) { + commMock := BuildTestCommunicatorPlugin("ssh") + commMock.On("Ready", mock.AnythingOfType("*core.Machine")).Return(true, nil) + commPlugin := plugin.TestPlugin(t, + commMock, + plugin.WithPluginName("ssh"), + plugin.WithPluginTypes(component.CommunicatorType), + ) + type test struct { config *component.ConfigData errors bool @@ -157,6 +165,7 @@ func TestMachineConfigedGuest(t *testing.T) { plugin.WithPluginName("myguest"), plugin.WithPluginTypes(component.GuestType), ), + commPlugin, ) for _, tc := range tests { @@ -178,6 +187,14 @@ func TestMachineConfigedGuest(t *testing.T) { } func TestMachineNoConfigGuest(t *testing.T) { + commMock := BuildTestCommunicatorPlugin("ssh") + commMock.On("Ready", mock.AnythingOfType("*core.Machine")).Return(true, nil) + commPlugin := plugin.TestPlugin(t, + commMock, + plugin.WithPluginName("ssh"), + plugin.WithPluginTypes(component.CommunicatorType), + ) + guestMock := BuildTestGuestPlugin("myguest", "") guestMock.On("Detect", mock.AnythingOfType("*core.Machine")).Return(true, nil) guestMock.On("Parent").Return("", nil) @@ -211,12 +228,12 @@ func TestMachineNoConfigGuest(t *testing.T) { } tests := []test{ - {plugins: []*plugin.Plugin{detectingPlugin}, errors: false, expectedPluginName: "myguest"}, - {plugins: []*plugin.Plugin{detectingChildPlugin}, errors: true, expectedPluginName: "myguest-child"}, - {plugins: []*plugin.Plugin{detectingChildPlugin, detectingPlugin}, errors: false, expectedPluginName: "myguest-child"}, - {plugins: []*plugin.Plugin{detectingPlugin, nonDetectingPlugin}, errors: false, expectedPluginName: "myguest"}, - {plugins: []*plugin.Plugin{nonDetectingPlugin}, errors: true}, - {plugins: []*plugin.Plugin{}, errors: true}, + {plugins: []*plugin.Plugin{commPlugin, detectingPlugin}, errors: false, expectedPluginName: "myguest"}, + {plugins: []*plugin.Plugin{commPlugin, detectingChildPlugin}, errors: true, expectedPluginName: "myguest-child"}, + {plugins: []*plugin.Plugin{commPlugin, detectingChildPlugin, detectingPlugin}, errors: false, expectedPluginName: "myguest-child"}, + {plugins: []*plugin.Plugin{commPlugin, detectingPlugin, nonDetectingPlugin}, errors: false, expectedPluginName: "myguest"}, + {plugins: []*plugin.Plugin{commPlugin, nonDetectingPlugin}, errors: true}, + {plugins: []*plugin.Plugin{commPlugin}, errors: true}, } for _, tc := range tests { @@ -230,13 +247,13 @@ func TestMachineNoConfigGuest(t *testing.T) { require.Nil(t, guest) require.Nil(t, tm.cache.Get("guest")) } else { + require.NoError(t, err) + require.NotNil(t, guest) + require.NotNil(t, tm.cache.Get("guest")) n, _ := guest.PluginName() if n != tc.expectedPluginName { t.Error("Found unexpected plugin, ", n) } - require.NoError(t, err) - require.NotNil(t, guest) - require.NotNil(t, tm.cache.Get("guest")) } } } diff --git a/internal/core/testing_basis.go b/internal/core/testing_basis.go index 080d9821f..e7593e54b 100644 --- a/internal/core/testing_basis.go +++ b/internal/core/testing_basis.go @@ -28,6 +28,11 @@ func (p *PluginWithParent) SetParentComponent(in interface{}) { p.parentPlugin = in } +type TestCommunicatorPlugin struct { + plugin.TestPluginWithFakeBroker + coremocks.Communicator +} + type TestGuestPlugin struct { PluginWithParent plugin.TestPluginWithFakeBroker @@ -46,6 +51,14 @@ type TestSyncedFolderPlugin struct { coremocks.SyncedFolder } +func BuildTestCommunicatorPlugin(name string) *TestCommunicatorPlugin { + c := &TestCommunicatorPlugin{} + c.On("Seed", mock.AnythingOfType("*core.Seeds")).Return(nil) + c.On("Seeds").Return(core.NewSeeds(), nil) + c.On("PluginName").Return(name, nil) + return c +} + func BuildTestGuestPlugin(name string, parent string) *TestGuestPlugin { p := &TestGuestPlugin{} p.On("SetPluginName", mock.AnythingOfType("string")).Return(nil)