quotas: refactor storage limit specification (#24785)

In anticipation of having quotas for dynamic host volumes, we want the user
experience of the storage limits to feel integrated with the other resource
limits. This is currently prevented by reusing the `Resources` type instead of
having a specific type for `QuotaResources`.

Update the quota limit/usage types to use a `QuotaResources` that includes a new
storage resources quota block. The wire format for the two types are compatible
such that we can migrate the existing variables limit in the FSM.

Also fixes improper parallelism in the quota init test where we change working
directory to avoid file write conflicts but this breaks when multiple tests are
executed in the same process.

Ref: https://github.com/hashicorp/nomad-enterprise/pull/2096

.changelog/24785.txt

@@ -0,0 +1,11 @@
+```release-note:breaking-change
+api: QuotaSpec.RegionLimit is now of type QuotaResources instead of Resources
+```
+
+```release-note:deprecation
+api: QuotaSpec.VariablesLimit field is deprecated and will be removed in Nomad 1.12.0. Use QuotaSpec.RegionLimit.Storage.Variables instead.
+```
+
+```release-note:deprecation
+quotas: the variables_limit field in the quota specification is deprecated and replaced by a new storage block under the region_limit block, with a variables field. The variables_limit field will be removed in Nomad 1.12.0
+```

api/quota.go

@@ -127,17 +127,38 @@ type QuotaLimit struct {
 	// referencing namespace in the region. A value of zero is treated as
 	// unlimited and a negative value is treated as fully disallowed. This is
 	// useful for once we support GPUs
-	RegionLimit *Resources
+	RegionLimit *QuotaResources
 
 	// VariablesLimit is the maximum total size of all variables
 	// Variable.EncryptedData. A value of zero is treated as unlimited and a
 	// negative value is treated as fully disallowed.
+	//
+	// DEPRECATED: use RegionLimit.Storage.VariablesMB instead. This field will
+	// be removed in Nomad 1.12.0.
 	VariablesLimit *int `mapstructure:"variables_limit" hcl:"variables_limit,optional"`
 
 	// Hash is the hash of the object and is used to make replication efficient.
 	Hash []byte
 }
 
+type QuotaResources struct {
+	CPU         *int                   `hcl:"cpu,optional"`
+	Cores       *int                   `hcl:"cores,optional"`
+	MemoryMB    *int                   `mapstructure:"memory" hcl:"memory,optional"`
+	MemoryMaxMB *int                   `mapstructure:"memory_max" hcl:"memory_max,optional"`
+	Devices     []*RequestedDevice     `hcl:"device,block"`
+	NUMA        *NUMAResource          `hcl:"numa,block"`
+	SecretsMB   *int                   `mapstructure:"secrets" hcl:"secrets,optional"`
+	Storage     *QuotaStorageResources `mapstructure:"storage" hcl:"storage,block"`
+}
+
+type QuotaStorageResources struct {
+	// VariablesMB is the maximum total size of all variables
+	// Variable.EncryptedData, in megabytes (2^20 bytes). A value of zero is
+	// treated as unlimited and a negative value is treated as fully disallowed.
+	VariablesMB int `hcl:"variables"`
+}
+
 // QuotaUsage is the resource usage of a Quota
 type QuotaUsage struct {
 	Name        string

api/util_test.go

@@ -116,7 +116,7 @@ func testQuotaSpec() *QuotaSpec {
 		Limits: []*QuotaLimit{
 			{
 				Region: "global",
-				RegionLimit: &Resources{
+				RegionLimit: &QuotaResources{
 					CPU:      pointerOf(2000),
 					MemoryMB: pointerOf(2000),
 					Devices: []*RequestedDevice{{

command/quota_apply.go

@@ -6,6 +6,7 @@ package command
 import (
 	"bytes"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"io"
 	"os"
@@ -231,7 +232,7 @@ func parseQuotaLimits(result *[]*api.QuotaLimit, list *ast.ObjectList) error {
 
 		// Parse limits
 		if o := listVal.Filter("region_limit"); len(o.Items) > 0 {
-			limit.RegionLimit = new(api.Resources)
+			limit.RegionLimit = new(api.QuotaResources)
 			if err := parseQuotaResource(limit.RegionLimit, o); err != nil {
 				return multierror.Prefix(err, "region_limit ->")
 			}
@@ -244,7 +245,7 @@ func parseQuotaLimits(result *[]*api.QuotaLimit, list *ast.ObjectList) error {
 }
 
 // parseQuotaResource parses the region_limit resources
-func parseQuotaResource(result *api.Resources, list *ast.ObjectList) error {
+func parseQuotaResource(result *api.QuotaResources, list *ast.ObjectList) error {
 	list = list.Elem()
 	if len(list.Items) == 0 {
 		return nil
@@ -271,6 +272,7 @@ func parseQuotaResource(result *api.Resources, list *ast.ObjectList) error {
 		"memory",
 		"memory_max",
 		"device",
+		"storage",
 	}
 	if err := helper.CheckHCLKeys(listVal, valid); err != nil {
 		return multierror.Prefix(err, "resources ->")
@@ -283,6 +285,7 @@ func parseQuotaResource(result *api.Resources, list *ast.ObjectList) error {
 
 	// Manually parse
 	delete(m, "device")
+	delete(m, "storage")
 
 	if err := mapstructure.WeakDecode(m, result); err != nil {
 		return err
@@ -296,9 +299,41 @@ func parseQuotaResource(result *api.Resources, list *ast.ObjectList) error {
 		}
 	}
 
+	// Parse storage block
+	storageBlocks := listVal.Filter("storage")
+	storage, err := parseStorageResource(storageBlocks)
+	if err != nil {
+		return multierror.Prefix(err, "storage ->")
+	}
+	result.Storage = storage
+
 	return nil
 }
 
+func parseStorageResource(storageBlocks *ast.ObjectList) (*api.QuotaStorageResources, error) {
+	switch len(storageBlocks.Items) {
+	case 0:
+		return nil, nil
+	case 1:
+	default:
+		return nil, errors.New("only one storage block is allowed")
+	}
+	block := storageBlocks.Items[0]
+	valid := []string{"variables"}
+	if err := helper.CheckHCLKeys(block.Val, valid); err != nil {
+		return nil, err
+	}
+	var storage api.QuotaStorageResources
+	var m map[string]interface{}
+	if err := hcl.DecodeObject(&storage, block.Val); err != nil {
+		return nil, err
+	}
+	if err := mapstructure.WeakDecode(m, &storage); err != nil {
+		return nil, err
+	}
+	return &storage, nil
+}
+
 func parseDeviceResource(result *[]*api.RequestedDevice, list *ast.ObjectList) error {
 	for idx, o := range list.Items {
 		if l := len(o.Keys); l == 0 {

command/quota_delete_test.go

@@ -101,7 +101,7 @@ func testQuotaSpec() *api.QuotaSpec {
 		Limits: []*api.QuotaLimit{
 			{
 				Region: "global",
-				RegionLimit: &api.Resources{
+				RegionLimit: &api.QuotaResources{
 					CPU: pointer.Of(100),
 				},
 			},

command/quota_init.go

@@ -126,8 +126,10 @@ limit {
     device "nvidia/gpu/1080ti" {
       count = 1
     }
+    storage {
+      variables = 1000
+    }
   }
-  variables_limit = 1000
 }
 `)
 
@@ -148,9 +150,11 @@ var defaultJsonQuotaSpec = strings.TrimSpace(`
             "Name": "nvidia/gpu/1080ti",
             "Count": 1
           }
-        ]
-      },
-      "VariablesLimit": 1000
+        ],
+       "Storage": {
+       "Variables": 1000
+}
+      }
     }
   ]
 }

command/quota_init_test.go

@@ -18,7 +18,6 @@ func TestQuotaInitCommand_Implements(t *testing.T) {
 }
 
 func TestQuotaInitCommand_Run_HCL(t *testing.T) {
-	ci.Parallel(t)
 	ui := cli.NewMockUi()
 	cmd := &QuotaInitCommand{Meta: Meta{Ui: ui}}
 
@@ -31,7 +30,7 @@ func TestQuotaInitCommand_Run_HCL(t *testing.T) {
 	// Ensure we change the cwd back
 	origDir, err := os.Getwd()
 	must.NoError(t, err)
-	defer os.Chdir(origDir)
+	t.Cleanup(func() { os.Chdir(origDir) })
 
 	// Create a temp dir and change into it
 	dir := t.TempDir()
@@ -65,7 +64,6 @@ func TestQuotaInitCommand_Run_HCL(t *testing.T) {
 }
 
 func TestQuotaInitCommand_Run_JSON(t *testing.T) {
-	ci.Parallel(t)
 	ui := cli.NewMockUi()
 	cmd := &QuotaInitCommand{Meta: Meta{Ui: ui}}
 
@@ -78,7 +76,7 @@ func TestQuotaInitCommand_Run_JSON(t *testing.T) {
 	// Ensure we change the cwd back
 	origDir, err := os.Getwd()
 	must.NoError(t, err)
-	defer os.Chdir(origDir)
+	t.Cleanup(func() { os.Chdir(origDir) })
 
 	// Create a temp dir and change into it
 	dir := t.TempDir()

website/content/docs/upgrade/upgrade-specific.mdx

@@ -13,6 +13,23 @@ upgrade. However, specific versions of Nomad may have more details provided for
 their upgrades as a result of new features or changed behavior. This page is
 used to document those details separately from the standard upgrade flow.
 
+## Nomad 1.10.0
+
+#### Quota specification variable_limits deprecated
+
+In Nomad 1.10.0, the quota specification's `variable_limits` field is
+deprecated. It is replaced by a new `storage` block with a `variables` field,
+under the `region_limit` block. Existing quotas will be automatically migrated
+during server upgrade. The `variables_limit` field will be removed from the
+quota specification in Nomad 1.12.0.
+
+#### Go SDK API change for quota limits
+
+In Nomad 1.10.0, the Go API for quotas has a breaking change. The
+`QuotaSpec.RegionLimit` field is now of type `QuotaResources` instead of
+`Resources`. The `QuotaSpec.VariablesLimit` field is deprecated in lieu of
+`QuotaSpec.RegionLimit.Storage.Variables` and will be removed in Nomad 1.12.0.
+
 ## Nomad 1.9.5
 
 #### CNI plugins