From 5dbef57a6b51fcaf1f616b564edf8f357799620f Mon Sep 17 00:00:00 2001 From: Tim Gross Date: Mon, 6 Jan 2025 11:25:38 -0500 Subject: [PATCH] quotas: refactor storage limit specification 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 | 11 ++++++ api/quota.go | 24 +++++++++++- api/util_test.go | 2 +- command/quota_apply.go | 39 ++++++++++++++++++- command/quota_delete_test.go | 2 +- command/quota_init.go | 12 ++++-- command/quota_init_test.go | 6 +-- .../content/docs/upgrade/upgrade-specific.mdx | 17 ++++++++ 8 files changed, 100 insertions(+), 13 deletions(-) create mode 100644 .changelog/24785.txt diff --git a/.changelog/24785.txt b/.changelog/24785.txt new file mode 100644 index 00000000000..8f43b9061ee --- /dev/null +++ b/.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 in lieu of QuotaSpec.RegionLimit.Storage.Variables and will be removed in Nomad 1.12.0 +``` + +```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 +``` diff --git a/api/quota.go b/api/quota.go index 261d3d1d101..6f73c9a329a 100644 --- a/api/quota.go +++ b/api/quota.go @@ -127,17 +127,39 @@ 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"` + DiskMB *int `mapstructure:"disk" hcl:"disk,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 diff --git a/api/util_test.go b/api/util_test.go index 0e179b307a6..769027020da 100644 --- a/api/util_test.go +++ b/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{{ diff --git a/command/quota_apply.go b/command/quota_apply.go index 84d23d931b4..1c0373d703c 100644 --- a/command/quota_apply.go +++ b/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 { diff --git a/command/quota_delete_test.go b/command/quota_delete_test.go index 1f087b8fdc9..2cbb64d4e6a 100644 --- a/command/quota_delete_test.go +++ b/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), }, }, diff --git a/command/quota_init.go b/command/quota_init.go index 2a7c46ebd26..a338e0cb86f 100644 --- a/command/quota_init.go +++ b/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 +} + } } ] } diff --git a/command/quota_init_test.go b/command/quota_init_test.go index c83f7ba2dbb..daa97927ce3 100644 --- a/command/quota_init_test.go +++ b/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() diff --git a/website/content/docs/upgrade/upgrade-specific.mdx b/website/content/docs/upgrade/upgrade-specific.mdx index 2734093f0be..5a1e08fbf34 100644 --- a/website/content/docs/upgrade/upgrade-specific.mdx +++ b/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.4 #### Security updates to default deny lists