Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Fixes #1006: Keep track of project status even if plans have been deleted #1005

Merged
merged 12 commits into from
Jun 24, 2020
61 changes: 61 additions & 0 deletions server/events/command_runner_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,12 @@ func setup(t *testing.T) *vcsmocks.MockClient {
projectCommandRunner = mocks.NewMockProjectCommandRunner()
workingDir = mocks.NewMockWorkingDir()
pendingPlanFinder = mocks.NewMockPendingPlanFinder()

tmp, cleanup := TempDir(t)
defer cleanup()
defaultBoltDB, err := db.New(tmp)
Ok(t, err)

drainer = &events.Drainer{}
When(logger.GetLevel()).ThenReturn(logging.Info)
When(logger.NewLogger("runatlantis/atlantis#1", true, logging.Info)).
Expand All @@ -79,6 +85,7 @@ func setup(t *testing.T) *vcsmocks.MockClient {
PendingPlanFinder: pendingPlanFinder,
WorkingDir: workingDir,
DisableApplyAll: false,
DB: defaultBoltDB,
Drainer: drainer,
}
return vcsClient
Expand Down Expand Up @@ -242,6 +249,60 @@ func TestRunAutoplanCommand_DeletePlans(t *testing.T) {
pendingPlanFinder.VerifyWasCalledOnce().DeletePlans(tmp)
}

func TestApplyWithAutoMerge_VSCMerge(t *testing.T) {
t.Log("if \"atlantis apply\" is run with automerge then a VCS merge is performed")

vcsClient := setup(t)
pull := &github.PullRequest{
State: github.String("open"),
}
modelPull := models.PullRequest{State: models.OpenPullState}
When(githubGetter.GetPullRequest(fixtures.GithubRepo, fixtures.Pull.Num)).ThenReturn(pull, nil)
When(eventParsing.ParseGithubPull(pull)).ThenReturn(modelPull, modelPull.BaseRepo, fixtures.GithubRepo, nil)
ch.GlobalAutomerge = true
lkysow marked this conversation as resolved.
Show resolved Hide resolved
defer func() { ch.GlobalAutomerge = false }()

ch.RunCommentCommand(fixtures.GithubRepo, &fixtures.GithubRepo, nil, fixtures.User, fixtures.Pull.Num, &events.CommentCommand{Name: models.ApplyCommand})
vcsClient.VerifyWasCalledOnce().MergePull(modelPull)
lkysow marked this conversation as resolved.
Show resolved Hide resolved
}

func TestRunApply_DiscardedProjects(t *testing.T) {
t.Log("if \"atlantis apply\" is run with automerge and at least one project" +
" has a discarded plan, automerge should not take place")
vcsClient := setup(t)
ch.GlobalAutomerge = true
defer func() { ch.GlobalAutomerge = false }()
tmp, cleanup := TempDir(t)
defer cleanup()
boltDB, err := db.New(tmp)
Ok(t, err)
ch.DB = boltDB
pull := fixtures.Pull
pull.BaseRepo = fixtures.GithubRepo
_, err = boltDB.UpdatePullWithResults(pull, []models.ProjectResult{
{
Command: models.PlanCommand,
RepoRelDir: ".",
Workspace: "default",
PlanSuccess: &models.PlanSuccess{
TerraformOutput: "tf-output",
LockURL: "lock-url",
},
},
})
Ok(t, err)
Ok(t, boltDB.UpdateProjectStatus(pull, "default", ".", models.DiscardedPlanStatus))
ghPull := &github.PullRequest{
State: github.String("open"),
}
When(githubGetter.GetPullRequest(fixtures.GithubRepo, fixtures.Pull.Num)).ThenReturn(ghPull, nil)
When(eventParsing.ParseGithubPull(ghPull)).ThenReturn(pull, pull.BaseRepo, fixtures.GithubRepo, nil)
When(workingDir.GetPullDir(matchers.AnyModelsRepo(), matchers.AnyModelsPullRequest())).
ThenReturn(tmp, nil)
ch.RunCommentCommand(fixtures.GithubRepo, &fixtures.GithubRepo, &pull, fixtures.User, fixtures.Pull.Num, &events.CommentCommand{Name: models.ApplyCommand})
vcsClient.VerifyWasCalled(Never()).MergePull(matchers.AnyModelsPullRequest())
}

func TestRunCommentCommand_DrainOngoing(t *testing.T) {
t.Log("if drain is ongoing then a message should be displayed")
vcsClient := setup(t)
Expand Down
23 changes: 10 additions & 13 deletions server/events/db/boltdb.go
Original file line number Diff line number Diff line change
Expand Up @@ -309,9 +309,8 @@ func (b *BoltDB) DeletePullStatus(pull models.PullRequest) error {
return errors.Wrap(err, "DB transaction failed")
}

// DeleteProjectStatus deletes all project statuses under pull that match
// workspace and repoRelDir.
func (b *BoltDB) DeleteProjectStatus(pull models.PullRequest, workspace string, repoRelDir string) error {
// UpdateProjectStatus updates project status.
func (b *BoltDB) UpdateProjectStatus(pull models.PullRequest, workspace string, repoRelDir string, newStatus models.ProjectPlanStatus) error {
key, err := b.pullKey(pull)
if err != nil {
return err
Expand All @@ -327,18 +326,16 @@ func (b *BoltDB) DeleteProjectStatus(pull models.PullRequest, workspace string,
}
currStatus := *currStatusPtr

// Create a new projectStatuses array without the ones we want to
// delete.
var newProjects []models.ProjectStatus
for _, p := range currStatus.Projects {
if p.Workspace == workspace && p.RepoRelDir == repoRelDir {
continue
// Update the status.
for i := range currStatus.Projects {
// NOTE: We're using a reference here because we are
// in-place updating its Status field.
proj := &currStatus.Projects[i]
if proj.Workspace == workspace && proj.RepoRelDir == repoRelDir {
proj.Status = newStatus
break
}
newProjects = append(newProjects, p)
}

// Overwrite the old pull status.
currStatus.Projects = newProjects
return b.writePullToBucket(bucket, key, currStatus)
})
return errors.Wrap(err, "DB transaction failed")
Expand Down
14 changes: 10 additions & 4 deletions server/events/db/boltdb_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -449,10 +449,10 @@ func TestPullStatus_UpdateDeleteGet(t *testing.T) {
Assert(t, maybeStatus == nil, "exp nil")
}

// Test we can create a status, delete a specific project's status within that
// Test we can create a status, update a specific project's status within that
// pull status, and when we get all the project statuses, that specific project
// should not be there.
func TestPullStatus_UpdateDeleteProject(t *testing.T) {
// should be updated.
func TestPullStatus_UpdateProject(t *testing.T) {
b, cleanup := newTestDB2(t)
defer cleanup()

Expand Down Expand Up @@ -492,14 +492,20 @@ func TestPullStatus_UpdateDeleteProject(t *testing.T) {
})
Ok(t, err)

err = b.DeleteProjectStatus(pull, "default", ".")
err = b.UpdateProjectStatus(pull, "default", ".", models.DiscardedPlanStatus)
Ok(t, err)

status, err := b.GetPullStatus(pull)
Ok(t, err)
Assert(t, status != nil, "exp non-nil")
Equals(t, pull, status.Pull) // nolint: staticcheck
Equals(t, []models.ProjectStatus{
{
Workspace: "default",
RepoRelDir: ".",
ProjectName: "",
Status: models.DiscardedPlanStatus,
},
{
Workspace: "staging",
RepoRelDir: ".",
Expand Down
7 changes: 6 additions & 1 deletion server/events/models/models.go
Original file line number Diff line number Diff line change
Expand Up @@ -480,12 +480,15 @@ const (
// PlannedPlanStatus means that a plan has been successfully generated but
// not yet applied.
PlannedPlanStatus
// ErrorApplyStatus means that a plan has been generated but there was an
// ErroredApplyStatus means that a plan has been generated but there was an
// error while applying it.
ErroredApplyStatus
// AppliedPlanStatus means that a plan has been generated and applied
// successfully.
AppliedPlanStatus
// DiscardedPlanStatus means that there was an unapplied plan that was
// discarded due to a project being unlocked
DiscardedPlanStatus
)

// String returns a string representation of the status.
Expand All @@ -499,6 +502,8 @@ func (p ProjectPlanStatus) String() string {
return "apply_errored"
case AppliedPlanStatus:
return "applied"
case DiscardedPlanStatus:
return "plan_discarded"
default:
panic("missing String() impl for ProjectPlanStatus")
}
Expand Down
4 changes: 4 additions & 0 deletions server/events/models/models_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -452,11 +452,15 @@ func TestPullStatus_StatusCount(t *testing.T) {
{
Status: models.ErroredApplyStatus,
},
{
Status: models.DiscardedPlanStatus,
},
},
}

Equals(t, 2, ps.StatusCount(models.PlannedPlanStatus))
Equals(t, 1, ps.StatusCount(models.AppliedPlanStatus))
Equals(t, 1, ps.StatusCount(models.ErroredApplyStatus))
Equals(t, 0, ps.StatusCount(models.ErroredPlanStatus))
Equals(t, 1, ps.StatusCount(models.DiscardedPlanStatus))
}
4 changes: 2 additions & 2 deletions server/locks_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,8 +108,8 @@ func (l *LocksController) DeleteLock(w http.ResponseWriter, r *http.Request) {
l.Logger.Err("unable to delete workspace: %s", err)
}
}
if err := l.DB.DeleteProjectStatus(lock.Pull, lock.Workspace, lock.Project.Path); err != nil {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should delete the DeleteProjectStatus method since it's not used anymore

l.Logger.Err("unable to delete project status: %s", err)
if err := l.DB.UpdateProjectStatus(lock.Pull, lock.Workspace, lock.Project.Path, models.DiscardedPlanStatus); err != nil {
l.Logger.Err("unable to update project status: %s", err)
}

// Once the lock has been deleted, comment back on the pull request.
Expand Down
66 changes: 66 additions & 0 deletions server/locks_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
. "github.com/petergtz/pegomock"
"github.com/runatlantis/atlantis/server"
"github.com/runatlantis/atlantis/server/events"

"github.com/runatlantis/atlantis/server/events/locking/mocks"
mocks2 "github.com/runatlantis/atlantis/server/events/mocks"
"github.com/runatlantis/atlantis/server/events/models"
Expand Down Expand Up @@ -192,6 +193,71 @@ func TestDeleteLock_OldFormat(t *testing.T) {
cp.VerifyWasCalled(Never()).CreateComment(AnyRepo(), AnyInt(), AnyString())
}

func TestDeleteLock_UpdateProjectStatus(t *testing.T) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you update to the following the you don't need the db mock:

func TestDeleteLock_UpdateProjectStatus(t *testing.T) {
	t.Log("When deleting a lock, pull status has to be updated to reflect discarded plan")
	RegisterMockTestingT(t)

	repoName := "owner/repo"
	projectPath := "path"
	workspaceName := "workspace"

	cp := vcsmocks.NewMockClient()
	l := mocks.NewMockLocker()
	workingDir := mocks2.NewMockWorkingDir()
	workingDirLocker := events.NewDefaultWorkingDirLocker()
	pull := models.PullRequest{
		BaseRepo: models.Repo{FullName: repoName},
	}
	When(l.Unlock("id")).ThenReturn(&models.ProjectLock{
		Pull:      pull,
		Workspace: workspaceName,
		Project: models.Project{
			Path:         projectPath,
			RepoFullName: repoName,
		},
	}, nil)
	tmp, cleanup := TempDir(t)
	defer cleanup()
	db, err := db.New(tmp)
	Ok(t, err)
	// Seed the DB with a successful plan for that project (that is later discarded).
	_, err = db.UpdatePullWithResults(pull, []models.ProjectResult{
		{
			Command:    models.PlanCommand,
			RepoRelDir: projectPath,
			Workspace:  workspaceName,
			PlanSuccess: &models.PlanSuccess{
				TerraformOutput: "tf-output",
				LockURL:         "lock-url",
			},
		},
	})
	Ok(t, err)
	lc := server.LocksController{
		Locker:           l,
		Logger:           logging.NewNoopLogger(),
		VCSClient:        cp,
		WorkingDirLocker: workingDirLocker,
		WorkingDir:       workingDir,
		DB:               db,
	}
	req, _ := http.NewRequest("GET", "", bytes.NewBuffer(nil))
	req = mux.SetURLVars(req, map[string]string{"id": "id"})
	w := httptest.NewRecorder()
	lc.DeleteLock(w, req)
	responseContains(t, w, http.StatusOK, "Deleted lock id \"id\"")
	status, err := db.GetPullStatus(pull)
	Ok(t, err)
	Assert(t, status != nil, "status was nil")
	Equals(t, []models.ProjectStatus{
		{
			Workspace:  workspaceName,
			RepoRelDir: projectPath,
			Status:     models.DiscardedPlanStatus,
		},
	}, status.Projects)
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, replaced 👍

t.Log("When deleting a lock, pull status has to be updated to reflect discarded plan")
RegisterMockTestingT(t)

repoName := "owner/repo"
projectPath := "path"
workspaceName := "workspace"

cp := vcsmocks.NewMockClient()
l := mocks.NewMockLocker()
workingDir := mocks2.NewMockWorkingDir()
workingDirLocker := events.NewDefaultWorkingDirLocker()
pull := models.PullRequest{
BaseRepo: models.Repo{FullName: repoName},
}
When(l.Unlock("id")).ThenReturn(&models.ProjectLock{
Pull: pull,
Workspace: workspaceName,
Project: models.Project{
Path: projectPath,
RepoFullName: repoName,
},
}, nil)
tmp, cleanup := TempDir(t)
defer cleanup()
db, err := db.New(tmp)
Ok(t, err)
// Seed the DB with a successful plan for that project (that is later discarded).
_, err = db.UpdatePullWithResults(pull, []models.ProjectResult{
{
Command: models.PlanCommand,
RepoRelDir: projectPath,
Workspace: workspaceName,
PlanSuccess: &models.PlanSuccess{
TerraformOutput: "tf-output",
LockURL: "lock-url",
},
},
})
Ok(t, err)
lc := server.LocksController{
Locker: l,
Logger: logging.NewNoopLogger(),
VCSClient: cp,
WorkingDirLocker: workingDirLocker,
WorkingDir: workingDir,
DB: db,
}
req, _ := http.NewRequest("GET", "", bytes.NewBuffer(nil))
req = mux.SetURLVars(req, map[string]string{"id": "id"})
w := httptest.NewRecorder()
lc.DeleteLock(w, req)
responseContains(t, w, http.StatusOK, "Deleted lock id \"id\"")
status, err := db.GetPullStatus(pull)
Ok(t, err)
Assert(t, status != nil, "status was nil")
Equals(t, []models.ProjectStatus{
{
Workspace: workspaceName,
RepoRelDir: projectPath,
Status: models.DiscardedPlanStatus,
},
}, status.Projects)
}

func TestDeleteLock_CommentFailed(t *testing.T) {
t.Log("If the commenting fails we return an error")
RegisterMockTestingT(t)
Expand Down