Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix provideRunner with no managers enabled. #704

Merged
merged 2 commits into from
Sep 28, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 10 additions & 2 deletions cmd/poseidon/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -341,6 +341,13 @@ func createManagerHandler(ctx context.Context, handler managerCreator, enabled b
return runnerManager, environmentManager
}

func createAbstractManager(ctx context.Context) (
runnerManager runner.Manager, environmentManager environment.ManagerHandler,
) {
runnerManager = runner.NewAbstractManager(ctx)
return runnerManager, environment.NewAbstractManager(runnerManager)
}

func createNomadManager(ctx context.Context) (runner.Manager, environment.ManagerHandler) {
// API initialization
nomadAPIClient, err := nomad.NewExecutorAPI(ctx, &config.Config.Nomad)
Expand Down Expand Up @@ -400,8 +407,9 @@ func createAWSManager(ctx context.Context) (

// initRouter builds a router that serves the API with the chain of responsibility for multiple managers.
func initRouter(ctx context.Context) *mux.Router {
runnerManager, environmentManager := createManagerHandler(ctx, createNomadManager, config.Config.Nomad.Enabled,
nil, nil)
runnerManager, environmentManager := createManagerHandler(ctx, createAbstractManager, true, nil, nil)
runnerManager, environmentManager = createManagerHandler(ctx, createNomadManager, config.Config.Nomad.Enabled,
runnerManager, environmentManager)
runnerManager, environmentManager = createManagerHandler(ctx, createAWSManager, config.Config.AWS.Enabled,
runnerManager, environmentManager)

Expand Down
1 change: 1 addition & 0 deletions internal/api/environments.go
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,7 @@ func (e *EnvironmentController) createOrUpdate(writer http.ResponseWriter, reque
})
if err != nil {
writeInternalServerError(request.Context(), writer, err, dto.ErrorUnknown)
return
}

if created {
Expand Down
10 changes: 9 additions & 1 deletion internal/environment/abstract_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,14 @@
runnerManager runner.Manager
}

// NewAbstractManager creates a new abstract runner manager that keeps track of all environments of one kind.
func NewAbstractManager(runnerManager runner.Manager) *AbstractManager {
return &AbstractManager{
nextHandler: nil,
runnerManager: runnerManager,
}
}

func (n *AbstractManager) SetNextHandler(next ManagerHandler) {
n.nextHandler = next
}
Expand All @@ -41,7 +49,7 @@
func (n *AbstractManager) CreateOrUpdate(_ context.Context, _ dto.EnvironmentID, _ dto.ExecutionEnvironmentRequest) (
bool, error,
) {
return false, nil
return false, dto.ErrNotSupported

Check warning on line 52 in internal/environment/abstract_manager.go

View check run for this annotation

Codecov / codecov/patch

internal/environment/abstract_manager.go#L52

Added line #L52 was not covered by tests
}

func (n *AbstractManager) Delete(environmentID dto.EnvironmentID) (bool, error) {
Expand Down
4 changes: 2 additions & 2 deletions internal/runner/abstract_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import (
"github.com/openHPI/poseidon/pkg/storage"
)

var ErrNullObject = errors.New("functionality not available for the null object")
var ErrUnknownExecutionEnvironment = errors.New("execution environment not found")

// AbstractManager is used to have a fallback runner manager in the chain of responsibility
// following the null object pattern.
Expand Down Expand Up @@ -99,7 +99,7 @@ func (n *AbstractManager) EnvironmentStatistics() map[dto.EnvironmentID]*dto.Sta
}

func (n *AbstractManager) Claim(_ dto.EnvironmentID, _ int) (Runner, error) {
return nil, ErrNullObject
return nil, ErrUnknownExecutionEnvironment
}

func (n *AbstractManager) Get(runnerID string) (Runner, error) {
Expand Down
14 changes: 9 additions & 5 deletions internal/runner/nomad_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,9 @@
const environmentMigrationDelay = time.Minute

var (
log = logging.GetLogger("runner")
ErrUnknownExecutionEnvironment = errors.New("execution environment not found")
ErrNoRunnersAvailable = errors.New("no runners available for this execution environment")
ErrRunnerNotFound = errors.New("no runner found with this id")
log = logging.GetLogger("runner")
ErrNoRunnersAvailable = errors.New("no runners available for this execution environment")
ErrRunnerNotFound = errors.New("no runner found with this id")
)

type alertData struct {
Expand All @@ -50,8 +49,13 @@
func (m *NomadRunnerManager) Claim(environmentID dto.EnvironmentID, duration int) (Runner, error) {
environment, ok := m.GetEnvironment(environmentID)
if !ok {
return nil, ErrUnknownExecutionEnvironment
r, err := m.NextHandler().Claim(environmentID, duration)
if err != nil {
return nil, fmt.Errorf("nomad wrapped: %w", err)
}
return r, nil

Check warning on line 56 in internal/runner/nomad_manager.go

View check run for this annotation

Codecov / codecov/patch

internal/runner/nomad_manager.go#L56

Added line #L56 was not covered by tests
}

runner, ok := environment.Sample()
if !ok {
return nil, ErrNoRunnersAvailable
Expand Down
2 changes: 1 addition & 1 deletion internal/runner/nomad_manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ func (s *ManagerTestSuite) TestSetEnvironmentAddsNewEnvironment() {
func (s *ManagerTestSuite) TestClaimReturnsNotFoundErrorIfEnvironmentNotFound() {
runner, err := s.nomadRunnerManager.Claim(anotherEnvironmentID, defaultInactivityTimeout)
s.Nil(runner)
s.Equal(ErrUnknownExecutionEnvironment, err)
s.ErrorIs(err, ErrUnknownExecutionEnvironment)
}

func (s *ManagerTestSuite) TestClaimReturnsRunnerIfAvailable() {
Expand Down
Loading