From f327d11af8f29876e87e9ea4090ac421636249e9 Mon Sep 17 00:00:00 2001 From: Aayush Gupta Date: Thu, 20 Jan 2022 13:03:53 -0800 Subject: [PATCH 01/19] Changing job identifier to UUID --- go.mod | 2 +- go.sum | 2 + .../events/events_controller_e2e_test.go | 2 + server/controllers/jobs_controller.go | 24 +++-- server/controllers/websocket/mux.go | 15 +-- server/events/mocks/mock_job_id_generator.go | 94 +++++++++++++++++++ server/events/models/models.go | 4 +- server/events/project_command_builder.go | 11 ++- .../project_command_builder_internal_test.go | 9 ++ server/events/project_command_builder_test.go | 29 ++++-- .../events/project_command_context_builder.go | 12 ++- .../project_command_context_builder_test.go | 1 + server/events/pull_closed_executor.go | 2 + .../project_command_output_handler.go | 75 +++++++-------- .../project_command_output_handler_test.go | 11 +-- server/router.go | 28 ++++-- server/router_test.go | 32 +++---- server/server.go | 7 +- 18 files changed, 258 insertions(+), 102 deletions(-) create mode 100644 server/events/mocks/mock_job_id_generator.go diff --git a/go.mod b/go.mod index 030639a9b..0d010d25a 100644 --- a/go.mod +++ b/go.mod @@ -36,7 +36,7 @@ require ( github.com/google/go-github/v29 v29.0.2 // indirect github.com/google/go-github/v31 v31.0.0 github.com/google/go-querystring v1.0.0 // indirect - github.com/google/uuid v1.1.2-0.20200519141726-cb32006e483f + github.com/google/uuid v1.3.0 github.com/googleapis/gax-go/v2 v2.0.5 // indirect github.com/gorilla/css v1.0.0 // indirect github.com/gorilla/mux v1.8.0 diff --git a/go.sum b/go.sum index 3529f4040..c03209869 100644 --- a/go.sum +++ b/go.sum @@ -203,6 +203,8 @@ github.com/google/renameio v0.1.0/go.mod h1:KWCgfxg9yswjAJkECMjeO8J8rahYeXnNhOm4 github.com/google/uuid v1.1.1/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= github.com/google/uuid v1.1.2-0.20200519141726-cb32006e483f h1:qa1wFcvZzVLbFVPdsdTsWL6k5IP6BEmFmd9SeahRQ5s= github.com/google/uuid v1.1.2-0.20200519141726-cb32006e483f/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= +github.com/google/uuid v1.3.0 h1:t6JiXgmwXMjEs8VusXIJk2BXHsn+wx8BZdTaoZ5fu7I= +github.com/google/uuid v1.3.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= github.com/googleapis/gax-go/v2 v2.0.4/go.mod h1:0Wqv26UfaUD9n4G6kQubkQ+KchISgw+vpHVxEJEs9eg= github.com/googleapis/gax-go/v2 v2.0.5 h1:sjZBwGj9Jlw33ImPtvFviGYvseOtDM7hkSKB7+Tv3SM= github.com/googleapis/gax-go/v2 v2.0.5/go.mod h1:DWXyrwAJ9X0FpwwEdw+IPEYBICEFu5mhpdKc/us6bOk= diff --git a/server/controllers/events/events_controller_e2e_test.go b/server/controllers/events/events_controller_e2e_test.go index 2f7c6f80e..47918ab18 100644 --- a/server/controllers/events/events_controller_e2e_test.go +++ b/server/controllers/events/events_controller_e2e_test.go @@ -18,6 +18,7 @@ import ( "github.com/hashicorp/go-version" . "github.com/petergtz/pegomock" "github.com/runatlantis/atlantis/server" + "github.com/runatlantis/atlantis/server/controllers" events_controllers "github.com/runatlantis/atlantis/server/controllers/events" "github.com/runatlantis/atlantis/server/core/db" "github.com/runatlantis/atlantis/server/core/locking" @@ -912,6 +913,7 @@ func setupE2E(t *testing.T, repoDir string) (events_controllers.VCSEventsControl "**/*.tf,**/*.tfvars,**/*.tfvars.json,**/terragrunt.hcl", statsScope, logger, + controllers.JobIDGenerator{}, ) showStepRunner, err := runtime.NewShowStepRunner(terraformClient, defaultTFVersion) diff --git a/server/controllers/jobs_controller.go b/server/controllers/jobs_controller.go index 7f9c5fe46..07ee41154 100644 --- a/server/controllers/jobs_controller.go +++ b/server/controllers/jobs_controller.go @@ -7,6 +7,7 @@ import ( "strconv" + "github.com/google/uuid" "github.com/gorilla/mux" "github.com/pkg/errors" "github.com/runatlantis/atlantis/server/controllers/templates" @@ -29,6 +30,12 @@ type JobsController struct { StatsScope tally.Scope } +type JobIDGenerator struct{} + +func (j JobIDGenerator) GenerateJobID() string { + return uuid.New().String() +} + type ProjectInfoKeyGenerator struct{} func (g ProjectInfoKeyGenerator) Generate(r *http.Request) (string, error) { @@ -111,20 +118,25 @@ func newProjectInfo(r *http.Request) (*projectInfo, error) { } func (j *JobsController) getProjectJobs(w http.ResponseWriter, r *http.Request) error { - projectInfo, err := newProjectInfo(r) - if err != nil { - j.respond(w, logging.Error, http.StatusInternalServerError, err.Error()) - return err + // projectInfo, err := newProjectInfo(r) + // if err != nil { + // j.respond(w, logging.Error, http.StatusInternalServerError, err.Error()) + // return err + // } + + jobID, ok := mux.Vars(r)["job-id"] + if !ok { + return fmt.Errorf("internal error: no job ID in route") } viewData := templates.ProjectJobData{ AtlantisVersion: j.AtlantisVersion, - ProjectPath: projectInfo.String(), + ProjectPath: jobID, CleanedBasePath: j.AtlantisURL.Path, ClearMsg: models.LogStreamingClearMsg, } - err = j.ProjectJobsTemplate.Execute(w, viewData) + err := j.ProjectJobsTemplate.Execute(w, viewData) if err != nil { j.Logger.Err(err.Error()) return err diff --git a/server/controllers/websocket/mux.go b/server/controllers/websocket/mux.go index ccfbdf99f..ceaaf13d1 100644 --- a/server/controllers/websocket/mux.go +++ b/server/controllers/websocket/mux.go @@ -1,8 +1,10 @@ package websocket import ( + "fmt" "net/http" + "github.com/gorilla/mux" "github.com/gorilla/websocket" "github.com/pkg/errors" "github.com/runatlantis/atlantis/server/logging" @@ -45,10 +47,11 @@ func NewMultiplexor(log logging.SimpleLogging, keyGenerator PartitionKeyGenerato // Handle should be called for a given websocket request. It blocks // while writing to the websocket until the buffer is closed. func (m *Multiplexor) Handle(w http.ResponseWriter, r *http.Request) error { - key, err := m.keyGenerator.Generate(r) + // key, err := m.keyGenerator.Generate(r) - if err != nil { - return errors.Wrapf(err, "generating partition key") + jobID, ok := mux.Vars(r)["job-id"] + if !ok { + return fmt.Errorf("internal error: no job ID in route") } // Buffer size set to 1000 to ensure messages get queued. @@ -56,8 +59,8 @@ func (m *Multiplexor) Handle(w http.ResponseWriter, r *http.Request) error { buffer := make(chan string, 1000) // spinning up a goroutine for this since we are attempting to block on the read side. - go m.registry.Register(key, buffer) - defer m.registry.Deregister(key, buffer) + go m.registry.Register(jobID, buffer) + defer m.registry.Deregister(jobID, buffer) - return errors.Wrapf(m.writer.Write(w, r, buffer), "writing to ws %s", key) + return errors.Wrapf(m.writer.Write(w, r, buffer), "writing to ws %s", jobID) } diff --git a/server/events/mocks/mock_job_id_generator.go b/server/events/mocks/mock_job_id_generator.go new file mode 100644 index 000000000..001d437f7 --- /dev/null +++ b/server/events/mocks/mock_job_id_generator.go @@ -0,0 +1,94 @@ +// Code generated by pegomock. DO NOT EDIT. +// Source: github.com/runatlantis/atlantis/server/events (interfaces: JobIDGenerator) + +package mocks + +import ( + pegomock "github.com/petergtz/pegomock" + "reflect" + "time" +) + +type MockJobIDGenerator struct { + fail func(message string, callerSkip ...int) +} + +func NewMockJobIDGenerator(options ...pegomock.Option) *MockJobIDGenerator { + mock := &MockJobIDGenerator{} + for _, option := range options { + option.Apply(mock) + } + return mock +} + +func (mock *MockJobIDGenerator) SetFailHandler(fh pegomock.FailHandler) { mock.fail = fh } +func (mock *MockJobIDGenerator) FailHandler() pegomock.FailHandler { return mock.fail } + +func (mock *MockJobIDGenerator) GenerateJobID() string { + if mock == nil { + panic("mock must not be nil. Use myMock := NewMockJobIDGenerator().") + } + params := []pegomock.Param{} + result := pegomock.GetGenericMockFrom(mock).Invoke("GenerateJobID", params, []reflect.Type{reflect.TypeOf((*string)(nil)).Elem()}) + var ret0 string + if len(result) != 0 { + if result[0] != nil { + ret0 = result[0].(string) + } + } + return ret0 +} + +func (mock *MockJobIDGenerator) VerifyWasCalledOnce() *VerifierMockJobIDGenerator { + return &VerifierMockJobIDGenerator{ + mock: mock, + invocationCountMatcher: pegomock.Times(1), + } +} + +func (mock *MockJobIDGenerator) VerifyWasCalled(invocationCountMatcher pegomock.InvocationCountMatcher) *VerifierMockJobIDGenerator { + return &VerifierMockJobIDGenerator{ + mock: mock, + invocationCountMatcher: invocationCountMatcher, + } +} + +func (mock *MockJobIDGenerator) VerifyWasCalledInOrder(invocationCountMatcher pegomock.InvocationCountMatcher, inOrderContext *pegomock.InOrderContext) *VerifierMockJobIDGenerator { + return &VerifierMockJobIDGenerator{ + mock: mock, + invocationCountMatcher: invocationCountMatcher, + inOrderContext: inOrderContext, + } +} + +func (mock *MockJobIDGenerator) VerifyWasCalledEventually(invocationCountMatcher pegomock.InvocationCountMatcher, timeout time.Duration) *VerifierMockJobIDGenerator { + return &VerifierMockJobIDGenerator{ + mock: mock, + invocationCountMatcher: invocationCountMatcher, + timeout: timeout, + } +} + +type VerifierMockJobIDGenerator struct { + mock *MockJobIDGenerator + invocationCountMatcher pegomock.InvocationCountMatcher + inOrderContext *pegomock.InOrderContext + timeout time.Duration +} + +func (verifier *VerifierMockJobIDGenerator) GenerateJobID() *MockJobIDGenerator_GenerateJobID_OngoingVerification { + params := []pegomock.Param{} + methodInvocations := pegomock.GetGenericMockFrom(verifier.mock).Verify(verifier.inOrderContext, verifier.invocationCountMatcher, "GenerateJobID", params, verifier.timeout) + return &MockJobIDGenerator_GenerateJobID_OngoingVerification{mock: verifier.mock, methodInvocations: methodInvocations} +} + +type MockJobIDGenerator_GenerateJobID_OngoingVerification struct { + mock *MockJobIDGenerator + methodInvocations []pegomock.MethodInvocation +} + +func (c *MockJobIDGenerator_GenerateJobID_OngoingVerification) GetCapturedArguments() { +} + +func (c *MockJobIDGenerator_GenerateJobID_OngoingVerification) GetAllCapturedArguments() { +} diff --git a/server/events/models/models.go b/server/events/models/models.go index 15913e26a..d0da2fe45 100644 --- a/server/events/models/models.go +++ b/server/events/models/models.go @@ -425,6 +425,8 @@ type ProjectCommandContext struct { PolicySets valid.PolicySets // DeleteSourceBranchOnMerge will attempt to allow a branch to be deleted when merged (AzureDevOps & GitLab Support Only) DeleteSourceBranchOnMerge bool + // JobID + JobID string } // ProjectCloneDir creates relative path to clone the repo to. If we are running @@ -713,7 +715,7 @@ func (c CommandName) TitleString() string { } type ProjectCmdOutputLine struct { - ProjectInfo string + JobID string Line string diff --git a/server/events/project_command_builder.go b/server/events/project_command_builder.go index 6cfa861e5..1930a4ba1 100644 --- a/server/events/project_command_builder.go +++ b/server/events/project_command_builder.go @@ -34,6 +34,11 @@ const ( InfiniteProjectsPerPR = -1 ) +//go:generate pegomock generate -m --use-experimental-model-gen --package mocks -o mocks/mock_job_id_generator.go JobIDGenerator +type JobIDGenerator interface { + GenerateJobID() string +} + func NewProjectCommandBuilder( policyChecksSupported bool, parserValidator *yaml.ParserValidator, @@ -49,6 +54,7 @@ func NewProjectCommandBuilder( AutoplanFileList string, scope tally.Scope, logger logging.SimpleLogging, + jobIDGenerator JobIDGenerator, ) ProjectCommandBuilder { return NewProjectCommandBuilderWithLimit( policyChecksSupported, @@ -66,6 +72,7 @@ func NewProjectCommandBuilder( scope, logger, InfiniteProjectsPerPR, + jobIDGenerator, ) } @@ -85,6 +92,7 @@ func NewProjectCommandBuilderWithLimit( scope tally.Scope, logger logging.SimpleLogging, limit int, + jobIDGenerator JobIDGenerator, ) ProjectCommandBuilder { var projectCommandBuilder ProjectCommandBuilder = &DefaultProjectCommandBuilder{ ParserValidator: parserValidator, @@ -97,10 +105,11 @@ func NewProjectCommandBuilderWithLimit( SkipCloneNoChanges: skipCloneNoChanges, EnableRegExpCmd: EnableRegExpCmd, AutoplanFileList: AutoplanFileList, - ProjectCommandContextBuilder: NewProjectCommandContextBulder( + ProjectCommandContextBuilder: NewProjectCommandContextBuilder( policyChecksSupported, commentBuilder, scope, + jobIDGenerator, ), } diff --git a/server/events/project_command_builder_internal_test.go b/server/events/project_command_builder_internal_test.go index 5bf93f0e4..81ea46765 100644 --- a/server/events/project_command_builder_internal_test.go +++ b/server/events/project_command_builder_internal_test.go @@ -17,6 +17,12 @@ import ( . "github.com/runatlantis/atlantis/testing" ) +type MockJobIDGenerator struct{} + +func (j MockJobIDGenerator) GenerateJobID() string { + return "" +} + // Test different permutations of global and repo config. func TestBuildProjectCmdCtx(t *testing.T) { logger := logging.NewNoopLogger(t) @@ -613,6 +619,7 @@ projects: SkipCloneNoChanges: false, ProjectCommandContextBuilder: &DefaultProjectCommandContextBuilder{ CommentBuilder: &CommentParser{}, + JobIDGenerator: MockJobIDGenerator{}, }, AutoplanFileList: "**/*.tf,**/*.tfvars,**/*.tfvars.json,**/terragrunt.hcl", EnableRegExpCmd: false, @@ -805,6 +812,7 @@ projects: SkipCloneNoChanges: true, ProjectCommandContextBuilder: &DefaultProjectCommandContextBuilder{ CommentBuilder: &CommentParser{}, + JobIDGenerator: MockJobIDGenerator{}, }, AutoplanFileList: "**/*.tf,**/*.tfvars,**/*.tfvars.json,**/terragrunt.hcl", EnableRegExpCmd: true, @@ -1026,6 +1034,7 @@ workflows: ProjectCommandContextBuilder: &PolicyCheckProjectCommandContextBuilder{ ProjectCommandContextBuilder: &DefaultProjectCommandContextBuilder{ CommentBuilder: &CommentParser{}, + JobIDGenerator: MockJobIDGenerator{}, }, CommentBuilder: &CommentParser{}, }, diff --git a/server/events/project_command_builder_test.go b/server/events/project_command_builder_test.go index 1394f1887..8194045a2 100644 --- a/server/events/project_command_builder_test.go +++ b/server/events/project_command_builder_test.go @@ -8,6 +8,7 @@ import ( "testing" . "github.com/petergtz/pegomock" + "github.com/runatlantis/atlantis/server/controllers" "github.com/runatlantis/atlantis/server/events" "github.com/runatlantis/atlantis/server/events/matchers" "github.com/runatlantis/atlantis/server/events/mocks" @@ -160,14 +161,15 @@ projects: "**/*.tf,**/*.tfvars,**/*.tfvars.json,**/terragrunt.hcl", scope, logger, + controllers.JobIDGenerator{}, ) ctxs, err := builder.BuildAutoplanCommands(&events.CommandContext{ PullRequestStatus: models.PullReqStatus{ Mergeable: true, }, - Log: logger, - Scope: scope, + Log: logger, + Scope: scope, }) Ok(t, err) Equals(t, len(c.exp), len(ctxs)) @@ -428,6 +430,7 @@ projects: "**/*.tf,**/*.tfvars,**/*.tfvars.json,**/terragrunt.hcl", scope, logger, + controllers.JobIDGenerator{}, ) var actCtxs []models.ProjectCommandContext @@ -583,6 +586,7 @@ projects: "**/*.tf,**/*.tfvars,**/*.tfvars.json,**/terragrunt.hcl", scope, logger, + controllers.JobIDGenerator{}, ) ctxs, err := builder.BuildPlanCommands( @@ -674,6 +678,7 @@ func TestDefaultProjectCommandBuilder_BuildMultiApply(t *testing.T) { "**/*.tf,**/*.tfvars,**/*.tfvars.json,**/terragrunt.hcl", scope, logger, + controllers.JobIDGenerator{}, ) ctxs, err := builder.BuildApplyCommands( @@ -759,6 +764,7 @@ projects: "**/*.tf,**/*.tfvars,**/*.tfvars.json,**/terragrunt.hcl", scope, logger, + controllers.JobIDGenerator{}, ) ctx := &events.CommandContext{ @@ -838,6 +844,7 @@ func TestDefaultProjectCommandBuilder_EscapeArgs(t *testing.T) { "**/*.tf,**/*.tfvars,**/*.tfvars.json,**/terragrunt.hcl", scope, logger, + controllers.JobIDGenerator{}, ) var actCtxs []models.ProjectCommandContext @@ -1021,6 +1028,7 @@ projects: "**/*.tf,**/*.tfvars,**/*.tfvars.json,**/terragrunt.hcl", scope, logger, + controllers.JobIDGenerator{}, ) actCtxs, err := builder.BuildPlanCommands( @@ -1088,19 +1096,20 @@ projects: "**/*.tf,**/*.tfvars,**/*.tfvars.json,**/terragrunt.hcl", scope, logger, + controllers.JobIDGenerator{}, ) var actCtxs []models.ProjectCommandContext var err error actCtxs, err = builder.BuildAutoplanCommands(&events.CommandContext{ - HeadRepo: models.Repo{}, - Pull: models.PullRequest{}, - User: models.User{}, - Log: logger, + HeadRepo: models.Repo{}, + Pull: models.PullRequest{}, + User: models.User{}, + Log: logger, PullRequestStatus: models.PullReqStatus{ Mergeable: true, }, - Scope: scope, + Scope: scope, }) Ok(t, err) Equals(t, 0, len(actCtxs)) @@ -1146,14 +1155,15 @@ func TestDefaultProjectCommandBuilder_WithPolicyCheckEnabled_BuildAutoplanComman "**/*.tf,**/*.tfvars,**/*.tfvars.json,**/terragrunt.hcl", scope, logger, + controllers.JobIDGenerator{}, ) ctxs, err := builder.BuildAutoplanCommands(&events.CommandContext{ PullRequestStatus: models.PullReqStatus{ Mergeable: true, }, - Log: logger, - Scope: scope, + Log: logger, + Scope: scope, }) Ok(t, err) @@ -1228,6 +1238,7 @@ func TestDefaultProjectCommandBuilder_BuildVersionCommand(t *testing.T) { "**/*.tf,**/*.tfvars,**/*.tfvars.json,**/terragrunt.hcl", scope, logger, + controllers.JobIDGenerator{}, ) ctxs, err := builder.BuildVersionCommands( diff --git a/server/events/project_command_context_builder.go b/server/events/project_command_context_builder.go index 59a11656b..5fdb559ff 100644 --- a/server/events/project_command_context_builder.go +++ b/server/events/project_command_context_builder.go @@ -6,14 +6,15 @@ import ( "github.com/hashicorp/go-version" "github.com/hashicorp/terraform-config-inspect/tfconfig" - "github.com/uber-go/tally" "github.com/runatlantis/atlantis/server/events/models" "github.com/runatlantis/atlantis/server/events/yaml/valid" + "github.com/uber-go/tally" ) -func NewProjectCommandContextBulder(policyCheckEnabled bool, commentBuilder CommentBuilder, scope tally.Scope) ProjectCommandContextBuilder { +func NewProjectCommandContextBuilder(policyCheckEnabled bool, commentBuilder CommentBuilder, scope tally.Scope, jobIDGenerator JobIDGenerator) ProjectCommandContextBuilder { projectCommandContextBuilder := &DefaultProjectCommandContextBuilder{ CommentBuilder: commentBuilder, + JobIDGenerator: jobIDGenerator, } contextBuilderWithStats := &CommandScopedStatsProjectCommandContextBuilder{ @@ -91,6 +92,7 @@ func (cb *CommandScopedStatsProjectCommandContextBuilder) BuildProjectContext( type DefaultProjectCommandContextBuilder struct { CommentBuilder CommentBuilder + JobIDGenerator JobIDGenerator } func (cb *DefaultProjectCommandContextBuilder) BuildProjectContext( @@ -135,6 +137,7 @@ func (cb *DefaultProjectCommandContextBuilder) BuildProjectContext( contextFlags, ctx.Scope, ctx.PullRequestStatus, + cb.JobIDGenerator.GenerateJobID(), ) projectCmds = append(projectCmds, projectCmdContext) @@ -189,6 +192,7 @@ func (cb *PolicyCheckProjectCommandContextBuilder) BuildProjectContext( contextFlags, ctx.Scope, ctx.PullRequestStatus, + "", // No Job ID for policy check commands )) } @@ -209,6 +213,7 @@ func newProjectCommandContext(ctx *CommandContext, contextFlags *ContextFlags, scope tally.Scope, pullStatus models.PullReqStatus, + jobID string, ) models.ProjectCommandContext { var projectPlanStatus models.ProjectPlanStatus @@ -257,7 +262,8 @@ func newProjectCommandContext(ctx *CommandContext, Workspace: projCfg.Workspace, PolicySets: policySets, Tags: projCfg.Tags, - PullReqStatus: pullStatus, + PullReqStatus: pullStatus, + JobID: jobID, } } diff --git a/server/events/project_command_context_builder_test.go b/server/events/project_command_context_builder_test.go index 74ab4a0d7..5d08339d1 100644 --- a/server/events/project_command_context_builder_test.go +++ b/server/events/project_command_context_builder_test.go @@ -17,6 +17,7 @@ func TestProjectCommandContextBuilder_PullStatus(t *testing.T) { mockCommentBuilder := mocks.NewMockCommentBuilder() subject := events.DefaultProjectCommandContextBuilder{ CommentBuilder: mockCommentBuilder, + JobIDGenerator: mocks.NewMockJobIDGenerator(), } projRepoRelDir := "dir1" diff --git a/server/events/pull_closed_executor.go b/server/events/pull_closed_executor.go index 955a573fe..642cbed1c 100644 --- a/server/events/pull_closed_executor.go +++ b/server/events/pull_closed_executor.go @@ -41,6 +41,8 @@ type PullCleaner interface { CleanUpPull(repo models.Repo, pull models.PullRequest) error } +// TODO: Retrieve job ID from the github checks + // PullClosedExecutor executes the tasks required to clean up a closed pull // request. type PullClosedExecutor struct { diff --git a/server/handlers/project_command_output_handler.go b/server/handlers/project_command_output_handler.go index 2504e9649..c98d92b56 100644 --- a/server/handlers/project_command_output_handler.go +++ b/server/handlers/project_command_output_handler.go @@ -1,9 +1,10 @@ package handlers import ( + "sync" + "github.com/runatlantis/atlantis/server/events/models" "github.com/runatlantis/atlantis/server/logging" - "sync" ) // AsyncProjectCommandOutputHandler is a handler to transport terraform client @@ -49,10 +50,10 @@ type ProjectCommandOutputHandler interface { // Register registers a channel and blocks until it is caught up. Callers should call this asynchronously when attempting // to read the channel in the same goroutine - Register(projectInfo string, receiver chan string) + Register(jobID string, receiver chan string) // Deregister removes a channel from successive updates and closes it. - Deregister(projectInfo string, receiver chan string) + Deregister(jobID string, receiver chan string) // Listens for msg from channel Handle() @@ -67,7 +68,7 @@ type ProjectCommandOutputHandler interface { //go:generate pegomock generate -m --use-experimental-model-gen --package mocks -o mocks/mock_resource_cleaner.go ResourceCleaner type ResourceCleaner interface { - CleanUp(pull string) + CleanUp(jobID string) } func NewAsyncProjectCommandOutputHandler( @@ -88,27 +89,27 @@ func NewAsyncProjectCommandOutputHandler( func (p *AsyncProjectCommandOutputHandler) Send(ctx models.ProjectCommandContext, msg string) { p.projectCmdOutput <- &models.ProjectCmdOutputLine{ - ProjectInfo: ctx.PullInfo(), - Line: msg, + JobID: ctx.JobID, + Line: msg, } } -func (p *AsyncProjectCommandOutputHandler) Register(projectInfo string, receiver chan string) { - p.addChan(receiver, projectInfo) +func (p *AsyncProjectCommandOutputHandler) Register(jobID string, receiver chan string) { + p.addChan(receiver, jobID) } func (p *AsyncProjectCommandOutputHandler) Handle() { for msg := range p.projectCmdOutput { if msg.ClearBuffBefore { - p.clearLogLines(msg.ProjectInfo) + p.clearLogLines(msg.JobID) } - p.writeLogLine(msg.ProjectInfo, msg.Line) + p.writeLogLine(msg.JobID, msg.Line) } } func (p *AsyncProjectCommandOutputHandler) Clear(ctx models.ProjectCommandContext) { p.projectCmdOutput <- &models.ProjectCmdOutputLine{ - ProjectInfo: ctx.PullInfo(), + JobID: ctx.JobID, Line: models.LogStreamingClearMsg, ClearBuffBefore: true, } @@ -123,15 +124,15 @@ func (p *AsyncProjectCommandOutputHandler) SetJobURLWithStatus(ctx models.Projec return p.projectStatusUpdater.UpdateProject(ctx, cmdName, status, url) } -func (p *AsyncProjectCommandOutputHandler) clearLogLines(pull string) { +func (p *AsyncProjectCommandOutputHandler) clearLogLines(jobID string) { p.projectOutputBuffersLock.Lock() - delete(p.projectOutputBuffers, pull) + delete(p.projectOutputBuffers, jobID) p.projectOutputBuffersLock.Unlock() } -func (p *AsyncProjectCommandOutputHandler) addChan(ch chan string, pull string) { +func (p *AsyncProjectCommandOutputHandler) addChan(ch chan string, jobID string) { p.projectOutputBuffersLock.RLock() - buffer := p.projectOutputBuffers[pull] + buffer := p.projectOutputBuffers[jobID] p.projectOutputBuffersLock.RUnlock() for _, line := range buffer { @@ -141,17 +142,17 @@ func (p *AsyncProjectCommandOutputHandler) addChan(ch chan string, pull string) // add the channel to our registry after we backfill the contents of the buffer, // to prevent new messages coming in interleaving with this backfill. p.receiverBuffersLock.Lock() - if p.receiverBuffers[pull] == nil { - p.receiverBuffers[pull] = map[chan string]bool{} + if p.receiverBuffers[jobID] == nil { + p.receiverBuffers[jobID] = map[chan string]bool{} } - p.receiverBuffers[pull][ch] = true + p.receiverBuffers[jobID][ch] = true p.receiverBuffersLock.Unlock() } //Add log line to buffer and send to all current channels -func (p *AsyncProjectCommandOutputHandler) writeLogLine(pull string, line string) { +func (p *AsyncProjectCommandOutputHandler) writeLogLine(jobID string, line string) { p.receiverBuffersLock.Lock() - for ch := range p.receiverBuffers[pull] { + for ch := range p.receiverBuffers[jobID] { select { case ch <- line: default: @@ -161,7 +162,7 @@ func (p *AsyncProjectCommandOutputHandler) writeLogLine(pull string, line string // 2. Client does not close the conn and the closeHandler() is not executed -> the // receiverChan will be blocking for N number of messages (equal to buffer size) // before we delete the channel and clean up the resources. - delete(p.receiverBuffers[pull], ch) + delete(p.receiverBuffers[jobID], ch) } } p.receiverBuffersLock.Unlock() @@ -172,39 +173,39 @@ func (p *AsyncProjectCommandOutputHandler) writeLogLine(pull string, line string } p.projectOutputBuffersLock.Lock() - if p.projectOutputBuffers[pull] == nil { - p.projectOutputBuffers[pull] = []string{} + if p.projectOutputBuffers[jobID] == nil { + p.projectOutputBuffers[jobID] = []string{} } - p.projectOutputBuffers[pull] = append(p.projectOutputBuffers[pull], line) + p.projectOutputBuffers[jobID] = append(p.projectOutputBuffers[jobID], line) p.projectOutputBuffersLock.Unlock() } //Remove channel, so client no longer receives Terraform output -func (p *AsyncProjectCommandOutputHandler) Deregister(pull string, ch chan string) { - p.logger.Debug("Removing channel for %s", pull) +func (p *AsyncProjectCommandOutputHandler) Deregister(jobID string, ch chan string) { + p.logger.Debug("Removing channel for %s", jobID) p.receiverBuffersLock.Lock() - delete(p.receiverBuffers[pull], ch) + delete(p.receiverBuffers[jobID], ch) p.receiverBuffersLock.Unlock() } -func (p *AsyncProjectCommandOutputHandler) GetReceiverBufferForPull(pull string) map[chan string]bool { - return p.receiverBuffers[pull] +func (p *AsyncProjectCommandOutputHandler) GetReceiverBufferForPull(jobID string) map[chan string]bool { + return p.receiverBuffers[jobID] } -func (p *AsyncProjectCommandOutputHandler) GetProjectOutputBuffer(pull string) []string { - return p.projectOutputBuffers[pull] +func (p *AsyncProjectCommandOutputHandler) GetProjectOutputBuffer(jobID string) []string { + return p.projectOutputBuffers[jobID] } -func (p *AsyncProjectCommandOutputHandler) CleanUp(pull string) { +func (p *AsyncProjectCommandOutputHandler) CleanUp(jobID string) { p.projectOutputBuffersLock.Lock() - delete(p.projectOutputBuffers, pull) + delete(p.projectOutputBuffers, jobID) p.projectOutputBuffersLock.Unlock() // Only delete the pull record from receiver buffers. // WS channel will be closed when the user closes the browser tab // in closeHanlder(). p.receiverBuffersLock.Lock() - delete(p.receiverBuffers, pull) + delete(p.receiverBuffers, jobID) p.receiverBuffersLock.Unlock() } @@ -214,8 +215,8 @@ type NoopProjectOutputHandler struct{} func (p *NoopProjectOutputHandler) Send(ctx models.ProjectCommandContext, msg string) { } -func (p *NoopProjectOutputHandler) Register(projectInfo string, receiver chan string) {} -func (p *NoopProjectOutputHandler) Deregister(projectInfo string, receiver chan string) {} +func (p *NoopProjectOutputHandler) Register(jobID string, receiver chan string) {} +func (p *NoopProjectOutputHandler) Deregister(jobID string, receiver chan string) {} func (p *NoopProjectOutputHandler) Handle() { } @@ -227,5 +228,5 @@ func (p *NoopProjectOutputHandler) SetJobURLWithStatus(ctx models.ProjectCommand return nil } -func (p *NoopProjectOutputHandler) CleanUp(pull string) { +func (p *NoopProjectOutputHandler) CleanUp(jobID string) { } diff --git a/server/handlers/project_command_output_handler_test.go b/server/handlers/project_command_output_handler_test.go index a37a2c90b..280130718 100644 --- a/server/handlers/project_command_output_handler_test.go +++ b/server/handlers/project_command_output_handler_test.go @@ -40,6 +40,7 @@ func createTestProjectCmdContext(t *testing.T) models.ProjectCommandContext { Workspace: "myworkspace", RepoRelDir: "test-dir", ProjectName: "test-project", + JobID: "1234", } } @@ -77,7 +78,7 @@ func TestProjectCommandOutputHandler(t *testing.T) { // Note: We call this synchronously because otherwise // there could be a race where we are unable to register the channel // before sending messages due to the way we lock our buffer memory cache - projectOutputHandler.Register(ctx.PullInfo(), ch) + projectOutputHandler.Register(ctx.JobID, ch) wg.Add(1) @@ -101,15 +102,13 @@ func TestProjectCommandOutputHandler(t *testing.T) { projectOutputHandler := createProjectCommandOutputHandler(t) - - ch := make(chan string) // register channel and backfill from buffer // Note: We call this synchronously because otherwise // there could be a race where we are unable to register the channel // before sending messages due to the way we lock our buffer memory cache - projectOutputHandler.Register(ctx.PullInfo(), ch) + projectOutputHandler.Register(ctx.JobID, ch) wg.Add(1) // read from channel asynchronously @@ -132,7 +131,7 @@ func TestProjectCommandOutputHandler(t *testing.T) { dfProjectOutputHandler, ok := projectOutputHandler.(*handlers.AsyncProjectCommandOutputHandler) assert.True(t, ok) - assert.Empty(t, dfProjectOutputHandler.GetProjectOutputBuffer(ctx.PullInfo())) + assert.Empty(t, dfProjectOutputHandler.GetProjectOutputBuffer(ctx.JobID)) }) t.Run("copies buffer to new channels", func(t *testing.T) { @@ -163,7 +162,7 @@ func TestProjectCommandOutputHandler(t *testing.T) { // Note: We call this synchronously because otherwise // there could be a race where we are unable to register the channel // before sending messages due to the way we lock our buffer memory cache - projectOutputHandler.Register(ctx.PullInfo(), ch) + projectOutputHandler.Register(ctx.JobID, ch) projectOutputHandler.Send(ctx, Msg) wg.Wait() diff --git a/server/router.go b/server/router.go index 560729470..e7728f942 100644 --- a/server/router.go +++ b/server/router.go @@ -1,7 +1,6 @@ package server import ( - "fmt" "net/url" "github.com/gorilla/mux" @@ -40,18 +39,27 @@ func (r *Router) GenerateLockURL(lockID string) string { } func (r *Router) GenerateProjectJobURL(ctx models.ProjectCommandContext) (string, error) { - pull := ctx.Pull - projectIdentifier := models.GetProjectIdentifier(ctx.RepoRelDir, ctx.ProjectName) - jobURL, err := r.Underlying.Get(r.ProjectJobsViewRouteName).URL( - "org", pull.BaseRepo.Owner, - "repo", pull.BaseRepo.Name, - "pull", fmt.Sprintf("%d", pull.Num), - "project", projectIdentifier, - "workspace", ctx.Workspace, + jobURL, err := r.Underlying.Get((r.ProjectJobsViewRouteName)).URL( + "job-id", ctx.JobID, ) if err != nil { - return "", errors.Wrapf(err, "creating job url for %s/%d/%s/%s", pull.BaseRepo.FullName, pull.Num, projectIdentifier, ctx.Workspace) + return "", errors.Wrapf(err, "creating job url for %s", ctx.JobID) } return r.AtlantisURL.String() + jobURL.String(), nil + + // pull := ctx.Pull + // projectIdentifier := models.GetProjectIdentifier(ctx.RepoRelDir, ctx.ProjectName) + // jobURL, err := r.Underlying.Get(r.ProjectJobsViewRouteName).URL( + // "org", pull.BaseRepo.Owner, + // "repo", pull.BaseRepo.Name, + // "pull", fmt.Sprintf("%d", pull.Num), + // "project", projectIdentifier, + // "workspace", ctx.Workspace, + // ) + // if err != nil { + // return "", errors.Wrapf(err, "creating job url for %s/%d/%s/%s", pull.BaseRepo.FullName, pull.Num, projectIdentifier, ctx.Workspace) + // } + + // return r.AtlantisURL.String() + jobURL.String(), nil } diff --git a/server/router_test.go b/server/router_test.go index ccabee44d..11aa4fd75 100644 --- a/server/router_test.go +++ b/server/router_test.go @@ -1,13 +1,16 @@ package server_test import ( + "fmt" "net/http" "testing" + "github.com/google/uuid" "github.com/gorilla/mux" "github.com/runatlantis/atlantis/server" "github.com/runatlantis/atlantis/server/events/models" . "github.com/runatlantis/atlantis/testing" + "github.com/stretchr/testify/assert" ) func TestRouter_GenerateLockURL(t *testing.T) { @@ -67,7 +70,7 @@ func setupJobsRouter(t *testing.T) *server.Router { Ok(t, err) underlyingRouter := mux.NewRouter() - underlyingRouter.HandleFunc("/jobs/{org}/{repo}/{pull}/{project}/{workspace}", func(_ http.ResponseWriter, _ *http.Request) {}).Methods("GET").Name("project-jobs-detail") + underlyingRouter.HandleFunc("/jobs/{job-id}", func(_ http.ResponseWriter, _ *http.Request) {}).Methods("GET").Name("project-jobs-detail") return &server.Router{ AtlantisURL: atlantisURL, @@ -76,27 +79,20 @@ func setupJobsRouter(t *testing.T) *server.Router { } } -func TestGenerateProjectJobURL_ShouldGenerateURLWithProjectNameWhenProjectNameSpecified(t *testing.T) { +func TestGenerateProjectJobURL_ShouldGenerateURLWhenJobIDSpecified(t *testing.T) { router := setupJobsRouter(t) + jobID := uuid.New().String() ctx := models.ProjectCommandContext{ - Pull: models.PullRequest{ - BaseRepo: models.Repo{ - Owner: "test-owner", - Name: "test-repo", - }, - Num: 1, - }, - ProjectName: "test-project", - Workspace: "default", + JobID: jobID, } - expectedURL := "http://localhost:4141/jobs/test-owner/test-repo/1/test-project/default" + expectedURL := fmt.Sprintf("http://localhost:4141/jobs/%s", jobID) gotURL, err := router.GenerateProjectJobURL(ctx) Ok(t, err) Equals(t, expectedURL, gotURL) } -func TestGenerateProjectJobURL_ShouldGenerateURLWithDirectoryAndWorkspaceWhenProjectNameNotSpecified(t *testing.T) { +func TestGenerateProjectJobURL_ShouldReturnErrorWhenJobIDNotSpecified(t *testing.T) { router := setupJobsRouter(t) ctx := models.ProjectCommandContext{ Pull: models.PullRequest{ @@ -106,12 +102,10 @@ func TestGenerateProjectJobURL_ShouldGenerateURLWithDirectoryAndWorkspaceWhenPro }, Num: 1, }, - RepoRelDir: "ops/terraform/test-root", - Workspace: "default", + RepoRelDir: "ops/terraform/", } - expectedURL := "http://localhost:4141/jobs/test-owner/test-repo/1/ops-terraform-test-root/default" - gotURL, err := router.GenerateProjectJobURL(ctx) - Ok(t, err) - Equals(t, expectedURL, gotURL) + gotURL, err := router.GenerateProjectJobURL(ctx) + assert.Error(t, err) + Equals(t, "", gotURL) } diff --git a/server/server.go b/server/server.go index bd9984124..a4849812b 100644 --- a/server/server.go +++ b/server/server.go @@ -58,9 +58,9 @@ import ( "github.com/runatlantis/atlantis/server/events" "github.com/runatlantis/atlantis/server/events/models" "github.com/runatlantis/atlantis/server/events/vcs" - lyft_vcs "github.com/runatlantis/atlantis/server/events/vcs/lyft" "github.com/runatlantis/atlantis/server/events/vcs/bitbucketcloud" "github.com/runatlantis/atlantis/server/events/vcs/bitbucketserver" + lyft_vcs "github.com/runatlantis/atlantis/server/events/vcs/lyft" "github.com/runatlantis/atlantis/server/events/webhooks" "github.com/runatlantis/atlantis/server/events/yaml" "github.com/runatlantis/atlantis/server/logging" @@ -518,6 +518,7 @@ func NewServer(userConfig UserConfig, config Config) (*Server, error) { statsScope, logger, userConfig.MaxProjectsPerPR, + controllers.JobIDGenerator{}, ) showStepRunner, err := runtime.NewShowStepRunner(terraformClient, defaultTfVersion) @@ -878,8 +879,8 @@ func (s *Server) Start() error { s.Router.HandleFunc("/locks", s.LocksController.DeleteLock).Methods("DELETE").Queries("id", "{id:.*}") s.Router.HandleFunc("/lock", s.LocksController.GetLock).Methods("GET"). Queries(LockViewRouteIDQueryParam, fmt.Sprintf("{%s}", LockViewRouteIDQueryParam)).Name(LockViewRouteName) - s.Router.HandleFunc("/jobs/{org}/{repo}/{pull}/{project}/{workspace}", s.JobsController.GetProjectJobs).Methods("GET").Name(ProjectJobsViewRouteName) - s.Router.HandleFunc("/jobs/{org}/{repo}/{pull}/{project}/{workspace}/ws", s.JobsController.GetProjectJobsWS).Methods("GET") + s.Router.HandleFunc("/jobs/{job-id}", s.JobsController.GetProjectJobs).Methods("GET").Name(ProjectJobsViewRouteName) + s.Router.HandleFunc("/jobs/{job-id}/ws", s.JobsController.GetProjectJobsWS).Methods("GET") n := negroni.New(&negroni.Recovery{ Logger: log.New(os.Stdout, "", log.LstdFlags), From 79c4fc4ab376065016d8a949a9be3a2732dd9d46 Mon Sep 17 00:00:00 2001 From: Aayush Gupta Date: Mon, 24 Jan 2022 11:05:38 -0800 Subject: [PATCH 02/19] Adding pullToJob mapping to ensure plan and apply have same job IDs --- .../events/events_controller_e2e_test.go | 3 +- server/events/models/models.go | 8 + server/events/project_command_builder.go | 10 +- .../project_command_builder_internal_test.go | 8 +- server/events/project_command_builder_test.go | 22 +- .../events/project_command_context_builder.go | 7 +- .../project_command_context_builder_test.go | 3 +- server/events/pull_closed_executor.go | 12 +- .../mocks/matchers/models_jobcontext.go | 33 +++ .../mocks/matchers/models_pullrequest.go | 33 +++ .../handlers/mocks/mock_job_id_generator.go | 113 +++++++++ .../mock_project_command_output_handler.go | 238 +++++++++++------- .../project_command_output_handler.go | 64 ++++- server/server.go | 2 +- 14 files changed, 420 insertions(+), 136 deletions(-) create mode 100644 server/handlers/mocks/matchers/models_jobcontext.go create mode 100644 server/handlers/mocks/matchers/models_pullrequest.go create mode 100644 server/handlers/mocks/mock_job_id_generator.go diff --git a/server/controllers/events/events_controller_e2e_test.go b/server/controllers/events/events_controller_e2e_test.go index 47918ab18..6b1981f6c 100644 --- a/server/controllers/events/events_controller_e2e_test.go +++ b/server/controllers/events/events_controller_e2e_test.go @@ -18,7 +18,6 @@ import ( "github.com/hashicorp/go-version" . "github.com/petergtz/pegomock" "github.com/runatlantis/atlantis/server" - "github.com/runatlantis/atlantis/server/controllers" events_controllers "github.com/runatlantis/atlantis/server/controllers/events" "github.com/runatlantis/atlantis/server/core/db" "github.com/runatlantis/atlantis/server/core/locking" @@ -913,7 +912,7 @@ func setupE2E(t *testing.T, repoDir string) (events_controllers.VCSEventsControl "**/*.tf,**/*.tfvars,**/*.tfvars.json,**/terragrunt.hcl", statsScope, logger, - controllers.JobIDGenerator{}, + projectCmdOutputHandler, ) showStepRunner, err := runtime.NewShowStepRunner(terraformClient, defaultTFVersion) diff --git a/server/events/models/models.go b/server/events/models/models.go index d0da2fe45..45e8f67bd 100644 --- a/server/events/models/models.go +++ b/server/events/models/models.go @@ -714,6 +714,14 @@ func (c CommandName) TitleString() string { return strings.Title(strings.ReplaceAll(strings.ToLower(c.String()), "_", " ")) } +type JobContext struct { + PullNum int + Repo string + ProjectName string + Workspace string + HeadCommit string +} + type ProjectCmdOutputLine struct { JobID string diff --git a/server/events/project_command_builder.go b/server/events/project_command_builder.go index 1930a4ba1..6f79a378c 100644 --- a/server/events/project_command_builder.go +++ b/server/events/project_command_builder.go @@ -5,6 +5,7 @@ import ( "os" "github.com/runatlantis/atlantis/server/events/yaml/valid" + "github.com/runatlantis/atlantis/server/handlers" "github.com/runatlantis/atlantis/server/logging" "github.com/uber-go/tally" @@ -34,11 +35,6 @@ const ( InfiniteProjectsPerPR = -1 ) -//go:generate pegomock generate -m --use-experimental-model-gen --package mocks -o mocks/mock_job_id_generator.go JobIDGenerator -type JobIDGenerator interface { - GenerateJobID() string -} - func NewProjectCommandBuilder( policyChecksSupported bool, parserValidator *yaml.ParserValidator, @@ -54,7 +50,7 @@ func NewProjectCommandBuilder( AutoplanFileList string, scope tally.Scope, logger logging.SimpleLogging, - jobIDGenerator JobIDGenerator, + jobIDGenerator handlers.JobIDGenerator, ) ProjectCommandBuilder { return NewProjectCommandBuilderWithLimit( policyChecksSupported, @@ -92,7 +88,7 @@ func NewProjectCommandBuilderWithLimit( scope tally.Scope, logger logging.SimpleLogging, limit int, - jobIDGenerator JobIDGenerator, + jobIDGenerator handlers.JobIDGenerator, ) ProjectCommandBuilder { var projectCommandBuilder ProjectCommandBuilder = &DefaultProjectCommandBuilder{ ParserValidator: parserValidator, diff --git a/server/events/project_command_builder_internal_test.go b/server/events/project_command_builder_internal_test.go index 81ea46765..37095db13 100644 --- a/server/events/project_command_builder_internal_test.go +++ b/server/events/project_command_builder_internal_test.go @@ -10,6 +10,8 @@ import ( "github.com/runatlantis/atlantis/server/events/matchers" "github.com/runatlantis/atlantis/server/events/models" vcsmocks "github.com/runatlantis/atlantis/server/events/vcs/mocks" + handlermocks "github.com/runatlantis/atlantis/server/handlers/mocks" + "github.com/runatlantis/atlantis/server/events/yaml" "github.com/runatlantis/atlantis/server/events/yaml/valid" "github.com/runatlantis/atlantis/server/logging" @@ -619,7 +621,7 @@ projects: SkipCloneNoChanges: false, ProjectCommandContextBuilder: &DefaultProjectCommandContextBuilder{ CommentBuilder: &CommentParser{}, - JobIDGenerator: MockJobIDGenerator{}, + JobIDGenerator: handlermocks.NewMockJobIDGenerator(), }, AutoplanFileList: "**/*.tf,**/*.tfvars,**/*.tfvars.json,**/terragrunt.hcl", EnableRegExpCmd: false, @@ -812,7 +814,7 @@ projects: SkipCloneNoChanges: true, ProjectCommandContextBuilder: &DefaultProjectCommandContextBuilder{ CommentBuilder: &CommentParser{}, - JobIDGenerator: MockJobIDGenerator{}, + JobIDGenerator: handlermocks.NewMockJobIDGenerator(), }, AutoplanFileList: "**/*.tf,**/*.tfvars,**/*.tfvars.json,**/terragrunt.hcl", EnableRegExpCmd: true, @@ -1034,7 +1036,7 @@ workflows: ProjectCommandContextBuilder: &PolicyCheckProjectCommandContextBuilder{ ProjectCommandContextBuilder: &DefaultProjectCommandContextBuilder{ CommentBuilder: &CommentParser{}, - JobIDGenerator: MockJobIDGenerator{}, + JobIDGenerator: handlermocks.NewMockJobIDGenerator(), }, CommentBuilder: &CommentParser{}, }, diff --git a/server/events/project_command_builder_test.go b/server/events/project_command_builder_test.go index 8194045a2..40fbf1781 100644 --- a/server/events/project_command_builder_test.go +++ b/server/events/project_command_builder_test.go @@ -8,7 +8,6 @@ import ( "testing" . "github.com/petergtz/pegomock" - "github.com/runatlantis/atlantis/server/controllers" "github.com/runatlantis/atlantis/server/events" "github.com/runatlantis/atlantis/server/events/matchers" "github.com/runatlantis/atlantis/server/events/mocks" @@ -16,6 +15,7 @@ import ( vcsmocks "github.com/runatlantis/atlantis/server/events/vcs/mocks" "github.com/runatlantis/atlantis/server/events/yaml" "github.com/runatlantis/atlantis/server/events/yaml/valid" + handlermocks "github.com/runatlantis/atlantis/server/handlers/mocks" "github.com/runatlantis/atlantis/server/logging" "github.com/runatlantis/atlantis/server/metrics" . "github.com/runatlantis/atlantis/testing" @@ -161,7 +161,7 @@ projects: "**/*.tf,**/*.tfvars,**/*.tfvars.json,**/terragrunt.hcl", scope, logger, - controllers.JobIDGenerator{}, + handlermocks.NewMockJobIDGenerator(), ) ctxs, err := builder.BuildAutoplanCommands(&events.CommandContext{ @@ -430,7 +430,7 @@ projects: "**/*.tf,**/*.tfvars,**/*.tfvars.json,**/terragrunt.hcl", scope, logger, - controllers.JobIDGenerator{}, + handlermocks.NewMockJobIDGenerator(), ) var actCtxs []models.ProjectCommandContext @@ -586,7 +586,7 @@ projects: "**/*.tf,**/*.tfvars,**/*.tfvars.json,**/terragrunt.hcl", scope, logger, - controllers.JobIDGenerator{}, + handlermocks.NewMockJobIDGenerator(), ) ctxs, err := builder.BuildPlanCommands( @@ -678,7 +678,7 @@ func TestDefaultProjectCommandBuilder_BuildMultiApply(t *testing.T) { "**/*.tf,**/*.tfvars,**/*.tfvars.json,**/terragrunt.hcl", scope, logger, - controllers.JobIDGenerator{}, + handlermocks.NewMockJobIDGenerator(), ) ctxs, err := builder.BuildApplyCommands( @@ -764,7 +764,7 @@ projects: "**/*.tf,**/*.tfvars,**/*.tfvars.json,**/terragrunt.hcl", scope, logger, - controllers.JobIDGenerator{}, + handlermocks.NewMockJobIDGenerator(), ) ctx := &events.CommandContext{ @@ -844,7 +844,7 @@ func TestDefaultProjectCommandBuilder_EscapeArgs(t *testing.T) { "**/*.tf,**/*.tfvars,**/*.tfvars.json,**/terragrunt.hcl", scope, logger, - controllers.JobIDGenerator{}, + handlermocks.NewMockJobIDGenerator(), ) var actCtxs []models.ProjectCommandContext @@ -1028,7 +1028,7 @@ projects: "**/*.tf,**/*.tfvars,**/*.tfvars.json,**/terragrunt.hcl", scope, logger, - controllers.JobIDGenerator{}, + handlermocks.NewMockJobIDGenerator(), ) actCtxs, err := builder.BuildPlanCommands( @@ -1096,7 +1096,7 @@ projects: "**/*.tf,**/*.tfvars,**/*.tfvars.json,**/terragrunt.hcl", scope, logger, - controllers.JobIDGenerator{}, + handlermocks.NewMockJobIDGenerator(), ) var actCtxs []models.ProjectCommandContext @@ -1155,7 +1155,7 @@ func TestDefaultProjectCommandBuilder_WithPolicyCheckEnabled_BuildAutoplanComman "**/*.tf,**/*.tfvars,**/*.tfvars.json,**/terragrunt.hcl", scope, logger, - controllers.JobIDGenerator{}, + handlermocks.NewMockJobIDGenerator(), ) ctxs, err := builder.BuildAutoplanCommands(&events.CommandContext{ @@ -1238,7 +1238,7 @@ func TestDefaultProjectCommandBuilder_BuildVersionCommand(t *testing.T) { "**/*.tf,**/*.tfvars,**/*.tfvars.json,**/terragrunt.hcl", scope, logger, - controllers.JobIDGenerator{}, + handlermocks.NewMockJobIDGenerator(), ) ctxs, err := builder.BuildVersionCommands( diff --git a/server/events/project_command_context_builder.go b/server/events/project_command_context_builder.go index 5fdb559ff..41625e36f 100644 --- a/server/events/project_command_context_builder.go +++ b/server/events/project_command_context_builder.go @@ -8,10 +8,11 @@ import ( "github.com/hashicorp/terraform-config-inspect/tfconfig" "github.com/runatlantis/atlantis/server/events/models" "github.com/runatlantis/atlantis/server/events/yaml/valid" + "github.com/runatlantis/atlantis/server/handlers" "github.com/uber-go/tally" ) -func NewProjectCommandContextBuilder(policyCheckEnabled bool, commentBuilder CommentBuilder, scope tally.Scope, jobIDGenerator JobIDGenerator) ProjectCommandContextBuilder { +func NewProjectCommandContextBuilder(policyCheckEnabled bool, commentBuilder CommentBuilder, scope tally.Scope, jobIDGenerator handlers.JobIDGenerator) ProjectCommandContextBuilder { projectCommandContextBuilder := &DefaultProjectCommandContextBuilder{ CommentBuilder: commentBuilder, JobIDGenerator: jobIDGenerator, @@ -92,7 +93,7 @@ func (cb *CommandScopedStatsProjectCommandContextBuilder) BuildProjectContext( type DefaultProjectCommandContextBuilder struct { CommentBuilder CommentBuilder - JobIDGenerator JobIDGenerator + JobIDGenerator handlers.JobIDGenerator } func (cb *DefaultProjectCommandContextBuilder) BuildProjectContext( @@ -137,7 +138,7 @@ func (cb *DefaultProjectCommandContextBuilder) BuildProjectContext( contextFlags, ctx.Scope, ctx.PullRequestStatus, - cb.JobIDGenerator.GenerateJobID(), + cb.JobIDGenerator.GenerateJobID(ctx.Pull, prjCfg.Name, prjCfg.Workspace), ) projectCmds = append(projectCmds, projectCmdContext) diff --git a/server/events/project_command_context_builder_test.go b/server/events/project_command_context_builder_test.go index 5d08339d1..7f8e86263 100644 --- a/server/events/project_command_context_builder_test.go +++ b/server/events/project_command_context_builder_test.go @@ -8,6 +8,7 @@ import ( "github.com/runatlantis/atlantis/server/events/mocks" "github.com/runatlantis/atlantis/server/events/models" "github.com/runatlantis/atlantis/server/events/yaml/valid" + handlermocks "github.com/runatlantis/atlantis/server/handlers/mocks" "github.com/runatlantis/atlantis/server/logging" "github.com/stretchr/testify/assert" ) @@ -17,7 +18,7 @@ func TestProjectCommandContextBuilder_PullStatus(t *testing.T) { mockCommentBuilder := mocks.NewMockCommentBuilder() subject := events.DefaultProjectCommandContextBuilder{ CommentBuilder: mockCommentBuilder, - JobIDGenerator: mocks.NewMockJobIDGenerator(), + JobIDGenerator: handlermocks.NewMockJobIDGenerator(), } projRepoRelDir := "dir1" diff --git a/server/events/pull_closed_executor.go b/server/events/pull_closed_executor.go index 642cbed1c..78652ae3b 100644 --- a/server/events/pull_closed_executor.go +++ b/server/events/pull_closed_executor.go @@ -90,8 +90,16 @@ func (p *PullClosedExecutor) CleanUpPull(repo models.Repo, pull models.PullReque // with same dir and workspace. If a project name has not been set, we'll use the dir and // workspace to build project key. // Source: https://www.runatlantis.io/docs/repo-level-atlantis-yaml.html#reference - projectKey := models.BuildPullInfo(pullStatus.Pull.BaseRepo.FullName, pull.Num, project.ProjectName, project.RepoRelDir, project.Workspace) - p.LogStreamResourceCleaner.CleanUp(projectKey) + // projectKey := models.BuildPullInfo(pullStatus.Pull.BaseRepo.FullName, pull.Num, project.ProjectName, project.RepoRelDir, project.Workspace) + + jobContext := models.JobContext{ + PullNum: pull.Num, + Repo: pull.BaseRepo.Name, + Workspace: project.Workspace, + ProjectName: project.ProjectName, + HeadCommit: pull.HeadCommit, + } + p.LogStreamResourceCleaner.CleanUp(jobContext) } } diff --git a/server/handlers/mocks/matchers/models_jobcontext.go b/server/handlers/mocks/matchers/models_jobcontext.go new file mode 100644 index 000000000..822da3bf2 --- /dev/null +++ b/server/handlers/mocks/matchers/models_jobcontext.go @@ -0,0 +1,33 @@ +// Code generated by pegomock. DO NOT EDIT. +package matchers + +import ( + "github.com/petergtz/pegomock" + "reflect" + + models "github.com/runatlantis/atlantis/server/events/models" +) + +func AnyModelsJobContext() models.JobContext { + pegomock.RegisterMatcher(pegomock.NewAnyMatcher(reflect.TypeOf((*(models.JobContext))(nil)).Elem())) + var nullValue models.JobContext + return nullValue +} + +func EqModelsJobContext(value models.JobContext) models.JobContext { + pegomock.RegisterMatcher(&pegomock.EqMatcher{Value: value}) + var nullValue models.JobContext + return nullValue +} + +func NotEqModelsJobContext(value models.JobContext) models.JobContext { + pegomock.RegisterMatcher(&pegomock.NotEqMatcher{Value: value}) + var nullValue models.JobContext + return nullValue +} + +func ModelsJobContextThat(matcher pegomock.ArgumentMatcher) models.JobContext { + pegomock.RegisterMatcher(matcher) + var nullValue models.JobContext + return nullValue +} diff --git a/server/handlers/mocks/matchers/models_pullrequest.go b/server/handlers/mocks/matchers/models_pullrequest.go new file mode 100644 index 000000000..9ae2a7e92 --- /dev/null +++ b/server/handlers/mocks/matchers/models_pullrequest.go @@ -0,0 +1,33 @@ +// Code generated by pegomock. DO NOT EDIT. +package matchers + +import ( + "github.com/petergtz/pegomock" + "reflect" + + models "github.com/runatlantis/atlantis/server/events/models" +) + +func AnyModelsPullRequest() models.PullRequest { + pegomock.RegisterMatcher(pegomock.NewAnyMatcher(reflect.TypeOf((*(models.PullRequest))(nil)).Elem())) + var nullValue models.PullRequest + return nullValue +} + +func EqModelsPullRequest(value models.PullRequest) models.PullRequest { + pegomock.RegisterMatcher(&pegomock.EqMatcher{Value: value}) + var nullValue models.PullRequest + return nullValue +} + +func NotEqModelsPullRequest(value models.PullRequest) models.PullRequest { + pegomock.RegisterMatcher(&pegomock.NotEqMatcher{Value: value}) + var nullValue models.PullRequest + return nullValue +} + +func ModelsPullRequestThat(matcher pegomock.ArgumentMatcher) models.PullRequest { + pegomock.RegisterMatcher(matcher) + var nullValue models.PullRequest + return nullValue +} diff --git a/server/handlers/mocks/mock_job_id_generator.go b/server/handlers/mocks/mock_job_id_generator.go new file mode 100644 index 000000000..e604ea8dc --- /dev/null +++ b/server/handlers/mocks/mock_job_id_generator.go @@ -0,0 +1,113 @@ +// Code generated by pegomock. DO NOT EDIT. +// Source: github.com/runatlantis/atlantis/server/handlers (interfaces: JobIDGenerator) + +package mocks + +import ( + pegomock "github.com/petergtz/pegomock" + models "github.com/runatlantis/atlantis/server/events/models" + "reflect" + "time" +) + +type MockJobIDGenerator struct { + fail func(message string, callerSkip ...int) +} + +func NewMockJobIDGenerator(options ...pegomock.Option) *MockJobIDGenerator { + mock := &MockJobIDGenerator{} + for _, option := range options { + option.Apply(mock) + } + return mock +} + +func (mock *MockJobIDGenerator) SetFailHandler(fh pegomock.FailHandler) { mock.fail = fh } +func (mock *MockJobIDGenerator) FailHandler() pegomock.FailHandler { return mock.fail } + +func (mock *MockJobIDGenerator) GenerateJobID(_param0 models.PullRequest, _param1 string, _param2 string) string { + if mock == nil { + panic("mock must not be nil. Use myMock := NewMockJobIDGenerator().") + } + params := []pegomock.Param{_param0, _param1, _param2} + result := pegomock.GetGenericMockFrom(mock).Invoke("GenerateJobID", params, []reflect.Type{reflect.TypeOf((*string)(nil)).Elem()}) + var ret0 string + if len(result) != 0 { + if result[0] != nil { + ret0 = result[0].(string) + } + } + return ret0 +} + +func (mock *MockJobIDGenerator) VerifyWasCalledOnce() *VerifierMockJobIDGenerator { + return &VerifierMockJobIDGenerator{ + mock: mock, + invocationCountMatcher: pegomock.Times(1), + } +} + +func (mock *MockJobIDGenerator) VerifyWasCalled(invocationCountMatcher pegomock.InvocationCountMatcher) *VerifierMockJobIDGenerator { + return &VerifierMockJobIDGenerator{ + mock: mock, + invocationCountMatcher: invocationCountMatcher, + } +} + +func (mock *MockJobIDGenerator) VerifyWasCalledInOrder(invocationCountMatcher pegomock.InvocationCountMatcher, inOrderContext *pegomock.InOrderContext) *VerifierMockJobIDGenerator { + return &VerifierMockJobIDGenerator{ + mock: mock, + invocationCountMatcher: invocationCountMatcher, + inOrderContext: inOrderContext, + } +} + +func (mock *MockJobIDGenerator) VerifyWasCalledEventually(invocationCountMatcher pegomock.InvocationCountMatcher, timeout time.Duration) *VerifierMockJobIDGenerator { + return &VerifierMockJobIDGenerator{ + mock: mock, + invocationCountMatcher: invocationCountMatcher, + timeout: timeout, + } +} + +type VerifierMockJobIDGenerator struct { + mock *MockJobIDGenerator + invocationCountMatcher pegomock.InvocationCountMatcher + inOrderContext *pegomock.InOrderContext + timeout time.Duration +} + +func (verifier *VerifierMockJobIDGenerator) GenerateJobID(_param0 models.PullRequest, _param1 string, _param2 string) *MockJobIDGenerator_GenerateJobID_OngoingVerification { + params := []pegomock.Param{_param0, _param1, _param2} + methodInvocations := pegomock.GetGenericMockFrom(verifier.mock).Verify(verifier.inOrderContext, verifier.invocationCountMatcher, "GenerateJobID", params, verifier.timeout) + return &MockJobIDGenerator_GenerateJobID_OngoingVerification{mock: verifier.mock, methodInvocations: methodInvocations} +} + +type MockJobIDGenerator_GenerateJobID_OngoingVerification struct { + mock *MockJobIDGenerator + methodInvocations []pegomock.MethodInvocation +} + +func (c *MockJobIDGenerator_GenerateJobID_OngoingVerification) GetCapturedArguments() (models.PullRequest, string, string) { + _param0, _param1, _param2 := c.GetAllCapturedArguments() + return _param0[len(_param0)-1], _param1[len(_param1)-1], _param2[len(_param2)-1] +} + +func (c *MockJobIDGenerator_GenerateJobID_OngoingVerification) GetAllCapturedArguments() (_param0 []models.PullRequest, _param1 []string, _param2 []string) { + params := pegomock.GetGenericMockFrom(c.mock).GetInvocationParams(c.methodInvocations) + if len(params) > 0 { + _param0 = make([]models.PullRequest, len(c.methodInvocations)) + for u, param := range params[0] { + _param0[u] = param.(models.PullRequest) + } + _param1 = make([]string, len(c.methodInvocations)) + for u, param := range params[1] { + _param1[u] = param.(string) + } + _param2 = make([]string, len(c.methodInvocations)) + for u, param := range params[2] { + _param2[u] = param.(string) + } + } + return +} diff --git a/server/handlers/mocks/mock_project_command_output_handler.go b/server/handlers/mocks/mock_project_command_output_handler.go index 08add4bd4..4d04a774a 100644 --- a/server/handlers/mocks/mock_project_command_output_handler.go +++ b/server/handlers/mocks/mock_project_command_output_handler.go @@ -25,36 +25,43 @@ func NewMockProjectCommandOutputHandler(options ...pegomock.Option) *MockProject func (mock *MockProjectCommandOutputHandler) SetFailHandler(fh pegomock.FailHandler) { mock.fail = fh } func (mock *MockProjectCommandOutputHandler) FailHandler() pegomock.FailHandler { return mock.fail } -func (mock *MockProjectCommandOutputHandler) Clear(ctx models.ProjectCommandContext) { +func (mock *MockProjectCommandOutputHandler) CleanUp(_param0 models.JobContext) { if mock == nil { panic("mock must not be nil. Use myMock := NewMockProjectCommandOutputHandler().") } - params := []pegomock.Param{ctx} - pegomock.GetGenericMockFrom(mock).Invoke("Clear", params, []reflect.Type{}) + params := []pegomock.Param{_param0} + pegomock.GetGenericMockFrom(mock).Invoke("CleanUp", params, []reflect.Type{}) } -func (mock *MockProjectCommandOutputHandler) Send(ctx models.ProjectCommandContext, msg string) { +func (mock *MockProjectCommandOutputHandler) Clear(_param0 models.ProjectCommandContext) { if mock == nil { panic("mock must not be nil. Use myMock := NewMockProjectCommandOutputHandler().") } - params := []pegomock.Param{ctx, msg} - pegomock.GetGenericMockFrom(mock).Invoke("Send", params, []reflect.Type{}) + params := []pegomock.Param{_param0} + pegomock.GetGenericMockFrom(mock).Invoke("Clear", params, []reflect.Type{}) } -func (mock *MockProjectCommandOutputHandler) Register(projectInfo string, receiver chan string) { +func (mock *MockProjectCommandOutputHandler) Deregister(_param0 string, _param1 chan string) { if mock == nil { panic("mock must not be nil. Use myMock := NewMockProjectCommandOutputHandler().") } - params := []pegomock.Param{projectInfo, receiver} - pegomock.GetGenericMockFrom(mock).Invoke("Register", params, []reflect.Type{}) + params := []pegomock.Param{_param0, _param1} + pegomock.GetGenericMockFrom(mock).Invoke("Deregister", params, []reflect.Type{}) } -func (mock *MockProjectCommandOutputHandler) Deregister(projectInfo string, receiver chan string) { +func (mock *MockProjectCommandOutputHandler) GenerateJobID(_param0 models.PullRequest, _param1 string, _param2 string) string { if mock == nil { panic("mock must not be nil. Use myMock := NewMockProjectCommandOutputHandler().") } - params := []pegomock.Param{projectInfo, receiver} - pegomock.GetGenericMockFrom(mock).Invoke("Deregister", params, []reflect.Type{}) + params := []pegomock.Param{_param0, _param1, _param2} + result := pegomock.GetGenericMockFrom(mock).Invoke("GenerateJobID", params, []reflect.Type{reflect.TypeOf((*string)(nil)).Elem()}) + var ret0 string + if len(result) != 0 { + if result[0] != nil { + ret0 = result[0].(string) + } + } + return ret0 } func (mock *MockProjectCommandOutputHandler) Handle() { @@ -65,11 +72,27 @@ func (mock *MockProjectCommandOutputHandler) Handle() { pegomock.GetGenericMockFrom(mock).Invoke("Handle", params, []reflect.Type{}) } -func (mock *MockProjectCommandOutputHandler) SetJobURLWithStatus(ctx models.ProjectCommandContext, cmdName models.CommandName, status models.CommitStatus) error { +func (mock *MockProjectCommandOutputHandler) Register(_param0 string, _param1 chan string) { + if mock == nil { + panic("mock must not be nil. Use myMock := NewMockProjectCommandOutputHandler().") + } + params := []pegomock.Param{_param0, _param1} + pegomock.GetGenericMockFrom(mock).Invoke("Register", params, []reflect.Type{}) +} + +func (mock *MockProjectCommandOutputHandler) Send(_param0 models.ProjectCommandContext, _param1 string) { if mock == nil { panic("mock must not be nil. Use myMock := NewMockProjectCommandOutputHandler().") } - params := []pegomock.Param{ctx, cmdName, status} + params := []pegomock.Param{_param0, _param1} + pegomock.GetGenericMockFrom(mock).Invoke("Send", params, []reflect.Type{}) +} + +func (mock *MockProjectCommandOutputHandler) SetJobURLWithStatus(_param0 models.ProjectCommandContext, _param1 models.CommandName, _param2 models.CommitStatus) error { + if mock == nil { + panic("mock must not be nil. Use myMock := NewMockProjectCommandOutputHandler().") + } + params := []pegomock.Param{_param0, _param1, _param2} result := pegomock.GetGenericMockFrom(mock).Invoke("SetJobURLWithStatus", params, []reflect.Type{reflect.TypeOf((*error)(nil)).Elem()}) var ret0 error if len(result) != 0 { @@ -80,14 +103,6 @@ func (mock *MockProjectCommandOutputHandler) SetJobURLWithStatus(ctx models.Proj return ret0 } -func (mock *MockProjectCommandOutputHandler) CleanUp(pull string) { - if mock == nil { - panic("mock must not be nil. Use myMock := NewMockProjectCommandOutputHandler().") - } - params := []pegomock.Param{pull} - pegomock.GetGenericMockFrom(mock).Invoke("CleanUp", params, []reflect.Type{}) -} - func (mock *MockProjectCommandOutputHandler) VerifyWasCalledOnce() *VerifierMockProjectCommandOutputHandler { return &VerifierMockProjectCommandOutputHandler{ mock: mock, @@ -125,81 +140,77 @@ type VerifierMockProjectCommandOutputHandler struct { timeout time.Duration } -func (verifier *VerifierMockProjectCommandOutputHandler) Clear(ctx models.ProjectCommandContext) *MockProjectCommandOutputHandler_Clear_OngoingVerification { - params := []pegomock.Param{ctx} - methodInvocations := pegomock.GetGenericMockFrom(verifier.mock).Verify(verifier.inOrderContext, verifier.invocationCountMatcher, "Clear", params, verifier.timeout) - return &MockProjectCommandOutputHandler_Clear_OngoingVerification{mock: verifier.mock, methodInvocations: methodInvocations} +func (verifier *VerifierMockProjectCommandOutputHandler) CleanUp(_param0 models.JobContext) *MockProjectCommandOutputHandler_CleanUp_OngoingVerification { + params := []pegomock.Param{_param0} + methodInvocations := pegomock.GetGenericMockFrom(verifier.mock).Verify(verifier.inOrderContext, verifier.invocationCountMatcher, "CleanUp", params, verifier.timeout) + return &MockProjectCommandOutputHandler_CleanUp_OngoingVerification{mock: verifier.mock, methodInvocations: methodInvocations} } -type MockProjectCommandOutputHandler_Clear_OngoingVerification struct { +type MockProjectCommandOutputHandler_CleanUp_OngoingVerification struct { mock *MockProjectCommandOutputHandler methodInvocations []pegomock.MethodInvocation } -func (c *MockProjectCommandOutputHandler_Clear_OngoingVerification) GetCapturedArguments() models.ProjectCommandContext { - ctx := c.GetAllCapturedArguments() - return ctx[len(ctx)-1] +func (c *MockProjectCommandOutputHandler_CleanUp_OngoingVerification) GetCapturedArguments() models.JobContext { + _param0 := c.GetAllCapturedArguments() + return _param0[len(_param0)-1] } -func (c *MockProjectCommandOutputHandler_Clear_OngoingVerification) GetAllCapturedArguments() (_param0 []models.ProjectCommandContext) { +func (c *MockProjectCommandOutputHandler_CleanUp_OngoingVerification) GetAllCapturedArguments() (_param0 []models.JobContext) { params := pegomock.GetGenericMockFrom(c.mock).GetInvocationParams(c.methodInvocations) if len(params) > 0 { - _param0 = make([]models.ProjectCommandContext, len(c.methodInvocations)) + _param0 = make([]models.JobContext, len(c.methodInvocations)) for u, param := range params[0] { - _param0[u] = param.(models.ProjectCommandContext) + _param0[u] = param.(models.JobContext) } } return } -func (verifier *VerifierMockProjectCommandOutputHandler) Send(ctx models.ProjectCommandContext, msg string) *MockProjectCommandOutputHandler_Send_OngoingVerification { - params := []pegomock.Param{ctx, msg} - methodInvocations := pegomock.GetGenericMockFrom(verifier.mock).Verify(verifier.inOrderContext, verifier.invocationCountMatcher, "Send", params, verifier.timeout) - return &MockProjectCommandOutputHandler_Send_OngoingVerification{mock: verifier.mock, methodInvocations: methodInvocations} +func (verifier *VerifierMockProjectCommandOutputHandler) Clear(_param0 models.ProjectCommandContext) *MockProjectCommandOutputHandler_Clear_OngoingVerification { + params := []pegomock.Param{_param0} + methodInvocations := pegomock.GetGenericMockFrom(verifier.mock).Verify(verifier.inOrderContext, verifier.invocationCountMatcher, "Clear", params, verifier.timeout) + return &MockProjectCommandOutputHandler_Clear_OngoingVerification{mock: verifier.mock, methodInvocations: methodInvocations} } -type MockProjectCommandOutputHandler_Send_OngoingVerification struct { +type MockProjectCommandOutputHandler_Clear_OngoingVerification struct { mock *MockProjectCommandOutputHandler methodInvocations []pegomock.MethodInvocation } -func (c *MockProjectCommandOutputHandler_Send_OngoingVerification) GetCapturedArguments() (models.ProjectCommandContext, string) { - ctx, msg := c.GetAllCapturedArguments() - return ctx[len(ctx)-1], msg[len(msg)-1] +func (c *MockProjectCommandOutputHandler_Clear_OngoingVerification) GetCapturedArguments() models.ProjectCommandContext { + _param0 := c.GetAllCapturedArguments() + return _param0[len(_param0)-1] } -func (c *MockProjectCommandOutputHandler_Send_OngoingVerification) GetAllCapturedArguments() (_param0 []models.ProjectCommandContext, _param1 []string) { +func (c *MockProjectCommandOutputHandler_Clear_OngoingVerification) GetAllCapturedArguments() (_param0 []models.ProjectCommandContext) { params := pegomock.GetGenericMockFrom(c.mock).GetInvocationParams(c.methodInvocations) if len(params) > 0 { _param0 = make([]models.ProjectCommandContext, len(c.methodInvocations)) for u, param := range params[0] { _param0[u] = param.(models.ProjectCommandContext) } - _param1 = make([]string, len(c.methodInvocations)) - for u, param := range params[1] { - _param1[u] = param.(string) - } } return } -func (verifier *VerifierMockProjectCommandOutputHandler) Register(projectInfo string, receiver chan string) *MockProjectCommandOutputHandler_Register_OngoingVerification { - params := []pegomock.Param{projectInfo, receiver} - methodInvocations := pegomock.GetGenericMockFrom(verifier.mock).Verify(verifier.inOrderContext, verifier.invocationCountMatcher, "Register", params, verifier.timeout) - return &MockProjectCommandOutputHandler_Register_OngoingVerification{mock: verifier.mock, methodInvocations: methodInvocations} +func (verifier *VerifierMockProjectCommandOutputHandler) Deregister(_param0 string, _param1 chan string) *MockProjectCommandOutputHandler_Deregister_OngoingVerification { + params := []pegomock.Param{_param0, _param1} + methodInvocations := pegomock.GetGenericMockFrom(verifier.mock).Verify(verifier.inOrderContext, verifier.invocationCountMatcher, "Deregister", params, verifier.timeout) + return &MockProjectCommandOutputHandler_Deregister_OngoingVerification{mock: verifier.mock, methodInvocations: methodInvocations} } -type MockProjectCommandOutputHandler_Register_OngoingVerification struct { +type MockProjectCommandOutputHandler_Deregister_OngoingVerification struct { mock *MockProjectCommandOutputHandler methodInvocations []pegomock.MethodInvocation } -func (c *MockProjectCommandOutputHandler_Register_OngoingVerification) GetCapturedArguments() (string, chan string) { - projectInfo, receiver := c.GetAllCapturedArguments() - return projectInfo[len(projectInfo)-1], receiver[len(receiver)-1] +func (c *MockProjectCommandOutputHandler_Deregister_OngoingVerification) GetCapturedArguments() (string, chan string) { + _param0, _param1 := c.GetAllCapturedArguments() + return _param0[len(_param0)-1], _param1[len(_param1)-1] } -func (c *MockProjectCommandOutputHandler_Register_OngoingVerification) GetAllCapturedArguments() (_param0 []string, _param1 []chan string) { +func (c *MockProjectCommandOutputHandler_Deregister_OngoingVerification) GetAllCapturedArguments() (_param0 []string, _param1 []chan string) { params := pegomock.GetGenericMockFrom(c.mock).GetInvocationParams(c.methodInvocations) if len(params) > 0 { _param0 = make([]string, len(c.methodInvocations)) @@ -214,32 +225,36 @@ func (c *MockProjectCommandOutputHandler_Register_OngoingVerification) GetAllCap return } -func (verifier *VerifierMockProjectCommandOutputHandler) Deregister(projectInfo string, receiver chan string) *MockProjectCommandOutputHandler_Deregister_OngoingVerification { - params := []pegomock.Param{projectInfo, receiver} - methodInvocations := pegomock.GetGenericMockFrom(verifier.mock).Verify(verifier.inOrderContext, verifier.invocationCountMatcher, "Deregister", params, verifier.timeout) - return &MockProjectCommandOutputHandler_Deregister_OngoingVerification{mock: verifier.mock, methodInvocations: methodInvocations} +func (verifier *VerifierMockProjectCommandOutputHandler) GenerateJobID(_param0 models.PullRequest, _param1 string, _param2 string) *MockProjectCommandOutputHandler_GenerateJobID_OngoingVerification { + params := []pegomock.Param{_param0, _param1, _param2} + methodInvocations := pegomock.GetGenericMockFrom(verifier.mock).Verify(verifier.inOrderContext, verifier.invocationCountMatcher, "GenerateJobID", params, verifier.timeout) + return &MockProjectCommandOutputHandler_GenerateJobID_OngoingVerification{mock: verifier.mock, methodInvocations: methodInvocations} } -type MockProjectCommandOutputHandler_Deregister_OngoingVerification struct { +type MockProjectCommandOutputHandler_GenerateJobID_OngoingVerification struct { mock *MockProjectCommandOutputHandler methodInvocations []pegomock.MethodInvocation } -func (c *MockProjectCommandOutputHandler_Deregister_OngoingVerification) GetCapturedArguments() (string, chan string) { - projectInfo, receiver := c.GetAllCapturedArguments() - return projectInfo[len(projectInfo)-1], receiver[len(receiver)-1] +func (c *MockProjectCommandOutputHandler_GenerateJobID_OngoingVerification) GetCapturedArguments() (models.PullRequest, string, string) { + _param0, _param1, _param2 := c.GetAllCapturedArguments() + return _param0[len(_param0)-1], _param1[len(_param1)-1], _param2[len(_param2)-1] } -func (c *MockProjectCommandOutputHandler_Deregister_OngoingVerification) GetAllCapturedArguments() (_param0 []string, _param1 []chan string) { +func (c *MockProjectCommandOutputHandler_GenerateJobID_OngoingVerification) GetAllCapturedArguments() (_param0 []models.PullRequest, _param1 []string, _param2 []string) { params := pegomock.GetGenericMockFrom(c.mock).GetInvocationParams(c.methodInvocations) if len(params) > 0 { - _param0 = make([]string, len(c.methodInvocations)) + _param0 = make([]models.PullRequest, len(c.methodInvocations)) for u, param := range params[0] { - _param0[u] = param.(string) + _param0[u] = param.(models.PullRequest) } - _param1 = make([]chan string, len(c.methodInvocations)) + _param1 = make([]string, len(c.methodInvocations)) for u, param := range params[1] { - _param1[u] = param.(chan string) + _param1[u] = param.(string) + } + _param2 = make([]string, len(c.methodInvocations)) + for u, param := range params[2] { + _param2[u] = param.(string) } } return @@ -262,63 +277,98 @@ func (c *MockProjectCommandOutputHandler_Handle_OngoingVerification) GetCaptured func (c *MockProjectCommandOutputHandler_Handle_OngoingVerification) GetAllCapturedArguments() { } -func (verifier *VerifierMockProjectCommandOutputHandler) SetJobURLWithStatus(ctx models.ProjectCommandContext, cmdName models.CommandName, status models.CommitStatus) *MockProjectCommandOutputHandler_SetJobURLWithStatus_OngoingVerification { - params := []pegomock.Param{ctx, cmdName, status} - methodInvocations := pegomock.GetGenericMockFrom(verifier.mock).Verify(verifier.inOrderContext, verifier.invocationCountMatcher, "SetJobURLWithStatus", params, verifier.timeout) - return &MockProjectCommandOutputHandler_SetJobURLWithStatus_OngoingVerification{mock: verifier.mock, methodInvocations: methodInvocations} +func (verifier *VerifierMockProjectCommandOutputHandler) Register(_param0 string, _param1 chan string) *MockProjectCommandOutputHandler_Register_OngoingVerification { + params := []pegomock.Param{_param0, _param1} + methodInvocations := pegomock.GetGenericMockFrom(verifier.mock).Verify(verifier.inOrderContext, verifier.invocationCountMatcher, "Register", params, verifier.timeout) + return &MockProjectCommandOutputHandler_Register_OngoingVerification{mock: verifier.mock, methodInvocations: methodInvocations} } -type MockProjectCommandOutputHandler_SetJobURLWithStatus_OngoingVerification struct { +type MockProjectCommandOutputHandler_Register_OngoingVerification struct { mock *MockProjectCommandOutputHandler methodInvocations []pegomock.MethodInvocation } -func (c *MockProjectCommandOutputHandler_SetJobURLWithStatus_OngoingVerification) GetCapturedArguments() (models.ProjectCommandContext, models.CommandName, models.CommitStatus) { - ctx, cmdName, status := c.GetAllCapturedArguments() - return ctx[len(ctx)-1], cmdName[len(cmdName)-1], status[len(status)-1] +func (c *MockProjectCommandOutputHandler_Register_OngoingVerification) GetCapturedArguments() (string, chan string) { + _param0, _param1 := c.GetAllCapturedArguments() + return _param0[len(_param0)-1], _param1[len(_param1)-1] } -func (c *MockProjectCommandOutputHandler_SetJobURLWithStatus_OngoingVerification) GetAllCapturedArguments() (_param0 []models.ProjectCommandContext, _param1 []models.CommandName, _param2 []models.CommitStatus) { +func (c *MockProjectCommandOutputHandler_Register_OngoingVerification) GetAllCapturedArguments() (_param0 []string, _param1 []chan string) { + params := pegomock.GetGenericMockFrom(c.mock).GetInvocationParams(c.methodInvocations) + if len(params) > 0 { + _param0 = make([]string, len(c.methodInvocations)) + for u, param := range params[0] { + _param0[u] = param.(string) + } + _param1 = make([]chan string, len(c.methodInvocations)) + for u, param := range params[1] { + _param1[u] = param.(chan string) + } + } + return +} + +func (verifier *VerifierMockProjectCommandOutputHandler) Send(_param0 models.ProjectCommandContext, _param1 string) *MockProjectCommandOutputHandler_Send_OngoingVerification { + params := []pegomock.Param{_param0, _param1} + methodInvocations := pegomock.GetGenericMockFrom(verifier.mock).Verify(verifier.inOrderContext, verifier.invocationCountMatcher, "Send", params, verifier.timeout) + return &MockProjectCommandOutputHandler_Send_OngoingVerification{mock: verifier.mock, methodInvocations: methodInvocations} +} + +type MockProjectCommandOutputHandler_Send_OngoingVerification struct { + mock *MockProjectCommandOutputHandler + methodInvocations []pegomock.MethodInvocation +} + +func (c *MockProjectCommandOutputHandler_Send_OngoingVerification) GetCapturedArguments() (models.ProjectCommandContext, string) { + _param0, _param1 := c.GetAllCapturedArguments() + return _param0[len(_param0)-1], _param1[len(_param1)-1] +} + +func (c *MockProjectCommandOutputHandler_Send_OngoingVerification) GetAllCapturedArguments() (_param0 []models.ProjectCommandContext, _param1 []string) { params := pegomock.GetGenericMockFrom(c.mock).GetInvocationParams(c.methodInvocations) if len(params) > 0 { _param0 = make([]models.ProjectCommandContext, len(c.methodInvocations)) for u, param := range params[0] { _param0[u] = param.(models.ProjectCommandContext) } - _param1 = make([]models.CommandName, len(c.methodInvocations)) + _param1 = make([]string, len(c.methodInvocations)) for u, param := range params[1] { - _param1[u] = param.(models.CommandName) - } - _param2 = make([]models.CommitStatus, len(c.methodInvocations)) - for u, param := range params[2] { - _param2[u] = param.(models.CommitStatus) + _param1[u] = param.(string) } } return } -func (verifier *VerifierMockProjectCommandOutputHandler) CleanUp(pull string) *MockProjectCommandOutputHandler_CleanUp_OngoingVerification { - params := []pegomock.Param{pull} - methodInvocations := pegomock.GetGenericMockFrom(verifier.mock).Verify(verifier.inOrderContext, verifier.invocationCountMatcher, "CleanUp", params, verifier.timeout) - return &MockProjectCommandOutputHandler_CleanUp_OngoingVerification{mock: verifier.mock, methodInvocations: methodInvocations} +func (verifier *VerifierMockProjectCommandOutputHandler) SetJobURLWithStatus(_param0 models.ProjectCommandContext, _param1 models.CommandName, _param2 models.CommitStatus) *MockProjectCommandOutputHandler_SetJobURLWithStatus_OngoingVerification { + params := []pegomock.Param{_param0, _param1, _param2} + methodInvocations := pegomock.GetGenericMockFrom(verifier.mock).Verify(verifier.inOrderContext, verifier.invocationCountMatcher, "SetJobURLWithStatus", params, verifier.timeout) + return &MockProjectCommandOutputHandler_SetJobURLWithStatus_OngoingVerification{mock: verifier.mock, methodInvocations: methodInvocations} } -type MockProjectCommandOutputHandler_CleanUp_OngoingVerification struct { +type MockProjectCommandOutputHandler_SetJobURLWithStatus_OngoingVerification struct { mock *MockProjectCommandOutputHandler methodInvocations []pegomock.MethodInvocation } -func (c *MockProjectCommandOutputHandler_CleanUp_OngoingVerification) GetCapturedArguments() string { - pull := c.GetAllCapturedArguments() - return pull[len(pull)-1] +func (c *MockProjectCommandOutputHandler_SetJobURLWithStatus_OngoingVerification) GetCapturedArguments() (models.ProjectCommandContext, models.CommandName, models.CommitStatus) { + _param0, _param1, _param2 := c.GetAllCapturedArguments() + return _param0[len(_param0)-1], _param1[len(_param1)-1], _param2[len(_param2)-1] } -func (c *MockProjectCommandOutputHandler_CleanUp_OngoingVerification) GetAllCapturedArguments() (_param0 []string) { +func (c *MockProjectCommandOutputHandler_SetJobURLWithStatus_OngoingVerification) GetAllCapturedArguments() (_param0 []models.ProjectCommandContext, _param1 []models.CommandName, _param2 []models.CommitStatus) { params := pegomock.GetGenericMockFrom(c.mock).GetInvocationParams(c.methodInvocations) if len(params) > 0 { - _param0 = make([]string, len(c.methodInvocations)) + _param0 = make([]models.ProjectCommandContext, len(c.methodInvocations)) for u, param := range params[0] { - _param0[u] = param.(string) + _param0[u] = param.(models.ProjectCommandContext) + } + _param1 = make([]models.CommandName, len(c.methodInvocations)) + for u, param := range params[1] { + _param1[u] = param.(models.CommandName) + } + _param2 = make([]models.CommitStatus, len(c.methodInvocations)) + for u, param := range params[2] { + _param2[u] = param.(models.CommitStatus) } } return diff --git a/server/handlers/project_command_output_handler.go b/server/handlers/project_command_output_handler.go index c98d92b56..6d2cc4fff 100644 --- a/server/handlers/project_command_output_handler.go +++ b/server/handlers/project_command_output_handler.go @@ -3,6 +3,7 @@ package handlers import ( "sync" + "github.com/google/uuid" "github.com/runatlantis/atlantis/server/events/models" "github.com/runatlantis/atlantis/server/logging" ) @@ -22,6 +23,13 @@ type AsyncProjectCommandOutputHandler struct { projectJobURLGenerator ProjectJobURLGenerator logger logging.SimpleLogging + + pullToJobMapping map[models.JobContext]string +} + +//go:generate pegomock generate -m --use-experimental-model-gen --package mocks -o mocks/mock_job_id_generator.go JobIDGenerator +type JobIDGenerator interface { + GenerateJobID(pull models.PullRequest, projectName string, workspace string) string } //go:generate pegomock generate -m --use-experimental-model-gen --package mocks -o mocks/mock_project_job_url_generator.go ProjectJobURLGenerator @@ -63,12 +71,14 @@ type ProjectCommandOutputHandler interface { SetJobURLWithStatus(ctx models.ProjectCommandContext, cmdName models.CommandName, status models.CommitStatus) error ResourceCleaner + + JobIDGenerator } //go:generate pegomock generate -m --use-experimental-model-gen --package mocks -o mocks/mock_resource_cleaner.go ResourceCleaner type ResourceCleaner interface { - CleanUp(jobID string) + CleanUp(jobContext models.JobContext) } func NewAsyncProjectCommandOutputHandler( @@ -84,6 +94,7 @@ func NewAsyncProjectCommandOutputHandler( projectStatusUpdater: projectStatusUpdater, projectJobURLGenerator: projectJobURLGenerator, projectOutputBuffers: map[string][]string{}, + pullToJobMapping: map[models.JobContext]string{}, } } @@ -196,17 +207,42 @@ func (p *AsyncProjectCommandOutputHandler) GetProjectOutputBuffer(jobID string) return p.projectOutputBuffers[jobID] } -func (p *AsyncProjectCommandOutputHandler) CleanUp(jobID string) { - p.projectOutputBuffersLock.Lock() - delete(p.projectOutputBuffers, jobID) - p.projectOutputBuffersLock.Unlock() +func (p *AsyncProjectCommandOutputHandler) CleanUp(jobContext models.JobContext) { - // Only delete the pull record from receiver buffers. - // WS channel will be closed when the user closes the browser tab - // in closeHanlder(). - p.receiverBuffersLock.Lock() - delete(p.receiverBuffers, jobID) - p.receiverBuffersLock.Unlock() + // Retrieve the jobID from the pullToJobMapping + if jobID, ok := p.pullToJobMapping[jobContext]; ok { + p.projectOutputBuffersLock.Lock() + delete(p.projectOutputBuffers, jobID) + p.projectOutputBuffersLock.Unlock() + + // Only delete the pull record from receiver buffers. + // WS channel will be closed when the user closes the browser tab + // in closeHanlder(). + p.receiverBuffersLock.Lock() + delete(p.receiverBuffers, jobID) + p.receiverBuffersLock.Unlock() + } +} + +func (p *AsyncProjectCommandOutputHandler) GenerateJobID(pull models.PullRequest, projectName string, workspace string) string { + // Check if the job context exists in the pullToJobMapping + + jobContext := models.JobContext{ + PullNum: pull.Num, + Repo: pull.BaseRepo.Name, + ProjectName: projectName, + Workspace: workspace, + HeadCommit: pull.HeadCommit, + } + + var jobID string + if val, ok := p.pullToJobMapping[jobContext]; ok { + jobID = val + } else { + jobID = uuid.New().String() + p.pullToJobMapping[jobContext] = jobID + } + return jobID } // NoopProjectOutputHandler is a mock that doesn't do anything @@ -228,5 +264,9 @@ func (p *NoopProjectOutputHandler) SetJobURLWithStatus(ctx models.ProjectCommand return nil } -func (p *NoopProjectOutputHandler) CleanUp(jobID string) { +func (p *NoopProjectOutputHandler) CleanUp(jobContext models.JobContext) { +} + +func (p *NoopProjectOutputHandler) GenerateJobID(pull models.PullRequest, projectName string, workspace string) string { + return "" } diff --git a/server/server.go b/server/server.go index a4849812b..c08261bd4 100644 --- a/server/server.go +++ b/server/server.go @@ -518,7 +518,7 @@ func NewServer(userConfig UserConfig, config Config) (*Server, error) { statsScope, logger, userConfig.MaxProjectsPerPR, - controllers.JobIDGenerator{}, + projectCmdOutputHandler, ) showStepRunner, err := runtime.NewShowStepRunner(terraformClient, defaultTfVersion) From 7bc52adda1174cf0f8f56156e274a357cf4f744b Mon Sep 17 00:00:00 2001 From: Aayush Gupta Date: Mon, 24 Jan 2022 11:16:52 -0800 Subject: [PATCH 03/19] Refactoring --- server/controllers/jobs_controller.go | 97 ------------------- server/controllers/websocket/mux.go | 17 +--- .../project_command_builder_internal_test.go | 6 -- server/events/pull_closed_executor.go | 9 -- server/router.go | 15 --- server/server.go | 1 - 6 files changed, 4 insertions(+), 141 deletions(-) diff --git a/server/controllers/jobs_controller.go b/server/controllers/jobs_controller.go index 07ee41154..7e62aef31 100644 --- a/server/controllers/jobs_controller.go +++ b/server/controllers/jobs_controller.go @@ -5,11 +5,7 @@ import ( "net/http" "net/url" - "strconv" - - "github.com/google/uuid" "github.com/gorilla/mux" - "github.com/pkg/errors" "github.com/runatlantis/atlantis/server/controllers/templates" "github.com/runatlantis/atlantis/server/controllers/websocket" "github.com/runatlantis/atlantis/server/core/db" @@ -30,100 +26,7 @@ type JobsController struct { StatsScope tally.Scope } -type JobIDGenerator struct{} - -func (j JobIDGenerator) GenerateJobID() string { - return uuid.New().String() -} - -type ProjectInfoKeyGenerator struct{} - -func (g ProjectInfoKeyGenerator) Generate(r *http.Request) (string, error) { - projectInfo, err := newProjectInfo(r) - - if err != nil { - return "", errors.Wrap(err, "creating project info") - } - - return projectInfo.String(), nil -} - -type pullInfo struct { - org string - repo string - pull int -} - -func (p *pullInfo) String() string { - return fmt.Sprintf("%s/%s/%d", p.org, p.repo, p.pull) -} - -type projectInfo struct { - projectName string - workspace string - pullInfo -} - -func (p *projectInfo) String() string { - return fmt.Sprintf("%s/%s/%d/%s/%s", p.org, p.repo, p.pull, p.projectName, p.workspace) -} - -func newPullInfo(r *http.Request) (*pullInfo, error) { - org, ok := mux.Vars(r)["org"] - if !ok { - return nil, fmt.Errorf("Internal error: no org in route") - } - repo, ok := mux.Vars(r)["repo"] - if !ok { - return nil, fmt.Errorf("Internal error: no repo in route") - } - pull, ok := mux.Vars(r)["pull"] - if !ok { - return nil, fmt.Errorf("Internal error: no pull in route") - } - pullNum, err := strconv.Atoi(pull) - if err != nil { - return nil, err - } - - return &pullInfo{ - org: org, - repo: repo, - pull: pullNum, - }, nil -} - -// Gets the PR information from the HTTP request params -func newProjectInfo(r *http.Request) (*projectInfo, error) { - pullInfo, err := newPullInfo(r) - if err != nil { - return nil, err - } - - project, ok := mux.Vars(r)["project"] - if !ok { - return nil, fmt.Errorf("Internal error: no project in route") - } - - workspace, ok := mux.Vars(r)["workspace"] - if !ok { - return nil, fmt.Errorf("Internal error: no workspace in route") - } - - return &projectInfo{ - pullInfo: *pullInfo, - projectName: project, - workspace: workspace, - }, nil -} - func (j *JobsController) getProjectJobs(w http.ResponseWriter, r *http.Request) error { - // projectInfo, err := newProjectInfo(r) - // if err != nil { - // j.respond(w, logging.Error, http.StatusInternalServerError, err.Error()) - // return err - // } - jobID, ok := mux.Vars(r)["job-id"] if !ok { return fmt.Errorf("internal error: no job ID in route") diff --git a/server/controllers/websocket/mux.go b/server/controllers/websocket/mux.go index ceaaf13d1..762b0e732 100644 --- a/server/controllers/websocket/mux.go +++ b/server/controllers/websocket/mux.go @@ -10,11 +10,6 @@ import ( "github.com/runatlantis/atlantis/server/logging" ) -// PartitionKeyGenerator generates partition keys for the multiplexor -type PartitionKeyGenerator interface { - Generate(r *http.Request) (string, error) -} - // PartitionRegistry is the registry holding each partition // and is responsible for registering/deregistering new buffers type PartitionRegistry interface { @@ -26,12 +21,11 @@ type PartitionRegistry interface { // and the registry. Note this is still a WIP as right now the registry is assumed to handle // everything. type Multiplexor struct { - writer *Writer - keyGenerator PartitionKeyGenerator - registry PartitionRegistry + writer *Writer + registry PartitionRegistry } -func NewMultiplexor(log logging.SimpleLogging, keyGenerator PartitionKeyGenerator, registry PartitionRegistry) *Multiplexor { +func NewMultiplexor(log logging.SimpleLogging, registry PartitionRegistry) *Multiplexor { upgrader := websocket.Upgrader{} upgrader.CheckOrigin = func(r *http.Request) bool { return true } return &Multiplexor{ @@ -39,16 +33,13 @@ func NewMultiplexor(log logging.SimpleLogging, keyGenerator PartitionKeyGenerato upgrader: upgrader, log: log, }, - keyGenerator: keyGenerator, - registry: registry, + registry: registry, } } // Handle should be called for a given websocket request. It blocks // while writing to the websocket until the buffer is closed. func (m *Multiplexor) Handle(w http.ResponseWriter, r *http.Request) error { - // key, err := m.keyGenerator.Generate(r) - jobID, ok := mux.Vars(r)["job-id"] if !ok { return fmt.Errorf("internal error: no job ID in route") diff --git a/server/events/project_command_builder_internal_test.go b/server/events/project_command_builder_internal_test.go index 37095db13..afd3d835c 100644 --- a/server/events/project_command_builder_internal_test.go +++ b/server/events/project_command_builder_internal_test.go @@ -19,12 +19,6 @@ import ( . "github.com/runatlantis/atlantis/testing" ) -type MockJobIDGenerator struct{} - -func (j MockJobIDGenerator) GenerateJobID() string { - return "" -} - // Test different permutations of global and repo config. func TestBuildProjectCmdCtx(t *testing.T) { logger := logging.NewNoopLogger(t) diff --git a/server/events/pull_closed_executor.go b/server/events/pull_closed_executor.go index 78652ae3b..564b7502e 100644 --- a/server/events/pull_closed_executor.go +++ b/server/events/pull_closed_executor.go @@ -41,8 +41,6 @@ type PullCleaner interface { CleanUpPull(repo models.Repo, pull models.PullRequest) error } -// TODO: Retrieve job ID from the github checks - // PullClosedExecutor executes the tasks required to clean up a closed pull // request. type PullClosedExecutor struct { @@ -85,13 +83,6 @@ func (p *PullClosedExecutor) CleanUpPull(repo models.Repo, pull models.PullReque if pullStatus != nil { for _, project := range pullStatus.Projects { - // TODO [ORCA-943]: Set projectName to "/" when project name is not set. - // Upstream atlantis only requires project name to be set if there's more than one project - // with same dir and workspace. If a project name has not been set, we'll use the dir and - // workspace to build project key. - // Source: https://www.runatlantis.io/docs/repo-level-atlantis-yaml.html#reference - // projectKey := models.BuildPullInfo(pullStatus.Pull.BaseRepo.FullName, pull.Num, project.ProjectName, project.RepoRelDir, project.Workspace) - jobContext := models.JobContext{ PullNum: pull.Num, Repo: pull.BaseRepo.Name, diff --git a/server/router.go b/server/router.go index e7728f942..e7373d156 100644 --- a/server/router.go +++ b/server/router.go @@ -47,19 +47,4 @@ func (r *Router) GenerateProjectJobURL(ctx models.ProjectCommandContext) (string } return r.AtlantisURL.String() + jobURL.String(), nil - - // pull := ctx.Pull - // projectIdentifier := models.GetProjectIdentifier(ctx.RepoRelDir, ctx.ProjectName) - // jobURL, err := r.Underlying.Get(r.ProjectJobsViewRouteName).URL( - // "org", pull.BaseRepo.Owner, - // "repo", pull.BaseRepo.Name, - // "pull", fmt.Sprintf("%d", pull.Num), - // "project", projectIdentifier, - // "workspace", ctx.Workspace, - // ) - // if err != nil { - // return "", errors.Wrapf(err, "creating job url for %s/%d/%s/%s", pull.BaseRepo.FullName, pull.Num, projectIdentifier, ctx.Workspace) - // } - - // return r.AtlantisURL.String() + jobURL.String(), nil } diff --git a/server/server.go b/server/server.go index c08261bd4..d329d28ea 100644 --- a/server/server.go +++ b/server/server.go @@ -752,7 +752,6 @@ func NewServer(userConfig UserConfig, config Config) (*Server, error) { wsMux := websocket.NewMultiplexor( logger, - controllers.ProjectInfoKeyGenerator{}, projectCmdOutputHandler, ) From 7127145fca60288b12410607f0e5193a7f877c22 Mon Sep 17 00:00:00 2001 From: Aayush Gupta Date: Tue, 25 Jan 2022 10:32:29 -0800 Subject: [PATCH 04/19] Adding tests --- server/controllers/jobs_controller.go | 4 ++- server/events/models/models.go | 2 +- .../project_command_output_handler_test.go | 31 +++++++++++++++++++ server/router.go | 4 +++ server/router_test.go | 4 +-- 5 files changed, 41 insertions(+), 4 deletions(-) diff --git a/server/controllers/jobs_controller.go b/server/controllers/jobs_controller.go index 7e62aef31..0bac9220e 100644 --- a/server/controllers/jobs_controller.go +++ b/server/controllers/jobs_controller.go @@ -29,7 +29,9 @@ type JobsController struct { func (j *JobsController) getProjectJobs(w http.ResponseWriter, r *http.Request) error { jobID, ok := mux.Vars(r)["job-id"] if !ok { - return fmt.Errorf("internal error: no job ID in route") + err := fmt.Errorf("internal error: no job ID in route") + j.respond(w, logging.Error, http.StatusBadRequest, err.Error()) + return err } viewData := templates.ProjectJobData{ diff --git a/server/events/models/models.go b/server/events/models/models.go index 45e8f67bd..9305ea4ac 100644 --- a/server/events/models/models.go +++ b/server/events/models/models.go @@ -425,7 +425,7 @@ type ProjectCommandContext struct { PolicySets valid.PolicySets // DeleteSourceBranchOnMerge will attempt to allow a branch to be deleted when merged (AzureDevOps & GitLab Support Only) DeleteSourceBranchOnMerge bool - // JobID + // UUID for atlantis logs JobID string } diff --git a/server/handlers/project_command_output_handler_test.go b/server/handlers/project_command_output_handler_test.go index 280130718..6fac1c729 100644 --- a/server/handlers/project_command_output_handler_test.go +++ b/server/handlers/project_command_output_handler_test.go @@ -212,4 +212,35 @@ func TestProjectCommandOutputHandler(t *testing.T) { err := prjCmdOutputHandler.SetJobURLWithStatus(ctx, models.PlanCommand, models.PendingCommitStatus) assert.Error(t, err) }) + + t.Run("return same Job ID when follow up operation", func(t *testing.T) { + RegisterMockTestingT(t) + logger := logging.NewNoopLogger(t) + prjCmdOutputChan := make(chan *models.ProjectCmdOutputLine) + projectStatusUpdater := mocks.NewMockProjectStatusUpdater() + projectJobURLGenerator := mocks.NewMockProjectJobURLGenerator() + prjCmdOutputHandler := handlers.NewAsyncProjectCommandOutputHandler( + prjCmdOutputChan, + projectStatusUpdater, + projectJobURLGenerator, + logger, + ) + + // Job ID generation during plan + pull := models.PullRequest{ + Num: 1, + BaseRepo: models.Repo{ + Name: "test-repo", + }, + HeadCommit: "head-sha", + } + projectName := "test-project" + workspaceName := "test-workspace" + jobID := prjCmdOutputHandler.GenerateJobID(pull, projectName, workspaceName) + + // Job ID generation during apply + newJobID := prjCmdOutputHandler.GenerateJobID(pull, projectName, workspaceName) + + assert.Equal(t, jobID, newJobID) + }) } diff --git a/server/router.go b/server/router.go index e7373d156..58e2d800a 100644 --- a/server/router.go +++ b/server/router.go @@ -1,6 +1,7 @@ package server import ( + "fmt" "net/url" "github.com/gorilla/mux" @@ -39,6 +40,9 @@ func (r *Router) GenerateLockURL(lockID string) string { } func (r *Router) GenerateProjectJobURL(ctx models.ProjectCommandContext) (string, error) { + if ctx.JobID == "" { + return "", fmt.Errorf("no job id in ctx") + } jobURL, err := r.Underlying.Get((r.ProjectJobsViewRouteName)).URL( "job-id", ctx.JobID, ) diff --git a/server/router_test.go b/server/router_test.go index 11aa4fd75..f5af05084 100644 --- a/server/router_test.go +++ b/server/router_test.go @@ -104,8 +104,8 @@ func TestGenerateProjectJobURL_ShouldReturnErrorWhenJobIDNotSpecified(t *testing }, RepoRelDir: "ops/terraform/", } - + expectedErrString := "no job id in ctx" gotURL, err := router.GenerateProjectJobURL(ctx) - assert.Error(t, err) + assert.EqualError(t, err, expectedErrString) Equals(t, "", gotURL) } From 299b6615b5b5f67357f9684b36c82a6f2c4e2334 Mon Sep 17 00:00:00 2001 From: Aayush Gupta Date: Wed, 26 Jan 2022 14:58:23 -0800 Subject: [PATCH 05/19] Addressing comments --- .../events/events_controller_e2e_test.go | 2 +- server/controllers/jobs_controller.go | 11 +++ server/controllers/websocket/mux.go | 30 ++++--- server/events/models/models.go | 10 ++- server/events/project_command_builder.go | 7 ++ .../events/project_command_context_builder.go | 2 +- server/events/pull_closed_executor.go | 3 +- .../mocks/matchers/models_pullcontext.go | 33 ++++++++ .../handlers/mocks/mock_job_id_generator.go | 31 ++----- .../mock_project_command_output_handler.go | 62 ++------------ .../project_command_output_handler.go | 80 +++++++++---------- .../project_command_output_handler_test.go | 69 +++++++++------- server/server.go | 3 +- 13 files changed, 174 insertions(+), 169 deletions(-) create mode 100644 server/handlers/mocks/matchers/models_pullcontext.go diff --git a/server/controllers/events/events_controller_e2e_test.go b/server/controllers/events/events_controller_e2e_test.go index 6b1981f6c..40f32c07b 100644 --- a/server/controllers/events/events_controller_e2e_test.go +++ b/server/controllers/events/events_controller_e2e_test.go @@ -912,7 +912,7 @@ func setupE2E(t *testing.T, repoDir string) (events_controllers.VCSEventsControl "**/*.tf,**/*.tfvars,**/*.tfvars.json,**/terragrunt.hcl", statsScope, logger, - projectCmdOutputHandler, + events.UUIDJobIdGenerator{}, ) showStepRunner, err := runtime.NewShowStepRunner(terraformClient, defaultTFVersion) diff --git a/server/controllers/jobs_controller.go b/server/controllers/jobs_controller.go index 0bac9220e..a9d5a679c 100644 --- a/server/controllers/jobs_controller.go +++ b/server/controllers/jobs_controller.go @@ -15,6 +15,17 @@ import ( "github.com/uber-go/tally" ) +type JobIDKeyGenerator struct{} + +func (g JobIDKeyGenerator) Generate(r *http.Request) (string, error) { + jobID, ok := mux.Vars(r)["job-id"] + if !ok { + return "", fmt.Errorf("internal error: no job-id in route") + } + + return jobID, nil +} + type JobsController struct { AtlantisVersion string AtlantisURL *url.URL diff --git a/server/controllers/websocket/mux.go b/server/controllers/websocket/mux.go index 762b0e732..ccfbdf99f 100644 --- a/server/controllers/websocket/mux.go +++ b/server/controllers/websocket/mux.go @@ -1,15 +1,18 @@ package websocket import ( - "fmt" "net/http" - "github.com/gorilla/mux" "github.com/gorilla/websocket" "github.com/pkg/errors" "github.com/runatlantis/atlantis/server/logging" ) +// PartitionKeyGenerator generates partition keys for the multiplexor +type PartitionKeyGenerator interface { + Generate(r *http.Request) (string, error) +} + // PartitionRegistry is the registry holding each partition // and is responsible for registering/deregistering new buffers type PartitionRegistry interface { @@ -21,11 +24,12 @@ type PartitionRegistry interface { // and the registry. Note this is still a WIP as right now the registry is assumed to handle // everything. type Multiplexor struct { - writer *Writer - registry PartitionRegistry + writer *Writer + keyGenerator PartitionKeyGenerator + registry PartitionRegistry } -func NewMultiplexor(log logging.SimpleLogging, registry PartitionRegistry) *Multiplexor { +func NewMultiplexor(log logging.SimpleLogging, keyGenerator PartitionKeyGenerator, registry PartitionRegistry) *Multiplexor { upgrader := websocket.Upgrader{} upgrader.CheckOrigin = func(r *http.Request) bool { return true } return &Multiplexor{ @@ -33,16 +37,18 @@ func NewMultiplexor(log logging.SimpleLogging, registry PartitionRegistry) *Mult upgrader: upgrader, log: log, }, - registry: registry, + keyGenerator: keyGenerator, + registry: registry, } } // Handle should be called for a given websocket request. It blocks // while writing to the websocket until the buffer is closed. func (m *Multiplexor) Handle(w http.ResponseWriter, r *http.Request) error { - jobID, ok := mux.Vars(r)["job-id"] - if !ok { - return fmt.Errorf("internal error: no job ID in route") + key, err := m.keyGenerator.Generate(r) + + if err != nil { + return errors.Wrapf(err, "generating partition key") } // Buffer size set to 1000 to ensure messages get queued. @@ -50,8 +56,8 @@ func (m *Multiplexor) Handle(w http.ResponseWriter, r *http.Request) error { buffer := make(chan string, 1000) // spinning up a goroutine for this since we are attempting to block on the read side. - go m.registry.Register(jobID, buffer) - defer m.registry.Deregister(jobID, buffer) + go m.registry.Register(key, buffer) + defer m.registry.Deregister(key, buffer) - return errors.Wrapf(m.writer.Write(w, r, buffer), "writing to ws %s", jobID) + return errors.Wrapf(m.writer.Write(w, r, buffer), "writing to ws %s", key) } diff --git a/server/events/models/models.go b/server/events/models/models.go index 9305ea4ac..6957a7778 100644 --- a/server/events/models/models.go +++ b/server/events/models/models.go @@ -714,17 +714,23 @@ func (c CommandName) TitleString() string { return strings.Title(strings.ReplaceAll(strings.ToLower(c.String()), "_", " ")) } -type JobContext struct { +type PullContext struct { PullNum int Repo string ProjectName string Workspace string - HeadCommit string +} + +type JobContext struct { + PullContext + HeadCommit string } type ProjectCmdOutputLine struct { JobID string + JobContext JobContext + Line string ClearBuffBefore bool diff --git a/server/events/project_command_builder.go b/server/events/project_command_builder.go index 6f79a378c..bedc6a77c 100644 --- a/server/events/project_command_builder.go +++ b/server/events/project_command_builder.go @@ -4,6 +4,7 @@ import ( "fmt" "os" + "github.com/google/uuid" "github.com/runatlantis/atlantis/server/events/yaml/valid" "github.com/runatlantis/atlantis/server/handlers" "github.com/runatlantis/atlantis/server/logging" @@ -35,6 +36,12 @@ const ( InfiniteProjectsPerPR = -1 ) +type UUIDJobIdGenerator struct{} + +func (u UUIDJobIdGenerator) GenerateJobID() string { + return uuid.New().String() +} + func NewProjectCommandBuilder( policyChecksSupported bool, parserValidator *yaml.ParserValidator, diff --git a/server/events/project_command_context_builder.go b/server/events/project_command_context_builder.go index 41625e36f..b62d6e67c 100644 --- a/server/events/project_command_context_builder.go +++ b/server/events/project_command_context_builder.go @@ -138,7 +138,7 @@ func (cb *DefaultProjectCommandContextBuilder) BuildProjectContext( contextFlags, ctx.Scope, ctx.PullRequestStatus, - cb.JobIDGenerator.GenerateJobID(ctx.Pull, prjCfg.Name, prjCfg.Workspace), + cb.JobIDGenerator.GenerateJobID(), ) projectCmds = append(projectCmds, projectCmdContext) diff --git a/server/events/pull_closed_executor.go b/server/events/pull_closed_executor.go index 564b7502e..6ef22bbf0 100644 --- a/server/events/pull_closed_executor.go +++ b/server/events/pull_closed_executor.go @@ -83,12 +83,11 @@ func (p *PullClosedExecutor) CleanUpPull(repo models.Repo, pull models.PullReque if pullStatus != nil { for _, project := range pullStatus.Projects { - jobContext := models.JobContext{ + jobContext := models.PullContext{ PullNum: pull.Num, Repo: pull.BaseRepo.Name, Workspace: project.Workspace, ProjectName: project.ProjectName, - HeadCommit: pull.HeadCommit, } p.LogStreamResourceCleaner.CleanUp(jobContext) } diff --git a/server/handlers/mocks/matchers/models_pullcontext.go b/server/handlers/mocks/matchers/models_pullcontext.go new file mode 100644 index 000000000..fb31e0bf7 --- /dev/null +++ b/server/handlers/mocks/matchers/models_pullcontext.go @@ -0,0 +1,33 @@ +// Code generated by pegomock. DO NOT EDIT. +package matchers + +import ( + "github.com/petergtz/pegomock" + "reflect" + + models "github.com/runatlantis/atlantis/server/events/models" +) + +func AnyModelsPullContext() models.PullContext { + pegomock.RegisterMatcher(pegomock.NewAnyMatcher(reflect.TypeOf((*(models.PullContext))(nil)).Elem())) + var nullValue models.PullContext + return nullValue +} + +func EqModelsPullContext(value models.PullContext) models.PullContext { + pegomock.RegisterMatcher(&pegomock.EqMatcher{Value: value}) + var nullValue models.PullContext + return nullValue +} + +func NotEqModelsPullContext(value models.PullContext) models.PullContext { + pegomock.RegisterMatcher(&pegomock.NotEqMatcher{Value: value}) + var nullValue models.PullContext + return nullValue +} + +func ModelsPullContextThat(matcher pegomock.ArgumentMatcher) models.PullContext { + pegomock.RegisterMatcher(matcher) + var nullValue models.PullContext + return nullValue +} diff --git a/server/handlers/mocks/mock_job_id_generator.go b/server/handlers/mocks/mock_job_id_generator.go index e604ea8dc..0b57d7bd0 100644 --- a/server/handlers/mocks/mock_job_id_generator.go +++ b/server/handlers/mocks/mock_job_id_generator.go @@ -5,7 +5,6 @@ package mocks import ( pegomock "github.com/petergtz/pegomock" - models "github.com/runatlantis/atlantis/server/events/models" "reflect" "time" ) @@ -25,11 +24,11 @@ func NewMockJobIDGenerator(options ...pegomock.Option) *MockJobIDGenerator { func (mock *MockJobIDGenerator) SetFailHandler(fh pegomock.FailHandler) { mock.fail = fh } func (mock *MockJobIDGenerator) FailHandler() pegomock.FailHandler { return mock.fail } -func (mock *MockJobIDGenerator) GenerateJobID(_param0 models.PullRequest, _param1 string, _param2 string) string { +func (mock *MockJobIDGenerator) GenerateJobID() string { if mock == nil { panic("mock must not be nil. Use myMock := NewMockJobIDGenerator().") } - params := []pegomock.Param{_param0, _param1, _param2} + params := []pegomock.Param{} result := pegomock.GetGenericMockFrom(mock).Invoke("GenerateJobID", params, []reflect.Type{reflect.TypeOf((*string)(nil)).Elem()}) var ret0 string if len(result) != 0 { @@ -77,8 +76,8 @@ type VerifierMockJobIDGenerator struct { timeout time.Duration } -func (verifier *VerifierMockJobIDGenerator) GenerateJobID(_param0 models.PullRequest, _param1 string, _param2 string) *MockJobIDGenerator_GenerateJobID_OngoingVerification { - params := []pegomock.Param{_param0, _param1, _param2} +func (verifier *VerifierMockJobIDGenerator) GenerateJobID() *MockJobIDGenerator_GenerateJobID_OngoingVerification { + params := []pegomock.Param{} methodInvocations := pegomock.GetGenericMockFrom(verifier.mock).Verify(verifier.inOrderContext, verifier.invocationCountMatcher, "GenerateJobID", params, verifier.timeout) return &MockJobIDGenerator_GenerateJobID_OngoingVerification{mock: verifier.mock, methodInvocations: methodInvocations} } @@ -88,26 +87,8 @@ type MockJobIDGenerator_GenerateJobID_OngoingVerification struct { methodInvocations []pegomock.MethodInvocation } -func (c *MockJobIDGenerator_GenerateJobID_OngoingVerification) GetCapturedArguments() (models.PullRequest, string, string) { - _param0, _param1, _param2 := c.GetAllCapturedArguments() - return _param0[len(_param0)-1], _param1[len(_param1)-1], _param2[len(_param2)-1] +func (c *MockJobIDGenerator_GenerateJobID_OngoingVerification) GetCapturedArguments() { } -func (c *MockJobIDGenerator_GenerateJobID_OngoingVerification) GetAllCapturedArguments() (_param0 []models.PullRequest, _param1 []string, _param2 []string) { - params := pegomock.GetGenericMockFrom(c.mock).GetInvocationParams(c.methodInvocations) - if len(params) > 0 { - _param0 = make([]models.PullRequest, len(c.methodInvocations)) - for u, param := range params[0] { - _param0[u] = param.(models.PullRequest) - } - _param1 = make([]string, len(c.methodInvocations)) - for u, param := range params[1] { - _param1[u] = param.(string) - } - _param2 = make([]string, len(c.methodInvocations)) - for u, param := range params[2] { - _param2[u] = param.(string) - } - } - return +func (c *MockJobIDGenerator_GenerateJobID_OngoingVerification) GetAllCapturedArguments() { } diff --git a/server/handlers/mocks/mock_project_command_output_handler.go b/server/handlers/mocks/mock_project_command_output_handler.go index 4d04a774a..7944b9b13 100644 --- a/server/handlers/mocks/mock_project_command_output_handler.go +++ b/server/handlers/mocks/mock_project_command_output_handler.go @@ -25,7 +25,7 @@ func NewMockProjectCommandOutputHandler(options ...pegomock.Option) *MockProject func (mock *MockProjectCommandOutputHandler) SetFailHandler(fh pegomock.FailHandler) { mock.fail = fh } func (mock *MockProjectCommandOutputHandler) FailHandler() pegomock.FailHandler { return mock.fail } -func (mock *MockProjectCommandOutputHandler) CleanUp(_param0 models.JobContext) { +func (mock *MockProjectCommandOutputHandler) CleanUp(_param0 models.PullContext) { if mock == nil { panic("mock must not be nil. Use myMock := NewMockProjectCommandOutputHandler().") } @@ -49,21 +49,6 @@ func (mock *MockProjectCommandOutputHandler) Deregister(_param0 string, _param1 pegomock.GetGenericMockFrom(mock).Invoke("Deregister", params, []reflect.Type{}) } -func (mock *MockProjectCommandOutputHandler) GenerateJobID(_param0 models.PullRequest, _param1 string, _param2 string) string { - if mock == nil { - panic("mock must not be nil. Use myMock := NewMockProjectCommandOutputHandler().") - } - params := []pegomock.Param{_param0, _param1, _param2} - result := pegomock.GetGenericMockFrom(mock).Invoke("GenerateJobID", params, []reflect.Type{reflect.TypeOf((*string)(nil)).Elem()}) - var ret0 string - if len(result) != 0 { - if result[0] != nil { - ret0 = result[0].(string) - } - } - return ret0 -} - func (mock *MockProjectCommandOutputHandler) Handle() { if mock == nil { panic("mock must not be nil. Use myMock := NewMockProjectCommandOutputHandler().") @@ -140,7 +125,7 @@ type VerifierMockProjectCommandOutputHandler struct { timeout time.Duration } -func (verifier *VerifierMockProjectCommandOutputHandler) CleanUp(_param0 models.JobContext) *MockProjectCommandOutputHandler_CleanUp_OngoingVerification { +func (verifier *VerifierMockProjectCommandOutputHandler) CleanUp(_param0 models.PullContext) *MockProjectCommandOutputHandler_CleanUp_OngoingVerification { params := []pegomock.Param{_param0} methodInvocations := pegomock.GetGenericMockFrom(verifier.mock).Verify(verifier.inOrderContext, verifier.invocationCountMatcher, "CleanUp", params, verifier.timeout) return &MockProjectCommandOutputHandler_CleanUp_OngoingVerification{mock: verifier.mock, methodInvocations: methodInvocations} @@ -151,17 +136,17 @@ type MockProjectCommandOutputHandler_CleanUp_OngoingVerification struct { methodInvocations []pegomock.MethodInvocation } -func (c *MockProjectCommandOutputHandler_CleanUp_OngoingVerification) GetCapturedArguments() models.JobContext { +func (c *MockProjectCommandOutputHandler_CleanUp_OngoingVerification) GetCapturedArguments() models.PullContext { _param0 := c.GetAllCapturedArguments() return _param0[len(_param0)-1] } -func (c *MockProjectCommandOutputHandler_CleanUp_OngoingVerification) GetAllCapturedArguments() (_param0 []models.JobContext) { +func (c *MockProjectCommandOutputHandler_CleanUp_OngoingVerification) GetAllCapturedArguments() (_param0 []models.PullContext) { params := pegomock.GetGenericMockFrom(c.mock).GetInvocationParams(c.methodInvocations) if len(params) > 0 { - _param0 = make([]models.JobContext, len(c.methodInvocations)) + _param0 = make([]models.PullContext, len(c.methodInvocations)) for u, param := range params[0] { - _param0[u] = param.(models.JobContext) + _param0[u] = param.(models.PullContext) } } return @@ -225,41 +210,6 @@ func (c *MockProjectCommandOutputHandler_Deregister_OngoingVerification) GetAllC return } -func (verifier *VerifierMockProjectCommandOutputHandler) GenerateJobID(_param0 models.PullRequest, _param1 string, _param2 string) *MockProjectCommandOutputHandler_GenerateJobID_OngoingVerification { - params := []pegomock.Param{_param0, _param1, _param2} - methodInvocations := pegomock.GetGenericMockFrom(verifier.mock).Verify(verifier.inOrderContext, verifier.invocationCountMatcher, "GenerateJobID", params, verifier.timeout) - return &MockProjectCommandOutputHandler_GenerateJobID_OngoingVerification{mock: verifier.mock, methodInvocations: methodInvocations} -} - -type MockProjectCommandOutputHandler_GenerateJobID_OngoingVerification struct { - mock *MockProjectCommandOutputHandler - methodInvocations []pegomock.MethodInvocation -} - -func (c *MockProjectCommandOutputHandler_GenerateJobID_OngoingVerification) GetCapturedArguments() (models.PullRequest, string, string) { - _param0, _param1, _param2 := c.GetAllCapturedArguments() - return _param0[len(_param0)-1], _param1[len(_param1)-1], _param2[len(_param2)-1] -} - -func (c *MockProjectCommandOutputHandler_GenerateJobID_OngoingVerification) GetAllCapturedArguments() (_param0 []models.PullRequest, _param1 []string, _param2 []string) { - params := pegomock.GetGenericMockFrom(c.mock).GetInvocationParams(c.methodInvocations) - if len(params) > 0 { - _param0 = make([]models.PullRequest, len(c.methodInvocations)) - for u, param := range params[0] { - _param0[u] = param.(models.PullRequest) - } - _param1 = make([]string, len(c.methodInvocations)) - for u, param := range params[1] { - _param1[u] = param.(string) - } - _param2 = make([]string, len(c.methodInvocations)) - for u, param := range params[2] { - _param2[u] = param.(string) - } - } - return -} - func (verifier *VerifierMockProjectCommandOutputHandler) Handle() *MockProjectCommandOutputHandler_Handle_OngoingVerification { params := []pegomock.Param{} methodInvocations := pegomock.GetGenericMockFrom(verifier.mock).Verify(verifier.inOrderContext, verifier.invocationCountMatcher, "Handle", params, verifier.timeout) diff --git a/server/handlers/project_command_output_handler.go b/server/handlers/project_command_output_handler.go index 6d2cc4fff..091aa6457 100644 --- a/server/handlers/project_command_output_handler.go +++ b/server/handlers/project_command_output_handler.go @@ -3,7 +3,6 @@ package handlers import ( "sync" - "github.com/google/uuid" "github.com/runatlantis/atlantis/server/events/models" "github.com/runatlantis/atlantis/server/logging" ) @@ -24,12 +23,14 @@ type AsyncProjectCommandOutputHandler struct { logger logging.SimpleLogging - pullToJobMapping map[models.JobContext]string + // Tracks all the jobs for a pull request + // Used for clean up after a pull request is closed. + pullToJobMapping map[models.PullContext]map[string]bool } //go:generate pegomock generate -m --use-experimental-model-gen --package mocks -o mocks/mock_job_id_generator.go JobIDGenerator type JobIDGenerator interface { - GenerateJobID(pull models.PullRequest, projectName string, workspace string) string + GenerateJobID() string } //go:generate pegomock generate -m --use-experimental-model-gen --package mocks -o mocks/mock_project_job_url_generator.go ProjectJobURLGenerator @@ -71,14 +72,12 @@ type ProjectCommandOutputHandler interface { SetJobURLWithStatus(ctx models.ProjectCommandContext, cmdName models.CommandName, status models.CommitStatus) error ResourceCleaner - - JobIDGenerator } //go:generate pegomock generate -m --use-experimental-model-gen --package mocks -o mocks/mock_resource_cleaner.go ResourceCleaner type ResourceCleaner interface { - CleanUp(jobContext models.JobContext) + CleanUp(jobContext models.PullContext) } func NewAsyncProjectCommandOutputHandler( @@ -94,14 +93,23 @@ func NewAsyncProjectCommandOutputHandler( projectStatusUpdater: projectStatusUpdater, projectJobURLGenerator: projectJobURLGenerator, projectOutputBuffers: map[string][]string{}, - pullToJobMapping: map[models.JobContext]string{}, + pullToJobMapping: map[models.PullContext]map[string]bool{}, } } func (p *AsyncProjectCommandOutputHandler) Send(ctx models.ProjectCommandContext, msg string) { p.projectCmdOutput <- &models.ProjectCmdOutputLine{ JobID: ctx.JobID, - Line: msg, + JobContext: models.JobContext{ + HeadCommit: ctx.Pull.HeadCommit, + PullContext: models.PullContext{ + PullNum: ctx.Pull.Num, + Repo: ctx.BaseRepo.Name, + ProjectName: ctx.ProjectName, + Workspace: ctx.Workspace, + }, + }, + Line: msg, } } @@ -113,6 +121,11 @@ func (p *AsyncProjectCommandOutputHandler) Handle() { for msg := range p.projectCmdOutput { if msg.ClearBuffBefore { p.clearLogLines(msg.JobID) + } else { + if p.pullToJobMapping[msg.JobContext.PullContext] == nil { + p.pullToJobMapping[msg.JobContext.PullContext] = map[string]bool{} + } + p.pullToJobMapping[msg.JobContext.PullContext][msg.JobID] = true } p.writeLogLine(msg.JobID, msg.Line) } @@ -207,42 +220,27 @@ func (p *AsyncProjectCommandOutputHandler) GetProjectOutputBuffer(jobID string) return p.projectOutputBuffers[jobID] } -func (p *AsyncProjectCommandOutputHandler) CleanUp(jobContext models.JobContext) { - - // Retrieve the jobID from the pullToJobMapping - if jobID, ok := p.pullToJobMapping[jobContext]; ok { - p.projectOutputBuffersLock.Lock() - delete(p.projectOutputBuffers, jobID) - p.projectOutputBuffersLock.Unlock() - - // Only delete the pull record from receiver buffers. - // WS channel will be closed when the user closes the browser tab - // in closeHanlder(). - p.receiverBuffersLock.Lock() - delete(p.receiverBuffers, jobID) - p.receiverBuffersLock.Unlock() - } +func (p *AsyncProjectCommandOutputHandler) GetJobIdMapForPullContext(pullContext models.PullContext) map[string]bool { + return p.pullToJobMapping[pullContext] } -func (p *AsyncProjectCommandOutputHandler) GenerateJobID(pull models.PullRequest, projectName string, workspace string) string { - // Check if the job context exists in the pullToJobMapping - - jobContext := models.JobContext{ - PullNum: pull.Num, - Repo: pull.BaseRepo.Name, - ProjectName: projectName, - Workspace: workspace, - HeadCommit: pull.HeadCommit, - } +func (p *AsyncProjectCommandOutputHandler) CleanUp(pullContext models.PullContext) { + if jobIdMap, ok := p.pullToJobMapping[pullContext]; ok { + for jobID := range jobIdMap { + p.projectOutputBuffersLock.Lock() + delete(p.projectOutputBuffers, jobID) + p.projectOutputBuffersLock.Unlock() + + // Only delete the pull record from receiver buffers. + // WS channel will be closed when the user closes the browser tab + // in closeHanlder(). + p.receiverBuffersLock.Lock() + delete(p.receiverBuffers, jobID) + p.receiverBuffersLock.Unlock() + } - var jobID string - if val, ok := p.pullToJobMapping[jobContext]; ok { - jobID = val - } else { - jobID = uuid.New().String() - p.pullToJobMapping[jobContext] = jobID + delete(p.pullToJobMapping, pullContext) } - return jobID } // NoopProjectOutputHandler is a mock that doesn't do anything @@ -264,7 +262,7 @@ func (p *NoopProjectOutputHandler) SetJobURLWithStatus(ctx models.ProjectCommand return nil } -func (p *NoopProjectOutputHandler) CleanUp(jobContext models.JobContext) { +func (p *NoopProjectOutputHandler) CleanUp(pullContext models.PullContext) { } func (p *NoopProjectOutputHandler) GenerateJobID(pull models.PullRequest, projectName string, workspace string) string { diff --git a/server/handlers/project_command_output_handler_test.go b/server/handlers/project_command_output_handler_test.go index 6fac1c729..fed12286c 100644 --- a/server/handlers/project_command_output_handler_test.go +++ b/server/handlers/project_command_output_handler_test.go @@ -5,15 +5,14 @@ import ( "sync" "testing" + . "github.com/petergtz/pegomock" "github.com/runatlantis/atlantis/server/events/models" "github.com/runatlantis/atlantis/server/handlers" "github.com/runatlantis/atlantis/server/handlers/mocks" "github.com/runatlantis/atlantis/server/handlers/mocks/matchers" "github.com/runatlantis/atlantis/server/logging" - "github.com/stretchr/testify/assert" - - . "github.com/petergtz/pegomock" . "github.com/runatlantis/atlantis/testing" + "github.com/stretchr/testify/assert" ) func createTestProjectCmdContext(t *testing.T) models.ProjectCommandContext { @@ -32,6 +31,7 @@ func createTestProjectCmdContext(t *testing.T) models.ProjectCommandContext { HeadBranch: "master", BaseBranch: "master", Author: "test-user", + HeadCommit: "234r232432", }, User: models.User{ Username: "test-user", @@ -213,34 +213,47 @@ func TestProjectCommandOutputHandler(t *testing.T) { assert.Error(t, err) }) - t.Run("return same Job ID when follow up operation", func(t *testing.T) { - RegisterMockTestingT(t) - logger := logging.NewNoopLogger(t) - prjCmdOutputChan := make(chan *models.ProjectCmdOutputLine) - projectStatusUpdater := mocks.NewMockProjectStatusUpdater() - projectJobURLGenerator := mocks.NewMockProjectJobURLGenerator() - prjCmdOutputHandler := handlers.NewAsyncProjectCommandOutputHandler( - prjCmdOutputChan, - projectStatusUpdater, - projectJobURLGenerator, - logger, - ) + // Close all jobs for a PR when clean up + t.Run("clean up all jobs when PR is closed", func(t *testing.T) { + var wg sync.WaitGroup + projectOutputHandler := createProjectCommandOutputHandler(t) + + ch := make(chan string) + + // register channel and backfill from buffer + // Note: We call this synchronously because otherwise + // there could be a race where we are unable to register the channel + // before sending messages due to the way we lock our buffer memory cache + projectOutputHandler.Register(ctx.JobID, ch) - // Job ID generation during plan - pull := models.PullRequest{ - Num: 1, - BaseRepo: models.Repo{ - Name: "test-repo", - }, - HeadCommit: "head-sha", + wg.Add(1) + + // read from channel + go func() { + for msg := range ch { + if msg == models.LogStreamingClearMsg { + wg.Done() + } + } + }() + + projectOutputHandler.Send(ctx, Msg) + projectOutputHandler.Send(ctx, models.LogStreamingClearMsg) + + pullContext := models.PullContext{ + PullNum: ctx.Pull.Num, + Repo: ctx.BaseRepo.Name, + ProjectName: ctx.ProjectName, + Workspace: ctx.Workspace, } - projectName := "test-project" - workspaceName := "test-workspace" - jobID := prjCmdOutputHandler.GenerateJobID(pull, projectName, workspaceName) + projectOutputHandler.CleanUp(pullContext) - // Job ID generation during apply - newJobID := prjCmdOutputHandler.GenerateJobID(pull, projectName, workspaceName) + // Check all the resources are cleaned up. + dfProjectOutputHandler, ok := projectOutputHandler.(*handlers.AsyncProjectCommandOutputHandler) + assert.True(t, ok) - assert.Equal(t, jobID, newJobID) + assert.Empty(t, dfProjectOutputHandler.GetProjectOutputBuffer(ctx.JobID)) + assert.Empty(t, dfProjectOutputHandler.GetReceiverBufferForPull(ctx.JobID)) + assert.Empty(t, dfProjectOutputHandler.GetJobIdMapForPullContext(pullContext)) }) } diff --git a/server/server.go b/server/server.go index d329d28ea..b86fa40c0 100644 --- a/server/server.go +++ b/server/server.go @@ -518,7 +518,7 @@ func NewServer(userConfig UserConfig, config Config) (*Server, error) { statsScope, logger, userConfig.MaxProjectsPerPR, - projectCmdOutputHandler, + events.UUIDJobIdGenerator{}, ) showStepRunner, err := runtime.NewShowStepRunner(terraformClient, defaultTfVersion) @@ -752,6 +752,7 @@ func NewServer(userConfig UserConfig, config Config) (*Server, error) { wsMux := websocket.NewMultiplexor( logger, + controllers.JobIDKeyGenerator{}, projectCmdOutputHandler, ) From 852ac4cea04d10b909c4293e3a62278a695b0e53 Mon Sep 17 00:00:00 2001 From: Aayush Gupta Date: Thu, 27 Jan 2022 09:21:59 -0800 Subject: [PATCH 06/19] Removing unnecessary methods --- server/handlers/project_command_output_handler.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/server/handlers/project_command_output_handler.go b/server/handlers/project_command_output_handler.go index 091aa6457..f6667f958 100644 --- a/server/handlers/project_command_output_handler.go +++ b/server/handlers/project_command_output_handler.go @@ -264,7 +264,3 @@ func (p *NoopProjectOutputHandler) SetJobURLWithStatus(ctx models.ProjectCommand func (p *NoopProjectOutputHandler) CleanUp(pullContext models.PullContext) { } - -func (p *NoopProjectOutputHandler) GenerateJobID(pull models.PullRequest, projectName string, workspace string) string { - return "" -} From aeb56b9c2bb5890fe8a44688de9a7d2af7e38c28 Mon Sep 17 00:00:00 2001 From: Aayush Gupta Date: Thu, 27 Jan 2022 11:36:16 -0800 Subject: [PATCH 07/19] Addressing comments --- server/controllers/jobs_controller.go | 14 +++---- .../project_command_output_handler.go | 10 +---- .../project_command_output_handler_test.go | 37 ------------------- server/server.go | 1 + 4 files changed, 9 insertions(+), 53 deletions(-) diff --git a/server/controllers/jobs_controller.go b/server/controllers/jobs_controller.go index a9d5a679c..0f96da3b0 100644 --- a/server/controllers/jobs_controller.go +++ b/server/controllers/jobs_controller.go @@ -6,6 +6,7 @@ import ( "net/url" "github.com/gorilla/mux" + "github.com/pkg/errors" "github.com/runatlantis/atlantis/server/controllers/templates" "github.com/runatlantis/atlantis/server/controllers/websocket" "github.com/runatlantis/atlantis/server/core/db" @@ -35,14 +36,14 @@ type JobsController struct { Db *db.BoltDB WsMux *websocket.Multiplexor StatsScope tally.Scope + KeyGenerator websocket.PartitionKeyGenerator } func (j *JobsController) getProjectJobs(w http.ResponseWriter, r *http.Request) error { - jobID, ok := mux.Vars(r)["job-id"] - if !ok { - err := fmt.Errorf("internal error: no job ID in route") - j.respond(w, logging.Error, http.StatusBadRequest, err.Error()) - return err + jobID, err := j.KeyGenerator.Generate(r) + + if err != nil { + return errors.Wrapf(err, "generating partition key") } viewData := templates.ProjectJobData{ @@ -52,8 +53,7 @@ func (j *JobsController) getProjectJobs(w http.ResponseWriter, r *http.Request) ClearMsg: models.LogStreamingClearMsg, } - err := j.ProjectJobsTemplate.Execute(w, viewData) - if err != nil { + if err = j.ProjectJobsTemplate.Execute(w, viewData); err != nil { j.Logger.Err(err.Error()) return err } diff --git a/server/handlers/project_command_output_handler.go b/server/handlers/project_command_output_handler.go index f6667f958..e84fdd142 100644 --- a/server/handlers/project_command_output_handler.go +++ b/server/handlers/project_command_output_handler.go @@ -119,9 +119,7 @@ func (p *AsyncProjectCommandOutputHandler) Register(jobID string, receiver chan func (p *AsyncProjectCommandOutputHandler) Handle() { for msg := range p.projectCmdOutput { - if msg.ClearBuffBefore { - p.clearLogLines(msg.JobID) - } else { + if !msg.ClearBuffBefore { if p.pullToJobMapping[msg.JobContext.PullContext] == nil { p.pullToJobMapping[msg.JobContext.PullContext] = map[string]bool{} } @@ -148,12 +146,6 @@ func (p *AsyncProjectCommandOutputHandler) SetJobURLWithStatus(ctx models.Projec return p.projectStatusUpdater.UpdateProject(ctx, cmdName, status, url) } -func (p *AsyncProjectCommandOutputHandler) clearLogLines(jobID string) { - p.projectOutputBuffersLock.Lock() - delete(p.projectOutputBuffers, jobID) - p.projectOutputBuffersLock.Unlock() -} - func (p *AsyncProjectCommandOutputHandler) addChan(ch chan string, jobID string) { p.projectOutputBuffersLock.RLock() buffer := p.projectOutputBuffers[jobID] diff --git a/server/handlers/project_command_output_handler_test.go b/server/handlers/project_command_output_handler_test.go index fed12286c..0c2af679b 100644 --- a/server/handlers/project_command_output_handler_test.go +++ b/server/handlers/project_command_output_handler_test.go @@ -97,43 +97,6 @@ func TestProjectCommandOutputHandler(t *testing.T) { Equals(t, expectedMsg, Msg) }) - t.Run("clear buffer", func(t *testing.T) { - var wg sync.WaitGroup - - projectOutputHandler := createProjectCommandOutputHandler(t) - - ch := make(chan string) - - // register channel and backfill from buffer - // Note: We call this synchronously because otherwise - // there could be a race where we are unable to register the channel - // before sending messages due to the way we lock our buffer memory cache - projectOutputHandler.Register(ctx.JobID, ch) - - wg.Add(1) - // read from channel asynchronously - go func() { - for msg := range ch { - // we are done once we receive the clear message. - // prior message doesn't matter for this test. - if msg == models.LogStreamingClearMsg { - wg.Done() - } - } - }() - - // send regular message followed by clear message - projectOutputHandler.Send(ctx, Msg) - projectOutputHandler.Clear(ctx) - wg.Wait() - close(ch) - - dfProjectOutputHandler, ok := projectOutputHandler.(*handlers.AsyncProjectCommandOutputHandler) - assert.True(t, ok) - - assert.Empty(t, dfProjectOutputHandler.GetProjectOutputBuffer(ctx.JobID)) - }) - t.Run("copies buffer to new channels", func(t *testing.T) { var wg sync.WaitGroup diff --git a/server/server.go b/server/server.go index b86fa40c0..4272c0d06 100644 --- a/server/server.go +++ b/server/server.go @@ -765,6 +765,7 @@ func NewServer(userConfig UserConfig, config Config) (*Server, error) { Db: boltdb, WsMux: wsMux, StatsScope: statsScope.SubScope("api"), + KeyGenerator: controllers.JobIDKeyGenerator{}, } eventsController := &events_controllers.VCSEventsController{ From 00d6a726d0b1bfab7c9336112296921b516a5a73 Mon Sep 17 00:00:00 2001 From: Aayush Gupta Date: Mon, 31 Jan 2022 11:17:42 -0800 Subject: [PATCH 08/19] Addressing comments --- server/controllers/jobs_controller.go | 2 +- server/events/models/models.go | 2 -- server/events/project_command_builder.go | 1 - .../project_command_builder_internal_test.go | 4 ---- .../events/project_command_context_builder.go | 8 +++---- .../project_command_context_builder_test.go | 2 -- server/handlers/concurrent_map.go | 0 .../project_command_output_handler.go | 22 ++++++++++++++----- 8 files changed, 21 insertions(+), 20 deletions(-) create mode 100644 server/handlers/concurrent_map.go diff --git a/server/controllers/jobs_controller.go b/server/controllers/jobs_controller.go index 0f96da3b0..a5e4fc037 100644 --- a/server/controllers/jobs_controller.go +++ b/server/controllers/jobs_controller.go @@ -36,7 +36,7 @@ type JobsController struct { Db *db.BoltDB WsMux *websocket.Multiplexor StatsScope tally.Scope - KeyGenerator websocket.PartitionKeyGenerator + KeyGenerator JobIDKeyGenerator } func (j *JobsController) getProjectJobs(w http.ResponseWriter, r *http.Request) error { diff --git a/server/events/models/models.go b/server/events/models/models.go index 6957a7778..1cca18572 100644 --- a/server/events/models/models.go +++ b/server/events/models/models.go @@ -732,8 +732,6 @@ type ProjectCmdOutputLine struct { JobContext JobContext Line string - - ClearBuffBefore bool } // String returns the string representation of c. diff --git a/server/events/project_command_builder.go b/server/events/project_command_builder.go index bedc6a77c..39401c115 100644 --- a/server/events/project_command_builder.go +++ b/server/events/project_command_builder.go @@ -112,7 +112,6 @@ func NewProjectCommandBuilderWithLimit( policyChecksSupported, commentBuilder, scope, - jobIDGenerator, ), } diff --git a/server/events/project_command_builder_internal_test.go b/server/events/project_command_builder_internal_test.go index afd3d835c..7ed5c8a21 100644 --- a/server/events/project_command_builder_internal_test.go +++ b/server/events/project_command_builder_internal_test.go @@ -10,7 +10,6 @@ import ( "github.com/runatlantis/atlantis/server/events/matchers" "github.com/runatlantis/atlantis/server/events/models" vcsmocks "github.com/runatlantis/atlantis/server/events/vcs/mocks" - handlermocks "github.com/runatlantis/atlantis/server/handlers/mocks" "github.com/runatlantis/atlantis/server/events/yaml" "github.com/runatlantis/atlantis/server/events/yaml/valid" @@ -615,7 +614,6 @@ projects: SkipCloneNoChanges: false, ProjectCommandContextBuilder: &DefaultProjectCommandContextBuilder{ CommentBuilder: &CommentParser{}, - JobIDGenerator: handlermocks.NewMockJobIDGenerator(), }, AutoplanFileList: "**/*.tf,**/*.tfvars,**/*.tfvars.json,**/terragrunt.hcl", EnableRegExpCmd: false, @@ -808,7 +806,6 @@ projects: SkipCloneNoChanges: true, ProjectCommandContextBuilder: &DefaultProjectCommandContextBuilder{ CommentBuilder: &CommentParser{}, - JobIDGenerator: handlermocks.NewMockJobIDGenerator(), }, AutoplanFileList: "**/*.tf,**/*.tfvars,**/*.tfvars.json,**/terragrunt.hcl", EnableRegExpCmd: true, @@ -1030,7 +1027,6 @@ workflows: ProjectCommandContextBuilder: &PolicyCheckProjectCommandContextBuilder{ ProjectCommandContextBuilder: &DefaultProjectCommandContextBuilder{ CommentBuilder: &CommentParser{}, - JobIDGenerator: handlermocks.NewMockJobIDGenerator(), }, CommentBuilder: &CommentParser{}, }, diff --git a/server/events/project_command_context_builder.go b/server/events/project_command_context_builder.go index b62d6e67c..376976200 100644 --- a/server/events/project_command_context_builder.go +++ b/server/events/project_command_context_builder.go @@ -4,18 +4,17 @@ import ( "path/filepath" "regexp" + "github.com/google/uuid" "github.com/hashicorp/go-version" "github.com/hashicorp/terraform-config-inspect/tfconfig" "github.com/runatlantis/atlantis/server/events/models" "github.com/runatlantis/atlantis/server/events/yaml/valid" - "github.com/runatlantis/atlantis/server/handlers" "github.com/uber-go/tally" ) -func NewProjectCommandContextBuilder(policyCheckEnabled bool, commentBuilder CommentBuilder, scope tally.Scope, jobIDGenerator handlers.JobIDGenerator) ProjectCommandContextBuilder { +func NewProjectCommandContextBuilder(policyCheckEnabled bool, commentBuilder CommentBuilder, scope tally.Scope) ProjectCommandContextBuilder { projectCommandContextBuilder := &DefaultProjectCommandContextBuilder{ CommentBuilder: commentBuilder, - JobIDGenerator: jobIDGenerator, } contextBuilderWithStats := &CommandScopedStatsProjectCommandContextBuilder{ @@ -93,7 +92,6 @@ func (cb *CommandScopedStatsProjectCommandContextBuilder) BuildProjectContext( type DefaultProjectCommandContextBuilder struct { CommentBuilder CommentBuilder - JobIDGenerator handlers.JobIDGenerator } func (cb *DefaultProjectCommandContextBuilder) BuildProjectContext( @@ -138,7 +136,7 @@ func (cb *DefaultProjectCommandContextBuilder) BuildProjectContext( contextFlags, ctx.Scope, ctx.PullRequestStatus, - cb.JobIDGenerator.GenerateJobID(), + uuid.New().String(), ) projectCmds = append(projectCmds, projectCmdContext) diff --git a/server/events/project_command_context_builder_test.go b/server/events/project_command_context_builder_test.go index 7f8e86263..74ab4a0d7 100644 --- a/server/events/project_command_context_builder_test.go +++ b/server/events/project_command_context_builder_test.go @@ -8,7 +8,6 @@ import ( "github.com/runatlantis/atlantis/server/events/mocks" "github.com/runatlantis/atlantis/server/events/models" "github.com/runatlantis/atlantis/server/events/yaml/valid" - handlermocks "github.com/runatlantis/atlantis/server/handlers/mocks" "github.com/runatlantis/atlantis/server/logging" "github.com/stretchr/testify/assert" ) @@ -18,7 +17,6 @@ func TestProjectCommandContextBuilder_PullStatus(t *testing.T) { mockCommentBuilder := mocks.NewMockCommentBuilder() subject := events.DefaultProjectCommandContextBuilder{ CommentBuilder: mockCommentBuilder, - JobIDGenerator: handlermocks.NewMockJobIDGenerator(), } projRepoRelDir := "dir1" diff --git a/server/handlers/concurrent_map.go b/server/handlers/concurrent_map.go new file mode 100644 index 000000000..e69de29bb diff --git a/server/handlers/project_command_output_handler.go b/server/handlers/project_command_output_handler.go index e84fdd142..1a276559a 100644 --- a/server/handlers/project_command_output_handler.go +++ b/server/handlers/project_command_output_handler.go @@ -28,6 +28,17 @@ type AsyncProjectCommandOutputHandler struct { pullToJobMapping map[models.PullContext]map[string]bool } +type LockedJobMap struct { + sync.RWMutex + internal map[string]bool +} + +func NewLockedJobMap() *LockedJobMap { + return &LockedJobMap{ + internal: map[string]bool{}, + } +} + //go:generate pegomock generate -m --use-experimental-model-gen --package mocks -o mocks/mock_job_id_generator.go JobIDGenerator type JobIDGenerator interface { GenerateJobID() string @@ -123,17 +134,18 @@ func (p *AsyncProjectCommandOutputHandler) Handle() { if p.pullToJobMapping[msg.JobContext.PullContext] == nil { p.pullToJobMapping[msg.JobContext.PullContext] = map[string]bool{} } - p.pullToJobMapping[msg.JobContext.PullContext][msg.JobID] = true - } + jobMapping, _ := p.pullToJobMapping.Load(msg.JobContext.PullContext) + jobMapping = jobMapping.(map[string]bool) + + p.pullToJobMapping[msg.JobContext.PullContext][msg.JobID] = true p.writeLogLine(msg.JobID, msg.Line) } } func (p *AsyncProjectCommandOutputHandler) Clear(ctx models.ProjectCommandContext) { p.projectCmdOutput <- &models.ProjectCmdOutputLine{ - JobID: ctx.JobID, - Line: models.LogStreamingClearMsg, - ClearBuffBefore: true, + JobID: ctx.JobID, + Line: models.LogStreamingClearMsg, } } From c4c1a8512e90f7d4093919939c29b802b616334c Mon Sep 17 00:00:00 2001 From: Aayush Gupta Date: Mon, 31 Jan 2022 15:19:37 -0800 Subject: [PATCH 09/19] Addressing comments --- server/controllers/jobs_controller.go | 2 - server/controllers/templates/web_templates.go | 8 --- server/events/models/models.go | 1 - server/events/project_command_runner.go | 1 - server/handlers/concurrent_map.go | 0 .../project_command_output_handler.go | 65 ++++++------------- .../project_command_output_handler_test.go | 4 +- 7 files changed, 22 insertions(+), 59 deletions(-) delete mode 100644 server/handlers/concurrent_map.go diff --git a/server/controllers/jobs_controller.go b/server/controllers/jobs_controller.go index a5e4fc037..ffcee9764 100644 --- a/server/controllers/jobs_controller.go +++ b/server/controllers/jobs_controller.go @@ -11,7 +11,6 @@ import ( "github.com/runatlantis/atlantis/server/controllers/websocket" "github.com/runatlantis/atlantis/server/core/db" "github.com/runatlantis/atlantis/server/events/metrics" - "github.com/runatlantis/atlantis/server/events/models" "github.com/runatlantis/atlantis/server/logging" "github.com/uber-go/tally" ) @@ -50,7 +49,6 @@ func (j *JobsController) getProjectJobs(w http.ResponseWriter, r *http.Request) AtlantisVersion: j.AtlantisVersion, ProjectPath: jobID, CleanedBasePath: j.AtlantisURL.Path, - ClearMsg: models.LogStreamingClearMsg, } if err = j.ProjectJobsTemplate.Execute(w, viewData); err != nil { diff --git a/server/controllers/templates/web_templates.go b/server/controllers/templates/web_templates.go index f5f3d1383..0df699d6d 100644 --- a/server/controllers/templates/web_templates.go +++ b/server/controllers/templates/web_templates.go @@ -357,7 +357,6 @@ type ProjectJobData struct { AtlantisVersion string ProjectPath string CleanedBasePath string - ClearMsg string } var ProjectJobsTemplate = template.Must(template.New("blank.html.tmpl").Parse(` @@ -422,13 +421,6 @@ var ProjectJobsTemplate = template.Must(template.New("blank.html.tmpl").Parse(` document.location.host + document.location.pathname + "/ws"); - socket.onmessage = function(event) { - var msg = String.fromCharCode.apply(null, new Uint8Array(event.data)) - if (msg.trim() === "-----Starting New Process-----") { - term.clear() - return - } - } window.addEventListener("unload", function(event) { websocket.close(); }) diff --git a/server/events/models/models.go b/server/events/models/models.go index 1cca18572..365f3eba1 100644 --- a/server/events/models/models.go +++ b/server/events/models/models.go @@ -35,7 +35,6 @@ import ( const ( planfileSlashReplace = "::" - LogStreamingClearMsg = "\n-----Starting New Process-----" ) type PullReqStatus struct { diff --git a/server/events/project_command_runner.go b/server/events/project_command_runner.go index fe3afb754..de5a2e44c 100644 --- a/server/events/project_command_runner.go +++ b/server/events/project_command_runner.go @@ -126,7 +126,6 @@ type ProjectOutputWrapper struct { func (p *ProjectOutputWrapper) Plan(ctx models.ProjectCommandContext) models.ProjectResult { // Reset the buffer when running the plan. We only need to do this for plan, // apply is a continuation of the same workflow - p.ProjectCmdOutputHandler.Clear(ctx) return p.updateProjectPRStatus(models.PlanCommand, ctx, p.ProjectCommandRunner.Plan) } diff --git a/server/handlers/concurrent_map.go b/server/handlers/concurrent_map.go deleted file mode 100644 index e69de29bb..000000000 diff --git a/server/handlers/project_command_output_handler.go b/server/handlers/project_command_output_handler.go index 1a276559a..d0e8b33b4 100644 --- a/server/handlers/project_command_output_handler.go +++ b/server/handlers/project_command_output_handler.go @@ -23,20 +23,8 @@ type AsyncProjectCommandOutputHandler struct { logger logging.SimpleLogging - // Tracks all the jobs for a pull request - // Used for clean up after a pull request is closed. - pullToJobMapping map[models.PullContext]map[string]bool -} - -type LockedJobMap struct { - sync.RWMutex - internal map[string]bool -} - -func NewLockedJobMap() *LockedJobMap { - return &LockedJobMap{ - internal: map[string]bool{}, - } + // Tracks all the jobs for a pull request which is used for clean up after a pull request is closed. + pullToJobMapping sync.Map } //go:generate pegomock generate -m --use-experimental-model-gen --package mocks -o mocks/mock_job_id_generator.go JobIDGenerator @@ -62,9 +50,6 @@ type ProjectStatusUpdater interface { //go:generate pegomock generate -m --use-experimental-model-gen --package mocks -o mocks/mock_project_command_output_handler.go ProjectCommandOutputHandler type ProjectCommandOutputHandler interface { - // Clear clears the buffer from previous terraform output lines - Clear(ctx models.ProjectCommandContext) - // Send will enqueue the msg and wait for Handle() to receive the message. Send(ctx models.ProjectCommandContext, msg string) @@ -104,7 +89,7 @@ func NewAsyncProjectCommandOutputHandler( projectStatusUpdater: projectStatusUpdater, projectJobURLGenerator: projectJobURLGenerator, projectOutputBuffers: map[string][]string{}, - pullToJobMapping: map[models.PullContext]map[string]bool{}, + pullToJobMapping: sync.Map{}, } } @@ -130,22 +115,15 @@ func (p *AsyncProjectCommandOutputHandler) Register(jobID string, receiver chan func (p *AsyncProjectCommandOutputHandler) Handle() { for msg := range p.projectCmdOutput { - if !msg.ClearBuffBefore { - if p.pullToJobMapping[msg.JobContext.PullContext] == nil { - p.pullToJobMapping[msg.JobContext.PullContext] = map[string]bool{} - } - jobMapping, _ := p.pullToJobMapping.Load(msg.JobContext.PullContext) - jobMapping = jobMapping.(map[string]bool) - - p.pullToJobMapping[msg.JobContext.PullContext][msg.JobID] = true - p.writeLogLine(msg.JobID, msg.Line) - } -} + // Add job to pullToJob mapping + if _, ok := p.pullToJobMapping.Load(msg.JobContext.PullContext); !ok { + p.pullToJobMapping.Store(msg.JobContext.PullContext, map[string]bool{}) + } + value, _ := p.pullToJobMapping.Load(msg.JobContext.PullContext) + jobMapping := value.(map[string]bool) + jobMapping[msg.JobID] = true -func (p *AsyncProjectCommandOutputHandler) Clear(ctx models.ProjectCommandContext) { - p.projectCmdOutput <- &models.ProjectCmdOutputLine{ - JobID: ctx.JobID, - Line: models.LogStreamingClearMsg, + p.writeLogLine(msg.JobID, msg.Line) } } @@ -195,11 +173,6 @@ func (p *AsyncProjectCommandOutputHandler) writeLogLine(jobID string, line strin } p.receiverBuffersLock.Unlock() - // No need to write to projectOutputBuffers if clear msg. - if line == models.LogStreamingClearMsg { - return - } - p.projectOutputBuffersLock.Lock() if p.projectOutputBuffers[jobID] == nil { p.projectOutputBuffers[jobID] = []string{} @@ -225,12 +198,16 @@ func (p *AsyncProjectCommandOutputHandler) GetProjectOutputBuffer(jobID string) } func (p *AsyncProjectCommandOutputHandler) GetJobIdMapForPullContext(pullContext models.PullContext) map[string]bool { - return p.pullToJobMapping[pullContext] + if value, ok := p.pullToJobMapping.Load(pullContext); ok { + return value.(map[string]bool) + } + return nil } func (p *AsyncProjectCommandOutputHandler) CleanUp(pullContext models.PullContext) { - if jobIdMap, ok := p.pullToJobMapping[pullContext]; ok { - for jobID := range jobIdMap { + if value, ok := p.pullToJobMapping.Load(pullContext); ok { + jobMapping := value.(map[string]bool) + for jobID := range jobMapping { p.projectOutputBuffersLock.Lock() delete(p.projectOutputBuffers, jobID) p.projectOutputBuffersLock.Unlock() @@ -243,7 +220,8 @@ func (p *AsyncProjectCommandOutputHandler) CleanUp(pullContext models.PullContex p.receiverBuffersLock.Unlock() } - delete(p.pullToJobMapping, pullContext) + // Remove job mapping + p.pullToJobMapping.Delete(pullContext) } } @@ -259,9 +237,6 @@ func (p *NoopProjectOutputHandler) Deregister(jobID string, receiver chan string func (p *NoopProjectOutputHandler) Handle() { } -func (p *NoopProjectOutputHandler) Clear(ctx models.ProjectCommandContext) { -} - func (p *NoopProjectOutputHandler) SetJobURLWithStatus(ctx models.ProjectCommandContext, cmdName models.CommandName, status models.CommitStatus) error { return nil } diff --git a/server/handlers/project_command_output_handler_test.go b/server/handlers/project_command_output_handler_test.go index 0c2af679b..5cf6f318b 100644 --- a/server/handlers/project_command_output_handler_test.go +++ b/server/handlers/project_command_output_handler_test.go @@ -194,14 +194,14 @@ func TestProjectCommandOutputHandler(t *testing.T) { // read from channel go func() { for msg := range ch { - if msg == models.LogStreamingClearMsg { + if msg == "Complete" { wg.Done() } } }() projectOutputHandler.Send(ctx, Msg) - projectOutputHandler.Send(ctx, models.LogStreamingClearMsg) + projectOutputHandler.Send(ctx, "Complete") pullContext := models.PullContext{ PullNum: ctx.Pull.Num, From d2859ca649f7a1585f54d08e696570364d1e42dc Mon Sep 17 00:00:00 2001 From: Aayush Gupta Date: Mon, 31 Jan 2022 15:26:34 -0800 Subject: [PATCH 10/19] Removing JobIDGenerator from project Command builder --- .../controllers/events/events_controller_e2e_test.go | 1 - server/events/project_command_builder.go | 4 ---- server/events/project_command_builder_test.go | 11 ----------- server/server.go | 1 - 4 files changed, 17 deletions(-) diff --git a/server/controllers/events/events_controller_e2e_test.go b/server/controllers/events/events_controller_e2e_test.go index 40f32c07b..2f7c6f80e 100644 --- a/server/controllers/events/events_controller_e2e_test.go +++ b/server/controllers/events/events_controller_e2e_test.go @@ -912,7 +912,6 @@ func setupE2E(t *testing.T, repoDir string) (events_controllers.VCSEventsControl "**/*.tf,**/*.tfvars,**/*.tfvars.json,**/terragrunt.hcl", statsScope, logger, - events.UUIDJobIdGenerator{}, ) showStepRunner, err := runtime.NewShowStepRunner(terraformClient, defaultTFVersion) diff --git a/server/events/project_command_builder.go b/server/events/project_command_builder.go index 39401c115..e7728ca2e 100644 --- a/server/events/project_command_builder.go +++ b/server/events/project_command_builder.go @@ -6,7 +6,6 @@ import ( "github.com/google/uuid" "github.com/runatlantis/atlantis/server/events/yaml/valid" - "github.com/runatlantis/atlantis/server/handlers" "github.com/runatlantis/atlantis/server/logging" "github.com/uber-go/tally" @@ -57,7 +56,6 @@ func NewProjectCommandBuilder( AutoplanFileList string, scope tally.Scope, logger logging.SimpleLogging, - jobIDGenerator handlers.JobIDGenerator, ) ProjectCommandBuilder { return NewProjectCommandBuilderWithLimit( policyChecksSupported, @@ -75,7 +73,6 @@ func NewProjectCommandBuilder( scope, logger, InfiniteProjectsPerPR, - jobIDGenerator, ) } @@ -95,7 +92,6 @@ func NewProjectCommandBuilderWithLimit( scope tally.Scope, logger logging.SimpleLogging, limit int, - jobIDGenerator handlers.JobIDGenerator, ) ProjectCommandBuilder { var projectCommandBuilder ProjectCommandBuilder = &DefaultProjectCommandBuilder{ ParserValidator: parserValidator, diff --git a/server/events/project_command_builder_test.go b/server/events/project_command_builder_test.go index 40fbf1781..97b3f1b93 100644 --- a/server/events/project_command_builder_test.go +++ b/server/events/project_command_builder_test.go @@ -15,7 +15,6 @@ import ( vcsmocks "github.com/runatlantis/atlantis/server/events/vcs/mocks" "github.com/runatlantis/atlantis/server/events/yaml" "github.com/runatlantis/atlantis/server/events/yaml/valid" - handlermocks "github.com/runatlantis/atlantis/server/handlers/mocks" "github.com/runatlantis/atlantis/server/logging" "github.com/runatlantis/atlantis/server/metrics" . "github.com/runatlantis/atlantis/testing" @@ -161,7 +160,6 @@ projects: "**/*.tf,**/*.tfvars,**/*.tfvars.json,**/terragrunt.hcl", scope, logger, - handlermocks.NewMockJobIDGenerator(), ) ctxs, err := builder.BuildAutoplanCommands(&events.CommandContext{ @@ -430,7 +428,6 @@ projects: "**/*.tf,**/*.tfvars,**/*.tfvars.json,**/terragrunt.hcl", scope, logger, - handlermocks.NewMockJobIDGenerator(), ) var actCtxs []models.ProjectCommandContext @@ -586,7 +583,6 @@ projects: "**/*.tf,**/*.tfvars,**/*.tfvars.json,**/terragrunt.hcl", scope, logger, - handlermocks.NewMockJobIDGenerator(), ) ctxs, err := builder.BuildPlanCommands( @@ -678,7 +674,6 @@ func TestDefaultProjectCommandBuilder_BuildMultiApply(t *testing.T) { "**/*.tf,**/*.tfvars,**/*.tfvars.json,**/terragrunt.hcl", scope, logger, - handlermocks.NewMockJobIDGenerator(), ) ctxs, err := builder.BuildApplyCommands( @@ -764,7 +759,6 @@ projects: "**/*.tf,**/*.tfvars,**/*.tfvars.json,**/terragrunt.hcl", scope, logger, - handlermocks.NewMockJobIDGenerator(), ) ctx := &events.CommandContext{ @@ -844,7 +838,6 @@ func TestDefaultProjectCommandBuilder_EscapeArgs(t *testing.T) { "**/*.tf,**/*.tfvars,**/*.tfvars.json,**/terragrunt.hcl", scope, logger, - handlermocks.NewMockJobIDGenerator(), ) var actCtxs []models.ProjectCommandContext @@ -1028,7 +1021,6 @@ projects: "**/*.tf,**/*.tfvars,**/*.tfvars.json,**/terragrunt.hcl", scope, logger, - handlermocks.NewMockJobIDGenerator(), ) actCtxs, err := builder.BuildPlanCommands( @@ -1096,7 +1088,6 @@ projects: "**/*.tf,**/*.tfvars,**/*.tfvars.json,**/terragrunt.hcl", scope, logger, - handlermocks.NewMockJobIDGenerator(), ) var actCtxs []models.ProjectCommandContext @@ -1155,7 +1146,6 @@ func TestDefaultProjectCommandBuilder_WithPolicyCheckEnabled_BuildAutoplanComman "**/*.tf,**/*.tfvars,**/*.tfvars.json,**/terragrunt.hcl", scope, logger, - handlermocks.NewMockJobIDGenerator(), ) ctxs, err := builder.BuildAutoplanCommands(&events.CommandContext{ @@ -1238,7 +1228,6 @@ func TestDefaultProjectCommandBuilder_BuildVersionCommand(t *testing.T) { "**/*.tf,**/*.tfvars,**/*.tfvars.json,**/terragrunt.hcl", scope, logger, - handlermocks.NewMockJobIDGenerator(), ) ctxs, err := builder.BuildVersionCommands( diff --git a/server/server.go b/server/server.go index 4272c0d06..71e7fc82b 100644 --- a/server/server.go +++ b/server/server.go @@ -518,7 +518,6 @@ func NewServer(userConfig UserConfig, config Config) (*Server, error) { statsScope, logger, userConfig.MaxProjectsPerPR, - events.UUIDJobIdGenerator{}, ) showStepRunner, err := runtime.NewShowStepRunner(terraformClient, defaultTfVersion) From a5fbe8f4518d90d2dd7283f5390af408b3435f27 Mon Sep 17 00:00:00 2001 From: Aayush Gupta Date: Mon, 31 Jan 2022 15:48:31 -0800 Subject: [PATCH 11/19] Fixing tests --- server/events/project_command_builder.go | 7 ------- .../events/project_command_builder_internal_test.go | 11 ++++++++++- server/events/project_command_context_builder.go | 5 +---- 3 files changed, 11 insertions(+), 12 deletions(-) diff --git a/server/events/project_command_builder.go b/server/events/project_command_builder.go index e7728ca2e..ac49b5cec 100644 --- a/server/events/project_command_builder.go +++ b/server/events/project_command_builder.go @@ -4,7 +4,6 @@ import ( "fmt" "os" - "github.com/google/uuid" "github.com/runatlantis/atlantis/server/events/yaml/valid" "github.com/runatlantis/atlantis/server/logging" "github.com/uber-go/tally" @@ -35,12 +34,6 @@ const ( InfiniteProjectsPerPR = -1 ) -type UUIDJobIdGenerator struct{} - -func (u UUIDJobIdGenerator) GenerateJobID() string { - return uuid.New().String() -} - func NewProjectCommandBuilder( policyChecksSupported bool, parserValidator *yaml.ParserValidator, diff --git a/server/events/project_command_builder_internal_test.go b/server/events/project_command_builder_internal_test.go index 7ed5c8a21..5653e8867 100644 --- a/server/events/project_command_builder_internal_test.go +++ b/server/events/project_command_builder_internal_test.go @@ -10,7 +10,6 @@ import ( "github.com/runatlantis/atlantis/server/events/matchers" "github.com/runatlantis/atlantis/server/events/models" vcsmocks "github.com/runatlantis/atlantis/server/events/vcs/mocks" - "github.com/runatlantis/atlantis/server/events/yaml" "github.com/runatlantis/atlantis/server/events/yaml/valid" "github.com/runatlantis/atlantis/server/logging" @@ -660,6 +659,9 @@ projects: c.expCtx.Steps = expSteps ctx.PolicySets = emptyPolicySets + // Job ID cannot be compared since its generated at random + ctx.JobID = "" + Equals(t, c.expCtx, ctx) // Equals() doesn't compare TF version properly so have to // use .String(). @@ -852,6 +854,10 @@ projects: // Init fields we couldn't in our cases map. c.expCtx.Steps = expSteps ctx.PolicySets = emptyPolicySets + + // Job ID cannot be compared since its generated at random + ctx.JobID = "" + Equals(t, c.expCtx, ctx) // Equals() doesn't compare TF version properly so have to // use .String(). @@ -1070,6 +1076,9 @@ workflows: c.expCtx.Steps = expSteps ctx.PolicySets = emptyPolicySets + // Job ID cannot be compared since its generated at random + ctx.JobID = "" + Equals(t, c.expCtx, ctx) // Equals() doesn't compare TF version properly so have to // use .String(). diff --git a/server/events/project_command_context_builder.go b/server/events/project_command_context_builder.go index 376976200..d40f95328 100644 --- a/server/events/project_command_context_builder.go +++ b/server/events/project_command_context_builder.go @@ -136,7 +136,6 @@ func (cb *DefaultProjectCommandContextBuilder) BuildProjectContext( contextFlags, ctx.Scope, ctx.PullRequestStatus, - uuid.New().String(), ) projectCmds = append(projectCmds, projectCmdContext) @@ -191,7 +190,6 @@ func (cb *PolicyCheckProjectCommandContextBuilder) BuildProjectContext( contextFlags, ctx.Scope, ctx.PullRequestStatus, - "", // No Job ID for policy check commands )) } @@ -212,7 +210,6 @@ func newProjectCommandContext(ctx *CommandContext, contextFlags *ContextFlags, scope tally.Scope, pullStatus models.PullReqStatus, - jobID string, ) models.ProjectCommandContext { var projectPlanStatus models.ProjectPlanStatus @@ -262,7 +259,7 @@ func newProjectCommandContext(ctx *CommandContext, PolicySets: policySets, Tags: projCfg.Tags, PullReqStatus: pullStatus, - JobID: jobID, + JobID: uuid.New().String(), } } From 388d98f330ad952acf03177e586bd7b242541fae Mon Sep 17 00:00:00 2001 From: Aayush Gupta Date: Mon, 31 Jan 2022 16:12:40 -0800 Subject: [PATCH 12/19] Updating mocks --- server/controllers/jobs_controller.go | 6 +- .../handlers/mocks/mock_job_id_generator.go | 94 --------- .../mock_project_command_output_handler.go | 191 +++++++----------- .../project_command_output_handler.go | 7 +- 4 files changed, 82 insertions(+), 216 deletions(-) delete mode 100644 server/handlers/mocks/mock_job_id_generator.go diff --git a/server/controllers/jobs_controller.go b/server/controllers/jobs_controller.go index ffcee9764..56fbfd1c3 100644 --- a/server/controllers/jobs_controller.go +++ b/server/controllers/jobs_controller.go @@ -6,7 +6,6 @@ import ( "net/url" "github.com/gorilla/mux" - "github.com/pkg/errors" "github.com/runatlantis/atlantis/server/controllers/templates" "github.com/runatlantis/atlantis/server/controllers/websocket" "github.com/runatlantis/atlantis/server/core/db" @@ -42,7 +41,8 @@ func (j *JobsController) getProjectJobs(w http.ResponseWriter, r *http.Request) jobID, err := j.KeyGenerator.Generate(r) if err != nil { - return errors.Wrapf(err, "generating partition key") + j.respond(w, logging.Error, http.StatusBadRequest, err.Error()) + return err } viewData := templates.ProjectJobData{ @@ -71,7 +71,7 @@ func (j *JobsController) getProjectJobsWS(w http.ResponseWriter, r *http.Request err := j.WsMux.Handle(w, r) if err != nil { - j.respond(w, logging.Error, http.StatusInternalServerError, err.Error()) + j.respond(w, logging.Error, http.StatusBadRequest, err.Error()) return err } diff --git a/server/handlers/mocks/mock_job_id_generator.go b/server/handlers/mocks/mock_job_id_generator.go deleted file mode 100644 index 0b57d7bd0..000000000 --- a/server/handlers/mocks/mock_job_id_generator.go +++ /dev/null @@ -1,94 +0,0 @@ -// Code generated by pegomock. DO NOT EDIT. -// Source: github.com/runatlantis/atlantis/server/handlers (interfaces: JobIDGenerator) - -package mocks - -import ( - pegomock "github.com/petergtz/pegomock" - "reflect" - "time" -) - -type MockJobIDGenerator struct { - fail func(message string, callerSkip ...int) -} - -func NewMockJobIDGenerator(options ...pegomock.Option) *MockJobIDGenerator { - mock := &MockJobIDGenerator{} - for _, option := range options { - option.Apply(mock) - } - return mock -} - -func (mock *MockJobIDGenerator) SetFailHandler(fh pegomock.FailHandler) { mock.fail = fh } -func (mock *MockJobIDGenerator) FailHandler() pegomock.FailHandler { return mock.fail } - -func (mock *MockJobIDGenerator) GenerateJobID() string { - if mock == nil { - panic("mock must not be nil. Use myMock := NewMockJobIDGenerator().") - } - params := []pegomock.Param{} - result := pegomock.GetGenericMockFrom(mock).Invoke("GenerateJobID", params, []reflect.Type{reflect.TypeOf((*string)(nil)).Elem()}) - var ret0 string - if len(result) != 0 { - if result[0] != nil { - ret0 = result[0].(string) - } - } - return ret0 -} - -func (mock *MockJobIDGenerator) VerifyWasCalledOnce() *VerifierMockJobIDGenerator { - return &VerifierMockJobIDGenerator{ - mock: mock, - invocationCountMatcher: pegomock.Times(1), - } -} - -func (mock *MockJobIDGenerator) VerifyWasCalled(invocationCountMatcher pegomock.InvocationCountMatcher) *VerifierMockJobIDGenerator { - return &VerifierMockJobIDGenerator{ - mock: mock, - invocationCountMatcher: invocationCountMatcher, - } -} - -func (mock *MockJobIDGenerator) VerifyWasCalledInOrder(invocationCountMatcher pegomock.InvocationCountMatcher, inOrderContext *pegomock.InOrderContext) *VerifierMockJobIDGenerator { - return &VerifierMockJobIDGenerator{ - mock: mock, - invocationCountMatcher: invocationCountMatcher, - inOrderContext: inOrderContext, - } -} - -func (mock *MockJobIDGenerator) VerifyWasCalledEventually(invocationCountMatcher pegomock.InvocationCountMatcher, timeout time.Duration) *VerifierMockJobIDGenerator { - return &VerifierMockJobIDGenerator{ - mock: mock, - invocationCountMatcher: invocationCountMatcher, - timeout: timeout, - } -} - -type VerifierMockJobIDGenerator struct { - mock *MockJobIDGenerator - invocationCountMatcher pegomock.InvocationCountMatcher - inOrderContext *pegomock.InOrderContext - timeout time.Duration -} - -func (verifier *VerifierMockJobIDGenerator) GenerateJobID() *MockJobIDGenerator_GenerateJobID_OngoingVerification { - params := []pegomock.Param{} - methodInvocations := pegomock.GetGenericMockFrom(verifier.mock).Verify(verifier.inOrderContext, verifier.invocationCountMatcher, "GenerateJobID", params, verifier.timeout) - return &MockJobIDGenerator_GenerateJobID_OngoingVerification{mock: verifier.mock, methodInvocations: methodInvocations} -} - -type MockJobIDGenerator_GenerateJobID_OngoingVerification struct { - mock *MockJobIDGenerator - methodInvocations []pegomock.MethodInvocation -} - -func (c *MockJobIDGenerator_GenerateJobID_OngoingVerification) GetCapturedArguments() { -} - -func (c *MockJobIDGenerator_GenerateJobID_OngoingVerification) GetAllCapturedArguments() { -} diff --git a/server/handlers/mocks/mock_project_command_output_handler.go b/server/handlers/mocks/mock_project_command_output_handler.go index 7944b9b13..456faa2be 100644 --- a/server/handlers/mocks/mock_project_command_output_handler.go +++ b/server/handlers/mocks/mock_project_command_output_handler.go @@ -25,27 +25,27 @@ func NewMockProjectCommandOutputHandler(options ...pegomock.Option) *MockProject func (mock *MockProjectCommandOutputHandler) SetFailHandler(fh pegomock.FailHandler) { mock.fail = fh } func (mock *MockProjectCommandOutputHandler) FailHandler() pegomock.FailHandler { return mock.fail } -func (mock *MockProjectCommandOutputHandler) CleanUp(_param0 models.PullContext) { +func (mock *MockProjectCommandOutputHandler) Send(ctx models.ProjectCommandContext, msg string) { if mock == nil { panic("mock must not be nil. Use myMock := NewMockProjectCommandOutputHandler().") } - params := []pegomock.Param{_param0} - pegomock.GetGenericMockFrom(mock).Invoke("CleanUp", params, []reflect.Type{}) + params := []pegomock.Param{ctx, msg} + pegomock.GetGenericMockFrom(mock).Invoke("Send", params, []reflect.Type{}) } -func (mock *MockProjectCommandOutputHandler) Clear(_param0 models.ProjectCommandContext) { +func (mock *MockProjectCommandOutputHandler) Register(jobID string, receiver chan string) { if mock == nil { panic("mock must not be nil. Use myMock := NewMockProjectCommandOutputHandler().") } - params := []pegomock.Param{_param0} - pegomock.GetGenericMockFrom(mock).Invoke("Clear", params, []reflect.Type{}) + params := []pegomock.Param{jobID, receiver} + pegomock.GetGenericMockFrom(mock).Invoke("Register", params, []reflect.Type{}) } -func (mock *MockProjectCommandOutputHandler) Deregister(_param0 string, _param1 chan string) { +func (mock *MockProjectCommandOutputHandler) Deregister(jobID string, receiver chan string) { if mock == nil { panic("mock must not be nil. Use myMock := NewMockProjectCommandOutputHandler().") } - params := []pegomock.Param{_param0, _param1} + params := []pegomock.Param{jobID, receiver} pegomock.GetGenericMockFrom(mock).Invoke("Deregister", params, []reflect.Type{}) } @@ -57,27 +57,11 @@ func (mock *MockProjectCommandOutputHandler) Handle() { pegomock.GetGenericMockFrom(mock).Invoke("Handle", params, []reflect.Type{}) } -func (mock *MockProjectCommandOutputHandler) Register(_param0 string, _param1 chan string) { - if mock == nil { - panic("mock must not be nil. Use myMock := NewMockProjectCommandOutputHandler().") - } - params := []pegomock.Param{_param0, _param1} - pegomock.GetGenericMockFrom(mock).Invoke("Register", params, []reflect.Type{}) -} - -func (mock *MockProjectCommandOutputHandler) Send(_param0 models.ProjectCommandContext, _param1 string) { - if mock == nil { - panic("mock must not be nil. Use myMock := NewMockProjectCommandOutputHandler().") - } - params := []pegomock.Param{_param0, _param1} - pegomock.GetGenericMockFrom(mock).Invoke("Send", params, []reflect.Type{}) -} - -func (mock *MockProjectCommandOutputHandler) SetJobURLWithStatus(_param0 models.ProjectCommandContext, _param1 models.CommandName, _param2 models.CommitStatus) error { +func (mock *MockProjectCommandOutputHandler) SetJobURLWithStatus(ctx models.ProjectCommandContext, cmdName models.CommandName, status models.CommitStatus) error { if mock == nil { panic("mock must not be nil. Use myMock := NewMockProjectCommandOutputHandler().") } - params := []pegomock.Param{_param0, _param1, _param2} + params := []pegomock.Param{ctx, cmdName, status} result := pegomock.GetGenericMockFrom(mock).Invoke("SetJobURLWithStatus", params, []reflect.Type{reflect.TypeOf((*error)(nil)).Elem()}) var ret0 error if len(result) != 0 { @@ -88,6 +72,14 @@ func (mock *MockProjectCommandOutputHandler) SetJobURLWithStatus(_param0 models. return ret0 } +func (mock *MockProjectCommandOutputHandler) CleanUp(pullContext models.PullContext) { + if mock == nil { + panic("mock must not be nil. Use myMock := NewMockProjectCommandOutputHandler().") + } + params := []pegomock.Param{pullContext} + pegomock.GetGenericMockFrom(mock).Invoke("CleanUp", params, []reflect.Type{}) +} + func (mock *MockProjectCommandOutputHandler) VerifyWasCalledOnce() *VerifierMockProjectCommandOutputHandler { return &VerifierMockProjectCommandOutputHandler{ mock: mock, @@ -125,62 +117,70 @@ type VerifierMockProjectCommandOutputHandler struct { timeout time.Duration } -func (verifier *VerifierMockProjectCommandOutputHandler) CleanUp(_param0 models.PullContext) *MockProjectCommandOutputHandler_CleanUp_OngoingVerification { - params := []pegomock.Param{_param0} - methodInvocations := pegomock.GetGenericMockFrom(verifier.mock).Verify(verifier.inOrderContext, verifier.invocationCountMatcher, "CleanUp", params, verifier.timeout) - return &MockProjectCommandOutputHandler_CleanUp_OngoingVerification{mock: verifier.mock, methodInvocations: methodInvocations} +func (verifier *VerifierMockProjectCommandOutputHandler) Send(ctx models.ProjectCommandContext, msg string) *MockProjectCommandOutputHandler_Send_OngoingVerification { + params := []pegomock.Param{ctx, msg} + methodInvocations := pegomock.GetGenericMockFrom(verifier.mock).Verify(verifier.inOrderContext, verifier.invocationCountMatcher, "Send", params, verifier.timeout) + return &MockProjectCommandOutputHandler_Send_OngoingVerification{mock: verifier.mock, methodInvocations: methodInvocations} } -type MockProjectCommandOutputHandler_CleanUp_OngoingVerification struct { +type MockProjectCommandOutputHandler_Send_OngoingVerification struct { mock *MockProjectCommandOutputHandler methodInvocations []pegomock.MethodInvocation } -func (c *MockProjectCommandOutputHandler_CleanUp_OngoingVerification) GetCapturedArguments() models.PullContext { - _param0 := c.GetAllCapturedArguments() - return _param0[len(_param0)-1] +func (c *MockProjectCommandOutputHandler_Send_OngoingVerification) GetCapturedArguments() (models.ProjectCommandContext, string) { + ctx, msg := c.GetAllCapturedArguments() + return ctx[len(ctx)-1], msg[len(msg)-1] } -func (c *MockProjectCommandOutputHandler_CleanUp_OngoingVerification) GetAllCapturedArguments() (_param0 []models.PullContext) { +func (c *MockProjectCommandOutputHandler_Send_OngoingVerification) GetAllCapturedArguments() (_param0 []models.ProjectCommandContext, _param1 []string) { params := pegomock.GetGenericMockFrom(c.mock).GetInvocationParams(c.methodInvocations) if len(params) > 0 { - _param0 = make([]models.PullContext, len(c.methodInvocations)) + _param0 = make([]models.ProjectCommandContext, len(c.methodInvocations)) for u, param := range params[0] { - _param0[u] = param.(models.PullContext) + _param0[u] = param.(models.ProjectCommandContext) + } + _param1 = make([]string, len(c.methodInvocations)) + for u, param := range params[1] { + _param1[u] = param.(string) } } return } -func (verifier *VerifierMockProjectCommandOutputHandler) Clear(_param0 models.ProjectCommandContext) *MockProjectCommandOutputHandler_Clear_OngoingVerification { - params := []pegomock.Param{_param0} - methodInvocations := pegomock.GetGenericMockFrom(verifier.mock).Verify(verifier.inOrderContext, verifier.invocationCountMatcher, "Clear", params, verifier.timeout) - return &MockProjectCommandOutputHandler_Clear_OngoingVerification{mock: verifier.mock, methodInvocations: methodInvocations} +func (verifier *VerifierMockProjectCommandOutputHandler) Register(jobID string, receiver chan string) *MockProjectCommandOutputHandler_Register_OngoingVerification { + params := []pegomock.Param{jobID, receiver} + methodInvocations := pegomock.GetGenericMockFrom(verifier.mock).Verify(verifier.inOrderContext, verifier.invocationCountMatcher, "Register", params, verifier.timeout) + return &MockProjectCommandOutputHandler_Register_OngoingVerification{mock: verifier.mock, methodInvocations: methodInvocations} } -type MockProjectCommandOutputHandler_Clear_OngoingVerification struct { +type MockProjectCommandOutputHandler_Register_OngoingVerification struct { mock *MockProjectCommandOutputHandler methodInvocations []pegomock.MethodInvocation } -func (c *MockProjectCommandOutputHandler_Clear_OngoingVerification) GetCapturedArguments() models.ProjectCommandContext { - _param0 := c.GetAllCapturedArguments() - return _param0[len(_param0)-1] +func (c *MockProjectCommandOutputHandler_Register_OngoingVerification) GetCapturedArguments() (string, chan string) { + jobID, receiver := c.GetAllCapturedArguments() + return jobID[len(jobID)-1], receiver[len(receiver)-1] } -func (c *MockProjectCommandOutputHandler_Clear_OngoingVerification) GetAllCapturedArguments() (_param0 []models.ProjectCommandContext) { +func (c *MockProjectCommandOutputHandler_Register_OngoingVerification) GetAllCapturedArguments() (_param0 []string, _param1 []chan string) { params := pegomock.GetGenericMockFrom(c.mock).GetInvocationParams(c.methodInvocations) if len(params) > 0 { - _param0 = make([]models.ProjectCommandContext, len(c.methodInvocations)) + _param0 = make([]string, len(c.methodInvocations)) for u, param := range params[0] { - _param0[u] = param.(models.ProjectCommandContext) + _param0[u] = param.(string) + } + _param1 = make([]chan string, len(c.methodInvocations)) + for u, param := range params[1] { + _param1[u] = param.(chan string) } } return } -func (verifier *VerifierMockProjectCommandOutputHandler) Deregister(_param0 string, _param1 chan string) *MockProjectCommandOutputHandler_Deregister_OngoingVerification { - params := []pegomock.Param{_param0, _param1} +func (verifier *VerifierMockProjectCommandOutputHandler) Deregister(jobID string, receiver chan string) *MockProjectCommandOutputHandler_Deregister_OngoingVerification { + params := []pegomock.Param{jobID, receiver} methodInvocations := pegomock.GetGenericMockFrom(verifier.mock).Verify(verifier.inOrderContext, verifier.invocationCountMatcher, "Deregister", params, verifier.timeout) return &MockProjectCommandOutputHandler_Deregister_OngoingVerification{mock: verifier.mock, methodInvocations: methodInvocations} } @@ -191,8 +191,8 @@ type MockProjectCommandOutputHandler_Deregister_OngoingVerification struct { } func (c *MockProjectCommandOutputHandler_Deregister_OngoingVerification) GetCapturedArguments() (string, chan string) { - _param0, _param1 := c.GetAllCapturedArguments() - return _param0[len(_param0)-1], _param1[len(_param1)-1] + jobID, receiver := c.GetAllCapturedArguments() + return jobID[len(jobID)-1], receiver[len(receiver)-1] } func (c *MockProjectCommandOutputHandler_Deregister_OngoingVerification) GetAllCapturedArguments() (_param0 []string, _param1 []chan string) { @@ -227,98 +227,63 @@ func (c *MockProjectCommandOutputHandler_Handle_OngoingVerification) GetCaptured func (c *MockProjectCommandOutputHandler_Handle_OngoingVerification) GetAllCapturedArguments() { } -func (verifier *VerifierMockProjectCommandOutputHandler) Register(_param0 string, _param1 chan string) *MockProjectCommandOutputHandler_Register_OngoingVerification { - params := []pegomock.Param{_param0, _param1} - methodInvocations := pegomock.GetGenericMockFrom(verifier.mock).Verify(verifier.inOrderContext, verifier.invocationCountMatcher, "Register", params, verifier.timeout) - return &MockProjectCommandOutputHandler_Register_OngoingVerification{mock: verifier.mock, methodInvocations: methodInvocations} -} - -type MockProjectCommandOutputHandler_Register_OngoingVerification struct { - mock *MockProjectCommandOutputHandler - methodInvocations []pegomock.MethodInvocation -} - -func (c *MockProjectCommandOutputHandler_Register_OngoingVerification) GetCapturedArguments() (string, chan string) { - _param0, _param1 := c.GetAllCapturedArguments() - return _param0[len(_param0)-1], _param1[len(_param1)-1] -} - -func (c *MockProjectCommandOutputHandler_Register_OngoingVerification) GetAllCapturedArguments() (_param0 []string, _param1 []chan string) { - params := pegomock.GetGenericMockFrom(c.mock).GetInvocationParams(c.methodInvocations) - if len(params) > 0 { - _param0 = make([]string, len(c.methodInvocations)) - for u, param := range params[0] { - _param0[u] = param.(string) - } - _param1 = make([]chan string, len(c.methodInvocations)) - for u, param := range params[1] { - _param1[u] = param.(chan string) - } - } - return -} - -func (verifier *VerifierMockProjectCommandOutputHandler) Send(_param0 models.ProjectCommandContext, _param1 string) *MockProjectCommandOutputHandler_Send_OngoingVerification { - params := []pegomock.Param{_param0, _param1} - methodInvocations := pegomock.GetGenericMockFrom(verifier.mock).Verify(verifier.inOrderContext, verifier.invocationCountMatcher, "Send", params, verifier.timeout) - return &MockProjectCommandOutputHandler_Send_OngoingVerification{mock: verifier.mock, methodInvocations: methodInvocations} +func (verifier *VerifierMockProjectCommandOutputHandler) SetJobURLWithStatus(ctx models.ProjectCommandContext, cmdName models.CommandName, status models.CommitStatus) *MockProjectCommandOutputHandler_SetJobURLWithStatus_OngoingVerification { + params := []pegomock.Param{ctx, cmdName, status} + methodInvocations := pegomock.GetGenericMockFrom(verifier.mock).Verify(verifier.inOrderContext, verifier.invocationCountMatcher, "SetJobURLWithStatus", params, verifier.timeout) + return &MockProjectCommandOutputHandler_SetJobURLWithStatus_OngoingVerification{mock: verifier.mock, methodInvocations: methodInvocations} } -type MockProjectCommandOutputHandler_Send_OngoingVerification struct { +type MockProjectCommandOutputHandler_SetJobURLWithStatus_OngoingVerification struct { mock *MockProjectCommandOutputHandler methodInvocations []pegomock.MethodInvocation } -func (c *MockProjectCommandOutputHandler_Send_OngoingVerification) GetCapturedArguments() (models.ProjectCommandContext, string) { - _param0, _param1 := c.GetAllCapturedArguments() - return _param0[len(_param0)-1], _param1[len(_param1)-1] +func (c *MockProjectCommandOutputHandler_SetJobURLWithStatus_OngoingVerification) GetCapturedArguments() (models.ProjectCommandContext, models.CommandName, models.CommitStatus) { + ctx, cmdName, status := c.GetAllCapturedArguments() + return ctx[len(ctx)-1], cmdName[len(cmdName)-1], status[len(status)-1] } -func (c *MockProjectCommandOutputHandler_Send_OngoingVerification) GetAllCapturedArguments() (_param0 []models.ProjectCommandContext, _param1 []string) { +func (c *MockProjectCommandOutputHandler_SetJobURLWithStatus_OngoingVerification) GetAllCapturedArguments() (_param0 []models.ProjectCommandContext, _param1 []models.CommandName, _param2 []models.CommitStatus) { params := pegomock.GetGenericMockFrom(c.mock).GetInvocationParams(c.methodInvocations) if len(params) > 0 { _param0 = make([]models.ProjectCommandContext, len(c.methodInvocations)) for u, param := range params[0] { _param0[u] = param.(models.ProjectCommandContext) } - _param1 = make([]string, len(c.methodInvocations)) + _param1 = make([]models.CommandName, len(c.methodInvocations)) for u, param := range params[1] { - _param1[u] = param.(string) + _param1[u] = param.(models.CommandName) + } + _param2 = make([]models.CommitStatus, len(c.methodInvocations)) + for u, param := range params[2] { + _param2[u] = param.(models.CommitStatus) } } return } -func (verifier *VerifierMockProjectCommandOutputHandler) SetJobURLWithStatus(_param0 models.ProjectCommandContext, _param1 models.CommandName, _param2 models.CommitStatus) *MockProjectCommandOutputHandler_SetJobURLWithStatus_OngoingVerification { - params := []pegomock.Param{_param0, _param1, _param2} - methodInvocations := pegomock.GetGenericMockFrom(verifier.mock).Verify(verifier.inOrderContext, verifier.invocationCountMatcher, "SetJobURLWithStatus", params, verifier.timeout) - return &MockProjectCommandOutputHandler_SetJobURLWithStatus_OngoingVerification{mock: verifier.mock, methodInvocations: methodInvocations} +func (verifier *VerifierMockProjectCommandOutputHandler) CleanUp(pullContext models.PullContext) *MockProjectCommandOutputHandler_CleanUp_OngoingVerification { + params := []pegomock.Param{pullContext} + methodInvocations := pegomock.GetGenericMockFrom(verifier.mock).Verify(verifier.inOrderContext, verifier.invocationCountMatcher, "CleanUp", params, verifier.timeout) + return &MockProjectCommandOutputHandler_CleanUp_OngoingVerification{mock: verifier.mock, methodInvocations: methodInvocations} } -type MockProjectCommandOutputHandler_SetJobURLWithStatus_OngoingVerification struct { +type MockProjectCommandOutputHandler_CleanUp_OngoingVerification struct { mock *MockProjectCommandOutputHandler methodInvocations []pegomock.MethodInvocation } -func (c *MockProjectCommandOutputHandler_SetJobURLWithStatus_OngoingVerification) GetCapturedArguments() (models.ProjectCommandContext, models.CommandName, models.CommitStatus) { - _param0, _param1, _param2 := c.GetAllCapturedArguments() - return _param0[len(_param0)-1], _param1[len(_param1)-1], _param2[len(_param2)-1] +func (c *MockProjectCommandOutputHandler_CleanUp_OngoingVerification) GetCapturedArguments() models.PullContext { + pullContext := c.GetAllCapturedArguments() + return pullContext[len(pullContext)-1] } -func (c *MockProjectCommandOutputHandler_SetJobURLWithStatus_OngoingVerification) GetAllCapturedArguments() (_param0 []models.ProjectCommandContext, _param1 []models.CommandName, _param2 []models.CommitStatus) { +func (c *MockProjectCommandOutputHandler_CleanUp_OngoingVerification) GetAllCapturedArguments() (_param0 []models.PullContext) { params := pegomock.GetGenericMockFrom(c.mock).GetInvocationParams(c.methodInvocations) if len(params) > 0 { - _param0 = make([]models.ProjectCommandContext, len(c.methodInvocations)) + _param0 = make([]models.PullContext, len(c.methodInvocations)) for u, param := range params[0] { - _param0[u] = param.(models.ProjectCommandContext) - } - _param1 = make([]models.CommandName, len(c.methodInvocations)) - for u, param := range params[1] { - _param1[u] = param.(models.CommandName) - } - _param2 = make([]models.CommitStatus, len(c.methodInvocations)) - for u, param := range params[2] { - _param2[u] = param.(models.CommitStatus) + _param0[u] = param.(models.PullContext) } } return diff --git a/server/handlers/project_command_output_handler.go b/server/handlers/project_command_output_handler.go index d0e8b33b4..c9f13843b 100644 --- a/server/handlers/project_command_output_handler.go +++ b/server/handlers/project_command_output_handler.go @@ -27,11 +27,6 @@ type AsyncProjectCommandOutputHandler struct { pullToJobMapping sync.Map } -//go:generate pegomock generate -m --use-experimental-model-gen --package mocks -o mocks/mock_job_id_generator.go JobIDGenerator -type JobIDGenerator interface { - GenerateJobID() string -} - //go:generate pegomock generate -m --use-experimental-model-gen --package mocks -o mocks/mock_project_job_url_generator.go ProjectJobURLGenerator // ProjectJobURLGenerator generates urls to view project's progress. @@ -73,7 +68,7 @@ type ProjectCommandOutputHandler interface { //go:generate pegomock generate -m --use-experimental-model-gen --package mocks -o mocks/mock_resource_cleaner.go ResourceCleaner type ResourceCleaner interface { - CleanUp(jobContext models.PullContext) + CleanUp(pullContext models.PullContext) } func NewAsyncProjectCommandOutputHandler( From cd7628234db2920f47c55140bf658c7eda82de80 Mon Sep 17 00:00:00 2001 From: Aayush Gupta Date: Tue, 1 Feb 2022 11:07:08 -0800 Subject: [PATCH 13/19] WIP --- server/controllers/websocket/writer.go | 27 -------- server/core/terraform/async_client.go | 4 +- server/events/models/models.go | 2 + server/events/project_command_runner.go | 14 +++- .../project_command_output_handler.go | 66 +++++++++++++++---- 5 files changed, 70 insertions(+), 43 deletions(-) diff --git a/server/controllers/websocket/writer.go b/server/controllers/websocket/writer.go index eca3f6ae6..323e081a8 100644 --- a/server/controllers/websocket/writer.go +++ b/server/controllers/websocket/writer.go @@ -31,18 +31,6 @@ func (w *Writer) Write(rw http.ResponseWriter, r *http.Request, input chan strin return errors.Wrap(err, "upgrading websocket connection") } - conn.SetCloseHandler(func(code int, text string) error { - // Close the channnel after websocket connection closed. - // Will gracefully exit the ProjectCommandOutputHandler.Register() call and cleanup. - // is it good practice to close at the receiver? Probably not, we should figure out a better - // way to handle this case - close(input) - return nil - }) - - // Add a reader goroutine to listen for socket.close() events. - go w.setReadHandler(conn) - // block on reading our input channel for msg := range input { if err := conn.WriteMessage(websocket.BinaryMessage, []byte("\r"+msg+"\n")); err != nil { @@ -53,18 +41,3 @@ func (w *Writer) Write(rw http.ResponseWriter, r *http.Request, input chan strin return nil } - -func (w *Writer) setReadHandler(c *websocket.Conn) { - for { - _, _, err := c.ReadMessage() - if err != nil { - // CloseGoingAway (1001) when a browser tab is closed. - // Expected behaviour since we have a CloseHandler(), log warning if not a CloseGoingAway - if websocket.IsUnexpectedCloseError(err, websocket.CloseGoingAway) { - w.log.Warn("Failed to read WS message: %s", err) - } - return - } - } - -} diff --git a/server/core/terraform/async_client.go b/server/core/terraform/async_client.go index 2db3a88d5..a94fbcae6 100644 --- a/server/core/terraform/async_client.go +++ b/server/core/terraform/async_client.go @@ -93,7 +93,7 @@ func (c *AsyncClient) RunCommandAsyncWithInput(ctx models.ProjectCommandContext, for s.Scan() { message := s.Text() outCh <- Line{Line: message} - c.projectCmdOutputHandler.Send(ctx, message) + c.projectCmdOutputHandler.Send(ctx, message, false) } wg.Done() }() @@ -102,7 +102,7 @@ func (c *AsyncClient) RunCommandAsyncWithInput(ctx models.ProjectCommandContext, for s.Scan() { message := s.Text() outCh <- Line{Line: message} - c.projectCmdOutputHandler.Send(ctx, message) + c.projectCmdOutputHandler.Send(ctx, message, false) } wg.Done() }() diff --git a/server/events/models/models.go b/server/events/models/models.go index 365f3eba1..5e208e824 100644 --- a/server/events/models/models.go +++ b/server/events/models/models.go @@ -731,6 +731,8 @@ type ProjectCmdOutputLine struct { JobContext JobContext Line string + + OperationComplete bool } // String returns the string representation of c. diff --git a/server/events/project_command_runner.go b/server/events/project_command_runner.go index de5a2e44c..006cf8663 100644 --- a/server/events/project_command_runner.go +++ b/server/events/project_command_runner.go @@ -126,11 +126,21 @@ type ProjectOutputWrapper struct { func (p *ProjectOutputWrapper) Plan(ctx models.ProjectCommandContext) models.ProjectResult { // Reset the buffer when running the plan. We only need to do this for plan, // apply is a continuation of the same workflow - return p.updateProjectPRStatus(models.PlanCommand, ctx, p.ProjectCommandRunner.Plan) + result := p.updateProjectPRStatus(models.PlanCommand, ctx, p.ProjectCommandRunner.Plan) + + if result.Error == nil && result.Failure == "" { + p.ProjectCmdOutputHandler.Send(ctx, "", true) + } + return result } func (p *ProjectOutputWrapper) Apply(ctx models.ProjectCommandContext) models.ProjectResult { - return p.updateProjectPRStatus(models.ApplyCommand, ctx, p.ProjectCommandRunner.Apply) + result := p.updateProjectPRStatus(models.ApplyCommand, ctx, p.ProjectCommandRunner.Apply) + + if result.Error == nil && result.Failure == "" { + p.ProjectCmdOutputHandler.Send(ctx, "", true) + } + return result } func (p *ProjectOutputWrapper) updateProjectPRStatus(commandName models.CommandName, ctx models.ProjectCommandContext, execute func(ctx models.ProjectCommandContext) models.ProjectResult) models.ProjectResult { diff --git a/server/handlers/project_command_output_handler.go b/server/handlers/project_command_output_handler.go index c9f13843b..e3b8e7a42 100644 --- a/server/handlers/project_command_output_handler.go +++ b/server/handlers/project_command_output_handler.go @@ -7,12 +7,17 @@ import ( "github.com/runatlantis/atlantis/server/logging" ) +type OutputBuffer struct { + isOperationComplete bool + buffer []string +} + // AsyncProjectCommandOutputHandler is a handler to transport terraform client // outputs to the front end. type AsyncProjectCommandOutputHandler struct { projectCmdOutput chan *models.ProjectCmdOutputLine - projectOutputBuffers map[string][]string + projectOutputBuffers map[string]OutputBuffer projectOutputBuffersLock sync.RWMutex receiverBuffers map[string]map[chan string]bool @@ -46,7 +51,7 @@ type ProjectStatusUpdater interface { type ProjectCommandOutputHandler interface { // Send will enqueue the msg and wait for Handle() to receive the message. - Send(ctx models.ProjectCommandContext, msg string) + Send(ctx models.ProjectCommandContext, msg string, isOperationComplete bool) // Register registers a channel and blocks until it is caught up. Callers should call this asynchronously when attempting // to read the channel in the same goroutine @@ -83,12 +88,12 @@ func NewAsyncProjectCommandOutputHandler( receiverBuffers: map[string]map[chan string]bool{}, projectStatusUpdater: projectStatusUpdater, projectJobURLGenerator: projectJobURLGenerator, - projectOutputBuffers: map[string][]string{}, + projectOutputBuffers: map[string]OutputBuffer{}, pullToJobMapping: sync.Map{}, } } -func (p *AsyncProjectCommandOutputHandler) Send(ctx models.ProjectCommandContext, msg string) { +func (p *AsyncProjectCommandOutputHandler) Send(ctx models.ProjectCommandContext, msg string, isOperationComplete bool) { p.projectCmdOutput <- &models.ProjectCmdOutputLine{ JobID: ctx.JobID, JobContext: models.JobContext{ @@ -100,7 +105,8 @@ func (p *AsyncProjectCommandOutputHandler) Send(ctx models.ProjectCommandContext Workspace: ctx.Workspace, }, }, - Line: msg, + Line: msg, + OperationComplete: isOperationComplete, } } @@ -110,6 +116,18 @@ func (p *AsyncProjectCommandOutputHandler) Register(jobID string, receiver chan func (p *AsyncProjectCommandOutputHandler) Handle() { for msg := range p.projectCmdOutput { + if msg.OperationComplete { + p.projectOutputBuffersLock.Lock() + if outputBuffer, ok := p.projectOutputBuffers[msg.JobID]; ok { + outputBuffer.isOperationComplete = true + p.projectOutputBuffers[msg.JobID] = outputBuffer + } + p.projectOutputBuffersLock.Unlock() + + // Close all channels for this job + p.closeChannelsForJob(msg.JobID) + continue + } // Add job to pullToJob mapping if _, ok := p.pullToJobMapping.Load(msg.JobContext.PullContext); !ok { p.pullToJobMapping.Store(msg.JobContext.PullContext, map[string]bool{}) @@ -122,6 +140,18 @@ func (p *AsyncProjectCommandOutputHandler) Handle() { } } +func (p *AsyncProjectCommandOutputHandler) closeChannelsForJob(jobID string) { + p.receiverBuffersLock.Lock() + defer func() { + p.receiverBuffersLock.Unlock() + }() + if openChannels, ok := p.receiverBuffers[jobID]; ok { + for ch := range openChannels { + close(ch) + } + } +} + func (p *AsyncProjectCommandOutputHandler) SetJobURLWithStatus(ctx models.ProjectCommandContext, cmdName models.CommandName, status models.CommitStatus) error { url, err := p.projectJobURLGenerator.GenerateProjectJobURL(ctx) @@ -133,13 +163,19 @@ func (p *AsyncProjectCommandOutputHandler) SetJobURLWithStatus(ctx models.Projec func (p *AsyncProjectCommandOutputHandler) addChan(ch chan string, jobID string) { p.projectOutputBuffersLock.RLock() - buffer := p.projectOutputBuffers[jobID] + outputBuffer := p.projectOutputBuffers[jobID] p.projectOutputBuffersLock.RUnlock() - for _, line := range buffer { + for _, line := range outputBuffer.buffer { ch <- line } + // No need register receiver since all the logs have been streamed + if outputBuffer.isOperationComplete { + close(ch) + return + } + // add the channel to our registry after we backfill the contents of the buffer, // to prevent new messages coming in interleaving with this backfill. p.receiverBuffersLock.Lock() @@ -169,10 +205,16 @@ func (p *AsyncProjectCommandOutputHandler) writeLogLine(jobID string, line strin p.receiverBuffersLock.Unlock() p.projectOutputBuffersLock.Lock() - if p.projectOutputBuffers[jobID] == nil { - p.projectOutputBuffers[jobID] = []string{} + if _, ok := p.projectOutputBuffers[jobID]; !ok { + p.projectOutputBuffers[jobID] = OutputBuffer{ + isOperationComplete: false, + buffer: []string{}, + } } - p.projectOutputBuffers[jobID] = append(p.projectOutputBuffers[jobID], line) + outputBuffer := p.projectOutputBuffers[jobID] + outputBuffer.buffer = append(outputBuffer.buffer, line) + p.projectOutputBuffers[jobID] = outputBuffer + p.projectOutputBuffersLock.Unlock() } @@ -189,7 +231,7 @@ func (p *AsyncProjectCommandOutputHandler) GetReceiverBufferForPull(jobID string } func (p *AsyncProjectCommandOutputHandler) GetProjectOutputBuffer(jobID string) []string { - return p.projectOutputBuffers[jobID] + return p.projectOutputBuffers[jobID].buffer } func (p *AsyncProjectCommandOutputHandler) GetJobIdMapForPullContext(pullContext models.PullContext) map[string]bool { @@ -223,7 +265,7 @@ func (p *AsyncProjectCommandOutputHandler) CleanUp(pullContext models.PullContex // NoopProjectOutputHandler is a mock that doesn't do anything type NoopProjectOutputHandler struct{} -func (p *NoopProjectOutputHandler) Send(ctx models.ProjectCommandContext, msg string) { +func (p *NoopProjectOutputHandler) Send(ctx models.ProjectCommandContext, msg string, isOperationComplete bool) { } func (p *NoopProjectOutputHandler) Register(jobID string, receiver chan string) {} From 1de87b68072e8c3d32f03265baff26cef933cb6d Mon Sep 17 00:00:00 2001 From: Aayush Gupta Date: Tue, 1 Feb 2022 11:18:44 -0800 Subject: [PATCH 14/19] Moving JobContext to log handler --- server/events/models/models.go | 20 -- server/events/pull_closed_executor.go | 2 +- server/events/pull_closed_executor_test.go | 2 +- .../mocks/matchers/handlers_pullcontext.go | 33 ++++ .../mocks/matchers/models_jobcontext.go | 33 ---- .../mocks/matchers/models_pullcontext.go | 33 ---- .../mock_project_command_output_handler.go | 187 +++++++++--------- .../handlers/mocks/mock_resource_cleaner.go | 21 +- .../project_command_output_handler.go | 38 +++- .../project_command_output_handler_test.go | 8 +- server/server.go | 2 +- 11 files changed, 174 insertions(+), 205 deletions(-) create mode 100644 server/handlers/mocks/matchers/handlers_pullcontext.go delete mode 100644 server/handlers/mocks/matchers/models_jobcontext.go delete mode 100644 server/handlers/mocks/matchers/models_pullcontext.go diff --git a/server/events/models/models.go b/server/events/models/models.go index 365f3eba1..4d8213ab2 100644 --- a/server/events/models/models.go +++ b/server/events/models/models.go @@ -713,26 +713,6 @@ func (c CommandName) TitleString() string { return strings.Title(strings.ReplaceAll(strings.ToLower(c.String()), "_", " ")) } -type PullContext struct { - PullNum int - Repo string - ProjectName string - Workspace string -} - -type JobContext struct { - PullContext - HeadCommit string -} - -type ProjectCmdOutputLine struct { - JobID string - - JobContext JobContext - - Line string -} - // String returns the string representation of c. func (c CommandName) String() string { switch c { diff --git a/server/events/pull_closed_executor.go b/server/events/pull_closed_executor.go index 6ef22bbf0..edb49255e 100644 --- a/server/events/pull_closed_executor.go +++ b/server/events/pull_closed_executor.go @@ -83,7 +83,7 @@ func (p *PullClosedExecutor) CleanUpPull(repo models.Repo, pull models.PullReque if pullStatus != nil { for _, project := range pullStatus.Projects { - jobContext := models.PullContext{ + jobContext := handlers.PullContext{ PullNum: pull.Num, Repo: pull.BaseRepo.Name, Workspace: project.Workspace, diff --git a/server/events/pull_closed_executor_test.go b/server/events/pull_closed_executor_test.go index a6d379367..a8bebe1a0 100644 --- a/server/events/pull_closed_executor_test.go +++ b/server/events/pull_closed_executor_test.go @@ -200,7 +200,7 @@ func TestCleanUpLogStreaming(t *testing.T) { prjJobURLGenerator := handlermocks.NewMockProjectJobURLGenerator() // Create Log streaming resources - prjCmdOutput := make(chan *models.ProjectCmdOutputLine) + prjCmdOutput := make(chan *handlers.ProjectCmdOutputLine) prjCmdOutHandler := handlers.NewAsyncProjectCommandOutputHandler(prjCmdOutput, prjStatusUpdater, prjJobURLGenerator, logger) ctx := models.ProjectCommandContext{ BaseRepo: fixtures.GithubRepo, diff --git a/server/handlers/mocks/matchers/handlers_pullcontext.go b/server/handlers/mocks/matchers/handlers_pullcontext.go new file mode 100644 index 000000000..b3322fbbb --- /dev/null +++ b/server/handlers/mocks/matchers/handlers_pullcontext.go @@ -0,0 +1,33 @@ +// Code generated by pegomock. DO NOT EDIT. +package matchers + +import ( + "github.com/petergtz/pegomock" + "reflect" + + handlers "github.com/runatlantis/atlantis/server/handlers" +) + +func AnyHandlersPullContext() handlers.PullContext { + pegomock.RegisterMatcher(pegomock.NewAnyMatcher(reflect.TypeOf((*(handlers.PullContext))(nil)).Elem())) + var nullValue handlers.PullContext + return nullValue +} + +func EqHandlersPullContext(value handlers.PullContext) handlers.PullContext { + pegomock.RegisterMatcher(&pegomock.EqMatcher{Value: value}) + var nullValue handlers.PullContext + return nullValue +} + +func NotEqHandlersPullContext(value handlers.PullContext) handlers.PullContext { + pegomock.RegisterMatcher(&pegomock.NotEqMatcher{Value: value}) + var nullValue handlers.PullContext + return nullValue +} + +func HandlersPullContextThat(matcher pegomock.ArgumentMatcher) handlers.PullContext { + pegomock.RegisterMatcher(matcher) + var nullValue handlers.PullContext + return nullValue +} diff --git a/server/handlers/mocks/matchers/models_jobcontext.go b/server/handlers/mocks/matchers/models_jobcontext.go deleted file mode 100644 index 822da3bf2..000000000 --- a/server/handlers/mocks/matchers/models_jobcontext.go +++ /dev/null @@ -1,33 +0,0 @@ -// Code generated by pegomock. DO NOT EDIT. -package matchers - -import ( - "github.com/petergtz/pegomock" - "reflect" - - models "github.com/runatlantis/atlantis/server/events/models" -) - -func AnyModelsJobContext() models.JobContext { - pegomock.RegisterMatcher(pegomock.NewAnyMatcher(reflect.TypeOf((*(models.JobContext))(nil)).Elem())) - var nullValue models.JobContext - return nullValue -} - -func EqModelsJobContext(value models.JobContext) models.JobContext { - pegomock.RegisterMatcher(&pegomock.EqMatcher{Value: value}) - var nullValue models.JobContext - return nullValue -} - -func NotEqModelsJobContext(value models.JobContext) models.JobContext { - pegomock.RegisterMatcher(&pegomock.NotEqMatcher{Value: value}) - var nullValue models.JobContext - return nullValue -} - -func ModelsJobContextThat(matcher pegomock.ArgumentMatcher) models.JobContext { - pegomock.RegisterMatcher(matcher) - var nullValue models.JobContext - return nullValue -} diff --git a/server/handlers/mocks/matchers/models_pullcontext.go b/server/handlers/mocks/matchers/models_pullcontext.go deleted file mode 100644 index fb31e0bf7..000000000 --- a/server/handlers/mocks/matchers/models_pullcontext.go +++ /dev/null @@ -1,33 +0,0 @@ -// Code generated by pegomock. DO NOT EDIT. -package matchers - -import ( - "github.com/petergtz/pegomock" - "reflect" - - models "github.com/runatlantis/atlantis/server/events/models" -) - -func AnyModelsPullContext() models.PullContext { - pegomock.RegisterMatcher(pegomock.NewAnyMatcher(reflect.TypeOf((*(models.PullContext))(nil)).Elem())) - var nullValue models.PullContext - return nullValue -} - -func EqModelsPullContext(value models.PullContext) models.PullContext { - pegomock.RegisterMatcher(&pegomock.EqMatcher{Value: value}) - var nullValue models.PullContext - return nullValue -} - -func NotEqModelsPullContext(value models.PullContext) models.PullContext { - pegomock.RegisterMatcher(&pegomock.NotEqMatcher{Value: value}) - var nullValue models.PullContext - return nullValue -} - -func ModelsPullContextThat(matcher pegomock.ArgumentMatcher) models.PullContext { - pegomock.RegisterMatcher(matcher) - var nullValue models.PullContext - return nullValue -} diff --git a/server/handlers/mocks/mock_project_command_output_handler.go b/server/handlers/mocks/mock_project_command_output_handler.go index 456faa2be..b3ec5ee89 100644 --- a/server/handlers/mocks/mock_project_command_output_handler.go +++ b/server/handlers/mocks/mock_project_command_output_handler.go @@ -6,6 +6,7 @@ package mocks import ( pegomock "github.com/petergtz/pegomock" models "github.com/runatlantis/atlantis/server/events/models" + handlers "github.com/runatlantis/atlantis/server/handlers" "reflect" "time" ) @@ -25,43 +26,51 @@ func NewMockProjectCommandOutputHandler(options ...pegomock.Option) *MockProject func (mock *MockProjectCommandOutputHandler) SetFailHandler(fh pegomock.FailHandler) { mock.fail = fh } func (mock *MockProjectCommandOutputHandler) FailHandler() pegomock.FailHandler { return mock.fail } -func (mock *MockProjectCommandOutputHandler) Send(ctx models.ProjectCommandContext, msg string) { +func (mock *MockProjectCommandOutputHandler) CleanUp(_param0 handlers.PullContext) { if mock == nil { panic("mock must not be nil. Use myMock := NewMockProjectCommandOutputHandler().") } - params := []pegomock.Param{ctx, msg} - pegomock.GetGenericMockFrom(mock).Invoke("Send", params, []reflect.Type{}) + params := []pegomock.Param{_param0} + pegomock.GetGenericMockFrom(mock).Invoke("CleanUp", params, []reflect.Type{}) } -func (mock *MockProjectCommandOutputHandler) Register(jobID string, receiver chan string) { +func (mock *MockProjectCommandOutputHandler) Deregister(_param0 string, _param1 chan string) { if mock == nil { panic("mock must not be nil. Use myMock := NewMockProjectCommandOutputHandler().") } - params := []pegomock.Param{jobID, receiver} - pegomock.GetGenericMockFrom(mock).Invoke("Register", params, []reflect.Type{}) + params := []pegomock.Param{_param0, _param1} + pegomock.GetGenericMockFrom(mock).Invoke("Deregister", params, []reflect.Type{}) } -func (mock *MockProjectCommandOutputHandler) Deregister(jobID string, receiver chan string) { +func (mock *MockProjectCommandOutputHandler) Handle() { if mock == nil { panic("mock must not be nil. Use myMock := NewMockProjectCommandOutputHandler().") } - params := []pegomock.Param{jobID, receiver} - pegomock.GetGenericMockFrom(mock).Invoke("Deregister", params, []reflect.Type{}) + params := []pegomock.Param{} + pegomock.GetGenericMockFrom(mock).Invoke("Handle", params, []reflect.Type{}) } -func (mock *MockProjectCommandOutputHandler) Handle() { +func (mock *MockProjectCommandOutputHandler) Register(_param0 string, _param1 chan string) { if mock == nil { panic("mock must not be nil. Use myMock := NewMockProjectCommandOutputHandler().") } - params := []pegomock.Param{} - pegomock.GetGenericMockFrom(mock).Invoke("Handle", params, []reflect.Type{}) + params := []pegomock.Param{_param0, _param1} + pegomock.GetGenericMockFrom(mock).Invoke("Register", params, []reflect.Type{}) +} + +func (mock *MockProjectCommandOutputHandler) Send(_param0 models.ProjectCommandContext, _param1 string) { + if mock == nil { + panic("mock must not be nil. Use myMock := NewMockProjectCommandOutputHandler().") + } + params := []pegomock.Param{_param0, _param1} + pegomock.GetGenericMockFrom(mock).Invoke("Send", params, []reflect.Type{}) } -func (mock *MockProjectCommandOutputHandler) SetJobURLWithStatus(ctx models.ProjectCommandContext, cmdName models.CommandName, status models.CommitStatus) error { +func (mock *MockProjectCommandOutputHandler) SetJobURLWithStatus(_param0 models.ProjectCommandContext, _param1 models.CommandName, _param2 models.CommitStatus) error { if mock == nil { panic("mock must not be nil. Use myMock := NewMockProjectCommandOutputHandler().") } - params := []pegomock.Param{ctx, cmdName, status} + params := []pegomock.Param{_param0, _param1, _param2} result := pegomock.GetGenericMockFrom(mock).Invoke("SetJobURLWithStatus", params, []reflect.Type{reflect.TypeOf((*error)(nil)).Elem()}) var ret0 error if len(result) != 0 { @@ -72,14 +81,6 @@ func (mock *MockProjectCommandOutputHandler) SetJobURLWithStatus(ctx models.Proj return ret0 } -func (mock *MockProjectCommandOutputHandler) CleanUp(pullContext models.PullContext) { - if mock == nil { - panic("mock must not be nil. Use myMock := NewMockProjectCommandOutputHandler().") - } - params := []pegomock.Param{pullContext} - pegomock.GetGenericMockFrom(mock).Invoke("CleanUp", params, []reflect.Type{}) -} - func (mock *MockProjectCommandOutputHandler) VerifyWasCalledOnce() *VerifierMockProjectCommandOutputHandler { return &VerifierMockProjectCommandOutputHandler{ mock: mock, @@ -117,54 +118,50 @@ type VerifierMockProjectCommandOutputHandler struct { timeout time.Duration } -func (verifier *VerifierMockProjectCommandOutputHandler) Send(ctx models.ProjectCommandContext, msg string) *MockProjectCommandOutputHandler_Send_OngoingVerification { - params := []pegomock.Param{ctx, msg} - methodInvocations := pegomock.GetGenericMockFrom(verifier.mock).Verify(verifier.inOrderContext, verifier.invocationCountMatcher, "Send", params, verifier.timeout) - return &MockProjectCommandOutputHandler_Send_OngoingVerification{mock: verifier.mock, methodInvocations: methodInvocations} +func (verifier *VerifierMockProjectCommandOutputHandler) CleanUp(_param0 handlers.PullContext) *MockProjectCommandOutputHandler_CleanUp_OngoingVerification { + params := []pegomock.Param{_param0} + methodInvocations := pegomock.GetGenericMockFrom(verifier.mock).Verify(verifier.inOrderContext, verifier.invocationCountMatcher, "CleanUp", params, verifier.timeout) + return &MockProjectCommandOutputHandler_CleanUp_OngoingVerification{mock: verifier.mock, methodInvocations: methodInvocations} } -type MockProjectCommandOutputHandler_Send_OngoingVerification struct { +type MockProjectCommandOutputHandler_CleanUp_OngoingVerification struct { mock *MockProjectCommandOutputHandler methodInvocations []pegomock.MethodInvocation } -func (c *MockProjectCommandOutputHandler_Send_OngoingVerification) GetCapturedArguments() (models.ProjectCommandContext, string) { - ctx, msg := c.GetAllCapturedArguments() - return ctx[len(ctx)-1], msg[len(msg)-1] +func (c *MockProjectCommandOutputHandler_CleanUp_OngoingVerification) GetCapturedArguments() handlers.PullContext { + _param0 := c.GetAllCapturedArguments() + return _param0[len(_param0)-1] } -func (c *MockProjectCommandOutputHandler_Send_OngoingVerification) GetAllCapturedArguments() (_param0 []models.ProjectCommandContext, _param1 []string) { +func (c *MockProjectCommandOutputHandler_CleanUp_OngoingVerification) GetAllCapturedArguments() (_param0 []handlers.PullContext) { params := pegomock.GetGenericMockFrom(c.mock).GetInvocationParams(c.methodInvocations) if len(params) > 0 { - _param0 = make([]models.ProjectCommandContext, len(c.methodInvocations)) + _param0 = make([]handlers.PullContext, len(c.methodInvocations)) for u, param := range params[0] { - _param0[u] = param.(models.ProjectCommandContext) - } - _param1 = make([]string, len(c.methodInvocations)) - for u, param := range params[1] { - _param1[u] = param.(string) + _param0[u] = param.(handlers.PullContext) } } return } -func (verifier *VerifierMockProjectCommandOutputHandler) Register(jobID string, receiver chan string) *MockProjectCommandOutputHandler_Register_OngoingVerification { - params := []pegomock.Param{jobID, receiver} - methodInvocations := pegomock.GetGenericMockFrom(verifier.mock).Verify(verifier.inOrderContext, verifier.invocationCountMatcher, "Register", params, verifier.timeout) - return &MockProjectCommandOutputHandler_Register_OngoingVerification{mock: verifier.mock, methodInvocations: methodInvocations} +func (verifier *VerifierMockProjectCommandOutputHandler) Deregister(_param0 string, _param1 chan string) *MockProjectCommandOutputHandler_Deregister_OngoingVerification { + params := []pegomock.Param{_param0, _param1} + methodInvocations := pegomock.GetGenericMockFrom(verifier.mock).Verify(verifier.inOrderContext, verifier.invocationCountMatcher, "Deregister", params, verifier.timeout) + return &MockProjectCommandOutputHandler_Deregister_OngoingVerification{mock: verifier.mock, methodInvocations: methodInvocations} } -type MockProjectCommandOutputHandler_Register_OngoingVerification struct { +type MockProjectCommandOutputHandler_Deregister_OngoingVerification struct { mock *MockProjectCommandOutputHandler methodInvocations []pegomock.MethodInvocation } -func (c *MockProjectCommandOutputHandler_Register_OngoingVerification) GetCapturedArguments() (string, chan string) { - jobID, receiver := c.GetAllCapturedArguments() - return jobID[len(jobID)-1], receiver[len(receiver)-1] +func (c *MockProjectCommandOutputHandler_Deregister_OngoingVerification) GetCapturedArguments() (string, chan string) { + _param0, _param1 := c.GetAllCapturedArguments() + return _param0[len(_param0)-1], _param1[len(_param1)-1] } -func (c *MockProjectCommandOutputHandler_Register_OngoingVerification) GetAllCapturedArguments() (_param0 []string, _param1 []chan string) { +func (c *MockProjectCommandOutputHandler_Deregister_OngoingVerification) GetAllCapturedArguments() (_param0 []string, _param1 []chan string) { params := pegomock.GetGenericMockFrom(c.mock).GetInvocationParams(c.methodInvocations) if len(params) > 0 { _param0 = make([]string, len(c.methodInvocations)) @@ -179,23 +176,40 @@ func (c *MockProjectCommandOutputHandler_Register_OngoingVerification) GetAllCap return } -func (verifier *VerifierMockProjectCommandOutputHandler) Deregister(jobID string, receiver chan string) *MockProjectCommandOutputHandler_Deregister_OngoingVerification { - params := []pegomock.Param{jobID, receiver} - methodInvocations := pegomock.GetGenericMockFrom(verifier.mock).Verify(verifier.inOrderContext, verifier.invocationCountMatcher, "Deregister", params, verifier.timeout) - return &MockProjectCommandOutputHandler_Deregister_OngoingVerification{mock: verifier.mock, methodInvocations: methodInvocations} +func (verifier *VerifierMockProjectCommandOutputHandler) Handle() *MockProjectCommandOutputHandler_Handle_OngoingVerification { + params := []pegomock.Param{} + methodInvocations := pegomock.GetGenericMockFrom(verifier.mock).Verify(verifier.inOrderContext, verifier.invocationCountMatcher, "Handle", params, verifier.timeout) + return &MockProjectCommandOutputHandler_Handle_OngoingVerification{mock: verifier.mock, methodInvocations: methodInvocations} } -type MockProjectCommandOutputHandler_Deregister_OngoingVerification struct { +type MockProjectCommandOutputHandler_Handle_OngoingVerification struct { mock *MockProjectCommandOutputHandler methodInvocations []pegomock.MethodInvocation } -func (c *MockProjectCommandOutputHandler_Deregister_OngoingVerification) GetCapturedArguments() (string, chan string) { - jobID, receiver := c.GetAllCapturedArguments() - return jobID[len(jobID)-1], receiver[len(receiver)-1] +func (c *MockProjectCommandOutputHandler_Handle_OngoingVerification) GetCapturedArguments() { } -func (c *MockProjectCommandOutputHandler_Deregister_OngoingVerification) GetAllCapturedArguments() (_param0 []string, _param1 []chan string) { +func (c *MockProjectCommandOutputHandler_Handle_OngoingVerification) GetAllCapturedArguments() { +} + +func (verifier *VerifierMockProjectCommandOutputHandler) Register(_param0 string, _param1 chan string) *MockProjectCommandOutputHandler_Register_OngoingVerification { + params := []pegomock.Param{_param0, _param1} + methodInvocations := pegomock.GetGenericMockFrom(verifier.mock).Verify(verifier.inOrderContext, verifier.invocationCountMatcher, "Register", params, verifier.timeout) + return &MockProjectCommandOutputHandler_Register_OngoingVerification{mock: verifier.mock, methodInvocations: methodInvocations} +} + +type MockProjectCommandOutputHandler_Register_OngoingVerification struct { + mock *MockProjectCommandOutputHandler + methodInvocations []pegomock.MethodInvocation +} + +func (c *MockProjectCommandOutputHandler_Register_OngoingVerification) GetCapturedArguments() (string, chan string) { + _param0, _param1 := c.GetAllCapturedArguments() + return _param0[len(_param0)-1], _param1[len(_param1)-1] +} + +func (c *MockProjectCommandOutputHandler_Register_OngoingVerification) GetAllCapturedArguments() (_param0 []string, _param1 []chan string) { params := pegomock.GetGenericMockFrom(c.mock).GetInvocationParams(c.methodInvocations) if len(params) > 0 { _param0 = make([]string, len(c.methodInvocations)) @@ -210,25 +224,39 @@ func (c *MockProjectCommandOutputHandler_Deregister_OngoingVerification) GetAllC return } -func (verifier *VerifierMockProjectCommandOutputHandler) Handle() *MockProjectCommandOutputHandler_Handle_OngoingVerification { - params := []pegomock.Param{} - methodInvocations := pegomock.GetGenericMockFrom(verifier.mock).Verify(verifier.inOrderContext, verifier.invocationCountMatcher, "Handle", params, verifier.timeout) - return &MockProjectCommandOutputHandler_Handle_OngoingVerification{mock: verifier.mock, methodInvocations: methodInvocations} +func (verifier *VerifierMockProjectCommandOutputHandler) Send(_param0 models.ProjectCommandContext, _param1 string) *MockProjectCommandOutputHandler_Send_OngoingVerification { + params := []pegomock.Param{_param0, _param1} + methodInvocations := pegomock.GetGenericMockFrom(verifier.mock).Verify(verifier.inOrderContext, verifier.invocationCountMatcher, "Send", params, verifier.timeout) + return &MockProjectCommandOutputHandler_Send_OngoingVerification{mock: verifier.mock, methodInvocations: methodInvocations} } -type MockProjectCommandOutputHandler_Handle_OngoingVerification struct { +type MockProjectCommandOutputHandler_Send_OngoingVerification struct { mock *MockProjectCommandOutputHandler methodInvocations []pegomock.MethodInvocation } -func (c *MockProjectCommandOutputHandler_Handle_OngoingVerification) GetCapturedArguments() { +func (c *MockProjectCommandOutputHandler_Send_OngoingVerification) GetCapturedArguments() (models.ProjectCommandContext, string) { + _param0, _param1 := c.GetAllCapturedArguments() + return _param0[len(_param0)-1], _param1[len(_param1)-1] } -func (c *MockProjectCommandOutputHandler_Handle_OngoingVerification) GetAllCapturedArguments() { +func (c *MockProjectCommandOutputHandler_Send_OngoingVerification) GetAllCapturedArguments() (_param0 []models.ProjectCommandContext, _param1 []string) { + params := pegomock.GetGenericMockFrom(c.mock).GetInvocationParams(c.methodInvocations) + if len(params) > 0 { + _param0 = make([]models.ProjectCommandContext, len(c.methodInvocations)) + for u, param := range params[0] { + _param0[u] = param.(models.ProjectCommandContext) + } + _param1 = make([]string, len(c.methodInvocations)) + for u, param := range params[1] { + _param1[u] = param.(string) + } + } + return } -func (verifier *VerifierMockProjectCommandOutputHandler) SetJobURLWithStatus(ctx models.ProjectCommandContext, cmdName models.CommandName, status models.CommitStatus) *MockProjectCommandOutputHandler_SetJobURLWithStatus_OngoingVerification { - params := []pegomock.Param{ctx, cmdName, status} +func (verifier *VerifierMockProjectCommandOutputHandler) SetJobURLWithStatus(_param0 models.ProjectCommandContext, _param1 models.CommandName, _param2 models.CommitStatus) *MockProjectCommandOutputHandler_SetJobURLWithStatus_OngoingVerification { + params := []pegomock.Param{_param0, _param1, _param2} methodInvocations := pegomock.GetGenericMockFrom(verifier.mock).Verify(verifier.inOrderContext, verifier.invocationCountMatcher, "SetJobURLWithStatus", params, verifier.timeout) return &MockProjectCommandOutputHandler_SetJobURLWithStatus_OngoingVerification{mock: verifier.mock, methodInvocations: methodInvocations} } @@ -239,8 +267,8 @@ type MockProjectCommandOutputHandler_SetJobURLWithStatus_OngoingVerification str } func (c *MockProjectCommandOutputHandler_SetJobURLWithStatus_OngoingVerification) GetCapturedArguments() (models.ProjectCommandContext, models.CommandName, models.CommitStatus) { - ctx, cmdName, status := c.GetAllCapturedArguments() - return ctx[len(ctx)-1], cmdName[len(cmdName)-1], status[len(status)-1] + _param0, _param1, _param2 := c.GetAllCapturedArguments() + return _param0[len(_param0)-1], _param1[len(_param1)-1], _param2[len(_param2)-1] } func (c *MockProjectCommandOutputHandler_SetJobURLWithStatus_OngoingVerification) GetAllCapturedArguments() (_param0 []models.ProjectCommandContext, _param1 []models.CommandName, _param2 []models.CommitStatus) { @@ -261,30 +289,3 @@ func (c *MockProjectCommandOutputHandler_SetJobURLWithStatus_OngoingVerification } return } - -func (verifier *VerifierMockProjectCommandOutputHandler) CleanUp(pullContext models.PullContext) *MockProjectCommandOutputHandler_CleanUp_OngoingVerification { - params := []pegomock.Param{pullContext} - methodInvocations := pegomock.GetGenericMockFrom(verifier.mock).Verify(verifier.inOrderContext, verifier.invocationCountMatcher, "CleanUp", params, verifier.timeout) - return &MockProjectCommandOutputHandler_CleanUp_OngoingVerification{mock: verifier.mock, methodInvocations: methodInvocations} -} - -type MockProjectCommandOutputHandler_CleanUp_OngoingVerification struct { - mock *MockProjectCommandOutputHandler - methodInvocations []pegomock.MethodInvocation -} - -func (c *MockProjectCommandOutputHandler_CleanUp_OngoingVerification) GetCapturedArguments() models.PullContext { - pullContext := c.GetAllCapturedArguments() - return pullContext[len(pullContext)-1] -} - -func (c *MockProjectCommandOutputHandler_CleanUp_OngoingVerification) GetAllCapturedArguments() (_param0 []models.PullContext) { - params := pegomock.GetGenericMockFrom(c.mock).GetInvocationParams(c.methodInvocations) - if len(params) > 0 { - _param0 = make([]models.PullContext, len(c.methodInvocations)) - for u, param := range params[0] { - _param0[u] = param.(models.PullContext) - } - } - return -} diff --git a/server/handlers/mocks/mock_resource_cleaner.go b/server/handlers/mocks/mock_resource_cleaner.go index 357faf36f..9ec4c0c70 100644 --- a/server/handlers/mocks/mock_resource_cleaner.go +++ b/server/handlers/mocks/mock_resource_cleaner.go @@ -5,6 +5,7 @@ package mocks import ( pegomock "github.com/petergtz/pegomock" + handlers "github.com/runatlantis/atlantis/server/handlers" "reflect" "time" ) @@ -24,11 +25,11 @@ func NewMockResourceCleaner(options ...pegomock.Option) *MockResourceCleaner { func (mock *MockResourceCleaner) SetFailHandler(fh pegomock.FailHandler) { mock.fail = fh } func (mock *MockResourceCleaner) FailHandler() pegomock.FailHandler { return mock.fail } -func (mock *MockResourceCleaner) CleanUp(pull string) { +func (mock *MockResourceCleaner) CleanUp(_param0 handlers.PullContext) { if mock == nil { panic("mock must not be nil. Use myMock := NewMockResourceCleaner().") } - params := []pegomock.Param{pull} + params := []pegomock.Param{_param0} pegomock.GetGenericMockFrom(mock).Invoke("CleanUp", params, []reflect.Type{}) } @@ -69,8 +70,8 @@ type VerifierMockResourceCleaner struct { timeout time.Duration } -func (verifier *VerifierMockResourceCleaner) CleanUp(pull string) *MockResourceCleaner_CleanUp_OngoingVerification { - params := []pegomock.Param{pull} +func (verifier *VerifierMockResourceCleaner) CleanUp(_param0 handlers.PullContext) *MockResourceCleaner_CleanUp_OngoingVerification { + params := []pegomock.Param{_param0} methodInvocations := pegomock.GetGenericMockFrom(verifier.mock).Verify(verifier.inOrderContext, verifier.invocationCountMatcher, "CleanUp", params, verifier.timeout) return &MockResourceCleaner_CleanUp_OngoingVerification{mock: verifier.mock, methodInvocations: methodInvocations} } @@ -80,17 +81,17 @@ type MockResourceCleaner_CleanUp_OngoingVerification struct { methodInvocations []pegomock.MethodInvocation } -func (c *MockResourceCleaner_CleanUp_OngoingVerification) GetCapturedArguments() string { - pull := c.GetAllCapturedArguments() - return pull[len(pull)-1] +func (c *MockResourceCleaner_CleanUp_OngoingVerification) GetCapturedArguments() handlers.PullContext { + _param0 := c.GetAllCapturedArguments() + return _param0[len(_param0)-1] } -func (c *MockResourceCleaner_CleanUp_OngoingVerification) GetAllCapturedArguments() (_param0 []string) { +func (c *MockResourceCleaner_CleanUp_OngoingVerification) GetAllCapturedArguments() (_param0 []handlers.PullContext) { params := pegomock.GetGenericMockFrom(c.mock).GetInvocationParams(c.methodInvocations) if len(params) > 0 { - _param0 = make([]string, len(c.methodInvocations)) + _param0 = make([]handlers.PullContext, len(c.methodInvocations)) for u, param := range params[0] { - _param0[u] = param.(string) + _param0[u] = param.(handlers.PullContext) } } return diff --git a/server/handlers/project_command_output_handler.go b/server/handlers/project_command_output_handler.go index c9f13843b..57dc62dd2 100644 --- a/server/handlers/project_command_output_handler.go +++ b/server/handlers/project_command_output_handler.go @@ -7,10 +7,30 @@ import ( "github.com/runatlantis/atlantis/server/logging" ) +type PullContext struct { + PullNum int + Repo string + ProjectName string + Workspace string +} + +type JobContext struct { + PullContext + HeadCommit string +} + +type ProjectCmdOutputLine struct { + JobID string + + JobContext JobContext + + Line string +} + // AsyncProjectCommandOutputHandler is a handler to transport terraform client // outputs to the front end. type AsyncProjectCommandOutputHandler struct { - projectCmdOutput chan *models.ProjectCmdOutputLine + projectCmdOutput chan *ProjectCmdOutputLine projectOutputBuffers map[string][]string projectOutputBuffersLock sync.RWMutex @@ -68,11 +88,11 @@ type ProjectCommandOutputHandler interface { //go:generate pegomock generate -m --use-experimental-model-gen --package mocks -o mocks/mock_resource_cleaner.go ResourceCleaner type ResourceCleaner interface { - CleanUp(pullContext models.PullContext) + CleanUp(pullContext PullContext) } func NewAsyncProjectCommandOutputHandler( - projectCmdOutput chan *models.ProjectCmdOutputLine, + projectCmdOutput chan *ProjectCmdOutputLine, projectStatusUpdater ProjectStatusUpdater, projectJobURLGenerator ProjectJobURLGenerator, logger logging.SimpleLogging, @@ -89,11 +109,11 @@ func NewAsyncProjectCommandOutputHandler( } func (p *AsyncProjectCommandOutputHandler) Send(ctx models.ProjectCommandContext, msg string) { - p.projectCmdOutput <- &models.ProjectCmdOutputLine{ + p.projectCmdOutput <- &ProjectCmdOutputLine{ JobID: ctx.JobID, - JobContext: models.JobContext{ + JobContext: JobContext{ HeadCommit: ctx.Pull.HeadCommit, - PullContext: models.PullContext{ + PullContext: PullContext{ PullNum: ctx.Pull.Num, Repo: ctx.BaseRepo.Name, ProjectName: ctx.ProjectName, @@ -192,14 +212,14 @@ func (p *AsyncProjectCommandOutputHandler) GetProjectOutputBuffer(jobID string) return p.projectOutputBuffers[jobID] } -func (p *AsyncProjectCommandOutputHandler) GetJobIdMapForPullContext(pullContext models.PullContext) map[string]bool { +func (p *AsyncProjectCommandOutputHandler) GetJobIdMapForPullContext(pullContext PullContext) map[string]bool { if value, ok := p.pullToJobMapping.Load(pullContext); ok { return value.(map[string]bool) } return nil } -func (p *AsyncProjectCommandOutputHandler) CleanUp(pullContext models.PullContext) { +func (p *AsyncProjectCommandOutputHandler) CleanUp(pullContext PullContext) { if value, ok := p.pullToJobMapping.Load(pullContext); ok { jobMapping := value.(map[string]bool) for jobID := range jobMapping { @@ -236,5 +256,5 @@ func (p *NoopProjectOutputHandler) SetJobURLWithStatus(ctx models.ProjectCommand return nil } -func (p *NoopProjectOutputHandler) CleanUp(pullContext models.PullContext) { +func (p *NoopProjectOutputHandler) CleanUp(pullContext PullContext) { } diff --git a/server/handlers/project_command_output_handler_test.go b/server/handlers/project_command_output_handler_test.go index 5cf6f318b..321ac65ff 100644 --- a/server/handlers/project_command_output_handler_test.go +++ b/server/handlers/project_command_output_handler_test.go @@ -46,7 +46,7 @@ func createTestProjectCmdContext(t *testing.T) models.ProjectCommandContext { func createProjectCommandOutputHandler(t *testing.T) handlers.ProjectCommandOutputHandler { logger := logging.NewNoopLogger(t) - prjCmdOutputChan := make(chan *models.ProjectCmdOutputLine) + prjCmdOutputChan := make(chan *handlers.ProjectCmdOutputLine) projectStatusUpdater := mocks.NewMockProjectStatusUpdater() projectJobURLGenerator := mocks.NewMockProjectJobURLGenerator() prjCmdOutputHandler := handlers.NewAsyncProjectCommandOutputHandler( @@ -141,7 +141,7 @@ func TestProjectCommandOutputHandler(t *testing.T) { t.Run("update project status with project jobs url", func(t *testing.T) { RegisterMockTestingT(t) logger := logging.NewNoopLogger(t) - prjCmdOutputChan := make(chan *models.ProjectCmdOutputLine) + prjCmdOutputChan := make(chan *handlers.ProjectCmdOutputLine) projectStatusUpdater := mocks.NewMockProjectStatusUpdater() projectJobURLGenerator := mocks.NewMockProjectJobURLGenerator() prjCmdOutputHandler := handlers.NewAsyncProjectCommandOutputHandler( @@ -161,7 +161,7 @@ func TestProjectCommandOutputHandler(t *testing.T) { t.Run("update project status with project jobs url error", func(t *testing.T) { RegisterMockTestingT(t) logger := logging.NewNoopLogger(t) - prjCmdOutputChan := make(chan *models.ProjectCmdOutputLine) + prjCmdOutputChan := make(chan *handlers.ProjectCmdOutputLine) projectStatusUpdater := mocks.NewMockProjectStatusUpdater() projectJobURLGenerator := mocks.NewMockProjectJobURLGenerator() prjCmdOutputHandler := handlers.NewAsyncProjectCommandOutputHandler( @@ -203,7 +203,7 @@ func TestProjectCommandOutputHandler(t *testing.T) { projectOutputHandler.Send(ctx, Msg) projectOutputHandler.Send(ctx, "Complete") - pullContext := models.PullContext{ + pullContext := handlers.PullContext{ PullNum: ctx.Pull.Num, Repo: ctx.BaseRepo.Name, ProjectName: ctx.ProjectName, diff --git a/server/server.go b/server/server.go index 71e7fc82b..75f7e4163 100644 --- a/server/server.go +++ b/server/server.go @@ -374,7 +374,7 @@ func NewServer(userConfig UserConfig, config Config) (*Server, error) { if userConfig.TFEToken != "" { projectCmdOutputHandler = &handlers.NoopProjectOutputHandler{} } else { - projectCmdOutput := make(chan *models.ProjectCmdOutputLine) + projectCmdOutput := make(chan *handlers.ProjectCmdOutputLine) projectCmdOutputHandler = handlers.NewAsyncProjectCommandOutputHandler( projectCmdOutput, commitStatusUpdater, From 867be62e69323f9f699a3d45deb4d5f3776ab0f1 Mon Sep 17 00:00:00 2001 From: Aayush Gupta Date: Tue, 1 Feb 2022 11:32:48 -0800 Subject: [PATCH 15/19] Fix tests --- server/handlers/project_command_output_handler_test.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/server/handlers/project_command_output_handler_test.go b/server/handlers/project_command_output_handler_test.go index 5cf6f318b..f526b4236 100644 --- a/server/handlers/project_command_output_handler_test.go +++ b/server/handlers/project_command_output_handler_test.go @@ -90,7 +90,7 @@ func TestProjectCommandOutputHandler(t *testing.T) { } }() - projectOutputHandler.Send(ctx, Msg) + projectOutputHandler.Send(ctx, Msg, false) wg.Wait() close(ch) @@ -103,7 +103,7 @@ func TestProjectCommandOutputHandler(t *testing.T) { projectOutputHandler := createProjectCommandOutputHandler(t) // send first message to populated the buffer - projectOutputHandler.Send(ctx, Msg) + projectOutputHandler.Send(ctx, Msg, false) ch := make(chan string) @@ -127,7 +127,7 @@ func TestProjectCommandOutputHandler(t *testing.T) { // before sending messages due to the way we lock our buffer memory cache projectOutputHandler.Register(ctx.JobID, ch) - projectOutputHandler.Send(ctx, Msg) + projectOutputHandler.Send(ctx, Msg, false) wg.Wait() close(ch) @@ -200,8 +200,8 @@ func TestProjectCommandOutputHandler(t *testing.T) { } }() - projectOutputHandler.Send(ctx, Msg) - projectOutputHandler.Send(ctx, "Complete") + projectOutputHandler.Send(ctx, Msg, false) + projectOutputHandler.Send(ctx, "Complete", false) pullContext := models.PullContext{ PullNum: ctx.Pull.Num, From d51c6fc356338b3a4ee504df199b277860fcd13d Mon Sep 17 00:00:00 2001 From: Aayush Gupta Date: Tue, 1 Feb 2022 12:15:53 -0800 Subject: [PATCH 16/19] Added tests --- server/events/pull_closed_executor_test.go | 2 +- .../mock_project_command_output_handler.go | 20 +++-- .../project_command_output_handler.go | 20 ++--- .../project_command_output_handler_test.go | 74 ++++++++++++++++++- 4 files changed, 96 insertions(+), 20 deletions(-) diff --git a/server/events/pull_closed_executor_test.go b/server/events/pull_closed_executor_test.go index a8bebe1a0..e3b1c200b 100644 --- a/server/events/pull_closed_executor_test.go +++ b/server/events/pull_closed_executor_test.go @@ -210,7 +210,7 @@ func TestCleanUpLogStreaming(t *testing.T) { } go prjCmdOutHandler.Handle() - prjCmdOutHandler.Send(ctx, "Test Message") + prjCmdOutHandler.Send(ctx, "Test Message", false) // Create boltdb and add pull request. var lockBucket = "bucket" diff --git a/server/handlers/mocks/mock_project_command_output_handler.go b/server/handlers/mocks/mock_project_command_output_handler.go index b3ec5ee89..fa8fb149b 100644 --- a/server/handlers/mocks/mock_project_command_output_handler.go +++ b/server/handlers/mocks/mock_project_command_output_handler.go @@ -58,11 +58,11 @@ func (mock *MockProjectCommandOutputHandler) Register(_param0 string, _param1 ch pegomock.GetGenericMockFrom(mock).Invoke("Register", params, []reflect.Type{}) } -func (mock *MockProjectCommandOutputHandler) Send(_param0 models.ProjectCommandContext, _param1 string) { +func (mock *MockProjectCommandOutputHandler) Send(_param0 models.ProjectCommandContext, _param1 string, _param2 bool) { if mock == nil { panic("mock must not be nil. Use myMock := NewMockProjectCommandOutputHandler().") } - params := []pegomock.Param{_param0, _param1} + params := []pegomock.Param{_param0, _param1, _param2} pegomock.GetGenericMockFrom(mock).Invoke("Send", params, []reflect.Type{}) } @@ -224,8 +224,8 @@ func (c *MockProjectCommandOutputHandler_Register_OngoingVerification) GetAllCap return } -func (verifier *VerifierMockProjectCommandOutputHandler) Send(_param0 models.ProjectCommandContext, _param1 string) *MockProjectCommandOutputHandler_Send_OngoingVerification { - params := []pegomock.Param{_param0, _param1} +func (verifier *VerifierMockProjectCommandOutputHandler) Send(_param0 models.ProjectCommandContext, _param1 string, _param2 bool) *MockProjectCommandOutputHandler_Send_OngoingVerification { + params := []pegomock.Param{_param0, _param1, _param2} methodInvocations := pegomock.GetGenericMockFrom(verifier.mock).Verify(verifier.inOrderContext, verifier.invocationCountMatcher, "Send", params, verifier.timeout) return &MockProjectCommandOutputHandler_Send_OngoingVerification{mock: verifier.mock, methodInvocations: methodInvocations} } @@ -235,12 +235,12 @@ type MockProjectCommandOutputHandler_Send_OngoingVerification struct { methodInvocations []pegomock.MethodInvocation } -func (c *MockProjectCommandOutputHandler_Send_OngoingVerification) GetCapturedArguments() (models.ProjectCommandContext, string) { - _param0, _param1 := c.GetAllCapturedArguments() - return _param0[len(_param0)-1], _param1[len(_param1)-1] +func (c *MockProjectCommandOutputHandler_Send_OngoingVerification) GetCapturedArguments() (models.ProjectCommandContext, string, bool) { + _param0, _param1, _param2 := c.GetAllCapturedArguments() + return _param0[len(_param0)-1], _param1[len(_param1)-1], _param2[len(_param2)-1] } -func (c *MockProjectCommandOutputHandler_Send_OngoingVerification) GetAllCapturedArguments() (_param0 []models.ProjectCommandContext, _param1 []string) { +func (c *MockProjectCommandOutputHandler_Send_OngoingVerification) GetAllCapturedArguments() (_param0 []models.ProjectCommandContext, _param1 []string, _param2 []bool) { params := pegomock.GetGenericMockFrom(c.mock).GetInvocationParams(c.methodInvocations) if len(params) > 0 { _param0 = make([]models.ProjectCommandContext, len(c.methodInvocations)) @@ -251,6 +251,10 @@ func (c *MockProjectCommandOutputHandler_Send_OngoingVerification) GetAllCapture for u, param := range params[1] { _param1[u] = param.(string) } + _param2 = make([]bool, len(c.methodInvocations)) + for u, param := range params[2] { + _param2[u] = param.(bool) + } } return } diff --git a/server/handlers/project_command_output_handler.go b/server/handlers/project_command_output_handler.go index e905fc579..663d96678 100644 --- a/server/handlers/project_command_output_handler.go +++ b/server/handlers/project_command_output_handler.go @@ -8,8 +8,8 @@ import ( ) type OutputBuffer struct { - isOperationComplete bool - buffer []string + IsOperationComplete bool + Buffer []string } type PullContext struct { @@ -140,7 +140,7 @@ func (p *AsyncProjectCommandOutputHandler) Handle() { if msg.OperationComplete { p.projectOutputBuffersLock.Lock() if outputBuffer, ok := p.projectOutputBuffers[msg.JobID]; ok { - outputBuffer.isOperationComplete = true + outputBuffer.IsOperationComplete = true p.projectOutputBuffers[msg.JobID] = outputBuffer } p.projectOutputBuffersLock.Unlock() @@ -187,12 +187,12 @@ func (p *AsyncProjectCommandOutputHandler) addChan(ch chan string, jobID string) outputBuffer := p.projectOutputBuffers[jobID] p.projectOutputBuffersLock.RUnlock() - for _, line := range outputBuffer.buffer { + for _, line := range outputBuffer.Buffer { ch <- line } // No need register receiver since all the logs have been streamed - if outputBuffer.isOperationComplete { + if outputBuffer.IsOperationComplete { close(ch) return } @@ -228,12 +228,12 @@ func (p *AsyncProjectCommandOutputHandler) writeLogLine(jobID string, line strin p.projectOutputBuffersLock.Lock() if _, ok := p.projectOutputBuffers[jobID]; !ok { p.projectOutputBuffers[jobID] = OutputBuffer{ - isOperationComplete: false, - buffer: []string{}, + IsOperationComplete: false, + Buffer: []string{}, } } outputBuffer := p.projectOutputBuffers[jobID] - outputBuffer.buffer = append(outputBuffer.buffer, line) + outputBuffer.Buffer = append(outputBuffer.Buffer, line) p.projectOutputBuffers[jobID] = outputBuffer p.projectOutputBuffersLock.Unlock() @@ -251,8 +251,8 @@ func (p *AsyncProjectCommandOutputHandler) GetReceiverBufferForPull(jobID string return p.receiverBuffers[jobID] } -func (p *AsyncProjectCommandOutputHandler) GetProjectOutputBuffer(jobID string) []string { - return p.projectOutputBuffers[jobID].buffer +func (p *AsyncProjectCommandOutputHandler) GetProjectOutputBuffer(jobID string) OutputBuffer { + return p.projectOutputBuffers[jobID] } func (p *AsyncProjectCommandOutputHandler) GetJobIdMapForPullContext(pullContext PullContext) map[string]bool { diff --git a/server/handlers/project_command_output_handler_test.go b/server/handlers/project_command_output_handler_test.go index 709174de9..4440ebf28 100644 --- a/server/handlers/project_command_output_handler_test.go +++ b/server/handlers/project_command_output_handler_test.go @@ -4,6 +4,7 @@ import ( "errors" "sync" "testing" + "time" . "github.com/petergtz/pegomock" "github.com/runatlantis/atlantis/server/events/models" @@ -176,7 +177,6 @@ func TestProjectCommandOutputHandler(t *testing.T) { assert.Error(t, err) }) - // Close all jobs for a PR when clean up t.Run("clean up all jobs when PR is closed", func(t *testing.T) { var wg sync.WaitGroup projectOutputHandler := createProjectCommandOutputHandler(t) @@ -219,4 +219,76 @@ func TestProjectCommandOutputHandler(t *testing.T) { assert.Empty(t, dfProjectOutputHandler.GetReceiverBufferForPull(ctx.JobID)) assert.Empty(t, dfProjectOutputHandler.GetJobIdMapForPullContext(pullContext)) }) + + t.Run("mark operation status complete and close conn buffers for the job", func(t *testing.T) { + projectOutputHandler := createProjectCommandOutputHandler(t) + + ch := make(chan string) + + // register channel and backfill from buffer + // Note: We call this synchronously because otherwise + // there could be a race where we are unable to register the channel + // before sending messages due to the way we lock our buffer memory cache + projectOutputHandler.Register(ctx.JobID, ch) + + // read from channel + go func() { + for _ = range ch { + } + }() + + projectOutputHandler.Send(ctx, Msg, false) + projectOutputHandler.Send(ctx, "", true) + + // Wait for the handler to process the message + time.Sleep(10 * time.Millisecond) + + dfProjectOutputHandler, ok := projectOutputHandler.(*handlers.AsyncProjectCommandOutputHandler) + assert.True(t, ok) + + outputBuffer := dfProjectOutputHandler.GetProjectOutputBuffer(ctx.JobID) + assert.True(t, outputBuffer.IsOperationComplete) + + _, ok = (<-ch) + assert.False(t, ok) + + }) + + t.Run("close conn buffer after streaming logs for completed operation", func(t *testing.T) { + projectOutputHandler := createProjectCommandOutputHandler(t) + + ch := make(chan string) + + // register channel and backfill from buffer + // Note: We call this synchronously because otherwise + // there could be a race where we are unable to register the channel + // before sending messages due to the way we lock our buffer memory cache + projectOutputHandler.Register(ctx.JobID, ch) + + // read from channel + go func() { + for _ = range ch { + } + }() + + projectOutputHandler.Send(ctx, Msg, false) + projectOutputHandler.Send(ctx, "", true) + + // Wait for the handler to process the message + time.Sleep(10 * time.Millisecond) + + ch_2 := make(chan string) + opComplete := make(chan bool) + + // buffer channel will be closed immediately after logs are streamed + go func() { + for _ = range ch_2 { + } + opComplete <- true + }() + + projectOutputHandler.Register(ctx.JobID, ch_2) + + assert.True(t, <-opComplete) + }) } From de7aef193bc90162c8528b4684bf9bf13a06784b Mon Sep 17 00:00:00 2001 From: Aayush Gupta Date: Tue, 1 Feb 2022 15:09:29 -0800 Subject: [PATCH 17/19] Refactoring --- server/controllers/websocket/writer.go | 4 ++ server/events/project_command_runner.go | 13 +++--- .../project_command_output_handler.go | 40 +++++++++++-------- .../project_command_output_handler_test.go | 2 +- 4 files changed, 36 insertions(+), 23 deletions(-) diff --git a/server/controllers/websocket/writer.go b/server/controllers/websocket/writer.go index 323e081a8..9058f69dd 100644 --- a/server/controllers/websocket/writer.go +++ b/server/controllers/websocket/writer.go @@ -39,5 +39,9 @@ func (w *Writer) Write(rw http.ResponseWriter, r *http.Request, input chan strin } } + // close ws conn after input channel is closed + if err = conn.Close(); err != nil { + w.log.Warn("Failed to close ws connection: %s", err) + } return nil } diff --git a/server/events/project_command_runner.go b/server/events/project_command_runner.go index f20a09a91..72642de96 100644 --- a/server/events/project_command_runner.go +++ b/server/events/project_command_runner.go @@ -127,20 +127,21 @@ func (p *ProjectOutputWrapper) Plan(ctx models.ProjectCommandContext) models.Pro // Reset the buffer when running the plan. We only need to do this for plan, // apply is a continuation of the same workflow result := p.updateProjectPRStatus(models.PlanCommand, ctx, p.ProjectCommandRunner.Plan) - - if result.Error == nil && result.Failure == "" { - p.ProjectCmdOutputHandler.Send(ctx, "", true) - } + p.updateOperationStatus(ctx, result) return result } func (p *ProjectOutputWrapper) Apply(ctx models.ProjectCommandContext) models.ProjectResult { result := p.updateProjectPRStatus(models.ApplyCommand, ctx, p.ProjectCommandRunner.Apply) + p.updateOperationStatus(ctx, result) + return result +} - if result.Error == nil && result.Failure == "" { +func (p *ProjectOutputWrapper) updateOperationStatus(ctx models.ProjectCommandContext, result models.ProjectResult) { + // No need to mark operation complete if failed operation. + if result.Failure == "" { p.ProjectCmdOutputHandler.Send(ctx, "", true) } - return result } func (p *ProjectOutputWrapper) updateProjectPRStatus(commandName models.CommandName, ctx models.ProjectCommandContext, execute func(ctx models.ProjectCommandContext) models.ProjectResult) models.ProjectResult { diff --git a/server/handlers/project_command_output_handler.go b/server/handlers/project_command_output_handler.go index 663d96678..1b03361cb 100644 --- a/server/handlers/project_command_output_handler.go +++ b/server/handlers/project_command_output_handler.go @@ -8,8 +8,8 @@ import ( ) type OutputBuffer struct { - IsOperationComplete bool - Buffer []string + OperationComplete bool + Buffer []string } type PullContext struct { @@ -72,7 +72,7 @@ type ProjectStatusUpdater interface { type ProjectCommandOutputHandler interface { // Send will enqueue the msg and wait for Handle() to receive the message. - Send(ctx models.ProjectCommandContext, msg string, isOperationComplete bool) + Send(ctx models.ProjectCommandContext, msg string, operationComplete bool) // Register registers a channel and blocks until it is caught up. Callers should call this asynchronously when attempting // to read the channel in the same goroutine @@ -114,7 +114,7 @@ func NewAsyncProjectCommandOutputHandler( } } -func (p *AsyncProjectCommandOutputHandler) Send(ctx models.ProjectCommandContext, msg string, isOperationComplete bool) { +func (p *AsyncProjectCommandOutputHandler) Send(ctx models.ProjectCommandContext, msg string, operationComplete bool) { p.projectCmdOutput <- &ProjectCmdOutputLine{ JobID: ctx.JobID, JobContext: JobContext{ @@ -127,7 +127,7 @@ func (p *AsyncProjectCommandOutputHandler) Send(ctx models.ProjectCommandContext }, }, Line: msg, - OperationComplete: isOperationComplete, + OperationComplete: operationComplete, } } @@ -138,17 +138,11 @@ func (p *AsyncProjectCommandOutputHandler) Register(jobID string, receiver chan func (p *AsyncProjectCommandOutputHandler) Handle() { for msg := range p.projectCmdOutput { if msg.OperationComplete { - p.projectOutputBuffersLock.Lock() - if outputBuffer, ok := p.projectOutputBuffers[msg.JobID]; ok { - outputBuffer.IsOperationComplete = true - p.projectOutputBuffers[msg.JobID] = outputBuffer - } - p.projectOutputBuffersLock.Unlock() - - // Close all channels for this job + p.updateOutputBufferOperationStatus(msg.JobID) p.closeChannelsForJob(msg.JobID) continue } + // Add job to pullToJob mapping if _, ok := p.pullToJobMapping.Load(msg.JobContext.PullContext); !ok { p.pullToJobMapping.Store(msg.JobContext.PullContext, map[string]bool{}) @@ -161,6 +155,20 @@ func (p *AsyncProjectCommandOutputHandler) Handle() { } } +// Sets the OperationComplete to true for the job +func (p *AsyncProjectCommandOutputHandler) updateOutputBufferOperationStatus(jobID string) { + p.projectOutputBuffersLock.Lock() + defer func() { + p.projectOutputBuffersLock.Unlock() + }() + + if outputBuffer, ok := p.projectOutputBuffers[jobID]; ok { + outputBuffer.OperationComplete = true + p.projectOutputBuffers[jobID] = outputBuffer + } +} + +// Closes all the buffered channels for the job func (p *AsyncProjectCommandOutputHandler) closeChannelsForJob(jobID string) { p.receiverBuffersLock.Lock() defer func() { @@ -192,7 +200,7 @@ func (p *AsyncProjectCommandOutputHandler) addChan(ch chan string, jobID string) } // No need register receiver since all the logs have been streamed - if outputBuffer.IsOperationComplete { + if outputBuffer.OperationComplete { close(ch) return } @@ -228,8 +236,8 @@ func (p *AsyncProjectCommandOutputHandler) writeLogLine(jobID string, line strin p.projectOutputBuffersLock.Lock() if _, ok := p.projectOutputBuffers[jobID]; !ok { p.projectOutputBuffers[jobID] = OutputBuffer{ - IsOperationComplete: false, - Buffer: []string{}, + OperationComplete: false, + Buffer: []string{}, } } outputBuffer := p.projectOutputBuffers[jobID] diff --git a/server/handlers/project_command_output_handler_test.go b/server/handlers/project_command_output_handler_test.go index 4440ebf28..a4d734b71 100644 --- a/server/handlers/project_command_output_handler_test.go +++ b/server/handlers/project_command_output_handler_test.go @@ -247,7 +247,7 @@ func TestProjectCommandOutputHandler(t *testing.T) { assert.True(t, ok) outputBuffer := dfProjectOutputHandler.GetProjectOutputBuffer(ctx.JobID) - assert.True(t, outputBuffer.IsOperationComplete) + assert.True(t, outputBuffer.OperationComplete) _, ok = (<-ch) assert.False(t, ok) From a56979fe4021287d62eb3046f9d5e1b9b838d5c9 Mon Sep 17 00:00:00 2001 From: Aayush Gupta Date: Wed, 2 Feb 2022 13:49:22 -0800 Subject: [PATCH 18/19] Addressing comments --- .../project_command_output_handler.go | 22 ++++++++----------- 1 file changed, 9 insertions(+), 13 deletions(-) diff --git a/server/handlers/project_command_output_handler.go b/server/handlers/project_command_output_handler.go index 1b03361cb..44a8e9ca7 100644 --- a/server/handlers/project_command_output_handler.go +++ b/server/handlers/project_command_output_handler.go @@ -138,8 +138,7 @@ func (p *AsyncProjectCommandOutputHandler) Register(jobID string, receiver chan func (p *AsyncProjectCommandOutputHandler) Handle() { for msg := range p.projectCmdOutput { if msg.OperationComplete { - p.updateOutputBufferOperationStatus(msg.JobID) - p.closeChannelsForJob(msg.JobID) + p.completeJob(msg.JobID) continue } @@ -151,34 +150,32 @@ func (p *AsyncProjectCommandOutputHandler) Handle() { jobMapping := value.(map[string]bool) jobMapping[msg.JobID] = true + // Forward new message to all receiver channels and output buffer p.writeLogLine(msg.JobID, msg.Line) } } -// Sets the OperationComplete to true for the job -func (p *AsyncProjectCommandOutputHandler) updateOutputBufferOperationStatus(jobID string) { +func (p *AsyncProjectCommandOutputHandler) completeJob(jobID string) { p.projectOutputBuffersLock.Lock() + p.receiverBuffersLock.Lock() defer func() { p.projectOutputBuffersLock.Unlock() + p.receiverBuffersLock.Unlock() }() + // Update operation status to complete if outputBuffer, ok := p.projectOutputBuffers[jobID]; ok { outputBuffer.OperationComplete = true p.projectOutputBuffers[jobID] = outputBuffer } -} -// Closes all the buffered channels for the job -func (p *AsyncProjectCommandOutputHandler) closeChannelsForJob(jobID string) { - p.receiverBuffersLock.Lock() - defer func() { - p.receiverBuffersLock.Unlock() - }() + // Close active receiver channels if openChannels, ok := p.receiverBuffers[jobID]; ok { for ch := range openChannels { close(ch) } } + } func (p *AsyncProjectCommandOutputHandler) SetJobURLWithStatus(ctx models.ProjectCommandContext, cmdName models.CommandName, status models.CommitStatus) error { @@ -236,8 +233,7 @@ func (p *AsyncProjectCommandOutputHandler) writeLogLine(jobID string, line strin p.projectOutputBuffersLock.Lock() if _, ok := p.projectOutputBuffers[jobID]; !ok { p.projectOutputBuffers[jobID] = OutputBuffer{ - OperationComplete: false, - Buffer: []string{}, + Buffer: []string{}, } } outputBuffer := p.projectOutputBuffers[jobID] From e86a27a38a58350d8e2fefa34360ecfb4812da7d Mon Sep 17 00:00:00 2001 From: Aayush Gupta Date: Wed, 2 Feb 2022 14:39:53 -0800 Subject: [PATCH 19/19] Removing unnecessary check for failed operation in project output wrapper --- server/events/project_command_runner.go | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/server/events/project_command_runner.go b/server/events/project_command_runner.go index 72642de96..33e0c445a 100644 --- a/server/events/project_command_runner.go +++ b/server/events/project_command_runner.go @@ -29,6 +29,8 @@ import ( "github.com/runatlantis/atlantis/server/lyft/feature" ) +const OperationComplete = true + // DirNotExistErr is an error caused by the directory not existing. type DirNotExistErr struct { RepoRelDir string @@ -124,26 +126,17 @@ type ProjectOutputWrapper struct { } func (p *ProjectOutputWrapper) Plan(ctx models.ProjectCommandContext) models.ProjectResult { - // Reset the buffer when running the plan. We only need to do this for plan, - // apply is a continuation of the same workflow result := p.updateProjectPRStatus(models.PlanCommand, ctx, p.ProjectCommandRunner.Plan) - p.updateOperationStatus(ctx, result) + p.ProjectCmdOutputHandler.Send(ctx, "", OperationComplete) return result } func (p *ProjectOutputWrapper) Apply(ctx models.ProjectCommandContext) models.ProjectResult { result := p.updateProjectPRStatus(models.ApplyCommand, ctx, p.ProjectCommandRunner.Apply) - p.updateOperationStatus(ctx, result) + p.ProjectCmdOutputHandler.Send(ctx, "", OperationComplete) return result } -func (p *ProjectOutputWrapper) updateOperationStatus(ctx models.ProjectCommandContext, result models.ProjectResult) { - // No need to mark operation complete if failed operation. - if result.Failure == "" { - p.ProjectCmdOutputHandler.Send(ctx, "", true) - } -} - func (p *ProjectOutputWrapper) updateProjectPRStatus(commandName models.CommandName, ctx models.ProjectCommandContext, execute func(ctx models.ProjectCommandContext) models.ProjectResult) models.ProjectResult { // Create a PR status to track project's plan status. The status will // include a link to view the progress of atlantis plan command in real