From 3a11a0b1e12b32de8b00f981113fdf447fd6b504 Mon Sep 17 00:00:00 2001 From: Tim Gross Date: Mon, 13 Jan 2025 09:25:00 -0500 Subject: [PATCH] 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 | 11 ++++++ api/quota.go | 23 ++++++++++- 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, 99 insertions(+), 13 deletions(-) create mode 100644 .changelog/24785.txt diff --git a/.changelog/24785.txt b/.changelog/24785.txt new file mode 100644 index 000000000..810e9df3f --- /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 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 +``` diff --git a/api/quota.go b/api/quota.go index 261d3d1d1..402c57a02 100644 --- a/api/quota.go +++ b/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 diff --git a/api/util_test.go b/api/util_test.go index 0e179b307..769027020 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 84d23d931..1c0373d70 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 1f087b8fd..2cbb64d4e 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 2a7c46ebd..a338e0cb8 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 c83f7ba2d..daa97927c 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 a724f458e..071635422 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.5 #### CNI plugins