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.

Sign up for GitHub

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

Jump to bottom

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

Merged
merged 12 commits into from
Jun 24, 2020
Merged

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

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
c85155e
Fix: Do not automerge with errors or noop
Apr 23, 2020
c5eda27
Refactoring model to allow NotPlanned status
Apr 26, 2020
ce1d577
Integrate PR feedback and tests adapted
May 9, 2020
ade9b31
Add BoltDB interface and some relevant tests
May 11, 2020
cb26355
Add the rest of the BoltDB interface methods
May 25, 2020
421b247
Integrate some more PR feedback including a test
Jun 3, 2020
9689dbb
Fix lint error
Jun 3, 2020
f546b69
Remove BoltDB interface and mock as not needed
Jun 4, 2020
036e1f6
Fix typo on boltDB reference
Jun 4, 2020
8a905f9
Typo in BoltDB test
Jun 4, 2020
ad512f1
Merge branch 'master' into fix-automerge-on-noapply
Jun 4, 2020
abd2161
Fix typo from master merge conflict
Jun 4, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion server/events/command_runner.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ type DefaultCommandRunner struct {
GlobalAutomerge bool
PendingPlanFinder PendingPlanFinder
WorkingDir WorkingDir
DB *db.BoltDB
DB db.BoltDB
}

// RunAutoplanCommand runs plan when a pull request is opened or updated.
Expand Down
21 changes: 21 additions & 0 deletions server/events/command_runner_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
"github.com/google/go-github/v28/github"
. "github.com/petergtz/pegomock"
"github.com/runatlantis/atlantis/server/events"
dbmocks "github.com/runatlantis/atlantis/server/events/db/mocks"
"github.com/runatlantis/atlantis/server/events/mocks"
"github.com/runatlantis/atlantis/server/events/mocks/matchers"
"github.com/runatlantis/atlantis/server/events/models"
Expand All @@ -43,6 +44,7 @@ var ch events.DefaultCommandRunner
var pullLogger *logging.SimpleLogger
var workingDir events.WorkingDir
var pendingPlanFinder *mocks.MockPendingPlanFinder
var boltDB *dbmocks.MockBoltDB

func setup(t *testing.T) *vcsmocks.MockClient {
RegisterMockTestingT(t)
Expand All @@ -57,6 +59,7 @@ func setup(t *testing.T) *vcsmocks.MockClient {
projectCommandRunner = mocks.NewMockProjectCommandRunner()
workingDir = mocks.NewMockWorkingDir()
pendingPlanFinder = mocks.NewMockPendingPlanFinder()
boltDB = dbmocks.NewMockBoltDB()
When(logger.GetLevel()).ThenReturn(logging.Info)
When(logger.NewLogger("runatlantis/atlantis#1", true, logging.Info)).
ThenReturn(pullLogger)
Expand All @@ -76,6 +79,7 @@ func setup(t *testing.T) *vcsmocks.MockClient {
PendingPlanFinder: pendingPlanFinder,
WorkingDir: workingDir,
DisableApplyAll: false,
DB: boltDB,
}
return vcsClient
}
Expand Down Expand Up @@ -234,3 +238,20 @@ func TestRunAutoplanCommand_DeletePlans(t *testing.T) {
ch.RunAutoplanCommand(fixtures.GithubRepo, fixtures.GithubRepo, fixtures.Pull, fixtures.User)
pendingPlanFinder.VerifyWasCalledOnce().DeletePlans(tmp)
}

func TestApplyWithAutoMerge_VSCMerge(t *testing.T) {
t.Log("if \"atlantis apply\" is run with automerge and at least one project" +
Copy link
Member

@lkysow lkysow May 27, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

comment here is not describing the test

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good spot, fixed

" has a discarded plan, automerge should not take place")

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

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
}
59 changes: 37 additions & 22 deletions server/events/db/boltdb.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,23 @@ import (
bolt "go.etcd.io/bbolt"
)

// BoltDB is a database using BoltDB
type BoltDB struct {
//go:generate pegomock generate -m --use-experimental-model-gen --package mocks -o mocks/mock_boltdb.go BoltDB

// BoltDB interface defines the set of methods the DB implements. Use this to allow DB mocking when testing
type BoltDB interface {
TryLock(newLock models.ProjectLock) (bool, models.ProjectLock, error)
Unlock(p models.Project, workspace string) (*models.ProjectLock, error)
List() ([]models.ProjectLock, error)
UnlockByPull(repoFullName string, pullNum int) ([]models.ProjectLock, error)
GetLock(p models.Project, workspace string) (*models.ProjectLock, error)
GetPullStatus(pull models.PullRequest) (*models.PullStatus, error)
UpdatePullWithResults(pull models.PullRequest, newResults []models.ProjectResult) (models.PullStatus, error)
DeletePullStatus(pull models.PullRequest) error
UpdateProjectStatus(pull models.PullRequest, workspace string, repoRelDir string, targetStatus models.ProjectPlanStatus) error
}

// DefaultBoltDB is a database using BoltDB
type DefaultBoltDB struct {
db *bolt.DB
locksBucketName []byte
pullsBucketName []byte
Expand All @@ -30,7 +45,7 @@ const (

// New returns a valid locker. We need to be able to write to dataDir
// since bolt stores its data as a file
func New(dataDir string) (*BoltDB, error) {
func New(dataDir string) (*DefaultBoltDB, error) {
if err := os.MkdirAll(dataDir, 0700); err != nil {
return nil, errors.Wrap(err, "creating data dir")
}
Expand All @@ -56,19 +71,19 @@ func New(dataDir string) (*BoltDB, error) {
return nil, errors.Wrap(err, "starting BoltDB")
}
// todo: close BoltDB when server is sigtermed
return &BoltDB{db: db, locksBucketName: []byte(locksBucketName), pullsBucketName: []byte(pullsBucketName)}, nil
return &DefaultBoltDB{db: db, locksBucketName: []byte(locksBucketName), pullsBucketName: []byte(pullsBucketName)}, nil
}

// NewWithDB is used for testing.
func NewWithDB(db *bolt.DB, bucket string) (*BoltDB, error) {
return &BoltDB{db: db, locksBucketName: []byte(bucket), pullsBucketName: []byte(pullsBucketName)}, nil
func NewWithDB(db *bolt.DB, bucket string) (*DefaultBoltDB, error) {
return &DefaultBoltDB{db: db, locksBucketName: []byte(bucket), pullsBucketName: []byte(pullsBucketName)}, nil
}

// TryLock attempts to create a new lock. If the lock is
// acquired, it will return true and the lock returned will be newLock.
// If the lock is not acquired, it will return false and the current
// lock that is preventing this lock from being acquired.
func (b *BoltDB) TryLock(newLock models.ProjectLock) (bool, models.ProjectLock, error) {
func (b *DefaultBoltDB) TryLock(newLock models.ProjectLock) (bool, models.ProjectLock, error) {
var lockAcquired bool
var currLock models.ProjectLock
key := b.lockKey(newLock.Project, newLock.Workspace)
Expand Down Expand Up @@ -105,7 +120,7 @@ func (b *BoltDB) TryLock(newLock models.ProjectLock) (bool, models.ProjectLock,
// If there is no lock, then it will return a nil pointer.
// If there is a lock, then it will delete it, and then return a pointer
// to the deleted lock.
func (b *BoltDB) Unlock(p models.Project, workspace string) (*models.ProjectLock, error) {
func (b *DefaultBoltDB) Unlock(p models.Project, workspace string) (*models.ProjectLock, error) {
var lock models.ProjectLock
foundLock := false
key := b.lockKey(p, workspace)
Expand All @@ -128,7 +143,7 @@ func (b *BoltDB) Unlock(p models.Project, workspace string) (*models.ProjectLock
}

// List lists all current locks.
func (b *BoltDB) List() ([]models.ProjectLock, error) {
func (b *DefaultBoltDB) List() ([]models.ProjectLock, error) {
var locks []models.ProjectLock
var locksBytes [][]byte
err := b.db.View(func(tx *bolt.Tx) error {
Expand Down Expand Up @@ -156,7 +171,7 @@ func (b *BoltDB) List() ([]models.ProjectLock, error) {
}

// UnlockByPull deletes all locks associated with that pull request and returns them.
func (b *BoltDB) UnlockByPull(repoFullName string, pullNum int) ([]models.ProjectLock, error) {
func (b *DefaultBoltDB) UnlockByPull(repoFullName string, pullNum int) ([]models.ProjectLock, error) {
var locks []models.ProjectLock
err := b.db.View(func(tx *bolt.Tx) error {
c := tx.Bucket(b.locksBucketName).Cursor()
Expand Down Expand Up @@ -188,7 +203,7 @@ func (b *BoltDB) UnlockByPull(repoFullName string, pullNum int) ([]models.Projec

// GetLock returns a pointer to the lock for that project and workspace.
// If there is no lock, it returns a nil pointer.
func (b *BoltDB) GetLock(p models.Project, workspace string) (*models.ProjectLock, error) {
func (b *DefaultBoltDB) GetLock(p models.Project, workspace string) (*models.ProjectLock, error) {
key := b.lockKey(p, workspace)
var lockBytes []byte
err := b.db.View(func(tx *bolt.Tx) error {
Expand Down Expand Up @@ -216,7 +231,7 @@ func (b *BoltDB) GetLock(p models.Project, workspace string) (*models.ProjectLoc

// UpdatePullWithResults updates pull's status with the latest project results.
// It returns the new PullStatus object.
func (b *BoltDB) UpdatePullWithResults(pull models.PullRequest, newResults []models.ProjectResult) (models.PullStatus, error) {
func (b *DefaultBoltDB) UpdatePullWithResults(pull models.PullRequest, newResults []models.ProjectResult) (models.PullStatus, error) {
key, err := b.pullKey(pull)
if err != nil {
return models.PullStatus{}, err
Expand Down Expand Up @@ -281,7 +296,7 @@ func (b *BoltDB) UpdatePullWithResults(pull models.PullRequest, newResults []mod

// GetPullStatus returns the status for pull.
// If there is no status, returns a nil pointer.
func (b *BoltDB) GetPullStatus(pull models.PullRequest) (*models.PullStatus, error) {
func (b *DefaultBoltDB) GetPullStatus(pull models.PullRequest) (*models.PullStatus, error) {
key, err := b.pullKey(pull)
if err != nil {
return nil, err
Expand All @@ -297,7 +312,7 @@ func (b *BoltDB) GetPullStatus(pull models.PullRequest) (*models.PullStatus, err
}

// DeletePullStatus deletes the status for pull.
func (b *BoltDB) DeletePullStatus(pull models.PullRequest) error {
func (b *DefaultBoltDB) DeletePullStatus(pull models.PullRequest) error {
key, err := b.pullKey(pull)
if err != nil {
return err
Expand All @@ -309,9 +324,9 @@ func (b *BoltDB) DeletePullStatus(pull models.PullRequest) error {
return errors.Wrap(err, "DB transaction failed")
}

// DeleteProjectStatus deletes all project statuses under pull that match
// UpdateProjectStatus updates all project statuses under pull that match
// workspace and repoRelDir.
func (b *BoltDB) DeleteProjectStatus(pull models.PullRequest, workspace string, repoRelDir string) error {
func (b *DefaultBoltDB) UpdateProjectStatus(pull models.PullRequest, workspace string, repoRelDir string, targetStatus models.ProjectPlanStatus) error {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested rewrite.

// UpdateProjectStatus updates project status.
func (b *DefaultBoltDB) UpdateProjectStatus(pull models.PullRequest, workspace string, repoRelDir string, newStatus models.ProjectPlanStatus) error {
	key, err := b.pullKey(pull)
	if err != nil {
		return err
	}
	err = b.db.Update(func(tx *bolt.Tx) error {
		bucket := tx.Bucket(b.pullsBucketName)
		currStatusPtr, err := b.getPullFromBucket(bucket, key)
		if err != nil {
			return err
		}
		if currStatusPtr == nil {
			return nil
		}
		currStatus := *currStatusPtr

		// 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
			}
		}
		return b.writePullToBucket(bucket, key, currStatus)
	})
	return errors.Wrap(err, "DB transaction failed")
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK

key, err := b.pullKey(pull)
if err != nil {
return err
Expand All @@ -332,7 +347,7 @@ func (b *BoltDB) DeleteProjectStatus(pull models.PullRequest, workspace string,
var newProjects []models.ProjectStatus
for _, p := range currStatus.Projects {
if p.Workspace == workspace && p.RepoRelDir == repoRelDir {
continue
p.Status = targetStatus
}
newProjects = append(newProjects, p)
}
Expand All @@ -344,7 +359,7 @@ func (b *BoltDB) DeleteProjectStatus(pull models.PullRequest, workspace string,
return errors.Wrap(err, "DB transaction failed")
}

func (b *BoltDB) pullKey(pull models.PullRequest) ([]byte, error) {
func (b *DefaultBoltDB) pullKey(pull models.PullRequest) ([]byte, error) {
hostname := pull.BaseRepo.VCSHost.Hostname
if strings.Contains(hostname, pullKeySeparator) {
return nil, fmt.Errorf("vcs hostname %q contains illegal string %q", hostname, pullKeySeparator)
Expand All @@ -358,11 +373,11 @@ func (b *BoltDB) pullKey(pull models.PullRequest) ([]byte, error) {
nil
}

func (b *BoltDB) lockKey(p models.Project, workspace string) string {
func (b *DefaultBoltDB) lockKey(p models.Project, workspace string) string {
return fmt.Sprintf("%s/%s/%s", p.RepoFullName, p.Path, workspace)
}

func (b *BoltDB) getPullFromBucket(bucket *bolt.Bucket, key []byte) (*models.PullStatus, error) {
func (b *DefaultBoltDB) getPullFromBucket(bucket *bolt.Bucket, key []byte) (*models.PullStatus, error) {
serialized := bucket.Get(key)
if serialized == nil {
return nil, nil
Expand All @@ -375,15 +390,15 @@ func (b *BoltDB) getPullFromBucket(bucket *bolt.Bucket, key []byte) (*models.Pul
return &p, nil
}

func (b *BoltDB) writePullToBucket(bucket *bolt.Bucket, key []byte, pull models.PullStatus) error {
func (b *DefaultBoltDB) writePullToBucket(bucket *bolt.Bucket, key []byte, pull models.PullStatus) error {
serialized, err := json.Marshal(pull)
if err != nil {
return errors.Wrap(err, "serializing")
}
return bucket.Put(key, serialized)
}

func (b *BoltDB) projectResultToProject(p models.ProjectResult) models.ProjectStatus {
func (b *DefaultBoltDB) projectResultToProject(p models.ProjectResult) models.ProjectStatus {
return models.ProjectStatus{
Workspace: p.Workspace,
RepoRelDir: p.RepoRelDir,
Expand Down
18 changes: 12 additions & 6 deletions server/events/db/boltdb_test.go
View file
Open in desktop
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 Expand Up @@ -686,7 +692,7 @@ func TestPullStatus_UpdateMerge(t *testing.T) {
}

// newTestDB returns a TestDB using a temporary path.
func newTestDB() (*bolt.DB, *db.BoltDB) {
func newTestDB() (*bolt.DB, *db.DefaultBoltDB) {
// Retrieve a temporary path.
f, err := ioutil.TempFile("", "")
if err != nil {
Expand All @@ -712,7 +718,7 @@ func newTestDB() (*bolt.DB, *db.BoltDB) {
return boltDB, b
}

func newTestDB2(t *testing.T) (*db.BoltDB, func()) {
func newTestDB2(t *testing.T) (*db.DefaultBoltDB, func()) {
tmp, cleanup := TempDir(t)
boltDB, err := db.New(tmp)
Ok(t, err)
Expand Down
20 changes: 20 additions & 0 deletions server/events/db/mocks/matchers/models_project.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

20 changes: 20 additions & 0 deletions server/events/db/mocks/matchers/models_projectlock.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

20 changes: 20 additions & 0 deletions server/events/db/mocks/matchers/models_projectplanstatus.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

20 changes: 20 additions & 0 deletions server/events/db/mocks/matchers/models_projectresult.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading