Merge remote-tracking branch 'upstream/2.7' into 2.8-merge-2.7

orbital-transfer-survey · Jan 26, 2020 · c04b861 · c04b861
2 parents 41f1c56 + ba9f119
commit c04b861
Show file tree

Hide file tree

Showing 32 changed files with 426 additions and 369 deletions.
diff --git a/acceptancetests/assess_destroy_model.py b/acceptancetests/assess_destroy_model.py
@@ -68,7 +68,7 @@ def destroy_model(client, new_client):
             if not_found_error in e.stderr:
                 log.info("Model fully removed")
                 break
-            removed_error = b'model "{}" has been removed from the controller'.format(old_model)
+            removed_error = b'model "admin/{}" has been removed from the controller'.format(old_model)
             if removed_error not in e.stderr:
                 error = 'unexpected error calling status\n{}'.format(e.stderr)
                 raise JujuAssertionError(error)

diff --git a/cloudconfig/userdatacfg_unix.go b/cloudconfig/userdatacfg_unix.go
@@ -361,7 +361,7 @@ func (w *unixConfigure) ConfigureJuju() error {
 
 	// We add the machine agent's configuration info
 	// before running bootstrap-state so that bootstrap-state
-	// has a chance to rerwrite it to change the password.
+	// has a chance to rewrite it to change the password.
 	// It would be cleaner to change bootstrap-state to
 	// be responsible for starting the machine agent itself,
 	// but this would not be backwardly compatible.

diff --git a/provider/vsphere/environ_policy.go b/provider/vsphere/environ_policy.go
@@ -33,8 +33,7 @@ func (env *sessionEnviron) PrecheckInstance(ctx context.ProviderCallContext, arg
 	if err := env.checkDatastore(ctx, args.Constraints.RootDiskSource); err != nil {
 		return errors.Trace(err)
 	}
-	err := env.checkExtendPermissions(ctx, args.Constraints.RootDisk)
-	return errors.Trace(err)
+	return nil
 }
 
 func (env *sessionEnviron) checkZones(ctx context.ProviderCallContext, zones *[]string) error {
@@ -74,25 +73,6 @@ func (env *sessionEnviron) checkDatastore(ctx context.ProviderCallContext, datas
 	return errors.NotFoundf("datastore %q", name)
 }
 
-func (env *sessionEnviron) checkExtendPermissions(ctx context.ProviderCallContext, rootDisk *uint64) error {
-	if rootDisk == nil || *rootDisk == 0 {
-		return nil
-	}
-	// If we're going to need to resize the root disk we need to have
-	// the System.Read privilege on the root level folder - this seems
-	// to be because the extend disk task doesn't get created with a
-	// target, so the root-level permissions are applied.
-	ok, err := env.client.UserHasRootLevelPrivilege(env.ctx, "System.Read")
-	if err != nil {
-		return errors.Trace(err)
-	}
-	if !ok {
-		user := env.cloud.Credential.Attributes()[credAttrUser]
-		return errors.Errorf("the System.Read privilege is required at the root level to extend disks - please grant the ReadOnly role to %q", user)
-	}
-	return nil
-}
-
 var unsupportedConstraints = []string{
 	constraints.Tags,
 	constraints.VirtType,

diff --git a/provider/vsphere/environ_policy_test.go b/provider/vsphere/environ_policy_test.go
@@ -4,12 +4,10 @@
 package vsphere_test
 
 import (
-	"github.com/juju/errors"
 	jc "github.com/juju/testing/checkers"
 	"github.com/vmware/govmomi/object"
 	"github.com/vmware/govmomi/vim25/mo"
 	"github.com/vmware/govmomi/vim25/types"
-	"golang.org/x/net/context"
 	gc "gopkg.in/check.v1"
 
 	"github.com/juju/juju/core/constraints"
@@ -138,24 +136,3 @@ func (s *environPolSuite) TestPrecheckInstanceChecksConstraintDatastore(c *gc.C)
 	})
 	c.Assert(err, jc.ErrorIsNil)
 }
-
-func (s *environPolSuite) TestPrecheckInstanceChecksForPrivIfRootDiskSet(c *gc.C) {
-	err := s.env.PrecheckInstance(s.callCtx, environs.PrecheckInstanceParams{
-		Constraints: constraints.MustParse("root-disk=30G"),
-	})
-	c.Assert(err, gc.ErrorMatches, `the System.Read privilege is required at the root level to extend disks - please grant the ReadOnly role to "user1"`)
-
-	s.client.SetErrors(errors.New("oops"))
-	err = s.env.PrecheckInstance(s.callCtx, environs.PrecheckInstanceParams{
-		Constraints: constraints.MustParse("root-disk=30G"),
-	})
-	c.Assert(err, gc.ErrorMatches, "oops")
-
-	s.client.hasPrivilege = true
-	err = s.env.PrecheckInstance(s.callCtx, environs.PrecheckInstanceParams{
-		Constraints: constraints.MustParse("root-disk=30G"),
-	})
-	c.Assert(err, jc.ErrorIsNil)
-
-	s.client.CheckCall(c, 0, "UserHasRootLevelPrivilege", context.Background(), "System.Read")
-}
diff --git a/provider/vsphere/internal/vsphereclient/client.go b/provider/vsphere/internal/vsphereclient/client.go
@@ -9,10 +9,12 @@ import (
 	"path"
 	"strings"
 
+	"github.com/dustin/go-humanize"
 	"github.com/juju/clock"
 	"github.com/juju/errors"
 	"github.com/juju/loggo"
 	"github.com/juju/mutex"
+	"github.com/kr/pretty"
 	"github.com/vmware/govmomi"
 	"github.com/vmware/govmomi/find"
 	"github.com/vmware/govmomi/list"
@@ -581,56 +583,46 @@ func customiseVAppConfig(
 	}, nil
 }
 
-func (c *Client) getDiskWithFileBacking(
+func (c *Client) getDisk(
 	ctx context.Context,
 	vm *object.VirtualMachine,
-) (*types.VirtualDisk, types.BaseVirtualDeviceFileBackingInfo, error) {
+) (*types.VirtualDisk, error) {
 	var mo mo.VirtualMachine
 	if err := c.client.RetrieveOne(ctx, vm.Reference(), []string{"config.hardware"}, &mo); err != nil {
-		return nil, nil, errors.Trace(err)
+		return nil, errors.Trace(err)
 	}
 	for _, dev := range mo.Config.Hardware.Device {
-		dev, ok := dev.(*types.VirtualDisk)
-		if !ok {
-			continue
+		if dev, ok := dev.(*types.VirtualDisk); ok {
+			return dev, nil
 		}
-		backing, ok := dev.Backing.(types.BaseVirtualDeviceFileBackingInfo)
-		if !ok {
-			continue
-		}
-		return dev, backing, nil
 	}
-	return nil, nil, errors.NotFoundf("disk")
+	return nil, errors.NotFoundf("disk")
 }
 
 func (c *Client) extendDisk(
 	ctx context.Context,
 	vm *object.VirtualMachine,
-	datacenter *object.Datacenter,
-	datastorePath string,
-	capacityKB int64,
-	taskWaiter *taskWaiter,
+	disk *types.VirtualDisk,
+	desiredCapacityKB int64,
 ) error {
-	// NOTE(axw) there's no ExtendVirtualDisk on the disk manager type,
-	// hence why we're dealing with request types directly. Send a patch
-	// to govmomi to add this to VirtualDiskManager.
+	prettySize := func(kb int64) string { return humanize.IBytes(uint64(kb) * 1024) }
+	c.logger.Debugf("extending disk from %q to %q", prettySize(disk.CapacityInKB), prettySize(desiredCapacityKB))
 
-	diskManager := object.NewVirtualDiskManager(c.client.Client)
-	dcref := datacenter.Reference()
-	req := types.ExtendVirtualDisk_Task{
-		This:          diskManager.Reference(),
-		Name:          datastorePath,
-		Datacenter:    &dcref,
-		NewCapacityKb: capacityKB,
-	}
+	// Resize the disk to desired size.
+	disk.CapacityInKB = desiredCapacityKB
 
-	res, err := methods.ExtendVirtualDisk_Task(ctx, c.client.Client, &req)
+	spec := types.VirtualMachineConfigSpec{}
+	spec.DeviceChange = append(spec.DeviceChange, &types.VirtualDeviceConfigSpec{
+		Device:        disk,
+		Operation:     types.VirtualDeviceConfigSpecOperationEdit,
+		FileOperation: "",
+	})
+	c.logger.Tracef("extending disk, config change -> %s", pretty.Sprint(spec))
+	task, err := vm.Reconfigure(ctx, spec)
 	if err != nil {
-		return errors.Trace(err)
+		return errors.Trace(&extendDiskError{err})
 	}
-	task := object.NewTask(c.client.Client, res.Returnval)
-	_, err = taskWaiter.waitTask(ctx, task, "extending disk")
-	if err != nil {
+	if err := task.Wait(ctx); err != nil {
 		return errors.Trace(&extendDiskError{err})
 	}
 	return nil

diff --git a/provider/vsphere/internal/vsphereclient/createvm.go b/provider/vsphere/internal/vsphereclient/createvm.go
@@ -398,7 +398,7 @@ func (c *Client) extendVMRootDisk(
 	sizeMB uint64,
 	taskWaiter *taskWaiter,
 ) error {
-	disk, backing, err := c.getDiskWithFileBacking(ctx, vm)
+	disk, err := c.getDisk(ctx, vm)
 	if err != nil {
 		return errors.Trace(err)
 	}
@@ -408,10 +408,7 @@ func (c *Client) extendVMRootDisk(
 		// user-specified size, so leave it alone.
 		return nil
 	}
-	datastorePath := backing.GetVirtualDeviceFileBackingInfo().FileName
-	return errors.Trace(c.extendDisk(
-		ctx, vm, datacenter, datastorePath, newCapacityInKB, taskWaiter,
-	))
+	return errors.Trace(c.extendDisk(ctx, vm, disk, newCapacityInKB))
 }
 
 func (c *Client) createImportSpec(

diff --git a/provider/vsphere/internal/vsphereclient/createvm_test.go b/provider/vsphere/internal/vsphereclient/createvm_test.go
@@ -197,8 +197,6 @@ func (s *clientSuite) TestCreateVirtualMachineDatastoreSpecified(c *gc.C) {
 	_, err := client.CreateVirtualMachine(context.Background(), args)
 	c.Assert(err, jc.ErrorIsNil)
 
-	//findStubCall(c, s.roundTripper.Calls(), "?")
-
 	cisp := baseCisp()
 	cisp.EntityName = vmTemplateName(args)
 	s.roundTripper.CheckCall(
@@ -455,10 +453,23 @@ func (s *clientSuite) TestCreateVirtualMachineRootDiskSize(c *gc.C) {
 	_, err := client.CreateVirtualMachine(context.Background(), args)
 	c.Assert(err, jc.ErrorIsNil)
 
-	call := findStubCall(c, s.roundTripper.Calls(), "ExtendVirtualDisk")
-	c.Assert(call.Args, jc.DeepEquals, []interface{}{
-		"disk.vmdk",
-		int64(rootDisk) * 1024, // in KiB
+	s.roundTripper.CheckCall(c, 36, "ReconfigVM_Task", types.VirtualMachineConfigSpec{
+		DeviceChange: []types.BaseVirtualDeviceConfigSpec{
+			&types.VirtualDeviceConfigSpec{
+				Operation:     types.VirtualDeviceConfigSpecOperationEdit,
+				FileOperation: "",
+				Device: &types.VirtualDisk{
+					VirtualDevice: types.VirtualDevice{
+						Backing: &types.VirtualDiskFlatVer2BackingInfo{
+							VirtualDeviceFileBackingInfo: types.VirtualDeviceFileBackingInfo{
+								FileName: "disk.vmdk",
+							},
+						},
+					},
+					CapacityInKB: 1024 * 1024 * 20, // 20 GiB
+				},
+			},
+		},
 	})
 }
 

diff --git a/provider/vsphere/internal/vsphereclient/mock_test.go b/provider/vsphere/internal/vsphereclient/mock_test.go
@@ -99,7 +99,8 @@ func (r *mockRoundTripper) RoundTrip(ctx context.Context, req, res soap.HasFault
 		r.MethodCall(r, "Logout")
 		res.Res = &types.LogoutResponse{}
 	case *methods.ReconfigVM_TaskBody:
-		r.MethodCall(r, "ReconfigVM_Task")
+		req := req.(*methods.ReconfigVM_TaskBody).Req
+		r.MethodCall(r, "ReconfigVM_Task", req.Spec)
 		res.Res = &types.ReconfigVM_TaskResponse{reconfigVMTask}
 	case *methods.Destroy_TaskBody:
 		r.MethodCall(r, "Destroy_Task")