htwkmooc/poseidon
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Implement review comments

Browse Source
This commit is contained in:
Maximilian Paß
2022-02-22 16:47:13 +01:00
parent df68461264
commit 2cf890ab91
15 changed files with 109 additions and 110 deletions
Download Patch File Download Diff File Expand all files Collapse all files

25
internal/environment/abstract_manager.go
View File

@ -1,6 +1,7 @@
package environment
import (
"fmt"
"github.com/openHPI/poseidon/internal/runner"
"github.com/openHPI/poseidon/pkg/dto"
)
@ -8,7 +9,8 @@ import (
// AbstractManager is used to have a fallback environment manager in the chain of responsibility
// following the null object pattern.
type AbstractManager struct {
nextHandler ManagerHandler
nextHandler ManagerHandler
runnerManager runner.Manager
}
func (n *AbstractManager) SetNextHandler(next ManagerHandler) {
@ -35,8 +37,25 @@ func (n *AbstractManager) CreateOrUpdate(_ dto.EnvironmentID, _ dto.ExecutionEnv
return false, nil
}
func (n *AbstractManager) Delete(_ dto.EnvironmentID) (bool, error) {
return false, nil
func (n *AbstractManager) Delete(id dto.EnvironmentID) (bool, error) {
e, ok := n.runnerManager.GetEnvironment(id)
if !ok {
if n.nextHandler != nil {
isFound, err := n.NextHandler().Delete(id)
if err != nil {
return false, fmt.Errorf("aws wrapped: %w", err)
}
return isFound, nil
} else {
return false, nil
}
}
n.runnerManager.DeleteEnvironment(id)
if err := e.Delete(); err != nil {
return true, fmt.Errorf("could not delete environment: %w", err)
}
return true, nil
}
func (n *AbstractManager) Statistics() map[dto.EnvironmentID]*dto.StatisticalExecutionEnvironmentData {

94
internal/environment/aws_environment.go
View File

@ -36,31 +36,6 @@ func (a *AWSEnvironment) SetID(id dto.EnvironmentID) {
a.id = id
}
func (a *AWSEnvironment) PrewarmingPoolSize() uint {
return 0
}
func (a *AWSEnvironment) SetPrewarmingPoolSize(_ uint) {}
func (a *AWSEnvironment) ApplyPrewarmingPoolSize() error {
return nil
}
func (a *AWSEnvironment) CPULimit() uint {
return 0
}
// SetCPULimit is disabled as one can only set the memory limit with AWS Lambda.
func (a *AWSEnvironment) SetCPULimit(_ uint) {}
func (a *AWSEnvironment) MemoryLimit() uint {
panic("implement me")
}
func (a *AWSEnvironment) SetMemoryLimit(_ uint) {
panic("implement me")
}
// Image is used to specify the AWS Endpoint Poseidon is connecting to.
func (a *AWSEnvironment) Image() string {
return a.awsEndpoint
@ -70,22 +45,6 @@ func (a *AWSEnvironment) SetImage(awsEndpoint string) {
a.awsEndpoint = awsEndpoint
}
func (a *AWSEnvironment) NetworkAccess() (enabled bool, mappedPorts []uint16) {
panic("implement me")
}
func (a *AWSEnvironment) SetNetworkAccess(_ bool, _ []uint16) {
panic("implement me")
}
func (a *AWSEnvironment) SetConfigFrom(_ runner.ExecutionEnvironment) {
panic("implement me")
}
func (a *AWSEnvironment) Register() error {
panic("implement me")
}
func (a *AWSEnvironment) Delete() error {
return nil
}
@ -98,14 +57,61 @@ func (a *AWSEnvironment) Sample() (r runner.Runner, ok bool) {
return workload, true
}
// The following methods are not supported at this moment.
// PrewarmingPoolSize is neither supported nor required. It is handled transparently by AWS.
func (a *AWSEnvironment) PrewarmingPoolSize() uint {
return 0
}
// SetPrewarmingPoolSize is neither supported nor required. It is handled transparently by AWS.
func (a *AWSEnvironment) SetPrewarmingPoolSize(_ uint) {}
// ApplyPrewarmingPoolSize is neither supported nor required. It is handled transparently by AWS.
func (a *AWSEnvironment) ApplyPrewarmingPoolSize() error {
return nil
}
// CPULimit is disabled as one can only set the memory limit with AWS Lambda.
func (a *AWSEnvironment) CPULimit() uint {
return 0
}
// SetCPULimit is disabled as one can only set the memory limit with AWS Lambda.
func (a *AWSEnvironment) SetCPULimit(_ uint) {}
func (a *AWSEnvironment) MemoryLimit() uint {
panic("not supported")
}
func (a *AWSEnvironment) SetMemoryLimit(_ uint) {
panic("not supported")
}
func (a *AWSEnvironment) NetworkAccess() (enabled bool, mappedPorts []uint16) {
panic("not supported")
}
func (a *AWSEnvironment) SetNetworkAccess(_ bool, _ []uint16) {
panic("not supported")
}
func (a *AWSEnvironment) SetConfigFrom(_ runner.ExecutionEnvironment) {
panic("not supported")
}
func (a *AWSEnvironment) Register() error {
panic("not supported")
}
func (a *AWSEnvironment) AddRunner(_ runner.Runner) {
panic("implement me")
panic("not supported")
}
func (a *AWSEnvironment) DeleteRunner(_ string) {
panic("implement me")
panic("not supported")
}
func (a *AWSEnvironment) IdleRunnerCount() int {
panic("implement me")
panic("not supported")
}

20
internal/environment/aws_manager.go
View File

@ -12,11 +12,10 @@ import (
// IMPROVE: Create Lambda functions dynamically.
type AWSEnvironmentManager struct {
*AbstractManager
runnerManager runner.Manager
}
func NewAWSEnvironmentManager(runnerManager runner.Manager) *AWSEnvironmentManager {
m := &AWSEnvironmentManager{&AbstractManager{nil}, runnerManager}
m := &AWSEnvironmentManager{&AbstractManager{nil, runnerManager}}
runnerManager.Load()
return m
}
@ -69,23 +68,6 @@ func isAWSEnvironment(request dto.ExecutionEnvironmentRequest) bool {
return false
}
func (a *AWSEnvironmentManager) Delete(id dto.EnvironmentID) (bool, error) {
e, ok := a.runnerManager.GetEnvironment(id)
if !ok {
isFound, err := a.NextHandler().Delete(id)
if err != nil {
return false, fmt.Errorf("aws wrapped: %w", err)
}
return isFound, nil
}
a.runnerManager.DeleteEnvironment(id)
if err := e.Delete(); err != nil {
return true, fmt.Errorf("could not delete environment: %w", err)
}
return true, nil
}
func (a *AWSEnvironmentManager) Statistics() map[dto.EnvironmentID]*dto.StatisticalExecutionEnvironmentData {
return a.NextHandler().Statistics()
}

6
internal/environment/aws_manager_test.go
View File

@ -28,7 +28,7 @@ func TestAWSEnvironmentManager_CreateOrUpdate(t *testing.T) {
assert.Equal(t, environment.Image(), uniqueImage)
})
t.Run("non handleable requests are forwarded to the next manager", func(t *testing.T) {
t.Run("non-handleable requests are forwarded to the next manager", func(t *testing.T) {
nextHandler := &ManagerHandlerMock{}
nextHandler.On("CreateOrUpdate", mock.AnythingOfType("dto.EnvironmentID"),
mock.AnythingOfType("dto.ExecutionEnvironmentRequest")).Return(true, nil)
@ -58,7 +58,7 @@ func TestAWSEnvironmentManager_Get(t *testing.T) {
})
t.Run("Returns error when not found", func(t *testing.T) {
nextHandler := &AbstractManager{nil}
nextHandler := &AbstractManager{nil, nil}
m.SetNextHandler(nextHandler)
_, err := m.Get(tests.DefaultEnvironmentIDAsInteger, false)
@ -80,7 +80,7 @@ func TestAWSEnvironmentManager_List(t *testing.T) {
runnerManager := runner.NewAWSRunnerManager()
m := NewAWSEnvironmentManager(runnerManager)
t.Run("returs also environments of the rest of the manager chain", func(t *testing.T) {
t.Run("also returns environments of the rest of the manager chain", func(t *testing.T) {
nextHandler := &ManagerHandlerMock{}
existingEnvironment := NewAWSEnvironment(nil)
nextHandler.On("List", mock.AnythingOfType("bool")).

6
internal/environment/nomad_environment.go
View File

@ -22,10 +22,10 @@ const (
var ErrScaleDown = errors.New("cannot scale down the environment")
type NomadEnvironment struct {
apiClient nomad.ExecutorAPI
jobHCL string
job *nomadApi.Job
idleRunners runner.Storage
apiClient nomad.ExecutorAPI
}
func NewNomadEnvironment(apiClient nomad.ExecutorAPI, jobHCL string) (*NomadEnvironment, error) {
@ -34,7 +34,7 @@ func NewNomadEnvironment(apiClient nomad.ExecutorAPI, jobHCL string) (*NomadEnvi
return nil, fmt.Errorf("error parsing Nomad job: %w", err)
}
return &NomadEnvironment{jobHCL, job, runner.NewLocalRunnerStorage(), apiClient}, nil
return &NomadEnvironment{apiClient, jobHCL, job, runner.NewLocalRunnerStorage()}, nil
}
func NewNomadEnvironmentFromRequest(
@ -252,7 +252,7 @@ func (n *NomadEnvironment) IdleRunnerCount() int {
}
// MarshalJSON implements the json.Marshaler interface.
// This converts the AWSEnvironment into the expected schema for dto.ExecutionEnvironmentData.
// This converts the NomadEnvironment into the expected schema for dto.ExecutionEnvironmentData.
func (n *NomadEnvironment) MarshalJSON() (res []byte, err error) {
networkAccess, exposedPorts := n.NetworkAccess()

18
internal/environment/nomad_environment_test.go
View File

@ -17,7 +17,7 @@ import (
func TestConfigureNetworkCreatesNewNetworkWhenNoNetworkExists(t *testing.T) {
_, job := helpers.CreateTemplateJob()
defaultTaskGroup := nomad.FindAndValidateDefaultTaskGroup(job)
environment := &NomadEnvironment{"", job, nil, nil}
environment := &NomadEnvironment{nil, "", job, nil}
if assert.Equal(t, 0, len(defaultTaskGroup.Networks)) {
environment.SetNetworkAccess(true, []uint16{})
@ -29,7 +29,7 @@ func TestConfigureNetworkCreatesNewNetworkWhenNoNetworkExists(t *testing.T) {
func TestConfigureNetworkDoesNotCreateNewNetworkWhenNetworkExists(t *testing.T) {
_, job := helpers.CreateTemplateJob()
defaultTaskGroup := nomad.FindAndValidateDefaultTaskGroup(job)
environment := &NomadEnvironment{"", job, nil, nil}
environment := &NomadEnvironment{nil, "", job, nil}
networkResource := &nomadApi.NetworkResource{Mode: "bridge"}
defaultTaskGroup.Networks = []*nomadApi.NetworkResource{networkResource}
@ -58,7 +58,7 @@ func TestConfigureNetworkSetsCorrectValues(t *testing.T) {
_, testJob := helpers.CreateTemplateJob()
testTaskGroup := nomad.FindAndValidateDefaultTaskGroup(testJob)
testTask := nomad.FindAndValidateDefaultTask(testTaskGroup)
testEnvironment := &NomadEnvironment{"", job, nil, nil}
testEnvironment := &NomadEnvironment{nil, "", job, nil}
testEnvironment.SetNetworkAccess(false, ports)
mode, ok := testTask.Config["network_mode"]
@ -73,7 +73,7 @@ func TestConfigureNetworkSetsCorrectValues(t *testing.T) {
_, testJob := helpers.CreateTemplateJob()
testTaskGroup := nomad.FindAndValidateDefaultTaskGroup(testJob)
testTask := nomad.FindAndValidateDefaultTask(testTaskGroup)
testEnvironment := &NomadEnvironment{"", testJob, nil, nil}
testEnvironment := &NomadEnvironment{nil, "", testJob, nil}
testEnvironment.SetNetworkAccess(true, ports)
require.Equal(t, 1, len(testTaskGroup.Networks))
@ -113,7 +113,7 @@ func TestRegisterFailsWhenNomadJobRegistrationFails(t *testing.T) {
apiClientMock.On("LoadRunnerIDs", mock.AnythingOfType("string")).Return([]string{}, nil)
apiClientMock.On("DeleteJob", mock.AnythingOfType("string")).Return(nil)
environment := &NomadEnvironment{"", &nomadApi.Job{}, runner.NewLocalRunnerStorage(), apiClientMock}
environment := &NomadEnvironment{apiClientMock, "", &nomadApi.Job{}, runner.NewLocalRunnerStorage()}
environment.SetID(tests.DefaultEnvironmentIDAsInteger)
err := environment.Register()
@ -130,7 +130,7 @@ func TestRegisterTemplateJobSucceedsWhenMonitoringEvaluationSucceeds(t *testing.
apiClientMock.On("LoadRunnerIDs", mock.AnythingOfType("string")).Return([]string{}, nil)
apiClientMock.On("DeleteJob", mock.AnythingOfType("string")).Return(nil)
environment := &NomadEnvironment{"", &nomadApi.Job{}, runner.NewLocalRunnerStorage(), apiClientMock}
environment := &NomadEnvironment{apiClientMock, "", &nomadApi.Job{}, runner.NewLocalRunnerStorage()}
environment.SetID(tests.DefaultEnvironmentIDAsInteger)
err := environment.Register()
@ -146,7 +146,7 @@ func TestRegisterTemplateJobReturnsErrorWhenMonitoringEvaluationFails(t *testing
apiClientMock.On("LoadRunnerIDs", mock.AnythingOfType("string")).Return([]string{}, nil)
apiClientMock.On("DeleteJob", mock.AnythingOfType("string")).Return(nil)
environment := &NomadEnvironment{"", &nomadApi.Job{}, runner.NewLocalRunnerStorage(), apiClientMock}
environment := &NomadEnvironment{apiClientMock, "", &nomadApi.Job{}, runner.NewLocalRunnerStorage()}
environment.SetID(tests.DefaultEnvironmentIDAsInteger)
err := environment.Register()
@ -172,7 +172,7 @@ func TestTwoSampleAddExactlyTwoRunners(t *testing.T) {
apiMock.On("RegisterRunnerJob", mock.AnythingOfType("*api.Job")).Return(nil)
_, job := helpers.CreateTemplateJob()
environment := &NomadEnvironment{templateEnvironmentJobHCL, job, runner.NewLocalRunnerStorage(), apiMock}
environment := &NomadEnvironment{apiMock, templateEnvironmentJobHCL, job, runner.NewLocalRunnerStorage()}
runner1 := &runner.RunnerMock{}
runner1.On("ID").Return(tests.DefaultRunnerID)
runner2 := &runner.RunnerMock{}
@ -205,7 +205,7 @@ func TestSampleDoesNotSetForcePullFlag(t *testing.T) {
})
_, job := helpers.CreateTemplateJob()
environment := &NomadEnvironment{templateEnvironmentJobHCL, job, runner.NewLocalRunnerStorage(), apiMock}
environment := &NomadEnvironment{apiMock, templateEnvironmentJobHCL, job, runner.NewLocalRunnerStorage()}
runner1 := &runner.RunnerMock{}
runner1.On("ID").Return(tests.DefaultRunnerID)
environment.AddRunner(runner1)

20
internal/environment/nomad_manager.go
View File

@ -21,7 +21,6 @@ var log = logging.GetLogger("environment")
type NomadEnvironmentManager struct {
*AbstractManager
runnerManager runner.Manager
api nomad.ExecutorAPI
templateEnvironmentHCL string
}
@ -35,7 +34,7 @@ func NewNomadEnvironmentManager(
return nil, err
}
m := &NomadEnvironmentManager{&AbstractManager{nil}, runnerManager,
m := &NomadEnvironmentManager{&AbstractManager{nil, runnerManager},
apiClient, templateEnvironmentJobHCL}
if err := m.Load(); err != nil {
log.WithError(err).Error("Error recovering the execution environments")
@ -121,19 +120,6 @@ func (m *NomadEnvironmentManager) CreateOrUpdate(id dto.EnvironmentID, request d
return !isExistingEnvironment, nil
}
func (m *NomadEnvironmentManager) Delete(id dto.EnvironmentID) (bool, error) {
executionEnvironment, ok := m.runnerManager.GetEnvironment(id)
if !ok {
return false, nil
}
m.runnerManager.DeleteEnvironment(id)
err := executionEnvironment.Delete()
if err != nil {
return true, fmt.Errorf("could not delete environment: %w", err)
}
return true, nil
}
func (m *NomadEnvironmentManager) Statistics() map[dto.EnvironmentID]*dto.StatisticalExecutionEnvironmentData {
return m.runnerManager.EnvironmentStatistics()
}
@ -156,10 +142,10 @@ func (m *NomadEnvironmentManager) Load() error {
continue
}
environment := &NomadEnvironment{
apiClient: m.api,
jobHCL: templateEnvironmentJobHCL,
job: job,
idleRunners: runner.NewLocalRunnerStorage(),
apiClient: m.api,
}
m.runnerManager.StoreEnvironment(environment)
jobLogger.Info("Successfully recovered environment")
@ -195,10 +181,10 @@ func fetchEnvironment(id dto.EnvironmentID, apiClient nomad.ExecutorAPI) (runner
}
if id == environmentID {
fetchedEnvironment = &NomadEnvironment{
apiClient: apiClient,
jobHCL: templateEnvironmentJobHCL,
job: job,
idleRunners: runner.NewLocalRunnerStorage(),
apiClient: apiClient,
}
}
}

2
internal/environment/nomad_manager_test.go
View File

@ -44,7 +44,7 @@ func (s *CreateOrUpdateTestSuite) SetupTest() {
}
s.manager = &NomadEnvironmentManager{
runnerManager: &s.runnerManagerMock,
AbstractManager: &AbstractManager{runnerManager: &s.runnerManagerMock},
api: &s.apiMock,
templateEnvironmentHCL: templateEnvironmentJobHCL,
}

4
internal/runner/aws_manager_test.go
View File

@ -45,7 +45,7 @@ func TestAWSRunnerManager_Claim(t *testing.T) {
assert.NotNil(t, r)
})
t.Run("forwards request for non AWS environments", func(t *testing.T) {
t.Run("forwards request for non-AWS environments", func(t *testing.T) {
nextHandler := &ManagerMock{}
nextHandler.On("Claim", mock.AnythingOfType("dto.EnvironmentID"), mock.AnythingOfType("int")).
Return(nil, nil)
@ -74,7 +74,7 @@ func TestAWSRunnerManager_Return(t *testing.T) {
assert.NotContains(t, m.usedRunners.List(), r)
})
t.Run("calls nextHandler for non AWS runner", func(t *testing.T) {
t.Run("calls nextHandler for non-AWS runner", func(t *testing.T) {
nextHandler := &ManagerMock{}
nextHandler.On("Return", mock.AnythingOfType("*runner.NomadJob")).Return(nil)
m.SetNextHandler(nextHandler)

10
internal/runner/aws_runner.go
View File

@ -13,7 +13,7 @@ import (
"io"
)
var ErrWrongMessageType = errors.New("received message that is not a text messages")
var ErrWrongMessageType = errors.New("received message that is not a text message")
type awsFunctionRequest struct {
Action string `json:"action"`
@ -22,6 +22,9 @@ type awsFunctionRequest struct {
}
// AWSFunctionWorkload is an abstraction to build a request to an AWS Lambda Function.
// It is not persisted on a Poseidon restart.
// The InactivityTimer is used actively. It stops listening to the Lambda function.
// AWS terminates the Lambda Function after the [Globals.Function.Timeout](deploy/aws/template.yaml).
type AWSFunctionWorkload struct {
InactivityTimer
id string
@ -88,6 +91,9 @@ func (w *AWSFunctionWorkload) ExecuteInteractively(id string, _ io.ReadWriter, s
}
// UpdateFileSystem copies Files into the executor.
// Current limitation: No files can be deleted apart from the previously added files.
// Future Work: Deduplication of the file systems, as the largest workload is likely to be used by additional
// CSV files or similar, which are the same for many executions.
func (w *AWSFunctionWorkload) UpdateFileSystem(request *dto.UpdateFileSystemRequest) error {
for _, path := range request.Delete {
delete(w.fs, path)
@ -136,7 +142,9 @@ func (w *AWSFunctionWorkload) executeCommand(ctx context.Context, command []stri
return
}
// receiveOutput listens for the execution timeout (or the exit code).
exitCode, err := w.receiveOutput(wsConn, stdout, stderr, ctx)
// TimeoutPassed checks the runner timeout
if w.TimeoutPassed() {
err = ErrorRunnerInactivityTimeout
}

4
internal/runner/aws_runner_test.go
View File

@ -67,7 +67,7 @@ func TestAWSFunctionWorkload_ExecuteInteractively(t *testing.T) {
s := httptest.NewServer(http.HandlerFunc(awsMock.handler))
t.Run("establishes WebSocket connection to AWS endpoint", func(t *testing.T) {
// Convert http://127.0.0.1 to ws://127.0.0.
// Convert http://127.0.0.1 to ws://127.0.0.1
config.Config.AWS.Endpoint = "ws" + strings.TrimPrefix(s.URL, "http")
awsMock.ctx, cancel = context.WithCancel(context.Background())
cancel()
@ -107,7 +107,7 @@ func TestAWSFunctionWorkload_UpdateFileSystem(t *testing.T) {
awsMock := &awsEndpointMock{}
s := httptest.NewServer(http.HandlerFunc(awsMock.handler))
// Convert http://127.0.0.1 to ws://127.0.0.
// Convert http://127.0.0.1 to ws://127.0.0.1
config.Config.AWS.Endpoint = "ws" + strings.TrimPrefix(s.URL, "http")
awsMock.ctx, cancel = context.WithTimeout(context.Background(), tests.ShortTimeout)
defer cancel()

2
internal/runner/manager.go
View File

@ -75,7 +75,7 @@ type EnvironmentAccessor interface {
EnvironmentStatistics() map[dto.EnvironmentID]*dto.StatisticalExecutionEnvironmentData
}
// AccessorHandler is one handler in te chain of responsibility of runner accessors.
// AccessorHandler is one handler in the chain of responsibility of runner accessors.
// Each runner accessor can handle different requests.
type AccessorHandler interface {
Accessor

2
internal/runner/nomad_runner.go
View File

@ -46,6 +46,8 @@ type NomadJob struct {
}
// NewNomadJob creates a new NomadJob with the provided id.
// The InactivityTimer is used actively. It executes onDestroy when it has expired.
// The InactivityTimer is persisted in Nomad by the runner manager's Claim Function.
func NewNomadJob(id string, portMappings []nomadApi.PortMapping,
apiClient nomad.ExecutorAPI, onDestroy DestroyRunnerHandler,
) *NomadJob {