From c22b76720c8229bc0ec983ef30bc3124edb39088 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maximilian=20Pa=C3=9F?= <22845248+mpass99@users.noreply.github.com> Date: Wed, 22 Dec 2021 14:41:53 +0100 Subject: [PATCH] Add documentation for guarding the Nomad tasks --- internal/environment/environment.go | 39 +++++++++++++----------- internal/environment/environment_test.go | 20 ++++++------ internal/environment/manager.go | 2 +- internal/environment/manager_test.go | 4 +-- internal/nomad/job.go | 24 ++++++++------- internal/nomad/job_test.go | 12 ++++---- internal/nomad/nomad.go | 2 +- 7 files changed, 54 insertions(+), 49 deletions(-) diff --git a/internal/environment/environment.go b/internal/environment/environment.go index 0318180..f3389d3 100644 --- a/internal/environment/environment.go +++ b/internal/environment/environment.go @@ -68,7 +68,7 @@ func (n *NomadEnvironment) SetID(id dto.EnvironmentID) { } func (n *NomadEnvironment) PrewarmingPoolSize() uint { - configTaskGroup := nomad.FindOrCreateConfigTaskGroup(n.job) + configTaskGroup := nomad.FindAndValidateConfigTaskGroup(n.job) count, err := strconv.Atoi(configTaskGroup.Meta[nomad.ConfigMetaPoolSizeKey]) if err != nil { log.WithError(err).Error("Prewarming pool size can not be parsed from Job") @@ -77,7 +77,7 @@ func (n *NomadEnvironment) PrewarmingPoolSize() uint { } func (n *NomadEnvironment) SetPrewarmingPoolSize(count uint) { - taskGroup := nomad.FindOrCreateConfigTaskGroup(n.job) + taskGroup := nomad.FindAndValidateConfigTaskGroup(n.job) if taskGroup.Meta == nil { taskGroup.Meta = make(map[string]string) @@ -86,36 +86,36 @@ func (n *NomadEnvironment) SetPrewarmingPoolSize(count uint) { } func (n *NomadEnvironment) CPULimit() uint { - defaultTaskGroup := nomad.FindOrCreateDefaultTaskGroup(n.job) - defaultTask := nomad.FindOrCreateDefaultTask(defaultTaskGroup) + defaultTaskGroup := nomad.FindAndValidateDefaultTaskGroup(n.job) + defaultTask := nomad.FindAndValidateDefaultTask(defaultTaskGroup) return uint(*defaultTask.Resources.CPU) } func (n *NomadEnvironment) SetCPULimit(limit uint) { - defaultTaskGroup := nomad.FindOrCreateDefaultTaskGroup(n.job) - defaultTask := nomad.FindOrCreateDefaultTask(defaultTaskGroup) + defaultTaskGroup := nomad.FindAndValidateDefaultTaskGroup(n.job) + defaultTask := nomad.FindAndValidateDefaultTask(defaultTaskGroup) integerCPULimit := int(limit) defaultTask.Resources.CPU = &integerCPULimit } func (n *NomadEnvironment) MemoryLimit() uint { - defaultTaskGroup := nomad.FindOrCreateDefaultTaskGroup(n.job) - defaultTask := nomad.FindOrCreateDefaultTask(defaultTaskGroup) + defaultTaskGroup := nomad.FindAndValidateDefaultTaskGroup(n.job) + defaultTask := nomad.FindAndValidateDefaultTask(defaultTaskGroup) return uint(*defaultTask.Resources.MemoryMB) } func (n *NomadEnvironment) SetMemoryLimit(limit uint) { - defaultTaskGroup := nomad.FindOrCreateDefaultTaskGroup(n.job) - defaultTask := nomad.FindOrCreateDefaultTask(defaultTaskGroup) + defaultTaskGroup := nomad.FindAndValidateDefaultTaskGroup(n.job) + defaultTask := nomad.FindAndValidateDefaultTask(defaultTaskGroup) integerMemoryLimit := int(limit) defaultTask.Resources.MemoryMB = &integerMemoryLimit } func (n *NomadEnvironment) Image() string { - defaultTaskGroup := nomad.FindOrCreateDefaultTaskGroup(n.job) - defaultTask := nomad.FindOrCreateDefaultTask(defaultTaskGroup) + defaultTaskGroup := nomad.FindAndValidateDefaultTaskGroup(n.job) + defaultTask := nomad.FindAndValidateDefaultTask(defaultTaskGroup) image, ok := defaultTask.Config["image"].(string) if !ok { image = "" @@ -124,14 +124,14 @@ func (n *NomadEnvironment) Image() string { } func (n *NomadEnvironment) SetImage(image string) { - defaultTaskGroup := nomad.FindOrCreateDefaultTaskGroup(n.job) - defaultTask := nomad.FindOrCreateDefaultTask(defaultTaskGroup) + defaultTaskGroup := nomad.FindAndValidateDefaultTaskGroup(n.job) + defaultTask := nomad.FindAndValidateDefaultTask(defaultTaskGroup) defaultTask.Config["image"] = image } func (n *NomadEnvironment) NetworkAccess() (allowed bool, ports []uint16) { - defaultTaskGroup := nomad.FindOrCreateDefaultTaskGroup(n.job) - defaultTask := nomad.FindOrCreateDefaultTask(defaultTaskGroup) + defaultTaskGroup := nomad.FindAndValidateDefaultTaskGroup(n.job) + defaultTask := nomad.FindAndValidateDefaultTask(defaultTaskGroup) allowed = defaultTask.Config["network_mode"] != "none" if len(defaultTaskGroup.Networks) > 0 { @@ -144,8 +144,8 @@ func (n *NomadEnvironment) NetworkAccess() (allowed bool, ports []uint16) { } func (n *NomadEnvironment) SetNetworkAccess(allow bool, exposedPorts []uint16) { - defaultTaskGroup := nomad.FindOrCreateDefaultTaskGroup(n.job) - defaultTask := nomad.FindOrCreateDefaultTask(defaultTaskGroup) + defaultTaskGroup := nomad.FindAndValidateDefaultTaskGroup(n.job) + defaultTask := nomad.FindAndValidateDefaultTask(defaultTaskGroup) if len(defaultTaskGroup.Tasks) == 0 { // This function is only used internally and must be called as last step when configuring the task. @@ -284,6 +284,9 @@ func (n *NomadEnvironment) DeepCopyJob() *nomadApi.Job { return copyEnvironment.job } +// SetConfigFrom gets the options from the environment job and saves it into another temporary job. +// IMPROVE: The getters use a validation function that theoretically could edit the environment job. +// But this modification might never been saved to Nomad. func (n *NomadEnvironment) SetConfigFrom(environment runner.ExecutionEnvironment) { n.SetID(environment.ID()) n.SetPrewarmingPoolSize(environment.PrewarmingPoolSize()) diff --git a/internal/environment/environment_test.go b/internal/environment/environment_test.go index 87a2e07..1f1268d 100644 --- a/internal/environment/environment_test.go +++ b/internal/environment/environment_test.go @@ -16,7 +16,7 @@ import ( func TestConfigureNetworkCreatesNewNetworkWhenNoNetworkExists(t *testing.T) { _, job := helpers.CreateTemplateJob() - defaultTaskGroup := nomad.FindOrCreateDefaultTaskGroup(job) + defaultTaskGroup := nomad.FindAndValidateDefaultTaskGroup(job) environment := &NomadEnvironment{"", job, nil} if assert.Equal(t, 0, len(defaultTaskGroup.Networks)) { @@ -28,7 +28,7 @@ func TestConfigureNetworkCreatesNewNetworkWhenNoNetworkExists(t *testing.T) { func TestConfigureNetworkDoesNotCreateNewNetworkWhenNetworkExists(t *testing.T) { _, job := helpers.CreateTemplateJob() - defaultTaskGroup := nomad.FindOrCreateDefaultTaskGroup(job) + defaultTaskGroup := nomad.FindAndValidateDefaultTaskGroup(job) environment := &NomadEnvironment{"", job, nil} networkResource := &nomadApi.NetworkResource{Mode: "bridge"} @@ -44,8 +44,8 @@ func TestConfigureNetworkDoesNotCreateNewNetworkWhenNetworkExists(t *testing.T) func TestConfigureNetworkSetsCorrectValues(t *testing.T) { _, job := helpers.CreateTemplateJob() - defaultTaskGroup := nomad.FindOrCreateDefaultTaskGroup(job) - defaultTask := nomad.FindOrCreateDefaultTask(defaultTaskGroup) + defaultTaskGroup := nomad.FindAndValidateDefaultTaskGroup(job) + defaultTask := nomad.FindAndValidateDefaultTask(defaultTaskGroup) mode, ok := defaultTask.Config["network_mode"] assert.True(t, ok) @@ -56,8 +56,8 @@ func TestConfigureNetworkSetsCorrectValues(t *testing.T) { t.Run("with no network access", func(t *testing.T) { for _, ports := range exposedPortsTests { _, testJob := helpers.CreateTemplateJob() - testTaskGroup := nomad.FindOrCreateDefaultTaskGroup(testJob) - testTask := nomad.FindOrCreateDefaultTask(testTaskGroup) + testTaskGroup := nomad.FindAndValidateDefaultTaskGroup(testJob) + testTask := nomad.FindAndValidateDefaultTask(testTaskGroup) testEnvironment := &NomadEnvironment{"", job, nil} testEnvironment.SetNetworkAccess(false, ports) @@ -71,8 +71,8 @@ func TestConfigureNetworkSetsCorrectValues(t *testing.T) { t.Run("with network access", func(t *testing.T) { for _, ports := range exposedPortsTests { _, testJob := helpers.CreateTemplateJob() - testTaskGroup := nomad.FindOrCreateDefaultTaskGroup(testJob) - testTask := nomad.FindOrCreateDefaultTask(testTaskGroup) + testTaskGroup := nomad.FindAndValidateDefaultTaskGroup(testJob) + testTask := nomad.FindAndValidateDefaultTask(testTaskGroup) testEnvironment := &NomadEnvironment{"", testJob, nil} testEnvironment.SetNetworkAccess(true, ports) @@ -197,8 +197,8 @@ func TestSampleDoesNotSetForcePullFlag(t *testing.T) { job, ok := args.Get(0).(*nomadApi.Job) assert.True(t, ok) - taskGroup := nomad.FindOrCreateDefaultTaskGroup(job) - task := nomad.FindOrCreateDefaultTask(taskGroup) + taskGroup := nomad.FindAndValidateDefaultTaskGroup(job) + task := nomad.FindAndValidateDefaultTask(taskGroup) assert.False(t, task.Config["force_pull"].(bool)) call.ReturnArguments = mock.Arguments{nil} diff --git a/internal/environment/manager.go b/internal/environment/manager.go index 1dd5321..d9e3c5d 100644 --- a/internal/environment/manager.go +++ b/internal/environment/manager.go @@ -178,7 +178,7 @@ func (m *NomadEnvironmentManager) Load() error { jobLogger.Info("Job not running, skipping ...") continue } - configTaskGroup := nomad.FindOrCreateConfigTaskGroup(job) + configTaskGroup := nomad.FindAndValidateConfigTaskGroup(job) if configTaskGroup == nil { jobLogger.Info("Couldn't find config task group in job, skipping ...") continue diff --git a/internal/environment/manager_test.go b/internal/environment/manager_test.go index 9e5a30d..72a0cab 100644 --- a/internal/environment/manager_test.go +++ b/internal/environment/manager_test.go @@ -79,8 +79,8 @@ func (s *CreateOrUpdateTestSuite) TestCreateOrUpdatesSetsForcePullFlag() { // The environment job itself has not the force_pull flag if count > 1 { - taskGroup := nomad.FindOrCreateDefaultTaskGroup(job) - task := nomad.FindOrCreateDefaultTask(taskGroup) + taskGroup := nomad.FindAndValidateDefaultTaskGroup(job) + task := nomad.FindAndValidateDefaultTask(taskGroup) s.True(task.Config["force_pull"].(bool)) } diff --git a/internal/nomad/job.go b/internal/nomad/job.go index 91e1acc..6bdfeda 100644 --- a/internal/nomad/job.go +++ b/internal/nomad/job.go @@ -37,7 +37,7 @@ var ( ) func (a *APIClient) RegisterRunnerJob(template *nomadApi.Job) error { - taskGroup := FindOrCreateConfigTaskGroup(template) + taskGroup := FindAndValidateConfigTaskGroup(template) taskGroup.Meta = make(map[string]string) taskGroup.Meta[ConfigMetaUsedKey] = ConfigMetaUnusedValue @@ -61,28 +61,29 @@ func FindTaskGroup(job *nomadApi.Job, name string) *nomadApi.TaskGroup { return nil } -func FindOrCreateDefaultTaskGroup(job *nomadApi.Job) *nomadApi.TaskGroup { +func FindAndValidateDefaultTaskGroup(job *nomadApi.Job) *nomadApi.TaskGroup { taskGroup := FindTaskGroup(job, TaskGroupName) if taskGroup == nil { taskGroup = nomadApi.NewTaskGroup(TaskGroupName, TaskCount) job.AddTaskGroup(taskGroup) } - FindOrCreateDefaultTask(taskGroup) + FindAndValidateDefaultTask(taskGroup) return taskGroup } -func FindOrCreateConfigTaskGroup(job *nomadApi.Job) *nomadApi.TaskGroup { +func FindAndValidateConfigTaskGroup(job *nomadApi.Job) *nomadApi.TaskGroup { taskGroup := FindTaskGroup(job, ConfigTaskGroupName) if taskGroup == nil { taskGroup = nomadApi.NewTaskGroup(ConfigTaskGroupName, 0) job.AddTaskGroup(taskGroup) } - FindOrCreateConfigTask(taskGroup) + FindAndValidateConfigTask(taskGroup) return taskGroup } -// FindOrCreateConfigTask ensures that a dummy task is in the task group so that the group is accepted by Nomad. -func FindOrCreateConfigTask(taskGroup *nomadApi.TaskGroup) *nomadApi.Task { +// FindAndValidateConfigTask returns the config task and +// ensures that a dummy task is in the task group so that the group is accepted by Nomad. It might modify the task. +func FindAndValidateConfigTask(taskGroup *nomadApi.TaskGroup) *nomadApi.Task { var task *nomadApi.Task for _, t := range taskGroup.Tasks { if t.Name == ConfigTaskName { @@ -107,8 +108,9 @@ func FindOrCreateConfigTask(taskGroup *nomadApi.TaskGroup) *nomadApi.Task { return task } -// FindOrCreateDefaultTask ensures that a default task is in the task group in that the executions are made. -func FindOrCreateDefaultTask(taskGroup *nomadApi.TaskGroup) *nomadApi.Task { +// FindAndValidateDefaultTask returns the default task and +// ensures that a default task is in the task group in that the executions are made. It might modify the task. +func FindAndValidateDefaultTask(taskGroup *nomadApi.TaskGroup) *nomadApi.Task { var task *nomadApi.Task for _, t := range taskGroup.Tasks { if t.Name == TaskName { @@ -142,8 +144,8 @@ func FindOrCreateDefaultTask(taskGroup *nomadApi.TaskGroup) *nomadApi.Task { // SetForcePullFlag sets the flag of a job if the image should be pulled again. func SetForcePullFlag(job *nomadApi.Job, value bool) { - taskGroup := FindOrCreateDefaultTaskGroup(job) - task := FindOrCreateDefaultTask(taskGroup) + taskGroup := FindAndValidateDefaultTaskGroup(job) + task := FindAndValidateDefaultTask(taskGroup) task.Config["force_pull"] = value } diff --git a/internal/nomad/job_test.go b/internal/nomad/job_test.go index ed64c14..86f9863 100644 --- a/internal/nomad/job_test.go +++ b/internal/nomad/job_test.go @@ -24,7 +24,7 @@ func TestFindTaskGroup(t *testing.T) { func TestFindOrCreateDefaultTask(t *testing.T) { t.Run("Adds default task group when not set", func(t *testing.T) { job := &nomadApi.Job{} - group := FindOrCreateDefaultTaskGroup(job) + group := FindAndValidateDefaultTaskGroup(job) assert.NotNil(t, group) assert.Equal(t, TaskGroupName, *group.Name) assert.Equal(t, 1, len(job.TaskGroups)) @@ -38,7 +38,7 @@ func TestFindOrCreateDefaultTask(t *testing.T) { expectedGroup := &nomadApi.TaskGroup{Name: &groupName} job.TaskGroups = []*nomadApi.TaskGroup{expectedGroup} - group := FindOrCreateDefaultTaskGroup(job) + group := FindAndValidateDefaultTaskGroup(job) assert.NotNil(t, group) assert.Equal(t, 1, len(job.TaskGroups)) assert.Equal(t, expectedGroup, group) @@ -48,7 +48,7 @@ func TestFindOrCreateDefaultTask(t *testing.T) { func TestFindOrCreateConfigTaskGroup(t *testing.T) { t.Run("Adds config task group when not set", func(t *testing.T) { job := &nomadApi.Job{} - group := FindOrCreateConfigTaskGroup(job) + group := FindAndValidateConfigTaskGroup(job) assert.NotNil(t, group) assert.Equal(t, group, job.TaskGroups[0]) assert.Equal(t, 1, len(job.TaskGroups)) @@ -63,7 +63,7 @@ func TestFindOrCreateConfigTaskGroup(t *testing.T) { expectedGroup := &nomadApi.TaskGroup{Name: &groupName} job.TaskGroups = []*nomadApi.TaskGroup{expectedGroup} - group := FindOrCreateConfigTaskGroup(job) + group := FindAndValidateConfigTaskGroup(job) assert.NotNil(t, group) assert.Equal(t, 1, len(job.TaskGroups)) assert.Equal(t, expectedGroup, group) @@ -77,7 +77,7 @@ func TestFindOrCreateTask(t *testing.T) { expectedTask := &nomadApi.Task{Name: TaskName} group.Tasks = []*nomadApi.Task{expectedTask} - task := FindOrCreateDefaultTask(group) + task := FindAndValidateDefaultTask(group) assert.NotNil(t, task) assert.Equal(t, 1, len(group.Tasks)) assert.Equal(t, expectedTask, task) @@ -89,7 +89,7 @@ func TestFindOrCreateTask(t *testing.T) { expectedTask := &nomadApi.Task{Name: ConfigTaskName} group.Tasks = []*nomadApi.Task{expectedTask} - task := FindOrCreateConfigTask(group) + task := FindAndValidateConfigTask(group) assert.NotNil(t, task) assert.Equal(t, 1, len(group.Tasks)) assert.Equal(t, expectedTask, task) diff --git a/internal/nomad/nomad.go b/internal/nomad/nomad.go index 7a238a1..031d3e2 100644 --- a/internal/nomad/nomad.go +++ b/internal/nomad/nomad.go @@ -311,7 +311,7 @@ func (a *APIClient) MarkRunnerAsUsed(runnerID string, duration int) error { if err != nil { return fmt.Errorf("couldn't retrieve job info: %w", err) } - configTaskGroup := FindOrCreateConfigTaskGroup(job) + configTaskGroup := FindAndValidateConfigTaskGroup(job) configTaskGroup.Meta[ConfigMetaUsedKey] = ConfigMetaUsedValue configTaskGroup.Meta[ConfigMetaTimeoutKey] = strconv.Itoa(duration)