From cb8e52b5dfe813ff1a21f3f5841f525c56b77fbd Mon Sep 17 00:00:00 2001 From: Charlie Voiselle <464492+angrycub@users.noreply.github.com> Date: Wed, 13 Oct 2021 21:23:13 -0400 Subject: [PATCH] Return SchedulerConfig instead of SchedulerConfigResponse struct (#10799) --- .changelog/10799.txt | 3 + api/operator.go | 4 +- command/agent/operator_endpoint.go | 2 +- command/agent/operator_endpoint_test.go | 12 ++-- e2e/oversubscription/oversubscription.go | 7 +-- internal/testing/apitests/operator_test.go | 32 +++++----- .../content/api-docs/operator/scheduler.mdx | 61 ++++++++++--------- 7 files changed, 63 insertions(+), 58 deletions(-) create mode 100644 .changelog/10799.txt diff --git a/.changelog/10799.txt b/.changelog/10799.txt new file mode 100644 index 000000000..943e781ca --- /dev/null +++ b/.changelog/10799.txt @@ -0,0 +1,3 @@ +```release-note:breaking-change +api: Return scheduler configuration as top-level result in the `/v1/operator/scheduler/configuration` +``` diff --git a/api/operator.go b/api/operator.go index 7adf79f1c..b2467ed35 100644 --- a/api/operator.go +++ b/api/operator.go @@ -171,8 +171,8 @@ type PreemptionConfig struct { } // SchedulerGetConfiguration is used to query the current Scheduler configuration. -func (op *Operator) SchedulerGetConfiguration(q *QueryOptions) (*SchedulerConfigurationResponse, *QueryMeta, error) { - var resp SchedulerConfigurationResponse +func (op *Operator) SchedulerGetConfiguration(q *QueryOptions) (*SchedulerConfiguration, *QueryMeta, error) { + var resp SchedulerConfiguration qm, err := op.c.query("/v1/operator/scheduler/configuration", &resp, q) if err != nil { return nil, nil, err diff --git a/command/agent/operator_endpoint.go b/command/agent/operator_endpoint.go index 78da1fd2c..fbb0368cb 100644 --- a/command/agent/operator_endpoint.go +++ b/command/agent/operator_endpoint.go @@ -246,7 +246,7 @@ func (s *HTTPServer) schedulerGetConfig(resp http.ResponseWriter, req *http.Requ } setMeta(resp, &reply.QueryMeta) - return reply, nil + return reply.SchedulerConfig, nil } func (s *HTTPServer) schedulerUpdateConfig(resp http.ResponseWriter, req *http.Request) (interface{}, error) { diff --git a/command/agent/operator_endpoint_test.go b/command/agent/operator_endpoint_test.go index b621d7d4e..71df383f8 100644 --- a/command/agent/operator_endpoint_test.go +++ b/command/agent/operator_endpoint_test.go @@ -277,15 +277,15 @@ func TestOperator_SchedulerGetConfiguration(t *testing.T) { obj, err := s.Server.OperatorSchedulerConfiguration(resp, req) require.Nil(err) require.Equal(200, resp.Code) - out, ok := obj.(structs.SchedulerConfigurationResponse) + out, ok := obj.(*structs.SchedulerConfiguration) require.True(ok) // Only system jobs can preempt other jobs by default. - require.True(out.SchedulerConfig.PreemptionConfig.SystemSchedulerEnabled) - require.False(out.SchedulerConfig.PreemptionConfig.SysBatchSchedulerEnabled) - require.False(out.SchedulerConfig.PreemptionConfig.BatchSchedulerEnabled) - require.False(out.SchedulerConfig.PreemptionConfig.ServiceSchedulerEnabled) - require.False(out.SchedulerConfig.MemoryOversubscriptionEnabled) + require.True(out.PreemptionConfig.SystemSchedulerEnabled) + require.False(out.PreemptionConfig.SysBatchSchedulerEnabled) + require.False(out.PreemptionConfig.BatchSchedulerEnabled) + require.False(out.PreemptionConfig.ServiceSchedulerEnabled) + require.False(out.MemoryOversubscriptionEnabled) }) } diff --git a/e2e/oversubscription/oversubscription.go b/e2e/oversubscription/oversubscription.go index e2c082bee..3d1c4ed40 100644 --- a/e2e/oversubscription/oversubscription.go +++ b/e2e/oversubscription/oversubscription.go @@ -41,11 +41,10 @@ func (tc *OversubscriptionTest) enableMemoryOversubscription(f *framework.F) { resp, _, err := tc.Nomad().Operator().SchedulerGetConfiguration(nil) f.NoError(err) - tc.initialSchedulerConfig = resp.SchedulerConfig + tc.initialSchedulerConfig = resp - conf := *resp.SchedulerConfig - conf.MemoryOversubscriptionEnabled = true - _, _, err = tc.Nomad().Operator().SchedulerSetConfiguration(&conf, nil) + resp.MemoryOversubscriptionEnabled = true + _, _, err = tc.Nomad().Operator().SchedulerSetConfiguration(resp, nil) f.NoError(err) } diff --git a/internal/testing/apitests/operator_test.go b/internal/testing/apitests/operator_test.go index bdf7477ad..e8c004a1c 100644 --- a/internal/testing/apitests/operator_test.go +++ b/internal/testing/apitests/operator_test.go @@ -15,15 +15,15 @@ func TestAPI_OperatorSchedulerGetSetConfiguration(t *testing.T) { defer s.Stop() operator := c.Operator() - var config *api.SchedulerConfigurationResponse + var config *api.SchedulerConfiguration retry.Run(t, func(r *retry.R) { var err error config, _, err = operator.SchedulerGetConfiguration(nil) r.Check(err) }) - require.True(config.SchedulerConfig.PreemptionConfig.SystemSchedulerEnabled) - require.False(config.SchedulerConfig.PreemptionConfig.BatchSchedulerEnabled) - require.False(config.SchedulerConfig.PreemptionConfig.ServiceSchedulerEnabled) + require.True(config.PreemptionConfig.SystemSchedulerEnabled) + require.False(config.PreemptionConfig.BatchSchedulerEnabled) + require.False(config.PreemptionConfig.ServiceSchedulerEnabled) // Change a config setting newConf := &api.SchedulerConfiguration{ @@ -41,9 +41,9 @@ func TestAPI_OperatorSchedulerGetSetConfiguration(t *testing.T) { config, _, err = operator.SchedulerGetConfiguration(nil) require.Nil(err) - require.False(config.SchedulerConfig.PreemptionConfig.SystemSchedulerEnabled) - require.True(config.SchedulerConfig.PreemptionConfig.BatchSchedulerEnabled) - require.True(config.SchedulerConfig.PreemptionConfig.ServiceSchedulerEnabled) + require.False(config.PreemptionConfig.SystemSchedulerEnabled) + require.True(config.PreemptionConfig.BatchSchedulerEnabled) + require.True(config.PreemptionConfig.ServiceSchedulerEnabled) } func TestAPI_OperatorSchedulerCASConfiguration(t *testing.T) { @@ -53,21 +53,21 @@ func TestAPI_OperatorSchedulerCASConfiguration(t *testing.T) { defer s.Stop() operator := c.Operator() - var config *api.SchedulerConfigurationResponse + var config *api.SchedulerConfiguration retry.Run(t, func(r *retry.R) { var err error config, _, err = operator.SchedulerGetConfiguration(nil) r.Check(err) }) - require.True(config.SchedulerConfig.PreemptionConfig.SystemSchedulerEnabled) - require.False(config.SchedulerConfig.PreemptionConfig.BatchSchedulerEnabled) - require.False(config.SchedulerConfig.PreemptionConfig.ServiceSchedulerEnabled) + require.True(config.PreemptionConfig.SystemSchedulerEnabled) + require.False(config.PreemptionConfig.BatchSchedulerEnabled) + require.False(config.PreemptionConfig.ServiceSchedulerEnabled) // Pass an invalid ModifyIndex { newConf := &api.SchedulerConfiguration{ PreemptionConfig: api.PreemptionConfig{SystemSchedulerEnabled: false, BatchSchedulerEnabled: true}, - ModifyIndex: config.SchedulerConfig.ModifyIndex - 1, + ModifyIndex: config.ModifyIndex - 1, } resp, wm, err := operator.SchedulerCASConfiguration(newConf, nil) require.Nil(err) @@ -79,7 +79,7 @@ func TestAPI_OperatorSchedulerCASConfiguration(t *testing.T) { { newConf := &api.SchedulerConfiguration{ PreemptionConfig: api.PreemptionConfig{SystemSchedulerEnabled: false, BatchSchedulerEnabled: true}, - ModifyIndex: config.SchedulerConfig.ModifyIndex, + ModifyIndex: config.ModifyIndex, } resp, wm, err := operator.SchedulerCASConfiguration(newConf, nil) require.Nil(err) @@ -89,7 +89,7 @@ func TestAPI_OperatorSchedulerCASConfiguration(t *testing.T) { config, _, err := operator.SchedulerGetConfiguration(nil) require.Nil(err) - require.False(config.SchedulerConfig.PreemptionConfig.SystemSchedulerEnabled) - require.True(config.SchedulerConfig.PreemptionConfig.BatchSchedulerEnabled) - require.False(config.SchedulerConfig.PreemptionConfig.ServiceSchedulerEnabled) + require.False(config.PreemptionConfig.SystemSchedulerEnabled) + require.True(config.PreemptionConfig.BatchSchedulerEnabled) + require.False(config.PreemptionConfig.ServiceSchedulerEnabled) } diff --git a/website/content/api-docs/operator/scheduler.mdx b/website/content/api-docs/operator/scheduler.mdx index cb44b2d0f..3b9365f3d 100644 --- a/website/content/api-docs/operator/scheduler.mdx +++ b/website/content/api-docs/operator/scheduler.mdx @@ -37,48 +37,51 @@ $ curl \ ```json { - "Index": 5, - "KnownLeader": true, - "LastContact": 0, - "SchedulerConfig": { - "CreateIndex": 5, - "ModifyIndex": 5, - "SchedulerAlgorithm": "spread", - "MemoryOversubscriptionEnabled": true, - "PreemptionConfig": { - "SystemSchedulerEnabled": true, - "BatchSchedulerEnabled": false, - "ServiceSchedulerEnabled": false - } - } + "CreateIndex": 5, + "MemoryOversubscriptionEnabled": false, + "ModifyIndex": 5, + "PreemptionConfig": { + "BatchSchedulerEnabled": false, + "ServiceSchedulerEnabled": false, + "SysBatchSchedulerEnabled": false, + "SystemSchedulerEnabled": true + }, + "SchedulerAlgorithm": "binpack" } ``` #### Field Reference -- `Index` `(int)` - The `Index` value is the Raft index corresponding to this - configuration. +- `SchedulerAlgorithm` `(string: "binpack")` - Specifies whether scheduler + binpacks or spreads allocations on available nodes. -- `SchedulerConfig` `(SchedulerConfig)` - The returned `SchedulerConfig` object has configuration - settings mentioned below. +- `MemoryOversubscriptionEnabled` `(bool: false)` 1.1 or later - When + `true`, tasks may exceed their reserved memory limit, if the client has excess + memory capacity. Tasks must specify + [`memory_max`](/docs/job-specification/resources#memory_max) to take advantage + of memory oversubscription. - - `SchedulerAlgorithm` `(string: "binpack")` - Specifies whether scheduler binpacks or spreads allocations on available nodes. +- `PreemptionConfig` `(PreemptionConfig)` - Options to enable preemption for + various schedulers. - - `MemoryOversubscriptionEnabled` `(bool: false)` 1.1 Beta - When `true`, tasks may exceed their reserved memory limit, if the client has excess memory capacity. Tasks must specify [`memory_max`](/docs/job-specification/resources#memory_max) to take advantage of memory oversubscription. + - `SystemSchedulerEnabled` `(bool: true)` - Specifies whether preemption for + system jobs is enabled. Note that this defaults to true. - - `PreemptionConfig` `(PreemptionConfig)` - Options to enable preemption for various schedulers. + - `SysBatchSchedulerEnabled` `(bool: false)` - Specifies whether preemption + for sysbatch jobs is enabled. Note that this defaults to false and must be + explicitly enabled. - - `SystemSchedulerEnabled` `(bool: true)` - Specifies whether preemption for system jobs is enabled. Note that - this defaults to true. + - `BatchSchedulerEnabled` `(bool: false)` - Specifies whether preemption for + batch jobs is enabled. Note that this defaults to false and must be explicitly + enabled. - - `BatchSchedulerEnabled` `(bool: false)` - Specifies whether preemption for batch jobs is enabled. Note that - this defaults to false and must be explicitly enabled. + - `ServiceSchedulerEnabled` `(bool: false)` - Specifies whether preemption for + service jobs is enabled. Note that this defaults to false and must be + explicitly enabled. - - `ServiceSchedulerEnabled` `(bool: false)` - Specifies whether preemption for service jobs is enabled. Note that - this defaults to false and must be explicitly enabled. +- `CreateIndex` - The Raft index at which the configuration was created. - - `CreateIndex` - The Raft index at which the config was created. - - `ModifyIndex` - The Raft index at which the config was modified. +- `ModifyIndex` - The Raft index at which the configuration was modified. ## Update Scheduler Configuration