Merge branch '2.6' into merge-2.6-20190921

jangocheng · Sep 20, 2019 · c555c64 · c555c64
2 parents 2f1e6cf + f190cd8
commit c555c64
Show file tree

Hide file tree

Showing 11 changed files with 238 additions and 38 deletions.
diff --git a/apiserver/common/storagecommon/blockdevices.go b/apiserver/common/storagecommon/blockdevices.go
@@ -4,6 +4,8 @@
 package storagecommon
 
 import (
+	"strings"
+
 	"github.com/juju/loggo"
 
 	"github.com/juju/juju/state"
@@ -16,17 +18,18 @@ var logger = loggo.GetLogger("juju.apiserver.storagecommon")
 // storage.BlockDevice.
 func BlockDeviceFromState(in state.BlockDeviceInfo) storage.BlockDevice {
 	return storage.BlockDevice{
-		in.DeviceName,
-		in.DeviceLinks,
-		in.Label,
-		in.UUID,
-		in.HardwareId,
-		in.WWN,
-		in.BusAddress,
-		in.Size,
-		in.FilesystemType,
-		in.InUse,
-		in.MountPoint,
+		DeviceName:     in.DeviceName,
+		DeviceLinks:    in.DeviceLinks,
+		Label:          in.Label,
+		UUID:           in.UUID,
+		HardwareId:     in.HardwareId,
+		WWN:            in.WWN,
+		BusAddress:     in.BusAddress,
+		Size:           in.Size,
+		FilesystemType: in.FilesystemType,
+		InUse:          in.InUse,
+		MountPoint:     in.MountPoint,
+		SerialId:       in.SerialId,
 	}
 }
 
@@ -76,6 +79,14 @@ func MatchingBlockDevice(
 			logger.Tracef("no match for block device hardware id: %v", dev.HardwareId)
 			continue
 		}
+		if volumeInfo.VolumeId != "" && dev.SerialId != "" {
+			if strings.HasPrefix(volumeInfo.VolumeId, dev.SerialId) {
+				logger.Tracef("serial id %v match on volume id %v", dev.SerialId, volumeInfo.VolumeId)
+				return &dev, true
+			}
+			logger.Tracef("no match for block device serial id: %v", dev.SerialId)
+			continue
+		}
 		if attachmentInfo.BusAddress != "" {
 			if attachmentInfo.BusAddress == dev.BusAddress {
 				logger.Tracef("bus address match on %v", attachmentInfo.BusAddress)

diff --git a/apiserver/common/storagecommon/blockdevices_test.go b/apiserver/common/storagecommon/blockdevices_test.go
@@ -0,0 +1,64 @@
+// Copyright 2019 Canonical Ltd.
+// Licensed under the AGPLv3, see LICENCE file for details.
+
+package storagecommon_test
+
+import (
+	jc "github.com/juju/testing/checkers"
+	gc "gopkg.in/check.v1"
+
+	"github.com/juju/juju/apiserver/common/storagecommon"
+	"github.com/juju/juju/state"
+)
+
+type BlockDeviceSuite struct {
+}
+
+var _ = gc.Suite(&BlockDeviceSuite{})
+
+func (s *BlockDeviceSuite) TestBlockDeviceMatchingSerialID(c *gc.C) {
+	blockDevices := []state.BlockDeviceInfo{
+		{
+			DeviceName: "sdb",
+			SerialId:   "543554ff-3b88-4",
+		},
+		{
+			DeviceName: "sdc",
+			WWN:        "wow",
+		},
+	}
+	volumeInfo := state.VolumeInfo{
+		VolumeId: "543554ff-3b88-43b9-83fc-4d69de28490b",
+	}
+	atachmentInfo := state.VolumeAttachmentInfo{}
+	planBlockInfo := state.BlockDeviceInfo{}
+	blockDeviceInfo, ok := storagecommon.MatchingBlockDevice(blockDevices, volumeInfo, atachmentInfo, planBlockInfo)
+	c.Assert(ok, jc.IsTrue)
+	c.Assert(blockDeviceInfo, jc.DeepEquals, &state.BlockDeviceInfo{
+		DeviceName: "sdb",
+		SerialId:   "543554ff-3b88-4",
+	})
+}
+
+func (s *BlockDeviceSuite) TestBlockDeviceMatchingHardwareID(c *gc.C) {
+	blockDevices := []state.BlockDeviceInfo{
+		{
+			DeviceName: "sdb",
+			HardwareId: "ide-543554ff-3b88-4",
+		},
+		{
+			DeviceName: "sdc",
+		},
+	}
+	volumeInfo := state.VolumeInfo{
+		HardwareId: "ide-543554ff-3b88-4",
+	}
+	atachmentInfo := state.VolumeAttachmentInfo{}
+	planBlockInfo := state.BlockDeviceInfo{}
+	blockDeviceInfo, ok := storagecommon.MatchingBlockDevice(blockDevices, volumeInfo, atachmentInfo, planBlockInfo)
+	c.Assert(ok, jc.IsTrue)
+	c.Assert(blockDeviceInfo, jc.DeepEquals, &state.BlockDeviceInfo{
+		DeviceName: "sdb",
+		HardwareId: "ide-543554ff-3b88-4",
+	})
+}
diff --git a/apiserver/facades/agent/diskmanager/diskmanager.go b/apiserver/facades/agent/diskmanager/diskmanager.go
@@ -90,17 +90,18 @@ func stateBlockDeviceInfo(devices []storage.BlockDevice) []state.BlockDeviceInfo
 	result := make([]state.BlockDeviceInfo, len(devices))
 	for i, dev := range devices {
 		result[i] = state.BlockDeviceInfo{
-			dev.DeviceName,
-			dev.DeviceLinks,
-			dev.Label,
-			dev.UUID,
-			dev.HardwareId,
-			dev.WWN,
-			dev.BusAddress,
-			dev.Size,
-			dev.FilesystemType,
-			dev.InUse,
-			dev.MountPoint,
+			DeviceName:     dev.DeviceName,
+			DeviceLinks:    dev.DeviceLinks,
+			Label:          dev.Label,
+			UUID:           dev.UUID,
+			HardwareId:     dev.HardwareId,
+			WWN:            dev.WWN,
+			BusAddress:     dev.BusAddress,
+			Size:           dev.Size,
+			FilesystemType: dev.FilesystemType,
+			InUse:          dev.InUse,
+			MountPoint:     dev.MountPoint,
+			SerialId:       dev.SerialId,
 		}
 	}
 	return result

diff --git a/apiserver/facades/schema.json b/apiserver/facades/schema.json
@@ -14784,6 +14784,9 @@
                         "MountPoint": {
                             "type": "string"
                         },
+                        "SerialId": {
+                            "type": "string"
+                        },
                         "Size": {
                             "type": "integer"
                         },
@@ -14806,7 +14809,8 @@
                         "Size",
                         "FilesystemType",
                         "InUse",
-                        "MountPoint"
+                        "MountPoint",
+                        "SerialId"
                     ]
                 },
                 "Error": {
@@ -31827,6 +31831,9 @@
                         "MountPoint": {
                             "type": "string"
                         },
+                        "SerialId": {
+                            "type": "string"
+                        },
                         "Size": {
                             "type": "integer"
                         },
@@ -31849,7 +31856,8 @@
                         "Size",
                         "FilesystemType",
                         "InUse",
-                        "MountPoint"
+                        "MountPoint",
+                        "SerialId"
                     ]
                 },
                 "BlockDeviceResult": {

diff --git a/state/blockdevices.go b/state/blockdevices.go
@@ -44,6 +44,7 @@ type BlockDeviceInfo struct {
 	FilesystemType string   `bson:"fstype,omitempty"`
 	InUse          bool     `bson:"inuse"`
 	MountPoint     string   `bson:"mountpoint,omitempty"`
+	SerialId       string   `bson:"serialid,omitempty"`
 }
 
 // WatchBlockDevices returns a new NotifyWatcher watching for

diff --git a/state/cleanup.go b/state/cleanup.go
@@ -529,9 +529,14 @@ func (st *State) removeApplicationsForDyingModel(args DestroyModelParams) (err e
 	//  it doesn't cause applications that are Dying to finish progressing to Dead.
 	application := Application{st: st}
 	sel := bson.D{{"life", Alive}}
+	force := args.Force != nil && *args.Force
+	if force {
+		// If we're forcing, propagate down to even dying
+		// applications, just in case they weren't originally forced.
+		sel = nil
+	}
 	iter := applications.Find(sel).Iter()
 	defer closeIter(iter, &err, "reading application document")
-	force := args.Force != nil && *args.Force
 	for iter.Next(&application.doc) {
 		op := application.DestroyOperation()
 		op.RemoveOffers = true

diff --git a/state/migration_internal_test.go b/state/migration_internal_test.go
@@ -629,6 +629,7 @@ func (s *MigrationSuite) TestBlockDeviceFields(c *gc.C) {
 		"FilesystemType",
 		"InUse",
 		"MountPoint",
+		"SerialId",
 	)
 	s.AssertExportedFields(c, BlockDeviceInfo{}, migrated)
 }

diff --git a/state/model_test.go b/state/model_test.go
@@ -13,6 +13,7 @@ import (
 	jc "github.com/juju/testing/checkers"
 	"github.com/juju/utils"
 	gc "gopkg.in/check.v1"
+	"gopkg.in/juju/charm.v6"
 	"gopkg.in/juju/names.v3"
 	"gopkg.in/mgo.v2/bson"
 
@@ -1567,6 +1568,89 @@ func (s *ModelSuite) TestSetEnvironVersionCannotDecrease(c *gc.C) {
 	c.Assert(m.EnvironVersion(), gc.Equals, 2)
 }
 
+func (s *ModelSuite) TestDestroyForceWorksWhenRemoteRelationScopesAreStuck(c *gc.C) {
+	mysqlEps := []charm.Relation{
+		{
+			Interface: "mysql",
+			Name:      "db",
+			Role:      charm.RoleProvider,
+			Scope:     charm.ScopeGlobal,
+		},
+	}
+	ms := s.Factory.MakeModel(c, nil)
+	defer ms.Close()
+	remoteApp, err := ms.AddRemoteApplication(state.AddRemoteApplicationParams{
+		Name:        "mysql",
+		SourceModel: s.Model.ModelTag(),
+		Token:       "t0",
+		Endpoints:   mysqlEps,
+	})
+	c.Assert(err, jc.ErrorIsNil)
+
+	wordpress := state.AddTestingApplication(c, ms, "wordpress", state.AddTestingCharm(c, ms, "wordpress"))
+	eps, err := ms.InferEndpoints("wordpress", "mysql")
+	c.Assert(err, jc.ErrorIsNil)
+	rel, err := ms.AddRelation(eps...)
+	c.Assert(err, jc.ErrorIsNil)
+
+	unit, err := wordpress.AddUnit(state.AddUnitParams{})
+	c.Assert(err, jc.ErrorIsNil)
+	f := factory.NewFactory(ms, s.StatePool)
+	machine := f.MakeMachine(c, nil)
+	err = unit.AssignToMachine(machine)
+	c.Assert(err, jc.ErrorIsNil)
+	localRelUnit, err := rel.Unit(unit)
+	c.Assert(err, jc.ErrorIsNil)
+	err = localRelUnit.EnterScope(nil)
+	c.Assert(err, jc.ErrorIsNil)
+
+	remoteRelUnit, err := rel.RemoteUnit("mysql/0")
+	c.Assert(err, jc.ErrorIsNil)
+	err = remoteRelUnit.EnterScope(nil)
+	c.Assert(err, jc.ErrorIsNil)
+
+	// Refetch the remoteapp to ensure that its relationcount is
+	// current. Otherwise it just silently fails? (See errRefresh
+	// handling in DestroyRemoteApplicationOperation.Build)
+	err = remoteApp.Refresh()
+	c.Assert(err, jc.ErrorIsNil)
+	err = remoteApp.Destroy()
+	c.Assert(err, jc.ErrorIsNil)
+	assertLife(c, remoteApp, state.Dying)
+
+	err = wordpress.Destroy()
+	c.Assert(err, jc.ErrorIsNil)
+
+	err = localRelUnit.LeaveScope()
+	c.Assert(err, jc.ErrorIsNil)
+	err = unit.EnsureDead()
+	c.Assert(err, jc.ErrorIsNil)
+	err = unit.Remove()
+	c.Assert(err, jc.ErrorIsNil)
+
+	// Cleanups
+	assertCleanupCount(c, ms, 1)
+
+	// wordpress is kept around because the relation can't be removed.
+	assertLife(c, wordpress, state.Dying)
+
+	// Force-destroying the model cleans them up.
+	model, err := ms.Model()
+	c.Assert(err, jc.ErrorIsNil)
+	force := true
+	err = model.Destroy(state.DestroyModelParams{
+		Force: &force,
+	})
+	c.Assert(err, jc.ErrorIsNil)
+
+	assertCleanupCount(c, ms, 4)
+	assertRemoved(c, wordpress)
+	c.Assert(model.Refresh(), jc.ErrorIsNil)
+	c.Assert(model.Life(), gc.Equals, state.Dying)
+	c.Assert(ms.ProcessDyingModel(), jc.ErrorIsNil)
+	c.Assert(ms.RemoveDyingModel(), jc.ErrorIsNil)
+}
+
 type ModelCloudValidationSuite struct {
 	gitjujutesting.MgoSuite
 }

diff --git a/storage/blockdevice.go b/storage/blockdevice.go
@@ -63,4 +63,7 @@ type BlockDevice struct {
 
 	// MountPoint is the path at which the block devices is mounted.
 	MountPoint string `yaml:"mountpoint,omitempty"`
+
+	// SerialId is the block devices serial id used for matching.
+	SerialId string `yaml:"serialid,omitempty"`
 }
diff --git a/worker/diskmanager/lsblk.go b/worker/diskmanager/lsblk.go
@@ -234,14 +234,17 @@ func addHardwareInfo(dev *storage.BlockDevice) error {
 		// and together they make up the symlink in /dev/disk/by-id.
 		dev.HardwareId = idBus + "-" + idSerial
 	}
+	if idSerial != "" {
+		dev.SerialId = idSerial
+	}
 
 	// For devices on the SCSI bus, we include the address. This is to
 	// support storage providers where the SCSI address may be specified,
 	// but the device name can not (and may change, depending on timing).
 	if idBus == "scsi" && devpath != "" {
 		// DEVPATH will be "<uninteresting stuff>/<SCSI address>/block/<device>".
 		re := regexp.MustCompile(fmt.Sprintf(
-			`^.*/(\d+):(\d+):(\d+):(\d+)/block/%s$`,
+			`^.*/(\d+):(\d+):(\d+):(\d+)/block/(?:\w+/|)%s$`,
 			regexp.QuoteMeta(dev.DeviceName),
 		))
 		submatch := re.FindStringSubmatch(devpath)