From 822b2af8a587a2a7612f14dea5c66fc43a6c6a5f Mon Sep 17 00:00:00 2001 From: Chris Roberts Date: Tue, 28 Jun 2022 11:07:52 -0700 Subject: [PATCH 1/6] Set target configuration from vagrantfile before save --- internal/core/target.go | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/internal/core/target.go b/internal/core/target.go index e80513b6d..001e5a414 100644 --- a/internal/core/target.go +++ b/internal/core/target.go @@ -270,12 +270,16 @@ func (t *Target) Save() (err error) { "name", t.target.Name, ) - result, err := t.Client().UpsertTarget(t.ctx, &vagrant_server.UpsertTargetRequest{ + t.target.Configuration, err = t.vagrantfile.rootToStore() + + result, uerr := t.Client().UpsertTarget(t.ctx, &vagrant_server.UpsertTargetRequest{ Target: t.target}) - if err != nil { + if uerr != nil { t.logger.Trace("failed to save target", "target", t.target.ResourceId, - "error", err) + "error", uerr) + + err = multierror.Append(err, uerr) return } From 8c40214d617a7957dcab89af228f45916aa29dcf Mon Sep 17 00:00:00 2001 From: Chris Roberts Date: Wed, 29 Jun 2022 10:29:43 -0700 Subject: [PATCH 2/6] Update project to use cleanup, not shutdown targets --- internal/core/basis.go | 1 + internal/core/project.go | 35 ++++++++--------------------------- 2 files changed, 9 insertions(+), 27 deletions(-) diff --git a/internal/core/basis.go b/internal/core/basis.go index 0b1191ff1..3ee1651a5 100644 --- a/internal/core/basis.go +++ b/internal/core/basis.go @@ -620,6 +620,7 @@ func (b *Basis) LoadProject(popts ...ProjectOption) (p *Project, err error) { // Create our project p = &Project{ ctx: b.ctx, + cleanup: cleanup.New(), basis: b, logger: b.logger, mappers: b.mappers, diff --git a/internal/core/project.go b/internal/core/project.go index f2e6c67e7..31efa1ebc 100644 --- a/internal/core/project.go +++ b/internal/core/project.go @@ -24,6 +24,7 @@ import ( "github.com/hashicorp/vagrant-plugin-sdk/helper/path" "github.com/hashicorp/vagrant-plugin-sdk/helper/paths" "github.com/hashicorp/vagrant-plugin-sdk/internal-shared/cacher" + "github.com/hashicorp/vagrant-plugin-sdk/internal-shared/cleanup" "github.com/hashicorp/vagrant-plugin-sdk/proto/vagrant_plugin_sdk" "github.com/hashicorp/vagrant-plugin-sdk/terminal" @@ -52,8 +53,8 @@ type Project struct { // This lock only needs to be held currently to protect closers. m sync.Mutex - // The below are resources we need to close when Close is called, if non-nil - closers []func() error + // Registered actions for cleanup on close + cleanup cleanup.Cleanup // UI is the terminal UI to use for messages related to the project // as a whole. These messages will show up unprefixed for example compared @@ -604,7 +605,7 @@ func (p *Project) seed(fn func(*core.Seeds)) { // Register functions to be called when closing this project func (p *Project) Closer(c func() error) { - p.closers = append(p.closers, c) + p.cleanup.Do(c) } // Close is called to clean up resources allocated by the project. @@ -613,31 +614,11 @@ func (p *Project) Close() (err error) { p.logger.Debug("closing project", "project", p) - // close all the loaded targets - for name, m := range p.targets { - p.logger.Trace("closing target", - "target", name) - - if cerr := m.Close(); cerr != nil { - p.logger.Warn("error closing target", - "target", name, - "err", cerr) - - err = multierror.Append(err, cerr) - } - } - - for _, f := range p.closers { - if cerr := f(); cerr != nil { - p.logger.Warn("error executing closer", - "error", cerr) - - err = multierror.Append(err, cerr) - } - } - // Remove this project from built project list in basis + // Remove this project from basis project list delete(p.basis.projects, p.Name()) - return + delete(p.basis.projects, p.project.ResourceId) + + return p.cleanup.Close() } // Saves the project to the db From aec4d13e2128cb3ee0792ad075cc184ba51b409c Mon Sep 17 00:00:00 2001 From: Chris Roberts Date: Wed, 29 Jun 2022 10:30:21 -0700 Subject: [PATCH 3/6] Set configuration after successful init --- internal/core/target.go | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/internal/core/target.go b/internal/core/target.go index 001e5a414..73e270052 100644 --- a/internal/core/target.go +++ b/internal/core/target.go @@ -270,8 +270,6 @@ func (t *Target) Save() (err error) { "name", t.target.Name, ) - t.target.Configuration, err = t.vagrantfile.rootToStore() - result, uerr := t.Client().UpsertTarget(t.ctx, &vagrant_server.UpsertTargetRequest{ Target: t.target}) if uerr != nil { @@ -462,6 +460,13 @@ func (t *Target) doOperation( // Initialize the target instance func (t *Target) init() (err error) { + // As long as no error is encountered, + // update the target configuration. + defer func() { + if err == nil { + t.target.Configuration, err = t.vagrantfile.rootToStore() + } + }() t.logger.Info("running init on target", "target", t.target.Name) // Name or resource id is required for a target to be loaded if t.target.Name == "" && t.target.ResourceId == "" { @@ -555,7 +560,6 @@ type TargetOption func(*Target) error func WithTargetVagrantfile(v *Vagrantfile) TargetOption { return func(t *Target) (err error) { t.vagrantfile = v - t.target.Configuration, err = v.rootToStore() return } } From b365827f73c030802dfd25e6a6953b3fc7e19df4 Mon Sep 17 00:00:00 2001 From: Chris Roberts Date: Thu, 30 Jun 2022 11:15:56 -0700 Subject: [PATCH 4/6] Save updated project after deleting target --- internal/server/singleprocess/state/target.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/server/singleprocess/state/target.go b/internal/server/singleprocess/state/target.go index e15f3a943..b9b7906b6 100644 --- a/internal/server/singleprocess/state/target.go +++ b/internal/server/singleprocess/state/target.go @@ -282,7 +282,7 @@ func (s *State) targetDelete( pp := serverptypes.Project{Project: p} if pp.DeleteTargetRef(ref) { - if err = s.projectPut(dbTxn, memTxn, p); err != nil { + if err = s.projectPut(dbTxn, memTxn, pp.Project); err != nil { return } } From e7df8b6c5248f4611183cdd640a59edc48ac090b Mon Sep 17 00:00:00 2001 From: Chris Roberts Date: Thu, 30 Jun 2022 11:16:09 -0700 Subject: [PATCH 5/6] Properly delete the target from the project --- internal/server/ptypes/project.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/internal/server/ptypes/project.go b/internal/server/ptypes/project.go index a6506912e..2a0e3db7c 100644 --- a/internal/server/ptypes/project.go +++ b/internal/server/ptypes/project.go @@ -19,7 +19,7 @@ func TestProject(t testing.T, src *vagrant_server.Project) *vagrant_server.Proje } require.NoError(t, mergo.Merge(src, &vagrant_server.Project{ - Name: "test", + Name: "test", Basis: &vagrant_plugin_sdk.Ref_Basis{}, })) @@ -84,8 +84,9 @@ func (p *Project) DeleteTargetRef(m *vagrant_plugin_sdk.Ref_Target) bool { if i < 0 { return false } - ms := p.Project.Targets - ms[len(ms)-1], ms[i] = ms[i], ms[len(ms)-1] + ms := make([]*vagrant_plugin_sdk.Ref_Target, len(p.Project.Targets)-1) + copy(ms[0:], p.Project.Targets[0:i]) + copy(ms[i:], p.Project.Targets[i+1:]) p.Project.Targets = ms return true } From 8b7d702dddfbfd740cf294a11bd5457f5971e73c Mon Sep 17 00:00:00 2001 From: Chris Roberts Date: Thu, 30 Jun 2022 11:16:23 -0700 Subject: [PATCH 6/6] Check existing target state and remove if not created --- internal/core/project.go | 49 +++++++++++++++++++++++++++++++++++++--- 1 file changed, 46 insertions(+), 3 deletions(-) diff --git a/internal/core/project.go b/internal/core/project.go index 31efa1ebc..de693fdb6 100644 --- a/internal/core/project.go +++ b/internal/core/project.go @@ -704,12 +704,15 @@ func (p *Project) InitTargets() (err error) { for _, t := range p.project.Targets { existingTargets = append(existingTargets, t.Name) } - p.logger.Trace("known targets within project", - "project", p.Name(), - "targets", existingTargets, + + p.logger.Trace("targets associated with project", + "project", p, + "existing", existingTargets, + "defined", targets, ) updated := false + seen := map[string]struct{}{} for _, t := range targets { _, err = p.createTarget(t) if err != nil { @@ -721,9 +724,49 @@ func (p *Project) InitTargets() (err error) { return } + seen[t] = struct{}{} updated = true } + // If any existing targets are not in the defined list and are + // not in a created state, delete them as they were removed + // from the vagrantfile + for _, existName := range existingTargets { + if _, ok := seen[existName]; ok { + continue + } + resp, err := p.Client().FindTarget(p.ctx, + &vagrant_server.FindTargetRequest{ + Target: &vagrant_server.Target{ + Name: existName, + Project: p.Ref().(*vagrant_plugin_sdk.Ref_Project), + }, + }, + ) + if err != nil { + return err + } + // If the state is not created or unknown, remove it + if resp.Target.State == vagrant_server.Operation_NOT_CREATED || + resp.Target.State == vagrant_server.Operation_UNKNOWN { + _, err := p.Client().DeleteTarget(p.ctx, + &vagrant_server.DeleteTargetRequest{ + Target: &vagrant_plugin_sdk.Ref_Target{ + Name: existName, + ResourceId: resp.Target.ResourceId, + Project: p.Ref().(*vagrant_plugin_sdk.Ref_Project), + }, + }, + ) + if err != nil && status.Code(err) != codes.NotFound { + return err + } else { + err = nil + } + updated = true + } + } + if updated { // If targets have been updated then refresh the project. This is required // since upserting targets will also update the project to have a reference