Revert "[Github Checks] Use boltdb to store CheckRun Status and elimi…

…nate fetching checkruns for every status update (#290)" (#295)

This reverts commit 426597b.

server/core/db/boltdb.go

@@ -22,16 +22,13 @@ type BoltDB struct {
 	locksBucketName       []byte
 	pullsBucketName       []byte
 	globalLocksBucketName []byte
-	checkRunsBucketName   []byte
 }
 
 const (
 	locksBucketName       = "runLocks"
 	pullsBucketName       = "pulls"
 	globalLocksBucketName = "globalLocks"
-	checkrunsBucketName   = "checkRuns"
 	pullKeySeparator      = "::"
-	checkRunKeySeparator  = "||"
 )
 
 // New returns a valid locker. We need to be able to write to dataDir
@@ -59,9 +56,6 @@ func New(dataDir string) (*BoltDB, error) {
 		if _, err = tx.CreateBucketIfNotExists([]byte(globalLocksBucketName)); err != nil {
 			return errors.Wrapf(err, "creating bucket %q", globalLocksBucketName)
 		}
-		if _, err = tx.CreateBucketIfNotExists([]byte(checkrunsBucketName)); err != nil {
-			return errors.Wrapf(err, "creating bucket %q", checkrunsBucketName)
-		}
 		return nil
 	})
 	if err != nil {
@@ -73,7 +67,6 @@ func New(dataDir string) (*BoltDB, error) {
 		locksBucketName:       []byte(locksBucketName),
 		pullsBucketName:       []byte(pullsBucketName),
 		globalLocksBucketName: []byte(globalLocksBucketName),
-		checkRunsBucketName:   []byte(checkrunsBucketName),
 	}, nil
 }
 
@@ -318,30 +311,6 @@ func (b *BoltDB) GetLock(p models.Project, workspace string) (*models.ProjectLoc
 	return &lock, nil
 }
 
-// Sets the checkRunID for a command
-func (b *BoltDB) UpdateCheckRunForStatus(statusName string, repo models.Repo, ref string, checkRunStatus models.CheckRunStatus) error {
-	key := b.checkRunKey(statusName, repo, ref)
-	return b.db.Update(func(tx *bolt.Tx) error {
-		bucket := tx.Bucket(b.checkRunsBucketName)
-		return b.writeCheckRunToBucket(bucket, []byte(key), checkRunStatus)
-	})
-}
-
-// Returns nil if the checkrun dne in the db
-func (b *BoltDB) GetCheckRunForStatus(statusName string, repo models.Repo, ref string) (*models.CheckRunStatus, error) {
-	key := b.checkRunKey(statusName, repo, ref)
-
-	var checkRun *models.CheckRunStatus
-	err := b.db.View(func(tx *bolt.Tx) error {
-		bucket := tx.Bucket(b.checkRunsBucketName)
-		var txErr error
-		checkRun, txErr = b.getCheckRunFromBucket(bucket, []byte(key))
-		return txErr
-	})
-
-	return checkRun, err
-}
-
 // UpdatePullWithResults updates pull's status with the latest project results.
 // It returns the new PullStatus object.
 func (b *BoltDB) UpdatePullWithResults(pull models.PullRequest, newResults []command.ProjectResult) (models.PullStatus, error) {
@@ -493,10 +462,6 @@ func (b *BoltDB) lockKey(p models.Project, workspace string) string {
 	return fmt.Sprintf("%s/%s/%s", p.RepoFullName, p.Path, workspace)
 }
 
-func (b *BoltDB) checkRunKey(statusName string, repo models.Repo, ref string) string {
-	return fmt.Sprintf("%s||%s||%s", repo.FullName, ref, statusName)
-}
-
 func (b *BoltDB) getPullFromBucket(bucket *bolt.Bucket, key []byte) (*models.PullStatus, error) {
 	serialized := bucket.Get(key)
 	if serialized == nil {
@@ -510,19 +475,6 @@ func (b *BoltDB) getPullFromBucket(bucket *bolt.Bucket, key []byte) (*models.Pul
 	return &p, nil
 }
 
-func (b *BoltDB) getCheckRunFromBucket(bucket *bolt.Bucket, key []byte) (*models.CheckRunStatus, error) {
-	serialized := bucket.Get(key)
-	if serialized == nil {
-		return nil, nil
-	}
-
-	var p models.CheckRunStatus
-	if err := json.Unmarshal(serialized, &p); err != nil {
-		return nil, errors.Wrapf(err, "deserializing checkrun at %q with contents %q", key, serialized)
-	}
-	return &p, nil
-}
-
 func (b *BoltDB) writePullToBucket(bucket *bolt.Bucket, key []byte, pull models.PullStatus) error {
 	serialized, err := json.Marshal(pull)
 	if err != nil {
@@ -531,15 +483,6 @@ func (b *BoltDB) writePullToBucket(bucket *bolt.Bucket, key []byte, pull models.
 	return bucket.Put(key, serialized)
 }
 
-func (b *BoltDB) writeCheckRunToBucket(bucket *bolt.Bucket, key []byte, checkRun models.CheckRunStatus) error {
-	serialized, err := json.Marshal(checkRun)
-	if err != nil {
-		return errors.Wrap(err, "serializing")
-	}
-
-	return bucket.Put(key, serialized)
-}
-
 func (b *BoltDB) projectResultToProject(p command.ProjectResult) models.ProjectStatus {
 	return models.ProjectStatus{
 		Workspace:   p.Workspace,

server/core/terraform/terraform_client_internal_test.go

@@ -64,7 +64,7 @@ func TestVersionLoader_buildsURL(t *testing.T) {
 	v, _ := version.NewVersion("0.15.0")
 
 	destPath := "some/path"
-	fullURL := fmt.Sprintf("https://releases.hashicorp.com/terraform/0.15.0/terraform_0.15.0_%s_%s.zip?checksum=file:https://releases.hashicorp.com/terraform/0.15.0/terraform_0.15.0_SHA256SUMS", runtime.GOOS, runtime.GOARCH)
+	fullURL := fmt.Sprintf("https://releases.hashicorp.com/terraform/0.15.0/terraform_0.15.0_%s_amd64.zip?checksum=file:https://releases.hashicorp.com/terraform/0.15.0/terraform_0.15.0_SHA256SUMS", runtime.GOOS)
 
 	RegisterMockTestingT(t)
 

server/events/apply_command_runner.go

@@ -80,7 +80,6 @@ func (a *ApplyCommandRunner) Run(ctx *command.Context, cmd *command.Comment) {
 		return
 	}
 
-	// Pending status creates a new checkrun
 	if err = a.commitStatusUpdater.UpdateCombined(context.TODO(), baseRepo, pull, models.PendingCommitStatus, cmd.CommandName()); err != nil {
 		ctx.Log.WarnContext(ctx.RequestCtx, fmt.Sprintf("unable to update commit status: %s", err))
 	}

server/events/approve_policies_command_runner.go

@@ -36,8 +36,7 @@ func (a *ApprovePoliciesCommandRunner) Run(ctx *command.Context, cmd *command.Co
 	baseRepo := ctx.Pull.BaseRepo
 	pull := ctx.Pull
 
-	// Set ApprovePolicies to Pending
-	if err := a.commitStatusUpdater.UpdateCombined(context.TODO(), baseRepo, pull, models.PendingCommitStatus, command.ApprovePolicies); err != nil {
+	if err := a.commitStatusUpdater.UpdateCombined(context.TODO(), baseRepo, pull, models.PendingCommitStatus, command.PolicyCheck); err != nil {
 		ctx.Log.WarnContext(ctx.RequestCtx, fmt.Sprintf("unable to update commit status: %s", err))
 	}
 

server/events/command_runner.go

@@ -140,9 +140,6 @@ func (c *DefaultCommandRunner) RunAutoplanCommand(ctx context.Context, baseRepo
 
 	if err := c.PreWorkflowHooksCommandRunner.RunPreHooks(ctx, cmdCtx); err != nil {
 		c.Logger.ErrorContext(ctx, "Error running pre-workflow hooks", fields.PullRequestWithErr(pull, err))
-		// Set to pending first to create a checkrun and populate the db
-		c.CommitStatusUpdater.UpdateCombined(ctx, cmdCtx.HeadRepo, cmdCtx.Pull, models.PendingCommitStatus, command.Plan)
-
 		c.CommitStatusUpdater.UpdateCombined(ctx, cmdCtx.HeadRepo, cmdCtx.Pull, models.FailedCommitStatus, command.Plan)
 		return
 	}

server/events/command_runner_test.go

@@ -381,7 +381,7 @@ func TestRunAutoplanCommand_PreWorkflowHookError(t *testing.T) {
 	ch.PreWorkflowHooksCommandRunner = preWorkflowHooksCommandRunner
 
 	ch.RunAutoplanCommand(ctx, fixtures.GithubRepo, fixtures.GithubRepo, fixtures.Pull, fixtures.User, time.Now())
-	_, _, _, status, cmdName := commitUpdater.VerifyWasCalled(&EqMatcher{Value: 2}).UpdateCombined(matchers.AnyContextContext(), matchers.AnyModelsRepo(), matchers.AnyModelsPullRequest(), matchers.AnyModelsCommitStatus(), matchers.AnyCommandName()).GetCapturedArguments()
+	_, _, _, status, cmdName := commitUpdater.VerifyWasCalledOnce().UpdateCombined(matchers.AnyContextContext(), matchers.AnyModelsRepo(), matchers.AnyModelsPullRequest(), matchers.AnyModelsCommitStatus(), matchers.AnyCommandName()).GetCapturedArguments()
 	Equals(t, models.FailedCommitStatus, status)
 	Equals(t, command.Plan, cmdName)
 }

server/events/models/models.go

@@ -394,17 +394,6 @@ type VersionSuccess struct {
 	VersionOutput string
 }
 
-// CheckRunStatus is the current status of a checkrun that is in progress
-// It keeps track of the jobURL and checkRunOutput
-type CheckRunStatus struct {
-	ID      string
-	JobsURL string
-
-	// Only need to persist for PolicyCheck commands since github does not persist the state of checkrun
-	// output
-	Output string
-}
-
 // PullStatus is the current status of a pull request that is in progress.
 type PullStatus struct {
 	// Projects are the projects that have been modified in this pull request.

server/events/plan_command_runner.go

@@ -52,11 +52,6 @@ func (p *PlanCommandRunner) runAutoplan(ctx *command.Context) {
 	baseRepo := ctx.Pull.BaseRepo
 	pull := ctx.Pull
 
-	// Pending status creates a new checkrun and populates the db
-	if err := p.commitStatusUpdater.UpdateCombined(context.TODO(), ctx.Pull.BaseRepo, ctx.Pull, models.PendingCommitStatus, command.Plan); err != nil {
-		ctx.Log.WarnContext(ctx.RequestCtx, fmt.Sprintf("unable to update commit status: %s", err))
-	}
-
 	projectCmds, err := p.prjCmdBuilder.BuildAutoplanCommands(ctx)
 	if err != nil {
 		if statusErr := p.commitStatusUpdater.UpdateCombined(context.TODO(), baseRepo, pull, models.FailedCommitStatus, command.Plan); statusErr != nil {
@@ -76,25 +71,20 @@ func (p *PlanCommandRunner) runAutoplan(ctx *command.Context) {
 		if err := p.commitStatusUpdater.UpdateCombinedCount(context.TODO(), baseRepo, pull, models.SuccessCommitStatus, command.Plan, 0, 0); err != nil {
 			ctx.Log.WarnContext(ctx.RequestCtx, fmt.Sprintf("unable to update commit status: %s", err))
 		}
-
-		// Pending status creates a new checkrun first
-		if err := p.commitStatusUpdater.UpdateCombinedCount(context.TODO(), baseRepo, pull, models.PendingCommitStatus, command.PolicyCheck, 0, 0); err != nil {
-			ctx.Log.WarnContext(ctx.RequestCtx, fmt.Sprintf("unable to update commit status: %s", err))
-		}
 		if err := p.commitStatusUpdater.UpdateCombinedCount(context.TODO(), baseRepo, pull, models.SuccessCommitStatus, command.PolicyCheck, 0, 0); err != nil {
 			ctx.Log.WarnContext(ctx.RequestCtx, fmt.Sprintf("unable to update commit status: %s", err))
 		}
-
-		// Pending status creates a new checkrun first
-		if err := p.commitStatusUpdater.UpdateCombinedCount(context.TODO(), baseRepo, pull, models.PendingCommitStatus, command.Apply, 0, 0); err != nil {
-			ctx.Log.WarnContext(ctx.RequestCtx, fmt.Sprintf("unable to update commit status: %s", err))
-		}
 		if err := p.commitStatusUpdater.UpdateCombinedCount(context.TODO(), baseRepo, pull, models.SuccessCommitStatus, command.Apply, 0, 0); err != nil {
 			ctx.Log.WarnContext(ctx.RequestCtx, fmt.Sprintf("unable to update commit status: %s", err))
 		}
 		return
 	}
 
+	// At this point we are sure Atlantis has work to do, so set commit status to pending
+	if err := p.commitStatusUpdater.UpdateCombined(context.TODO(), ctx.Pull.BaseRepo, ctx.Pull, models.PendingCommitStatus, command.Plan); err != nil {
+		ctx.Log.WarnContext(ctx.RequestCtx, fmt.Sprintf("unable to update commit status: %s", err))
+	}
+
 	// Only run commands in parallel if enabled
 	var result command.Result
 	if p.isParallelEnabled(projectCmds) {

server/events/policy_check_command_runner.go

@@ -33,12 +33,6 @@ type PolicyCheckCommandRunner struct {
 }
 
 func (p *PolicyCheckCommandRunner) Run(ctx *command.Context, cmds []command.ProjectContext) {
-
-	// Set policy_check commit status to pending
-	if err := p.commitStatusUpdater.UpdateCombined(context.TODO(), ctx.Pull.BaseRepo, ctx.Pull, models.PendingCommitStatus, command.PolicyCheck); err != nil {
-		ctx.Log.WarnContext(ctx.RequestCtx, fmt.Sprintf("unable to update commit status: %s", err))
-	}
-
 	if len(cmds) == 0 {
 		ctx.Log.InfoContext(ctx.RequestCtx, "no projects to run policy_check in")
 		// If there were no projects modified, we set successful commit statuses
@@ -50,6 +44,11 @@ func (p *PolicyCheckCommandRunner) Run(ctx *command.Context, cmds []command.Proj
 		return
 	}
 
+	// So set policy_check commit status to pending
+	if err := p.commitStatusUpdater.UpdateCombined(context.TODO(), ctx.Pull.BaseRepo, ctx.Pull, models.PendingCommitStatus, command.PolicyCheck); err != nil {
+		ctx.Log.WarnContext(ctx.RequestCtx, fmt.Sprintf("unable to update commit status: %s", err))
+	}
+
 	var result command.Result
 	if p.isParallelEnabled(cmds) {
 		ctx.Log.InfoContext(ctx.RequestCtx, "Running policy_checks in parallel")
@@ -58,9 +57,6 @@ func (p *PolicyCheckCommandRunner) Run(ctx *command.Context, cmds []command.Proj
 		result = runProjectCmds(cmds, p.prjCmdRunner.PolicyCheck)
 	}
 
-	// Set project level statuses to pending to simplify handling status updates in checks/github_client
-	p.setProjectLevelStatusesToPending(*ctx, result)
-
 	p.outputUpdater.UpdateOutput(ctx, PolicyCheckCommand{}, result)
 
 	pullStatus, err := p.dbUpdater.updateDB(ctx, ctx.Pull, result.ProjectResults)
@@ -91,22 +87,3 @@ func (p *PolicyCheckCommandRunner) updateCommitStatus(ctx *command.Context, pull
 func (p *PolicyCheckCommandRunner) isParallelEnabled(cmds []command.ProjectContext) bool {
 	return len(cmds) > 0 && cmds[0].ParallelPolicyCheckEnabled
 }
-
-func (p *PolicyCheckCommandRunner) setProjectLevelStatusesToPending(ctx command.Context, result command.Result) {
-
-	for _, prjResult := range result.ProjectResults {
-
-		// Rebuild the project ctx after result
-		prjCtx := command.ProjectContext{
-			ProjectName: prjResult.ProjectName,
-			RepoRelDir:  prjResult.RepoRelDir,
-			Workspace:   prjResult.Workspace,
-			BaseRepo:    ctx.Pull.BaseRepo,
-			Pull:        ctx.Pull,
-		}
-
-		if err := p.commitStatusUpdater.UpdateProject(ctx.RequestCtx, prjCtx, prjResult.Command, models.PendingCommitStatus, ""); err != nil {
-			ctx.Log.WarnContext(ctx.RequestCtx, fmt.Sprintf("unable to update commit status: %s", err))
-		}
-	}
-}