[ARVADOS] created: 1.3.0-970-g924fb5067

Git user git at public.curoverse.com
Thu May 30 15:31:35 UTC 2019


        at  924fb506752776e59528685e44ece83192301f73 (commit)


commit 924fb506752776e59528685e44ece83192301f73
Author: Tom Clegg <tclegg at veritasgenetics.com>
Date:   Thu May 30 10:49:57 2019 -0400

    14931: Pass custom tags to cloud driver for shared resources.
    
    Neither driver uses them yet: the only shared resources being created
    are AWS key pairs, which don't support tags.
    
    https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/Using_Tags.html
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tclegg at veritasgenetics.com>

diff --git a/lib/cloud/azure/azure.go b/lib/cloud/azure/azure.go
index dce8b61b7..ab14d6681 100644
--- a/lib/cloud/azure/azure.go
+++ b/lib/cloud/azure/azure.go
@@ -211,7 +211,7 @@ type azureInstanceSet struct {
 	logger       logrus.FieldLogger
 }
 
-func newAzureInstanceSet(config json.RawMessage, dispatcherID cloud.InstanceSetID, logger logrus.FieldLogger) (prv cloud.InstanceSet, err error) {
+func newAzureInstanceSet(config json.RawMessage, dispatcherID cloud.InstanceSetID, _ cloud.SharedResourceTags, logger logrus.FieldLogger) (prv cloud.InstanceSet, err error) {
 	azcfg := azureInstanceSetConfig{}
 	err = json.Unmarshal(config, &azcfg)
 	if err != nil {
diff --git a/lib/cloud/azure/azure_test.go b/lib/cloud/azure/azure_test.go
index 8cedca295..152b7e73b 100644
--- a/lib/cloud/azure/azure_test.go
+++ b/lib/cloud/azure/azure_test.go
@@ -144,7 +144,7 @@ func GetInstanceSet() (cloud.InstanceSet, cloud.ImageID, arvados.Cluster, error)
 			return nil, cloud.ImageID(""), cluster, err
 		}
 
-		ap, err := newAzureInstanceSet(exampleCfg.DriverParameters, "test123", logrus.StandardLogger())
+		ap, err := newAzureInstanceSet(exampleCfg.DriverParameters, "test123", nil, logrus.StandardLogger())
 		return ap, cloud.ImageID(exampleCfg.ImageIDForTestSuite), cluster, err
 	}
 	ap := azureInstanceSet{
diff --git a/lib/cloud/ec2/ec2.go b/lib/cloud/ec2/ec2.go
index c630e9541..079c32802 100644
--- a/lib/cloud/ec2/ec2.go
+++ b/lib/cloud/ec2/ec2.go
@@ -56,7 +56,7 @@ type ec2InstanceSet struct {
 	keys          map[string]string
 }
 
-func newEC2InstanceSet(config json.RawMessage, instanceSetID cloud.InstanceSetID, logger logrus.FieldLogger) (prv cloud.InstanceSet, err error) {
+func newEC2InstanceSet(config json.RawMessage, instanceSetID cloud.InstanceSetID, _ cloud.SharedResourceTags, logger logrus.FieldLogger) (prv cloud.InstanceSet, err error) {
 	instanceSet := &ec2InstanceSet{
 		instanceSetID: instanceSetID,
 		logger:        logger,
diff --git a/lib/cloud/ec2/ec2_test.go b/lib/cloud/ec2/ec2_test.go
index 62a0b8564..8b754eaca 100644
--- a/lib/cloud/ec2/ec2_test.go
+++ b/lib/cloud/ec2/ec2_test.go
@@ -121,7 +121,7 @@ func GetInstanceSet() (cloud.InstanceSet, cloud.ImageID, arvados.Cluster, error)
 			return nil, cloud.ImageID(""), cluster, err
 		}
 
-		ap, err := newEC2InstanceSet(exampleCfg.DriverParameters, "test123", logrus.StandardLogger())
+		ap, err := newEC2InstanceSet(exampleCfg.DriverParameters, "test123", nil, logrus.StandardLogger())
 		return ap, cloud.ImageID(exampleCfg.ImageIDForTestSuite), cluster, err
 	}
 	ap := ec2InstanceSet{
diff --git a/lib/cloud/interfaces.go b/lib/cloud/interfaces.go
index 804de667e..7410f9d0e 100644
--- a/lib/cloud/interfaces.go
+++ b/lib/cloud/interfaces.go
@@ -36,6 +36,7 @@ type QuotaError interface {
 	error
 }
 
+type SharedResourceTags map[string]string
 type InstanceSetID string
 type InstanceTags map[string]string
 type InstanceID string
@@ -145,6 +146,10 @@ type InitCommand string
 // A Driver returns an InstanceSet that uses the given InstanceSetID
 // and driver-dependent configuration parameters.
 //
+// If the driver creates cloud resources that aren't attached to a
+// single VM instance (like SSH key pairs on AWS) and support tagging,
+// they should be tagged with the provided SharedResourceTags.
+//
 // The supplied id will be of the form "zzzzz-zzzzz-zzzzzzzzzzzzzzz"
 // where each z can be any alphanum. The returned InstanceSet must use
 // this id to tag long-lived cloud resources that it creates, and must
@@ -175,7 +180,7 @@ type InitCommand string
 //
 //	type exampleDriver struct {}
 //
-//	func (*exampleDriver) InstanceSet(config json.RawMessage, id InstanceSetID) (InstanceSet, error) {
+//	func (*exampleDriver) InstanceSet(config json.RawMessage, id cloud.InstanceSetID, tags cloud.SharedResourceTags, logger logrus.FieldLogger) (cloud.InstanceSet, error) {
 //		var is exampleInstanceSet
 //		if err := json.Unmarshal(config, &is); err != nil {
 //			return nil, err
@@ -186,17 +191,17 @@ type InitCommand string
 //
 //	var _ = registerCloudDriver("example", &exampleDriver{})
 type Driver interface {
-	InstanceSet(config json.RawMessage, id InstanceSetID, logger logrus.FieldLogger) (InstanceSet, error)
+	InstanceSet(config json.RawMessage, id InstanceSetID, tags SharedResourceTags, logger logrus.FieldLogger) (InstanceSet, error)
 }
 
 // DriverFunc makes a Driver using the provided function as its
 // InstanceSet method. This is similar to http.HandlerFunc.
-func DriverFunc(fn func(config json.RawMessage, id InstanceSetID, logger logrus.FieldLogger) (InstanceSet, error)) Driver {
+func DriverFunc(fn func(config json.RawMessage, id InstanceSetID, tags SharedResourceTags, logger logrus.FieldLogger) (InstanceSet, error)) Driver {
 	return driverFunc(fn)
 }
 
-type driverFunc func(config json.RawMessage, id InstanceSetID, logger logrus.FieldLogger) (InstanceSet, error)
+type driverFunc func(config json.RawMessage, id InstanceSetID, tags SharedResourceTags, logger logrus.FieldLogger) (InstanceSet, error)
 
-func (df driverFunc) InstanceSet(config json.RawMessage, id InstanceSetID, logger logrus.FieldLogger) (InstanceSet, error) {
-	return df(config, id, logger)
+func (df driverFunc) InstanceSet(config json.RawMessage, id InstanceSetID, tags SharedResourceTags, logger logrus.FieldLogger) (InstanceSet, error) {
+	return df(config, id, tags, logger)
 }
diff --git a/lib/dispatchcloud/driver.go b/lib/dispatchcloud/driver.go
index 36b8e8008..b67b5d054 100644
--- a/lib/dispatchcloud/driver.go
+++ b/lib/dispatchcloud/driver.go
@@ -26,7 +26,8 @@ func newInstanceSet(cluster *arvados.Cluster, setID cloud.InstanceSetID, logger
 	if !ok {
 		return nil, fmt.Errorf("unsupported cloud driver %q", cluster.Containers.CloudVMs.Driver)
 	}
-	is, err := driver.InstanceSet(cluster.Containers.CloudVMs.DriverParameters, setID, logger)
+	sharedResourceTags := cloud.SharedResourceTags(cluster.Containers.CloudVMs.ResourceTags)
+	is, err := driver.InstanceSet(cluster.Containers.CloudVMs.DriverParameters, setID, sharedResourceTags, logger)
 	if maxops := cluster.Containers.CloudVMs.MaxCloudOpsPerSecond; maxops > 0 {
 		is = rateLimitedInstanceSet{
 			InstanceSet: is,
diff --git a/lib/dispatchcloud/test/stub_driver.go b/lib/dispatchcloud/test/stub_driver.go
index 873d98732..a9a5a429f 100644
--- a/lib/dispatchcloud/test/stub_driver.go
+++ b/lib/dispatchcloud/test/stub_driver.go
@@ -56,7 +56,7 @@ type StubDriver struct {
 }
 
 // InstanceSet returns a new *StubInstanceSet.
-func (sd *StubDriver) InstanceSet(params json.RawMessage, id cloud.InstanceSetID, logger logrus.FieldLogger) (cloud.InstanceSet, error) {
+func (sd *StubDriver) InstanceSet(params json.RawMessage, id cloud.InstanceSetID, _ cloud.SharedResourceTags, logger logrus.FieldLogger) (cloud.InstanceSet, error) {
 	if sd.holdCloudOps == nil {
 		sd.holdCloudOps = make(chan bool)
 	}
diff --git a/lib/dispatchcloud/worker/pool_test.go b/lib/dispatchcloud/worker/pool_test.go
index 300b4730f..4b87ce503 100644
--- a/lib/dispatchcloud/worker/pool_test.go
+++ b/lib/dispatchcloud/worker/pool_test.go
@@ -66,7 +66,7 @@ func (suite *PoolSuite) TestResumeAfterRestart(c *check.C) {
 	logger := ctxlog.TestLogger(c)
 	driver := &test.StubDriver{}
 	instanceSetID := cloud.InstanceSetID("test-instance-set-id")
-	is, err := driver.InstanceSet(nil, instanceSetID, logger)
+	is, err := driver.InstanceSet(nil, instanceSetID, nil, logger)
 	c.Assert(err, check.IsNil)
 
 	newExecutor := func(cloud.Instance) Executor {
@@ -147,7 +147,7 @@ func (suite *PoolSuite) TestResumeAfterRestart(c *check.C) {
 func (suite *PoolSuite) TestCreateUnallocShutdown(c *check.C) {
 	logger := ctxlog.TestLogger(c)
 	driver := test.StubDriver{HoldCloudOps: true}
-	instanceSet, err := driver.InstanceSet(nil, "", logger)
+	instanceSet, err := driver.InstanceSet(nil, "test-instance-set-id", nil, logger)
 	c.Assert(err, check.IsNil)
 
 	type1 := arvados.InstanceType{Name: "a1s", ProviderType: "a1.small", VCPUs: 1, RAM: 1 * GiB, Price: .01}
diff --git a/lib/dispatchcloud/worker/worker_test.go b/lib/dispatchcloud/worker/worker_test.go
index 15a2a894c..4f9ba911c 100644
--- a/lib/dispatchcloud/worker/worker_test.go
+++ b/lib/dispatchcloud/worker/worker_test.go
@@ -25,7 +25,7 @@ func (suite *WorkerSuite) TestProbeAndUpdate(c *check.C) {
 	bootTimeout := time.Minute
 	probeTimeout := time.Second
 
-	is, err := (&test.StubDriver{}).InstanceSet(nil, "", logger)
+	is, err := (&test.StubDriver{}).InstanceSet(nil, "test-instance-set-id", nil, logger)
 	c.Assert(err, check.IsNil)
 	inst, err := is.Create(arvados.InstanceType{}, "", nil, "echo InitCommand", nil)
 	c.Assert(err, check.IsNil)

commit 88ed1932d79b4dfc2acedeea16ccef435aaa9fa7
Author: Tom Clegg <tclegg at veritasgenetics.com>
Date:   Thu May 30 10:13:07 2019 -0400

    14931: Add configurable prefix for built-in tags.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tclegg at veritasgenetics.com>

diff --git a/lib/config/config.default.yml b/lib/config/config.default.yml
index b1ed91d4b..5a482e2d2 100644
--- a/lib/config/config.default.yml
+++ b/lib/config/config.default.yml
@@ -537,6 +537,17 @@ Clusters:
         ResourceTags:
           SAMPLE: "tag value"
 
+        # Prefix for predefined tags used by Arvados (InstanceSetID,
+        # InstanceType, InstanceSecret, IdleBehavior). For example,
+        # set to "arvados-" to use tag key "arvados-InstanceSecret"
+        # instead of "InstanceSecret".
+        #
+        # This should only be changed while no cloud resources are in
+        # use and the cloud dispatcher is not running. Otherwise,
+        # VMs/resources that were added using the old tag prefix will
+        # need to be detected and cleaned up manually.
+        TagKeyPrefix: ""
+
         # Cloud driver: "azure" (Microsoft Azure) or "ec2" (Amazon AWS).
         Driver: ec2
 
diff --git a/lib/config/generated_config.go b/lib/config/generated_config.go
index 513eee362..e34203dc8 100644
--- a/lib/config/generated_config.go
+++ b/lib/config/generated_config.go
@@ -543,6 +543,17 @@ Clusters:
         ResourceTags:
           SAMPLE: "tag value"
 
+        # Prefix for predefined tags used by Arvados (InstanceSetID,
+        # InstanceType, InstanceSecret, IdleBehavior). For example,
+        # set to "arvados-" to use tag key "arvados-InstanceSecret"
+        # instead of "InstanceSecret".
+        #
+        # This should only be changed while no cloud resources are in
+        # use and the cloud dispatcher is not running. Otherwise,
+        # VMs/resources that were added using the old tag prefix will
+        # need to be detected and cleaned up manually.
+        TagKeyPrefix: ""
+
         # Cloud driver: "azure" (Microsoft Azure) or "ec2" (Amazon AWS).
         Driver: ec2
 
diff --git a/lib/dispatchcloud/dispatcher_test.go b/lib/dispatchcloud/dispatcher_test.go
index 11786bd3c..012621f12 100644
--- a/lib/dispatchcloud/dispatcher_test.go
+++ b/lib/dispatchcloud/dispatcher_test.go
@@ -66,6 +66,7 @@ func (s *DispatcherSuite) SetUpTest(c *check.C) {
 				TimeoutSignal:        arvados.Duration(3 * time.Millisecond),
 				TimeoutTERM:          arvados.Duration(20 * time.Millisecond),
 				ResourceTags:         map[string]string{"testtag": "test value"},
+				TagKeyPrefix:         "test:",
 			},
 		},
 		InstanceTypes: arvados.InstanceTypeMap{
diff --git a/lib/dispatchcloud/worker/pool.go b/lib/dispatchcloud/worker/pool.go
index 8af103712..0ee36a96f 100644
--- a/lib/dispatchcloud/worker/pool.go
+++ b/lib/dispatchcloud/worker/pool.go
@@ -112,6 +112,7 @@ func NewPool(logger logrus.FieldLogger, arvClient *arvados.Client, reg *promethe
 		timeoutTERM:        duration(cluster.Containers.CloudVMs.TimeoutTERM, defaultTimeoutTERM),
 		timeoutSignal:      duration(cluster.Containers.CloudVMs.TimeoutSignal, defaultTimeoutSignal),
 		installPublicKey:   installPublicKey,
+		tagKeyPrefix:       cluster.Containers.CloudVMs.TagKeyPrefix,
 		stop:               make(chan bool),
 	}
 	wp.registerMetrics(reg)
@@ -146,6 +147,7 @@ type Pool struct {
 	timeoutTERM        time.Duration
 	timeoutSignal      time.Duration
 	installPublicKey   ssh.PublicKey
+	tagKeyPrefix       string
 
 	// private state
 	subscribers  map[<-chan struct{}]chan<- struct{}
@@ -284,10 +286,10 @@ func (wp *Pool) Create(it arvados.InstanceType) bool {
 	go func() {
 		defer wp.notify()
 		tags := cloud.InstanceTags{
-			wp.tagPrefix + tagKeyInstanceSetID:  string(wp.instanceSetID),
-			wp.tagPrefix + tagKeyInstanceType:   it.Name,
-			wp.tagPrefix + tagKeyIdleBehavior:   string(IdleBehaviorRun),
-			wp.tagPrefix + tagKeyInstanceSecret: secret,
+			wp.tagKeyPrefix + tagKeyInstanceSetID:  string(wp.instanceSetID),
+			wp.tagKeyPrefix + tagKeyInstanceType:   it.Name,
+			wp.tagKeyPrefix + tagKeyIdleBehavior:   string(IdleBehaviorRun),
+			wp.tagKeyPrefix + tagKeyInstanceSecret: secret,
 		}
 		initCmd := cloud.InitCommand(fmt.Sprintf("umask 0177 && echo -n %q >%s", secret, instanceSecretFilename))
 		inst, err := wp.instanceSet.Create(it, wp.imageID, tags, initCmd, wp.installPublicKey)
@@ -342,7 +344,8 @@ func (wp *Pool) SetIdleBehavior(id cloud.InstanceID, idleBehavior IdleBehavior)
 //
 // Caller must have lock.
 func (wp *Pool) updateWorker(inst cloud.Instance, it arvados.InstanceType) (*worker, bool) {
-	inst = tagVerifier{inst}
+	secret := inst.Tags()[wp.tagKeyPrefix+tagKeyInstanceSecret]
+	inst = tagVerifier{inst, secret}
 	id := inst.ID()
 	if wkr := wp.workers[id]; wkr != nil {
 		wkr.executor.SetTarget(inst)
@@ -353,7 +356,7 @@ func (wp *Pool) updateWorker(inst cloud.Instance, it arvados.InstanceType) (*wor
 	}
 
 	state := StateUnknown
-	if _, ok := wp.creating[inst.Tags()[tagKeyInstanceSecret]]; ok {
+	if _, ok := wp.creating[secret]; ok {
 		state = StateBooting
 	}
 
@@ -363,7 +366,7 @@ func (wp *Pool) updateWorker(inst cloud.Instance, it arvados.InstanceType) (*wor
 	// process); otherwise, default to "run". After this,
 	// wkr.idleBehavior is the source of truth, and will only be
 	// changed via SetIdleBehavior().
-	idleBehavior := IdleBehavior(inst.Tags()[tagKeyIdleBehavior])
+	idleBehavior := IdleBehavior(inst.Tags()[wp.tagKeyPrefix+tagKeyIdleBehavior])
 	if !validIdleBehavior[idleBehavior] {
 		idleBehavior = IdleBehaviorRun
 	}
@@ -732,7 +735,7 @@ func (wp *Pool) getInstancesAndSync() error {
 	}
 	wp.logger.Debug("getting instance list")
 	threshold := time.Now()
-	instances, err := wp.instanceSet.Instances(cloud.InstanceTags{tagKeyInstanceSetID: string(wp.instanceSetID)})
+	instances, err := wp.instanceSet.Instances(cloud.InstanceTags{wp.tagKeyPrefix + tagKeyInstanceSetID: string(wp.instanceSetID)})
 	if err != nil {
 		wp.instanceSet.throttleInstances.CheckRateLimitError(err, wp.logger, "list instances", wp.notify)
 		return err
@@ -752,7 +755,7 @@ func (wp *Pool) sync(threshold time.Time, instances []cloud.Instance) {
 	notify := false
 
 	for _, inst := range instances {
-		itTag := inst.Tags()[tagKeyInstanceType]
+		itTag := inst.Tags()[wp.tagKeyPrefix+tagKeyInstanceType]
 		it, ok := wp.instanceTypes[itTag]
 		if !ok {
 			wp.logger.WithField("Instance", inst).Errorf("unknown InstanceType tag %q --- ignoring", itTag)
diff --git a/lib/dispatchcloud/worker/pool_test.go b/lib/dispatchcloud/worker/pool_test.go
index 8ab4c9875..300b4730f 100644
--- a/lib/dispatchcloud/worker/pool_test.go
+++ b/lib/dispatchcloud/worker/pool_test.go
@@ -83,6 +83,7 @@ func (suite *PoolSuite) TestResumeAfterRestart(c *check.C) {
 				MaxProbesPerSecond: 1000,
 				ProbeInterval:      arvados.Duration(time.Millisecond * 10),
 				SyncInterval:       arvados.Duration(time.Millisecond * 10),
+				TagKeyPrefix:       "testprefix:",
 			},
 		},
 		InstanceTypes: arvados.InstanceTypeMap{
@@ -107,13 +108,14 @@ func (suite *PoolSuite) TestResumeAfterRestart(c *check.C) {
 		}
 	}
 	// Wait for the tags to save to the cloud provider
+	tagKey := cluster.Containers.CloudVMs.TagKeyPrefix + tagKeyIdleBehavior
 	deadline := time.Now().Add(time.Second)
 	for !func() bool {
 		pool.mtx.RLock()
 		defer pool.mtx.RUnlock()
 		for _, wkr := range pool.workers {
 			if wkr.instType == type2 {
-				return wkr.instance.Tags()[tagKeyIdleBehavior] == string(IdleBehaviorHold)
+				return wkr.instance.Tags()[tagKey] == string(IdleBehaviorHold)
 			}
 		}
 		return false
diff --git a/lib/dispatchcloud/worker/verify.go b/lib/dispatchcloud/worker/verify.go
index e22c85d00..330071951 100644
--- a/lib/dispatchcloud/worker/verify.go
+++ b/lib/dispatchcloud/worker/verify.go
@@ -23,11 +23,11 @@ var (
 
 type tagVerifier struct {
 	cloud.Instance
+	secret string
 }
 
 func (tv tagVerifier) VerifyHostKey(pubKey ssh.PublicKey, client *ssh.Client) error {
-	expectSecret := tv.Instance.Tags()[tagKeyInstanceSecret]
-	if err := tv.Instance.VerifyHostKey(pubKey, client); err != cloud.ErrNotImplemented || expectSecret == "" {
+	if err := tv.Instance.VerifyHostKey(pubKey, client); err != cloud.ErrNotImplemented || tv.secret == "" {
 		// If the wrapped instance indicates it has a way to
 		// verify the key, return that decision.
 		return err
@@ -49,7 +49,7 @@ func (tv tagVerifier) VerifyHostKey(pubKey ssh.PublicKey, client *ssh.Client) er
 	if err != nil {
 		return err
 	}
-	if stdout.String() != expectSecret {
+	if stdout.String() != tv.secret {
 		return errBadInstanceSecret
 	}
 	return nil
diff --git a/lib/dispatchcloud/worker/worker.go b/lib/dispatchcloud/worker/worker.go
index 49c5057b3..03ab15176 100644
--- a/lib/dispatchcloud/worker/worker.go
+++ b/lib/dispatchcloud/worker/worker.go
@@ -455,8 +455,8 @@ func (wkr *worker) saveTags() {
 	instance := wkr.instance
 	tags := instance.Tags()
 	update := cloud.InstanceTags{
-		tagKeyInstanceType: wkr.instType.Name,
-		tagKeyIdleBehavior: string(wkr.idleBehavior),
+		wkr.wp.tagKeyPrefix + tagKeyInstanceType: wkr.instType.Name,
+		wkr.wp.tagKeyPrefix + tagKeyIdleBehavior: string(wkr.idleBehavior),
 	}
 	save := false
 	for k, v := range update {
diff --git a/sdk/go/arvados/config.go b/sdk/go/arvados/config.go
index 819afdbc2..541dd9784 100644
--- a/sdk/go/arvados/config.go
+++ b/sdk/go/arvados/config.go
@@ -170,6 +170,7 @@ type CloudVMsConfig struct {
 	TimeoutSignal        Duration
 	TimeoutTERM          Duration
 	ResourceTags         map[string]string
+	TagKeyPrefix         string
 
 	Driver           string
 	DriverParameters json.RawMessage

commit 031f7397b7c0493c7ab33bd40152e59db263b148
Author: Tom Clegg <tclegg at veritasgenetics.com>
Date:   Wed May 29 16:01:36 2019 -0400

    14931: Tag and filter instances by SetID, so driver doesn't need to.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tclegg at veritasgenetics.com>

diff --git a/lib/cloud/azure/azure.go b/lib/cloud/azure/azure.go
index 03d2550bb..dce8b61b7 100644
--- a/lib/cloud/azure/azure.go
+++ b/lib/cloud/azure/azure.go
@@ -477,18 +477,16 @@ func (az *azureInstanceSet) Instances(cloud.InstanceTags) ([]cloud.Instance, err
 		return nil, wrapAzureError(err)
 	}
 
-	instances := make([]cloud.Instance, 0)
-
+	var instances []cloud.Instance
 	for ; result.NotDone(); err = result.Next() {
 		if err != nil {
 			return nil, wrapAzureError(err)
 		}
-		if strings.HasPrefix(*result.Value().Name, az.namePrefix) {
-			instances = append(instances, &azureInstance{
-				provider: az,
-				vm:       result.Value(),
-				nic:      interfaces[*(*result.Value().NetworkProfile.NetworkInterfaces)[0].ID]})
-		}
+		instances = append(instances, &azureInstance{
+			provider: az,
+			vm:       result.Value(),
+			nic:      interfaces[*(*result.Value().NetworkProfile.NetworkInterfaces)[0].ID],
+		})
 	}
 	return instances, nil
 }
diff --git a/lib/cloud/azure/azure_test.go b/lib/cloud/azure/azure_test.go
index 96bfb4fef..8cedca295 100644
--- a/lib/cloud/azure/azure_test.go
+++ b/lib/cloud/azure/azure_test.go
@@ -39,6 +39,7 @@ import (
 	"net"
 	"net/http"
 	"os"
+	"strings"
 	"testing"
 	"time"
 
@@ -66,6 +67,8 @@ type AzureInstanceSetSuite struct{}
 
 var _ = check.Suite(&AzureInstanceSetSuite{})
 
+const testNamePrefix = "compute-test123-"
+
 type VirtualMachinesClientStub struct{}
 
 func (*VirtualMachinesClientStub) createOrUpdate(ctx context.Context,
@@ -149,7 +152,7 @@ func GetInstanceSet() (cloud.InstanceSet, cloud.ImageID, arvados.Cluster, error)
 			BlobContainer: "vhds",
 		},
 		dispatcherID: "test123",
-		namePrefix:   "compute-test123-",
+		namePrefix:   testNamePrefix,
 		logger:       logrus.StandardLogger(),
 		deleteNIC:    make(chan string),
 		deleteBlob:   make(chan storage.Blob),
@@ -228,7 +231,7 @@ func (*AzureInstanceSetSuite) TestDestroyInstances(c *check.C) {
 	l, err := ap.Instances(nil)
 	c.Assert(err, check.IsNil)
 
-	for _, i := range l {
+	for _, i := range filterInstances(c, l) {
 		c.Check(i.Destroy(), check.IsNil)
 	}
 }
@@ -287,17 +290,20 @@ func (*AzureInstanceSetSuite) TestSetTags(c *check.C) {
 	if err != nil {
 		c.Fatal("Error making provider", err)
 	}
+
 	l, err := ap.Instances(nil)
 	c.Assert(err, check.IsNil)
-
+	l = filterInstances(c, l)
 	if len(l) > 0 {
 		err = l[0].SetTags(map[string]string{"foo": "bar"})
 		if err != nil {
 			c.Fatal("Error setting tags", err)
 		}
 	}
+
 	l, err = ap.Instances(nil)
 	c.Assert(err, check.IsNil)
+	l = filterInstances(c, l)
 
 	if len(l) > 0 {
 		tg := l[0].Tags()
@@ -312,6 +318,7 @@ func (*AzureInstanceSetSuite) TestSSH(c *check.C) {
 	}
 	l, err := ap.Instances(nil)
 	c.Assert(err, check.IsNil)
+	l = filterInstances(c, l)
 
 	if len(l) > 0 {
 		sshclient, err := SetupSSHClient(c, l[0])
@@ -372,3 +379,15 @@ func SetupSSHClient(c *check.C, inst cloud.Instance) (*ssh.Client, error) {
 
 	return client, nil
 }
+
+func filterInstances(c *check.C, instances []cloud.Instance) []cloud.Instance {
+	var r []cloud.Instance
+	for _, i := range instances {
+		if !strings.HasPrefix(i.String(), testNamePrefix) {
+			c.Logf("ignoring instance %s", i)
+			continue
+		}
+		r = append(r, i)
+	}
+	return r
+}
diff --git a/lib/cloud/ec2/ec2.go b/lib/cloud/ec2/ec2.go
index e2ad6b42b..c630e9541 100644
--- a/lib/cloud/ec2/ec2.go
+++ b/lib/cloud/ec2/ec2.go
@@ -25,8 +25,6 @@ import (
 	"golang.org/x/crypto/ssh"
 )
 
-const tagKeyInstanceSetID = "arvados-dispatch-id"
-
 // Driver is the ec2 implementation of the cloud.Driver interface.
 var Driver = cloud.DriverFunc(newEC2InstanceSet)
 
@@ -155,12 +153,7 @@ func (instanceSet *ec2InstanceSet) Create(
 	}
 	instanceSet.keysMtx.Unlock()
 
-	ec2tags := []*ec2.Tag{
-		&ec2.Tag{
-			Key:   aws.String(tagKeyInstanceSetID),
-			Value: aws.String(string(instanceSet.instanceSetID)),
-		},
-	}
+	ec2tags := []*ec2.Tag{}
 	for k, v := range newTags {
 		ec2tags = append(ec2tags, &ec2.Tag{
 			Key:   aws.String(k),
@@ -224,13 +217,15 @@ func (instanceSet *ec2InstanceSet) Create(
 	}, nil
 }
 
-func (instanceSet *ec2InstanceSet) Instances(cloud.InstanceTags) (instances []cloud.Instance, err error) {
-	dii := &ec2.DescribeInstancesInput{
-		Filters: []*ec2.Filter{&ec2.Filter{
-			Name:   aws.String("tag:" + tagKeyInstanceSetID),
-			Values: []*string{aws.String(string(instanceSet.instanceSetID))},
-		}}}
-
+func (instanceSet *ec2InstanceSet) Instances(tags cloud.InstanceTags) (instances []cloud.Instance, err error) {
+	var filters []*ec2.Filter
+	for k, v := range tags {
+		filters = append(filters, &ec2.Filter{
+			Name:   aws.String("tag:" + k),
+			Values: []*string{aws.String(v)},
+		})
+	}
+	dii := &ec2.DescribeInstancesInput{Filters: filters}
 	for {
 		dio, err := instanceSet.client.DescribeInstances(dii)
 		if err != nil {
@@ -272,12 +267,7 @@ func (inst *ec2Instance) ProviderType() string {
 }
 
 func (inst *ec2Instance) SetTags(newTags cloud.InstanceTags) error {
-	ec2tags := []*ec2.Tag{
-		&ec2.Tag{
-			Key:   aws.String(tagKeyInstanceSetID),
-			Value: aws.String(string(inst.provider.instanceSetID)),
-		},
-	}
+	var ec2tags []*ec2.Tag
 	for k, v := range newTags {
 		ec2tags = append(ec2tags, &ec2.Tag{
 			Key:   aws.String(k),
diff --git a/lib/cloud/interfaces.go b/lib/cloud/interfaces.go
index 792e737a9..804de667e 100644
--- a/lib/cloud/interfaces.go
+++ b/lib/cloud/interfaces.go
@@ -154,13 +154,17 @@ type InitCommand string
 // other mechanism. The tags must be visible to another instance of
 // the same driver running on a different host.
 //
-// The returned InstanceSet must ignore existing resources that are
-// visible but not tagged with the given id, except that it should log
-// a summary of such resources -- only once -- when it starts
-// up. Thus, two identically configured InstanceSets running on
-// different hosts with different ids should log about the existence
-// of each other's resources at startup, but will not interfere with
-// each other.
+// The returned InstanceSet must not modify or delete cloud resources
+// unless they are tagged with the given InstanceSetID or the caller
+// (dispatcher) calls Destroy() on them. It may log a summary of
+// untagged resources once at startup, though. Thus, two identically
+// configured InstanceSets running on different hosts with different
+// ids should log about the existence of each other's resources at
+// startup, but will not interfere with each other.
+//
+// The dispatcher always passes the InstanceSetID as a tag when
+// calling Create() and Instances(), so the driver does not need to
+// tag/filter VMs by InstanceSetID itself.
 //
 // Example:
 //
diff --git a/lib/dispatchcloud/dispatcher.go b/lib/dispatchcloud/dispatcher.go
index 3bf0ee9bd..bc699d928 100644
--- a/lib/dispatchcloud/dispatcher.go
+++ b/lib/dispatchcloud/dispatcher.go
@@ -138,7 +138,7 @@ func (disp *dispatcher) initialize() {
 	}
 	disp.instanceSet = instanceSet
 	disp.reg = prometheus.NewRegistry()
-	disp.pool = worker.NewPool(disp.logger, disp.ArvClient, disp.reg, disp.instanceSet, disp.newExecutor, disp.sshKey.PublicKey(), disp.Cluster)
+	disp.pool = worker.NewPool(disp.logger, disp.ArvClient, disp.reg, disp.InstanceSetID, disp.instanceSet, disp.newExecutor, disp.sshKey.PublicKey(), disp.Cluster)
 	disp.queue = container.NewQueue(disp.logger, disp.reg, disp.typeChooser, disp.ArvClient)
 
 	if disp.Cluster.ManagementToken == "" {
diff --git a/lib/dispatchcloud/driver.go b/lib/dispatchcloud/driver.go
index 3f1601188..36b8e8008 100644
--- a/lib/dispatchcloud/driver.go
+++ b/lib/dispatchcloud/driver.go
@@ -28,7 +28,7 @@ func newInstanceSet(cluster *arvados.Cluster, setID cloud.InstanceSetID, logger
 	}
 	is, err := driver.InstanceSet(cluster.Containers.CloudVMs.DriverParameters, setID, logger)
 	if maxops := cluster.Containers.CloudVMs.MaxCloudOpsPerSecond; maxops > 0 {
-		is = &rateLimitedInstanceSet{
+		is = rateLimitedInstanceSet{
 			InstanceSet: is,
 			ticker:      time.NewTicker(time.Second / time.Duration(maxops)),
 		}
@@ -37,6 +37,10 @@ func newInstanceSet(cluster *arvados.Cluster, setID cloud.InstanceSetID, logger
 		InstanceSet: is,
 		defaultTags: cloud.InstanceTags(cluster.Containers.CloudVMs.ResourceTags),
 	}
+	is = filteringInstanceSet{
+		InstanceSet: is,
+		logger:      logger,
+	}
 	return is, err
 }
 
@@ -77,3 +81,34 @@ func (is defaultTaggingInstanceSet) Create(it arvados.InstanceType, image cloud.
 	}
 	return is.InstanceSet.Create(it, image, allTags, init, pk)
 }
+
+// Filters the instances returned by the wrapped InstanceSet's
+// Instances() method (in case the wrapped InstanceSet didn't do this
+// itself).
+type filteringInstanceSet struct {
+	cloud.InstanceSet
+	logger logrus.FieldLogger
+}
+
+func (is filteringInstanceSet) Instances(tags cloud.InstanceTags) ([]cloud.Instance, error) {
+	instances, err := is.InstanceSet.Instances(tags)
+
+	skipped := 0
+	var returning []cloud.Instance
+nextInstance:
+	for _, inst := range instances {
+		instTags := inst.Tags()
+		for k, v := range tags {
+			if instTags[k] != v {
+				skipped++
+				continue nextInstance
+			}
+		}
+		returning = append(returning, inst)
+	}
+	is.logger.WithFields(logrus.Fields{
+		"returning": len(returning),
+		"skipped":   skipped,
+	}).WithError(err).Debugf("filteringInstanceSet returning instances")
+	return returning, err
+}
diff --git a/lib/dispatchcloud/worker/pool.go b/lib/dispatchcloud/worker/pool.go
index 84b61fc00..8af103712 100644
--- a/lib/dispatchcloud/worker/pool.go
+++ b/lib/dispatchcloud/worker/pool.go
@@ -25,6 +25,7 @@ const (
 	tagKeyInstanceType   = "InstanceType"
 	tagKeyIdleBehavior   = "IdleBehavior"
 	tagKeyInstanceSecret = "InstanceSecret"
+	tagKeyInstanceSetID  = "InstanceSetID"
 )
 
 // An InstanceView shows a worker's current state and recent activity.
@@ -91,10 +92,11 @@ func duration(conf arvados.Duration, def time.Duration) time.Duration {
 //
 // New instances are configured and set up according to the given
 // cluster configuration.
-func NewPool(logger logrus.FieldLogger, arvClient *arvados.Client, reg *prometheus.Registry, instanceSet cloud.InstanceSet, newExecutor func(cloud.Instance) Executor, installPublicKey ssh.PublicKey, cluster *arvados.Cluster) *Pool {
+func NewPool(logger logrus.FieldLogger, arvClient *arvados.Client, reg *prometheus.Registry, instanceSetID cloud.InstanceSetID, instanceSet cloud.InstanceSet, newExecutor func(cloud.Instance) Executor, installPublicKey ssh.PublicKey, cluster *arvados.Cluster) *Pool {
 	wp := &Pool{
 		logger:             logger,
 		arvClient:          arvClient,
+		instanceSetID:      instanceSetID,
 		instanceSet:        &throttledInstanceSet{InstanceSet: instanceSet},
 		newExecutor:        newExecutor,
 		bootProbeCommand:   cluster.Containers.CloudVMs.BootProbeCommand,
@@ -128,6 +130,7 @@ type Pool struct {
 	// configuration
 	logger             logrus.FieldLogger
 	arvClient          *arvados.Client
+	instanceSetID      cloud.InstanceSetID
 	instanceSet        *throttledInstanceSet
 	newExecutor        func(cloud.Instance) Executor
 	bootProbeCommand   string
@@ -281,9 +284,10 @@ func (wp *Pool) Create(it arvados.InstanceType) bool {
 	go func() {
 		defer wp.notify()
 		tags := cloud.InstanceTags{
-			tagKeyInstanceType:   it.Name,
-			tagKeyIdleBehavior:   string(IdleBehaviorRun),
-			tagKeyInstanceSecret: secret,
+			wp.tagPrefix + tagKeyInstanceSetID:  string(wp.instanceSetID),
+			wp.tagPrefix + tagKeyInstanceType:   it.Name,
+			wp.tagPrefix + tagKeyIdleBehavior:   string(IdleBehaviorRun),
+			wp.tagPrefix + tagKeyInstanceSecret: secret,
 		}
 		initCmd := cloud.InitCommand(fmt.Sprintf("umask 0177 && echo -n %q >%s", secret, instanceSecretFilename))
 		inst, err := wp.instanceSet.Create(it, wp.imageID, tags, initCmd, wp.installPublicKey)
@@ -728,7 +732,7 @@ func (wp *Pool) getInstancesAndSync() error {
 	}
 	wp.logger.Debug("getting instance list")
 	threshold := time.Now()
-	instances, err := wp.instanceSet.Instances(cloud.InstanceTags{})
+	instances, err := wp.instanceSet.Instances(cloud.InstanceTags{tagKeyInstanceSetID: string(wp.instanceSetID)})
 	if err != nil {
 		wp.instanceSet.throttleInstances.CheckRateLimitError(err, wp.logger, "list instances", wp.notify)
 		return err
diff --git a/lib/dispatchcloud/worker/pool_test.go b/lib/dispatchcloud/worker/pool_test.go
index 693953668..8ab4c9875 100644
--- a/lib/dispatchcloud/worker/pool_test.go
+++ b/lib/dispatchcloud/worker/pool_test.go
@@ -65,7 +65,8 @@ func (suite *PoolSuite) TestResumeAfterRestart(c *check.C) {
 
 	logger := ctxlog.TestLogger(c)
 	driver := &test.StubDriver{}
-	is, err := driver.InstanceSet(nil, "", logger)
+	instanceSetID := cloud.InstanceSetID("test-instance-set-id")
+	is, err := driver.InstanceSet(nil, instanceSetID, logger)
 	c.Assert(err, check.IsNil)
 
 	newExecutor := func(cloud.Instance) Executor {
@@ -91,7 +92,7 @@ func (suite *PoolSuite) TestResumeAfterRestart(c *check.C) {
 		},
 	}
 
-	pool := NewPool(logger, arvados.NewClientFromEnv(), prometheus.NewRegistry(), is, newExecutor, nil, cluster)
+	pool := NewPool(logger, arvados.NewClientFromEnv(), prometheus.NewRegistry(), instanceSetID, is, newExecutor, nil, cluster)
 	notify := pool.Subscribe()
 	defer pool.Unsubscribe(notify)
 	pool.Create(type1)
@@ -126,7 +127,7 @@ func (suite *PoolSuite) TestResumeAfterRestart(c *check.C) {
 
 	c.Log("------- starting new pool, waiting to recover state")
 
-	pool2 := NewPool(logger, arvados.NewClientFromEnv(), prometheus.NewRegistry(), is, newExecutor, nil, cluster)
+	pool2 := NewPool(logger, arvados.NewClientFromEnv(), prometheus.NewRegistry(), instanceSetID, is, newExecutor, nil, cluster)
 	notify2 := pool2.Subscribe()
 	defer pool2.Unsubscribe(notify2)
 	waitForIdle(pool2, notify2)

commit 8a097f03e9687b136f091d361e5cc1949a2647de
Author: Tom Clegg <tclegg at veritasgenetics.com>
Date:   Tue May 28 15:39:11 2019 -0400

    14931: Support extra tags on resources created by dispatchcloud.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tclegg at veritasgenetics.com>

diff --git a/lib/cloud/azure/azure.go b/lib/cloud/azure/azure.go
index ac7ff14cc..03d2550bb 100644
--- a/lib/cloud/azure/azure.go
+++ b/lib/cloud/azure/azure.go
@@ -50,8 +50,6 @@ type azureInstanceSetConfig struct {
 	AdminUsername                string
 }
 
-const tagKeyInstanceSecret = "InstanceSecret"
-
 type containerWrapper interface {
 	GetBlobReference(name string) *storage.Blob
 	ListBlobs(params storage.ListBlobsParameters) (storage.BlobListResponse, error)
@@ -352,14 +350,11 @@ func (az *azureInstanceSet) Create(
 
 	name = az.namePrefix + name
 
-	timestamp := time.Now().Format(time.RFC3339Nano)
-
-	tags := make(map[string]*string)
-	tags["created-at"] = &timestamp
+	tags := map[string]*string{}
 	for k, v := range newTags {
-		newstr := v
-		tags["dispatch-"+k] = &newstr
+		tags[k] = to.StringPtr(v)
 	}
+	tags["created-at"] = to.StringPtr(time.Now().Format(time.RFC3339Nano))
 
 	nicParameters := network.Interface{
 		Location: &az.azconfig.Location,
@@ -499,9 +494,9 @@ func (az *azureInstanceSet) Instances(cloud.InstanceTags) ([]cloud.Instance, err
 }
 
 // ManageNics returns a list of Azure network interface resources.
-// Also performs garbage collection of NICs which have "namePrefix", are
-// not associated with a virtual machine and have a "create-at" time
-// more than DeleteDanglingResourcesAfter (to prevent racing and
+// Also performs garbage collection of NICs which have "namePrefix",
+// are not associated with a virtual machine and have a "created-at"
+// time more than DeleteDanglingResourcesAfter (to prevent racing and
 // deleting newly created NICs) in the past are deleted.
 func (az *azureInstanceSet) manageNics() (map[string]network.Interface, error) {
 	az.stopWg.Add(1)
@@ -603,16 +598,12 @@ func (ai *azureInstance) SetTags(newTags cloud.InstanceTags) error {
 	ai.provider.stopWg.Add(1)
 	defer ai.provider.stopWg.Done()
 
-	tags := make(map[string]*string)
-
+	tags := map[string]*string{}
 	for k, v := range ai.vm.Tags {
-		if !strings.HasPrefix(k, "dispatch-") {
-			tags[k] = v
-		}
+		tags[k] = v
 	}
 	for k, v := range newTags {
-		newstr := v
-		tags["dispatch-"+k] = &newstr
+		tags[k] = to.StringPtr(v)
 	}
 
 	vmParameters := compute.VirtualMachine{
@@ -629,14 +620,10 @@ func (ai *azureInstance) SetTags(newTags cloud.InstanceTags) error {
 }
 
 func (ai *azureInstance) Tags() cloud.InstanceTags {
-	tags := make(map[string]string)
-
+	tags := cloud.InstanceTags{}
 	for k, v := range ai.vm.Tags {
-		if strings.HasPrefix(k, "dispatch-") {
-			tags[k[9:]] = *v
-		}
+		tags[k] = *v
 	}
-
 	return tags
 }
 
diff --git a/lib/cloud/ec2/ec2.go b/lib/cloud/ec2/ec2.go
index c5565d424..e2ad6b42b 100644
--- a/lib/cloud/ec2/ec2.go
+++ b/lib/cloud/ec2/ec2.go
@@ -13,7 +13,6 @@ import (
 	"encoding/json"
 	"fmt"
 	"math/big"
-	"strings"
 	"sync"
 
 	"git.curoverse.com/arvados.git/lib/cloud"
@@ -26,8 +25,7 @@ import (
 	"golang.org/x/crypto/ssh"
 )
 
-const arvadosDispatchID = "arvados-dispatch-id"
-const tagPrefix = "arvados-dispatch-tag-"
+const tagKeyInstanceSetID = "arvados-dispatch-id"
 
 // Driver is the ec2 implementation of the cloud.Driver interface.
 var Driver = cloud.DriverFunc(newEC2InstanceSet)
@@ -52,18 +50,18 @@ type ec2Interface interface {
 }
 
 type ec2InstanceSet struct {
-	ec2config    ec2InstanceSetConfig
-	dispatcherID cloud.InstanceSetID
-	logger       logrus.FieldLogger
-	client       ec2Interface
-	keysMtx      sync.Mutex
-	keys         map[string]string
+	ec2config     ec2InstanceSetConfig
+	instanceSetID cloud.InstanceSetID
+	logger        logrus.FieldLogger
+	client        ec2Interface
+	keysMtx       sync.Mutex
+	keys          map[string]string
 }
 
-func newEC2InstanceSet(config json.RawMessage, dispatcherID cloud.InstanceSetID, logger logrus.FieldLogger) (prv cloud.InstanceSet, err error) {
+func newEC2InstanceSet(config json.RawMessage, instanceSetID cloud.InstanceSetID, logger logrus.FieldLogger) (prv cloud.InstanceSet, err error) {
 	instanceSet := &ec2InstanceSet{
-		dispatcherID: dispatcherID,
-		logger:       logger,
+		instanceSetID: instanceSetID,
+		logger:        logger,
 	}
 	err = json.Unmarshal(config, &instanceSet.ec2config)
 	if err != nil {
@@ -159,17 +157,13 @@ func (instanceSet *ec2InstanceSet) Create(
 
 	ec2tags := []*ec2.Tag{
 		&ec2.Tag{
-			Key:   aws.String(arvadosDispatchID),
-			Value: aws.String(string(instanceSet.dispatcherID)),
-		},
-		&ec2.Tag{
-			Key:   aws.String("arvados-class"),
-			Value: aws.String("dynamic-compute"),
+			Key:   aws.String(tagKeyInstanceSetID),
+			Value: aws.String(string(instanceSet.instanceSetID)),
 		},
 	}
 	for k, v := range newTags {
 		ec2tags = append(ec2tags, &ec2.Tag{
-			Key:   aws.String(tagPrefix + k),
+			Key:   aws.String(k),
 			Value: aws.String(v),
 		})
 	}
@@ -191,12 +185,12 @@ func (instanceSet *ec2InstanceSet) Create(
 			}},
 		DisableApiTermination:             aws.Bool(false),
 		InstanceInitiatedShutdownBehavior: aws.String("terminate"),
-		UserData: aws.String(base64.StdEncoding.EncodeToString([]byte("#!/bin/sh\n" + initCommand + "\n"))),
 		TagSpecifications: []*ec2.TagSpecification{
 			&ec2.TagSpecification{
 				ResourceType: aws.String("instance"),
 				Tags:         ec2tags,
 			}},
+		UserData: aws.String(base64.StdEncoding.EncodeToString([]byte("#!/bin/sh\n" + initCommand + "\n"))),
 	}
 
 	if instanceType.AddedScratch > 0 {
@@ -233,8 +227,8 @@ func (instanceSet *ec2InstanceSet) Create(
 func (instanceSet *ec2InstanceSet) Instances(cloud.InstanceTags) (instances []cloud.Instance, err error) {
 	dii := &ec2.DescribeInstancesInput{
 		Filters: []*ec2.Filter{&ec2.Filter{
-			Name:   aws.String("tag:" + arvadosDispatchID),
-			Values: []*string{aws.String(string(instanceSet.dispatcherID))},
+			Name:   aws.String("tag:" + tagKeyInstanceSetID),
+			Values: []*string{aws.String(string(instanceSet.instanceSetID))},
 		}}}
 
 	for {
@@ -280,13 +274,13 @@ func (inst *ec2Instance) ProviderType() string {
 func (inst *ec2Instance) SetTags(newTags cloud.InstanceTags) error {
 	ec2tags := []*ec2.Tag{
 		&ec2.Tag{
-			Key:   aws.String(arvadosDispatchID),
-			Value: aws.String(string(inst.provider.dispatcherID)),
+			Key:   aws.String(tagKeyInstanceSetID),
+			Value: aws.String(string(inst.provider.instanceSetID)),
 		},
 	}
 	for k, v := range newTags {
 		ec2tags = append(ec2tags, &ec2.Tag{
-			Key:   aws.String(tagPrefix + k),
+			Key:   aws.String(k),
 			Value: aws.String(v),
 		})
 	}
@@ -303,9 +297,7 @@ func (inst *ec2Instance) Tags() cloud.InstanceTags {
 	tags := make(map[string]string)
 
 	for _, t := range inst.instance.Tags {
-		if strings.HasPrefix(*t.Key, tagPrefix) {
-			tags[(*t.Key)[len(tagPrefix):]] = *t.Value
-		}
+		tags[*t.Key] = *t.Value
 	}
 
 	return tags
diff --git a/lib/cloud/ec2/ec2_test.go b/lib/cloud/ec2/ec2_test.go
index 50ba01174..62a0b8564 100644
--- a/lib/cloud/ec2/ec2_test.go
+++ b/lib/cloud/ec2/ec2_test.go
@@ -125,11 +125,11 @@ func GetInstanceSet() (cloud.InstanceSet, cloud.ImageID, arvados.Cluster, error)
 		return ap, cloud.ImageID(exampleCfg.ImageIDForTestSuite), cluster, err
 	}
 	ap := ec2InstanceSet{
-		ec2config:    ec2InstanceSetConfig{},
-		dispatcherID: "test123",
-		logger:       logrus.StandardLogger(),
-		client:       &ec2stub{},
-		keys:         make(map[string]string),
+		ec2config:     ec2InstanceSetConfig{},
+		instanceSetID: "test123",
+		logger:        logrus.StandardLogger(),
+		client:        &ec2stub{},
+		keys:          make(map[string]string),
 	}
 	return &ap, cloud.ImageID("blob"), cluster, nil
 }
diff --git a/lib/config/config.default.yml b/lib/config/config.default.yml
index f19238f42..b1ed91d4b 100644
--- a/lib/config/config.default.yml
+++ b/lib/config/config.default.yml
@@ -530,6 +530,13 @@ Clusters:
         # Worker VM image ID.
         ImageID: ami-01234567890abcdef
 
+        # Tags to add on all resources (VMs, NICs, disks) created by
+        # the container dispatcher. (Arvados's own tags --
+        # InstanceType, IdleBehavior, and InstanceSecret -- will also
+        # be added.)
+        ResourceTags:
+          SAMPLE: "tag value"
+
         # Cloud driver: "azure" (Microsoft Azure) or "ec2" (Amazon AWS).
         Driver: ec2
 
diff --git a/lib/config/generated_config.go b/lib/config/generated_config.go
index 59feeec49..513eee362 100644
--- a/lib/config/generated_config.go
+++ b/lib/config/generated_config.go
@@ -536,6 +536,13 @@ Clusters:
         # Worker VM image ID.
         ImageID: ami-01234567890abcdef
 
+        # Tags to add on all resources (VMs, NICs, disks) created by
+        # the container dispatcher. (Arvados's own tags --
+        # InstanceType, IdleBehavior, and InstanceSecret -- will also
+        # be added.)
+        ResourceTags:
+          SAMPLE: "tag value"
+
         # Cloud driver: "azure" (Microsoft Azure) or "ec2" (Amazon AWS).
         Driver: ec2
 
diff --git a/lib/dispatchcloud/dispatcher_test.go b/lib/dispatchcloud/dispatcher_test.go
index 6b8620ade..11786bd3c 100644
--- a/lib/dispatchcloud/dispatcher_test.go
+++ b/lib/dispatchcloud/dispatcher_test.go
@@ -65,6 +65,7 @@ func (s *DispatcherSuite) SetUpTest(c *check.C) {
 				MaxProbesPerSecond:   1000,
 				TimeoutSignal:        arvados.Duration(3 * time.Millisecond),
 				TimeoutTERM:          arvados.Duration(20 * time.Millisecond),
+				ResourceTags:         map[string]string{"testtag": "test value"},
 			},
 		},
 		InstanceTypes: arvados.InstanceTypeMap{
diff --git a/lib/dispatchcloud/driver.go b/lib/dispatchcloud/driver.go
index 5ec0f73e7..3f1601188 100644
--- a/lib/dispatchcloud/driver.go
+++ b/lib/dispatchcloud/driver.go
@@ -33,6 +33,10 @@ func newInstanceSet(cluster *arvados.Cluster, setID cloud.InstanceSetID, logger
 			ticker:      time.NewTicker(time.Second / time.Duration(maxops)),
 		}
 	}
+	is = defaultTaggingInstanceSet{
+		InstanceSet: is,
+		defaultTags: cloud.InstanceTags(cluster.Containers.CloudVMs.ResourceTags),
+	}
 	return is, err
 }
 
@@ -56,3 +60,20 @@ func (inst *rateLimitedInstance) Destroy() error {
 	<-inst.ticker.C
 	return inst.Instance.Destroy()
 }
+
+// Adds the specified defaultTags to every Create() call.
+type defaultTaggingInstanceSet struct {
+	cloud.InstanceSet
+	defaultTags cloud.InstanceTags
+}
+
+func (is defaultTaggingInstanceSet) Create(it arvados.InstanceType, image cloud.ImageID, tags cloud.InstanceTags, init cloud.InitCommand, pk ssh.PublicKey) (cloud.Instance, error) {
+	allTags := cloud.InstanceTags{}
+	for k, v := range is.defaultTags {
+		allTags[k] = v
+	}
+	for k, v := range tags {
+		allTags[k] = v
+	}
+	return is.InstanceSet.Create(it, image, allTags, init, pk)
+}
diff --git a/sdk/go/arvados/config.go b/sdk/go/arvados/config.go
index 4936aa270..819afdbc2 100644
--- a/sdk/go/arvados/config.go
+++ b/sdk/go/arvados/config.go
@@ -169,6 +169,7 @@ type CloudVMsConfig struct {
 	TimeoutShutdown      Duration
 	TimeoutSignal        Duration
 	TimeoutTERM          Duration
+	ResourceTags         map[string]string
 
 	Driver           string
 	DriverParameters json.RawMessage

-----------------------------------------------------------------------


hooks/post-receive
-- 




More information about the arvados-commits mailing list