From: Ward Vandewege <ward@curii.com>
Date: Fri, 13 May 2022 14:13:29 +0000 (-0400)
Subject: Merge branch '19127-remove-warnings'
X-Git-Tag: 2.5.0~181
X-Git-Url: https://git.arvados.org/arvados.git/commitdiff_plain/44a296b1c976ebf546cbd4b2444ec2b41c571f88?hp=01f48e4a0d3e979c8007ecf91d3386873961c256

Merge branch '19127-remove-warnings'

closes #19127

Arvados-DCO-1.1-Signed-off-by: Ward Vandewege <ward@curii.com>
---

diff --git a/lib/config/load_test.go b/lib/config/load_test.go
index abf3217056..4ae9a513c8 100644
--- a/lib/config/load_test.go
+++ b/lib/config/load_test.go
@@ -316,7 +316,11 @@ Clusters:
   ManagementToken: aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
   SystemRootToken: aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
   Collections:
-   BlobSigningKey: aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa`, &logbuf).Load()
+   BlobSigningKey: aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
+  InstanceTypes:
+   abc:
+    IncludedScratch: 123456
+`, &logbuf).Load()
 	c.Assert(err, check.IsNil)
 	yaml, err := yaml.Marshal(cfg)
 	c.Assert(err, check.IsNil)
diff --git a/sdk/go/arvados/config.go b/sdk/go/arvados/config.go
index f0adcda5f1..6a90c30ce4 100644
--- a/sdk/go/arvados/config.go
+++ b/sdk/go/arvados/config.go
@@ -424,11 +424,11 @@ type CUDAFeatures struct {
 }
 
 type InstanceType struct {
-	Name            string
+	Name            string `json:"-"`
 	ProviderType    string
 	VCPUs           int
 	RAM             ByteSize
-	Scratch         ByteSize
+	Scratch         ByteSize `json:"-"`
 	IncludedScratch ByteSize
 	AddedScratch    ByteSize
 	Price           float64
@@ -528,49 +528,23 @@ 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.
+// UnmarshalJSON does special handling of InstanceTypes:
+// * populate computed fields (Name and Scratch)
+// * error out if InstancesTypes are populated as an array, which was
+//   deprecated in Arvados 1.2.0
 func (it *InstanceTypeMap) UnmarshalJSON(data []byte) error {
 	fixup := func(t InstanceType) (InstanceType, error) {
 		if t.ProviderType == "" {
 			t.ProviderType = t.Name
 		}
-		if t.Scratch == 0 {
-			t.Scratch = t.IncludedScratch + t.AddedScratch
-		} else if t.AddedScratch == 0 {
-			t.AddedScratch = t.Scratch - t.IncludedScratch
-		} else if t.IncludedScratch == 0 {
-			t.IncludedScratch = t.Scratch - t.AddedScratch
-		}
-
-		if t.Scratch != (t.IncludedScratch + t.AddedScratch) {
-			return t, fmt.Errorf("InstanceType %q: Scratch != (IncludedScratch + AddedScratch)", t.Name)
-		}
+		// If t.Scratch is set in the configuration file, it will be ignored and overwritten.
+		// It will also generate a "deprecated or unknown config entry" warning.
+		t.Scratch = t.IncludedScratch + t.AddedScratch
 		return t, nil
 	}
 
 	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
-			}
-			t, err := fixup(t)
-			if err != nil {
-				return err
-			}
-			(*it)[t.Name] = t
-		}
-		return nil
+		return fmt.Errorf("InstanceTypes must be specified as a map, not an array, see https://doc.arvados.org/admin/config.html")
 	}
 	var hash map[string]InstanceType
 	err := json.Unmarshal(data, &hash)
diff --git a/sdk/go/arvados/config_test.go b/sdk/go/arvados/config_test.go
index 8c77e29287..58f4b961bb 100644
--- a/sdk/go/arvados/config_test.go
+++ b/sdk/go/arvados/config_test.go
@@ -15,7 +15,7 @@ var _ = check.Suite(&ConfigSuite{})
 
 type ConfigSuite struct{}
 
-func (s *ConfigSuite) TestInstanceTypesAsArray(c *check.C) {
+func (s *ConfigSuite) TestStringSetAsArray(c *check.C) {
 	var cluster Cluster
 	yaml.Unmarshal([]byte(`
 API:
@@ -25,13 +25,6 @@ API:
 	c.Check(ok, check.Equals, true)
 }
 
-func (s *ConfigSuite) TestStringSetAsArray(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)
@@ -42,18 +35,16 @@ func (s *ConfigSuite) TestInstanceTypesAsHash(c *check.C) {
 
 func (s *ConfigSuite) TestInstanceTypeSize(c *check.C) {
 	var it InstanceType
-	err := yaml.Unmarshal([]byte("Name: foo\nScratch: 4GB\nRAM: 4GiB\n"), &it)
+	err := yaml.Unmarshal([]byte("Name: foo\nIncludedScratch: 4GB\nRAM: 4GiB\n"), &it)
 	c.Check(err, check.IsNil)
-	c.Check(int64(it.Scratch), check.Equals, int64(4000000000))
+	c.Check(int64(it.IncludedScratch), check.Equals, int64(4000000000))
 	c.Check(int64(it.RAM), check.Equals, int64(4294967296))
 }
 
 func (s *ConfigSuite) TestInstanceTypeFixup(c *check.C) {
 	for _, confdata := range []string{
 		// Current format: map of entries
-		`{foo4: {IncludedScratch: 4GB}, foo8: {ProviderType: foo_8, Scratch: 8GB}}`,
-		// Legacy format: array of entries with key in "Name" field
-		`[{Name: foo4, IncludedScratch: 4GB}, {Name: foo8, ProviderType: foo_8, Scratch: 8GB}]`,
+		`{foo4: {IncludedScratch: 4GB}, foo8: {ProviderType: foo_8, AddedScratch: 8GB}}`,
 	} {
 		c.Log(confdata)
 		var itm InstanceTypeMap