13674: Change InstanceTypes config from array to hash.

Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tclegg@veritasgenetics.com>

diff --git a/lib/dispatchcloud/node_size.go b/lib/dispatchcloud/node_size.go
index 4329f4f139e14fc916935a945e2c0935db7f4860..5173cc79716867683c125f6da2925aa210391284 100644 (file)
--- a/lib/dispatchcloud/node_size.go
+++ b/lib/dispatchcloud/node_size.go
@@ -47,8 +47,10 @@ func ChooseInstanceType(cc *arvados.Cluster, ctr *arvados.Container) (best arvad
        needRAM := ctr.RuntimeConstraints.RAM + ctr.RuntimeConstraints.KeepCacheRAM
        needRAM = (needRAM * 100) / int64(100-discountConfiguredRAMPercent)
 
-       availableTypes := make([]arvados.InstanceType, len(cc.InstanceTypes))
-       copy(availableTypes, cc.InstanceTypes)
+       availableTypes := make([]arvados.InstanceType, 0, len(cc.InstanceTypes))
+       for _, t := range cc.InstanceTypes {
+               availableTypes = append(availableTypes, t)
+       }
        sort.Slice(availableTypes, func(a, b int) bool {
                return availableTypes[a].Price < availableTypes[b].Price
        })

diff --git a/lib/dispatchcloud/node_size_test.go b/lib/dispatchcloud/node_size_test.go
index 1484f07a29c6754f80b1fb88faaebcb59290938b..881c1f523d6048148035c9ade0d2d8a8d240f869 100644 (file)
--- a/lib/dispatchcloud/node_size_test.go
+++ b/lib/dispatchcloud/node_size_test.go
@@ -27,10 +27,10 @@ func (*NodeSizeSuite) TestChooseNotConfigured(c *check.C) {
 
 func (*NodeSizeSuite) TestChooseUnsatisfiable(c *check.C) {
        checkUnsatisfiable := func(ctr *arvados.Container) {
-               _, err := ChooseInstanceType(&arvados.Cluster{InstanceTypes: []arvados.InstanceType{
-                       {Price: 1.1, RAM: 1000000000, VCPUs: 2, Name: "small1"},
-                       {Price: 2.2, RAM: 2000000000, VCPUs: 4, Name: "small2"},
-                       {Price: 4.4, RAM: 4000000000, VCPUs: 8, Name: "small4", Scratch: GiB},
+               _, err := ChooseInstanceType(&arvados.Cluster{InstanceTypes: map[string]arvados.InstanceType{
+                       "small1": {Price: 1.1, RAM: 1000000000, VCPUs: 2, Name: "small1"},
+                       "small2": {Price: 2.2, RAM: 2000000000, VCPUs: 4, Name: "small2"},
+                       "small4": {Price: 4.4, RAM: 4000000000, VCPUs: 8, Name: "small4", Scratch: GiB},
                }}, ctr)
                c.Check(err, check.FitsTypeOf, ConstraintsNotSatisfiableError{})
        }
@@ -49,29 +49,29 @@ func (*NodeSizeSuite) TestChooseUnsatisfiable(c *check.C) {
 }
 
 func (*NodeSizeSuite) TestChoose(c *check.C) {
-       for _, menu := range [][]arvados.InstanceType{
+       for _, menu := range []map[string]arvados.InstanceType{
                {
-                       {Price: 4.4, RAM: 4000000000, VCPUs: 8, Scratch: 2 * GiB, Name: "costly"},
-                       {Price: 2.2, RAM: 2000000000, VCPUs: 4, Scratch: 2 * GiB, Name: "best"},
-                       {Price: 1.1, RAM: 1000000000, VCPUs: 2, Scratch: 2 * GiB, Name: "small"},
+                       "costly": {Price: 4.4, RAM: 4000000000, VCPUs: 8, Scratch: 2 * GiB, Name: "costly"},
+                       "best":   {Price: 2.2, RAM: 2000000000, VCPUs: 4, Scratch: 2 * GiB, Name: "best"},
+                       "small":  {Price: 1.1, RAM: 1000000000, VCPUs: 2, Scratch: 2 * GiB, Name: "small"},
                },
                {
-                       {Price: 4.4, RAM: 4000000000, VCPUs: 8, Scratch: 2 * GiB, Name: "costly"},
-                       {Price: 2.2, RAM: 2000000000, VCPUs: 4, Scratch: 2 * GiB, Name: "goodenough"},
-                       {Price: 2.2, RAM: 4000000000, VCPUs: 4, Scratch: 2 * GiB, Name: "best"},
-                       {Price: 1.1, RAM: 1000000000, VCPUs: 2, Scratch: 2 * GiB, Name: "small"},
+                       "costly":     {Price: 4.4, RAM: 4000000000, VCPUs: 8, Scratch: 2 * GiB, Name: "costly"},
+                       "goodenough": {Price: 2.2, RAM: 2000000000, VCPUs: 4, Scratch: 2 * GiB, Name: "goodenough"},
+                       "best":       {Price: 2.2, RAM: 4000000000, VCPUs: 4, Scratch: 2 * GiB, Name: "best"},
+                       "small":      {Price: 1.1, RAM: 1000000000, VCPUs: 2, Scratch: 2 * GiB, Name: "small"},
                },
                {
-                       {Price: 1.1, RAM: 1000000000, VCPUs: 2, Scratch: 2 * GiB, Name: "small"},
-                       {Price: 2.2, RAM: 2000000000, VCPUs: 4, Scratch: 2 * GiB, Name: "goodenough"},
-                       {Price: 2.2, RAM: 4000000000, VCPUs: 4, Scratch: 2 * GiB, Name: "best"},
-                       {Price: 4.4, RAM: 4000000000, VCPUs: 8, Scratch: 2 * GiB, Name: "costly"},
+                       "small":      {Price: 1.1, RAM: 1000000000, VCPUs: 2, Scratch: 2 * GiB, Name: "small"},
+                       "goodenough": {Price: 2.2, RAM: 2000000000, VCPUs: 4, Scratch: 2 * GiB, Name: "goodenough"},
+                       "best":       {Price: 2.2, RAM: 4000000000, VCPUs: 4, Scratch: 2 * GiB, Name: "best"},
+                       "costly":     {Price: 4.4, RAM: 4000000000, VCPUs: 8, Scratch: 2 * GiB, Name: "costly"},
                },
                {
-                       {Price: 1.1, RAM: 1000000000, VCPUs: 2, Scratch: GiB, Name: "small"},
-                       {Price: 2.2, RAM: 2000000000, VCPUs: 4, Scratch: GiB, Name: "nearly"},
-                       {Price: 3.3, RAM: 4000000000, VCPUs: 4, Scratch: 2 * GiB, Name: "best"},
-                       {Price: 4.4, RAM: 4000000000, VCPUs: 8, Scratch: 2 * GiB, Name: "costly"},
+                       "small":  {Price: 1.1, RAM: 1000000000, VCPUs: 2, Scratch: GiB, Name: "small"},
+                       "nearly": {Price: 2.2, RAM: 2000000000, VCPUs: 4, Scratch: GiB, Name: "nearly"},
+                       "best":   {Price: 3.3, RAM: 4000000000, VCPUs: 4, Scratch: 2 * GiB, Name: "best"},
+                       "costly": {Price: 4.4, RAM: 4000000000, VCPUs: 8, Scratch: 2 * GiB, Name: "costly"},
                },
        } {
                best, err := ChooseInstanceType(&arvados.Cluster{InstanceTypes: menu}, &arvados.Container{
@@ -92,12 +92,12 @@ func (*NodeSizeSuite) TestChoose(c *check.C) {
        }
 }
 
-func (*NodeSizeSuite) TestChoosePreemptible(c *check.C) {
-       menu := []arvados.InstanceType{
-               {Price: 4.4, RAM: 4000000000, VCPUs: 8, Scratch: 2 * GiB, Preemptible: true, Name: "costly"},
-               {Price: 2.2, RAM: 2000000000, VCPUs: 4, Scratch: 2 * GiB, Name: "almost best"},
-               {Price: 2.2, RAM: 2000000000, VCPUs: 4, Scratch: 2 * GiB, Preemptible: true, Name: "best"},
-               {Price: 1.1, RAM: 1000000000, VCPUs: 2, Scratch: 2 * GiB, Preemptible: true, Name: "small"},
+func (*NodeSizeSuite) TestChoosePreemptable(c *check.C) {
+       menu := map[string]arvados.InstanceType{
+               "costly":      {Price: 4.4, RAM: 4000000000, VCPUs: 8, Scratch: 2 * GiB, Preemptible: true, Name: "costly"},
+               "almost best": {Price: 2.2, RAM: 2000000000, VCPUs: 4, Scratch: 2 * GiB, Name: "almost best"},
+               "best":        {Price: 2.2, RAM: 2000000000, VCPUs: 4, Scratch: 2 * GiB, Preemptible: true, Name: "best"},
+               "small":       {Price: 1.1, RAM: 1000000000, VCPUs: 2, Scratch: 2 * GiB, Preemptible: true, Name: "small"},
        }
        best, err := ChooseInstanceType(&arvados.Cluster{InstanceTypes: menu}, &arvados.Container{
                Mounts: map[string]arvados.Mount{

diff --git a/sdk/go/arvados/config.go b/sdk/go/arvados/config.go
index 182cf8433b2ef0152381b7b7b0acab263685e807..af09d1af181a68ae962f20487220b134ae9dd4e1 100644 (file)
--- a/sdk/go/arvados/config.go
+++ b/sdk/go/arvados/config.go
@@ -5,6 +5,8 @@
 package arvados
 
 import (
+       "encoding/json"
+       "errors"
        "fmt"
        "os"
 
@@ -52,7 +54,7 @@ type Cluster struct {
        ClusterID          string `json:"-"`
        ManagementToken    string
        NodeProfiles       map[string]NodeProfile
-       InstanceTypes      []InstanceType
+       InstanceTypes      InstanceTypeMap
        HTTPRequestTimeout Duration
 }
 
@@ -66,6 +68,46 @@ type InstanceType struct {
        Preemptible  bool
 }
 
+type InstanceTypeMap map[string]InstanceType
+
+var errDuplicateInstanceTypeName = errors.New("duplicate instance type name")
+
+// UnmarshalJSON handles old config files that provide an array of
+// instance types instead of a hash.
+func (it *InstanceTypeMap) UnmarshalJSON(data []byte) error {
+       if len(data) > 0 && data[0] == '[' {
+               var arr []InstanceType
+               err := json.Unmarshal(data, &arr)
+               if err != nil {
+                       return err
+               }
+               if len(arr) == 0 {
+                       *it = nil
+                       return nil
+               }
+               *it = make(map[string]InstanceType, len(arr))
+               for _, t := range arr {
+                       if _, ok := (*it)[t.Name]; ok {
+                               return errDuplicateInstanceTypeName
+                       }
+                       (*it)[t.Name] = t
+               }
+               return nil
+       }
+       var hash map[string]InstanceType
+       err := json.Unmarshal(data, &hash)
+       if err != nil {
+               return err
+       }
+       // Fill in Name field using hash key.
+       *it = InstanceTypeMap(hash)
+       for name, t := range *it {
+               t.Name = name
+               (*it)[name] = t
+       }
+       return nil
+}
+
 // GetNodeProfile returns a NodeProfile for the given hostname. An
 // error is returned if the appropriate configuration can't be
 // determined (e.g., this does not appear to be a system node). If

diff --git a/sdk/go/arvados/config_test.go b/sdk/go/arvados/config_test.go
new file mode 100644 (file)
index 0000000..697b02d
--- /dev/null
+++ b/sdk/go/arvados/config_test.go
@@ -0,0 +1,29 @@
+// Copyright (C) The Arvados Authors. All rights reserved.
+//
+// SPDX-License-Identifier: Apache-2.0
+
+package arvados
+
+import (
+       "github.com/ghodss/yaml"
+       check "gopkg.in/check.v1"
+)
+
+var _ = check.Suite(&ConfigSuite{})
+
+type ConfigSuite struct{}
+
+func (s *ConfigSuite) TestInstanceTypesAsArray(c *check.C) {
+       var cluster Cluster
+       yaml.Unmarshal([]byte("InstanceTypes:\n- Name: foo\n"), &cluster)
+       c.Check(len(cluster.InstanceTypes), check.Equals, 1)
+       c.Check(cluster.InstanceTypes["foo"].Name, check.Equals, "foo")
+}
+
+func (s *ConfigSuite) TestInstanceTypesAsHash(c *check.C) {
+       var cluster Cluster
+       yaml.Unmarshal([]byte("InstanceTypes:\n  foo:\n    ProviderType: bar\n"), &cluster)
+       c.Check(len(cluster.InstanceTypes), check.Equals, 1)
+       c.Check(cluster.InstanceTypes["foo"].Name, check.Equals, "foo")
+       c.Check(cluster.InstanceTypes["foo"].ProviderType, check.Equals, "bar")
+}

diff --git a/services/crunch-dispatch-slurm/crunch-dispatch-slurm_test.go b/services/crunch-dispatch-slurm/crunch-dispatch-slurm_test.go
index b4033e78b00abee87e2fb7423281021be5233577..23a8a0ca01124df89575c5724a4b6cb6650527fb 100644 (file)
--- a/services/crunch-dispatch-slurm/crunch-dispatch-slurm_test.go
+++ b/services/crunch-dispatch-slurm/crunch-dispatch-slurm_test.go
@@ -367,17 +367,17 @@ func (s *StubbedSuite) TestSbatchInstanceTypeConstraint(c *C) {
        }
 
        for _, trial := range []struct {
-               types      []arvados.InstanceType
+               types      map[string]arvados.InstanceType
                sbatchArgs []string
                err        error
        }{
                // Choose node type => use --constraint arg
                {
-                       types: []arvados.InstanceType{
-                               {Name: "a1.tiny", Price: 0.02, RAM: 128000000, VCPUs: 1},
-                               {Name: "a1.small", Price: 0.04, RAM: 256000000, VCPUs: 2},
-                               {Name: "a1.medium", Price: 0.08, RAM: 512000000, VCPUs: 4},
-                               {Name: "a1.large", Price: 0.16, RAM: 1024000000, VCPUs: 8},
+                       types: map[string]arvados.InstanceType{
+                               "a1.tiny":   {Name: "a1.tiny", Price: 0.02, RAM: 128000000, VCPUs: 1},
+                               "a1.small":  {Name: "a1.small", Price: 0.04, RAM: 256000000, VCPUs: 2},
+                               "a1.medium": {Name: "a1.medium", Price: 0.08, RAM: 512000000, VCPUs: 4},
+                               "a1.large":  {Name: "a1.large", Price: 0.16, RAM: 1024000000, VCPUs: 8},
                        },
                        sbatchArgs: []string{"--constraint=instancetype=a1.medium"},
                },
@@ -388,8 +388,8 @@ func (s *StubbedSuite) TestSbatchInstanceTypeConstraint(c *C) {
                },
                // No node type is big enough => error
                {
-                       types: []arvados.InstanceType{
-                               {Name: "a1.tiny", Price: 0.02, RAM: 128000000, VCPUs: 1},
+                       types: map[string]arvados.InstanceType{
+                               "a1.tiny": {Name: "a1.tiny", Price: 0.02, RAM: 128000000, VCPUs: 1},
                        },
                        err: dispatchcloud.ConstraintsNotSatisfiableError{},
                },