Merge pull request #296 from hashicorp/case-of-the-missing-target

Set target configuration from vagrantfile before save

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,

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
@@ -723,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 {
@@ -740,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

internal/core/target.go

@@ -283,12 +283,14 @@ func (t *Target) Save() (err error) {
 		"name", t.target.Name,
 	)
 
-	result, err := t.Client().UpsertTarget(t.ctx, &vagrant_server.UpsertTargetRequest{
+	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
 	}
@@ -471,6 +473,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 == "" {
@@ -564,7 +573,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
 	}
 }

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
 }

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
 		}
 	}