From 67ebdbd6500a39be427263018bbe6c44068f29cf Mon Sep 17 00:00:00 2001 From: sirkrypt0 <22522058+sirkrypt0@users.noreply.github.com> Date: Tue, 27 Jul 2021 15:30:02 +0200 Subject: [PATCH] Add option to configure template job HCL file Previously, the template job HCL file was hardcoded using go:embed in the binary. However, this did not allow users running Poseidon to change its content. Now, users can change the content of the template job HCL file using the configuration option. --- cmd/poseidon/main.go | 6 ++- configuration.example.yaml | 2 + internal/config/config.go | 2 + internal/environment/manager.go | 36 +++++++++++--- internal/environment/manager_test.go | 74 ++++++++++++++++++++++------ 5 files changed, 96 insertions(+), 24 deletions(-) diff --git a/cmd/poseidon/main.go b/cmd/poseidon/main.go index 426db48..c79e50b 100644 --- a/cmd/poseidon/main.go +++ b/cmd/poseidon/main.go @@ -51,7 +51,11 @@ func initServer() *http.Server { } runnerManager := runner.NewNomadRunnerManager(nomadAPIClient, context.Background()) - environmentManager := environment.NewNomadEnvironmentManager(runnerManager, nomadAPIClient) + environmentManager, err := environment. + NewNomadEnvironmentManager(runnerManager, nomadAPIClient, config.Config.Server.TemplateJobFile) + if err != nil { + log.WithError(err).Fatal("Error initializing environment manager") + } return &http.Server{ Addr: config.Config.Server.URL().Host, diff --git a/configuration.example.yaml b/configuration.example.yaml index 6a6e014..951d151 100644 --- a/configuration.example.yaml +++ b/configuration.example.yaml @@ -16,6 +16,8 @@ server: keyfile: ./poseidon.key # If true, an additional WebSocket connection will be opened to split stdout and stderr when executing interactively interactiveStderr: true + # If set, the file at the given path overwrites the default Nomad job file in internal/environment/template-environment-job.hcl + templateJobFile: ./poseidon.hcl # Configuration of the used Nomad cluster nomad: diff --git a/internal/config/config.go b/internal/config/config.go index 359b5ca..32e8a5b 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -28,6 +28,7 @@ var ( KeyFile: "", }, InteractiveStderr: true, + TemplateJobFile: "", }, Nomad: Nomad{ Address: "127.0.0.1", @@ -63,6 +64,7 @@ type server struct { Token string TLS TLS InteractiveStderr bool + TemplateJobFile string } // URL returns the URL of the Poseidon webserver. diff --git a/internal/environment/manager.go b/internal/environment/manager.go index 8d8c765..2deb3bf 100644 --- a/internal/environment/manager.go +++ b/internal/environment/manager.go @@ -10,6 +10,7 @@ import ( "gitlab.hpi.de/codeocean/codemoon/poseidon/internal/runner" "gitlab.hpi.de/codeocean/codemoon/poseidon/pkg/dto" "gitlab.hpi.de/codeocean/codemoon/poseidon/pkg/logging" + "os" "strconv" ) @@ -39,13 +40,35 @@ type Manager interface { func NewNomadEnvironmentManager( runnerManager runner.Manager, apiClient nomad.ExecutorAPI, -) *NomadEnvironmentManager { - m := &NomadEnvironmentManager{runnerManager, apiClient, *parseJob(templateEnvironmentJobHCL)} + templateJobFile string, +) (*NomadEnvironmentManager, error) { + if err := loadTemplateEnvironmentJobHCL(templateJobFile); err != nil { + return nil, err + } + templateEnvironmentJob, err := parseJob(templateEnvironmentJobHCL) + if err != nil { + return nil, err + } + m := &NomadEnvironmentManager{runnerManager, apiClient, *templateEnvironmentJob} if err := m.Load(); err != nil { log.WithError(err).Error("Error recovering the execution environments") } runnerManager.Load() - return m + return m, nil +} + +// loadTemplateEnvironmentJobHCL loads the template environment job HCL from the given path. +// If the path is empty, the embedded default file is used. +func loadTemplateEnvironmentJobHCL(path string) error { + if path == "" { + return nil + } + data, err := os.ReadFile(path) + if err != nil { + return fmt.Errorf("error loading template environment job: %w", err) + } + templateEnvironmentJobHCL = string(data) + return nil } type NomadEnvironmentManager struct { @@ -116,7 +139,7 @@ func (m *NomadEnvironmentManager) Load() error { return nil } -func parseJob(jobHCL string) *nomadApi.Job { +func parseJob(jobHCL string) (*nomadApi.Job, error) { config := jobspec2.ParseConfig{ Body: []byte(jobHCL), AllowFS: false, @@ -124,9 +147,8 @@ func parseJob(jobHCL string) *nomadApi.Job { } job, err := jobspec2.ParseWithConfig(&config) if err != nil { - log.WithError(err).Fatal("Error parsing Nomad job") - return nil + return nil, fmt.Errorf("error parsing Nomad job: %w", err) } - return job + return job, nil } diff --git a/internal/environment/manager_test.go b/internal/environment/manager_test.go index 7cfe245..623d906 100644 --- a/internal/environment/manager_test.go +++ b/internal/environment/manager_test.go @@ -2,15 +2,15 @@ package environment import ( nomadApi "github.com/hashicorp/nomad/api" - "github.com/sirupsen/logrus" - "github.com/sirupsen/logrus/hooks/test" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" "github.com/stretchr/testify/suite" "gitlab.hpi.de/codeocean/codemoon/poseidon/internal/nomad" "gitlab.hpi.de/codeocean/codemoon/poseidon/internal/runner" "gitlab.hpi.de/codeocean/codemoon/poseidon/pkg/dto" "gitlab.hpi.de/codeocean/codemoon/poseidon/tests" + "os" "testing" ) @@ -119,24 +119,66 @@ func (s *CreateOrUpdateTestSuite) TestReturnsFalseIfCreatesOrUpdateEnvironmentRe } func TestParseJob(t *testing.T) { - exited := false - logger, hook := test.NewNullLogger() - logger.ExitFunc = func(i int) { - exited = true - } - - log = logger.WithField("pkg", "nomad") - t.Run("parses the given default job", func(t *testing.T) { - job := parseJob(templateEnvironmentJobHCL) - assert.False(t, exited) + job, err := parseJob(templateEnvironmentJobHCL) + assert.NoError(t, err) assert.NotNil(t, job) }) - t.Run("fatals when given wrong job", func(t *testing.T) { - job := parseJob("") - assert.True(t, exited) + t.Run("returns error when given wrong job", func(t *testing.T) { + job, err := parseJob("") + assert.Error(t, err) assert.Nil(t, job) - assert.Equal(t, logrus.FatalLevel, hook.LastEntry().Level) }) } + +func TestNewNomadEnvironmentManager(t *testing.T) { + executorAPIMock := &nomad.ExecutorAPIMock{} + executorAPIMock.On("LoadEnvironmentJobs").Return([]*nomadApi.Job{}, nil) + + runnerManagerMock := &runner.ManagerMock{} + runnerManagerMock.On("Load").Return() + + previousTemplateEnvironmentJobHCL := templateEnvironmentJobHCL + + t.Run("returns error if template file does not exist", func(t *testing.T) { + _, err := NewNomadEnvironmentManager(runnerManagerMock, executorAPIMock, "/non-existent/file") + assert.Error(t, err) + }) + + t.Run("loads template environment job from file", func(t *testing.T) { + templateJobHCL := "job \"test\" {}" + expectedJob, err := parseJob(templateJobHCL) + require.NoError(t, err) + f := createTempFile(t, templateJobHCL) + defer os.Remove(f.Name()) + + m, err := NewNomadEnvironmentManager(runnerManagerMock, executorAPIMock, f.Name()) + assert.NoError(t, err) + assert.NotNil(t, m) + assert.Equal(t, templateJobHCL, templateEnvironmentJobHCL) + assert.Equal(t, *expectedJob, m.templateEnvironmentJob) + }) + + t.Run("returns error if template file is invalid", func(t *testing.T) { + templateJobHCL := "invalid hcl file" + f := createTempFile(t, templateJobHCL) + defer os.Remove(f.Name()) + + _, err := NewNomadEnvironmentManager(runnerManagerMock, executorAPIMock, f.Name()) + assert.Error(t, err) + }) + + templateEnvironmentJobHCL = previousTemplateEnvironmentJobHCL +} + +func createTempFile(t *testing.T, content string) *os.File { + t.Helper() + f, err := os.CreateTemp("", "test") + require.NoError(t, err) + n, err := f.WriteString(content) + require.NoError(t, err) + require.Equal(t, len(content), n) + + return f +}