Destroy environ if new model was not created;

sdg0919 · Feb 10, 2020 · 05d5dd1 · 05d5dd1
1 parent 2dbf785
commit 05d5dd1
Show file tree

Hide file tree

Showing 7 changed files with 29 additions and 43 deletions.
diff --git a/apiserver/facades/client/modelmanager/modelmanager.go b/apiserver/facades/client/modelmanager/modelmanager.go
@@ -520,8 +520,6 @@ func (m *ModelManagerAPI) CreateModel(args params.ModelCreateArgs) (params.Model
 		return result, errors.Trace(err)
 	}
 
-	// TODO: check model name already exist!!!!!!!!!
-
 	var model common.Model
 	if jujucloud.CloudIsCAAS(cloud) {
 		model, err = m.newCAASModel(
@@ -664,6 +662,10 @@ func (m *ModelManagerAPI) newModel(
 		EnvironVersion:          env.Provider().Version(),
 	})
 	if err != nil {
+		// Clean up the environ.
+		if e := env.Destroy(m.callContext); e != nil {
+			logger.Warningf("failed to destroy environ, error %v", e)
+		}
 		return nil, errors.Annotate(err, "failed to create new model")
 	}
 	defer st.Close()

diff --git a/provider/vsphere/config.go b/provider/vsphere/config.go
@@ -87,7 +87,6 @@ func (c *environConfig) externalNetwork() string {
 
 func (c *environConfig) datastore() string {
 	ds, _ := c.attrs[cfgDatastore].(string)
-	logger.Criticalf("c.attrs %#v, datastore %q", c.attrs, ds)
 	return ds
 }
 

diff --git a/provider/vsphere/environ.go b/provider/vsphere/environ.go
@@ -200,7 +200,9 @@ func (env *sessionEnviron) Destroy(ctx callcontext.ProviderCallContext) error {
 		// further down the stack.
 		return errors.Trace(err)
 	}
-	err := env.client.DestroyVMFolder(env.ctx, path.Join(env.getVMFolder(), controllerFolderName("*"), env.modelFolderName()))
+	err := env.client.DestroyVMFolder(env.ctx,
+		path.Join(env.getVMFolder(), controllerFolderName("*"), env.modelFolderName()),
+	)
 	HandleCredentialError(err, env, ctx)
 	return err
 }
@@ -218,7 +220,9 @@ func (env *sessionEnviron) DestroyController(ctx callcontext.ProviderCallContext
 		return errors.Trace(err)
 	}
 	controllerFolderName := controllerFolderName(controllerUUID)
-	if err := env.client.RemoveVirtualMachines(env.ctx, path.Join(env.getVMFolder(), controllerFolderName, modelFolderName("*", "*"), "*")); err != nil {
+	if err := env.client.RemoveVirtualMachines(env.ctx,
+		path.Join(env.getVMFolder(), controllerFolderName, modelFolderName("*", "*"), "*"),
+	); err != nil {
 		HandleCredentialError(err, env, ctx)
 		return errors.Annotate(err, "removing VMs")
 	}
@@ -246,11 +250,7 @@ func (env *sessionEnviron) DestroyController(ctx callcontext.ProviderCallContext
 }
 
 func (env *sessionEnviron) getVMFolder() string {
-	attrs := env.environ.cloud.Credential.Attributes()
-	if attrs == nil {
-		return ""
-	}
-	return attrs[credAttrVMFolder]
+	return env.environ.cloud.Credential.Attributes()[credAttrVMFolder]
 }
 
 func (env *sessionEnviron) accessibleDatastores(ctx callcontext.ProviderCallContext) ([]*mo.Datastore, error) {

diff --git a/provider/vsphere/environ_broker.go b/provider/vsphere/environ_broker.go
@@ -199,7 +199,6 @@ func (env *sessionEnviron) newRawInstance(
 	if (cons.RootDiskSource == nil || *cons.RootDiskSource == "") && defaultDatastore != "" {
 		cons.RootDiskSource = &defaultDatastore
 	}
-	logger.Criticalf("newRawInstance defaultDatastore %q, cons.RootDiskSource %+v", defaultDatastore, cons.RootDiskSource)
 
 	// Download and extract the OVA file. If we're bootstrapping we use
 	// a temporary directory, otherwise we cache the image for future use.

diff --git a/provider/vsphere/internal/vsphereclient/client.go b/provider/vsphere/internal/vsphereclient/client.go
@@ -108,7 +108,6 @@ func (c *Client) FindFolder(ctx context.Context, folderPath string) (vmFolder *o
 		return nil, errors.Trace(err)
 	}
 	dcfolders, err := datacenter.Folders(ctx)
-	c.logger.Criticalf("FindFolder dcfolders %+v, folderPath %q, err %+v", dcfolders, folderPath, err)
 	if err != nil {
 		return nil, errors.Trace(err)
 	}
@@ -124,15 +123,13 @@ func (c *Client) FindFolder(ctx context.Context, folderPath string) (vmFolder *o
 	}
 
 	vmFolder, err = fi.Folder(ctx, folderPath)
-	c.logger.Criticalf("FindFolder vmFolder %+v, fullpath %q, err %+v", vmFolder, folderPath, err)
-	if err != nil {
-		if _, ok := err.(*find.NotFoundError); ok {
-			c.logger.Debugf("%q not found", folderPath)
-			return nil, errors.NotFoundf("folder path %q", folderPath)
-		}
-		return nil, errors.Trace(err)
+	if err == nil {
+		return vmFolder, nil
 	}
-	return vmFolder, nil
+	if _, ok := err.(*find.NotFoundError); ok {
+		return nil, errors.NotFoundf("folder path %q", folderPath)
+	}
+	return nil, errors.Trace(err)
 }
 
 func (c *Client) finder(ctx context.Context) (*find.Finder, *object.Datacenter, error) {
@@ -340,16 +337,15 @@ func (c *Client) EnsureVMFolder(ctx context.Context, credAttrFolder string, fold
 
 	// LP: #1849194
 	// User do not necessarily own permission to create credAttrFolder
-	// since that demands Add_folder permissions from the root-folder's DC
-	// Join paths for folders.VMFolder and credentials attribute
+	// since that demands Add_folder permissions from the root-folder's DC.
+	// Join paths folders.VMFolder and credentials attribute.
 	parentFolder, err := c.FindFolder(ctx, credAttrFolder)
-	c.logger.Criticalf("EnsureVMFolder FindFolder credAttrFolder %q, err %+v", credAttrFolder, err)
 	// Consider the case where folder from credential attribute comes empty:
-	// path.Join ignores empty entries
+	// path.Join ignores empty entries.
 	if err != nil {
 		return nil, errors.Trace(err)
 	}
-	// Creating "Juju Controller (...)" folder and then model folder, for example
+	// Creating "Juju Controller (...)" folder and then model folder, for example.
 	for _, name := range strings.Split(folderPath, "/") {
 		folder, err := createFolder(parentFolder, name)
 		if err != nil {
@@ -360,13 +356,13 @@ func (c *Client) EnsureVMFolder(ctx context.Context, credAttrFolder string, fold
 	return parentFolder, nil
 }
 
-// DestroyVMFolder destroys a folder rooted at the datacenter's base VM folder.
+// DestroyVMFolder destroys a folder(folderPath could be either relative path of vmfolder of datacenter or full path).
 func (c *Client) DestroyVMFolder(ctx context.Context, folderPath string) error {
 	folder, err := c.FindFolder(ctx, folderPath)
+	if errors.IsNotFound(err) {
+		return nil
+	}
 	if err != nil {
-		if _, ok := err.(*find.NotFoundError); ok {
-			return nil
-		}
 		return errors.Trace(err)
 	}
 
@@ -382,12 +378,12 @@ func (c *Client) DestroyVMFolder(ctx context.Context, folderPath string) error {
 }
 
 // MoveVMFolderInto moves one VM folder into another.
-func (c *Client) MoveVMFolderInto(ctx context.Context, from, to string) error {
-	parent, err := c.FindFolder(ctx, from)
+func (c *Client) MoveVMFolderInto(ctx context.Context, parentPath, childPath string) error {
+	parent, err := c.FindFolder(ctx, parentPath)
 	if err != nil {
 		return errors.Trace(err)
 	}
-	child, err := c.FindFolder(ctx, to)
+	child, err := c.FindFolder(ctx, childPath)
 	if err != nil {
 		return errors.Trace(err)
 	}

diff --git a/provider/vsphere/internal/vsphereclient/client_test.go b/provider/vsphere/internal/vsphereclient/client_test.go
@@ -237,15 +237,6 @@ func (s *clientSuite) SetUpTest(c *gc.C) {
 				{Name: "name", Val: "foo"},
 			},
 		}},
-		"FakeVMDKSFolder": {{
-			Obj: types.ManagedObjectReference{
-				Type:  "Folder",
-				Value: "FakeVmFolder",
-			},
-			PropSet: []types.DynamicProperty{
-				{Name: "name", Val: "juju-vmdks"},
-			},
-		}},
 		"FakeControllerVmFolder": {{
 			Obj: types.ManagedObjectReference{
 				Type:  "Folder",

diff --git a/provider/vsphere/internal/vsphereclient/createvm.go b/provider/vsphere/internal/vsphereclient/createvm.go
@@ -170,8 +170,7 @@ func (c *Client) ensureTemplateVM(
 	if err != nil {
 		return nil, errors.Trace(err)
 	}
-	// Finding the folder can be tricky since user may pass same
-	// value in multiple forms.
+
 	templateFolder, err := c.FindFolder(ctx, path.Join(args.Folder, vmTemplatePath(args)))
 	if err != nil && !errors.IsNotFound(err) {
 		return nil, errors.Trace(err)