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@veritasgenetics.com>

diff --git a/lib/cloud/azure/azure.go b/lib/cloud/azure/azure.go
index dce8b61b71d7e2d52c4865af7df218bf4c3e57c7..ab14d6681eb34b30f1845dd47f4a37f3afc7aced 100644 (file)
--- 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 8cedca295a7a9a2e4deab2b9627fb97b101ce1a9..152b7e73b7a32f2af80862fcde1d4e68b43df9e1 100644 (file)
--- 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 c630e95418f03e590dc052ea0880104ea845b85c..079c32802ca4d3a038b1a395b5d56188b99a7cce 100644 (file)
--- 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 62a0b8564f6e1c0b21e66a126a0ce33255740d0c..8b754eacac454b0993e5961b01754538c150980e 100644 (file)
--- 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 804de667ee010e81b5710ff140a3fa2f988e596f..7410f9d0e0ea2fe61e5ac6dfedecd3ac740f7ebe 100644 (file)
--- 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 36b8e8008249e0a37910642ff286c88309204c4c..b67b5d054b57d172b940255a8318b76dd21af3b8 100644 (file)
--- 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 873d987327eafed2a53f6d63f0dcc17230dbeb0d..a9a5a429f3e941a0973ee394d6693f952e4e780c 100644 (file)
--- 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 300b4730fc3b0dd9107803df1931bb95a4b52f6a..4b87ce503157a6d873e5d7d13eed018fc0ad35af 100644 (file)
--- 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 15a2a894c5bceb89bdae4f6c5e4146d317dca083..4f9ba911cd3c112463f749738dcb70d1dfb47619 100644 (file)
--- 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)