Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions controllers/evacuation_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ func (h *EvacuationController) EvacuateClaimedActualLRP(ctx context.Context, log
}

// this is an ordinary LRP
before, after, err := h.actualLRPDB.UnclaimActualLRP(ctx, logger, actualLRPKey)
before, after, err := h.actualLRPDB.UnclaimActualLRP(ctx, logger, false, actualLRPKey)
bbsErr := models.ConvertError(err)
if bbsErr != nil {
if bbsErr.Type == models.Error_ResourceNotFound {
Expand Down Expand Up @@ -430,7 +430,7 @@ func (h *EvacuationController) evacuateInstance(ctx context.Context, logger lage
return nil
}

_, after, err := h.actualLRPDB.UnclaimActualLRP(ctx, logger, &actualLRP.ActualLRPKey)
_, after, err := h.actualLRPDB.UnclaimActualLRP(ctx, logger, false, &actualLRP.ActualLRPKey)
if err != nil {
return err
}
Expand Down
4 changes: 2 additions & 2 deletions controllers/evacuation_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -261,7 +261,7 @@ var _ = Describe("Evacuation Controller", func() {

It("unclaims and reauctions the lrp", func() {
Expect(fakeActualLRPDB.UnclaimActualLRPCallCount()).To(Equal(1))
_, _, key := fakeActualLRPDB.UnclaimActualLRPArgsForCall(0)
_, _, _, key := fakeActualLRPDB.UnclaimActualLRPArgsForCall(0)
Expect(key).To(Equal(lrpKey))

Expect(fakeDesiredLRPDB.DesiredLRPSchedulingInfoByProcessGuidCallCount()).To(Equal(1))
Expand Down Expand Up @@ -1143,7 +1143,7 @@ var _ = Describe("Evacuation Controller", func() {

It("unclaims the lrp and requests an auction", func() {
Expect(fakeActualLRPDB.UnclaimActualLRPCallCount()).To(Equal(1))
_, _, lrpKey := fakeActualLRPDB.UnclaimActualLRPArgsForCall(0)
_, _, _, lrpKey := fakeActualLRPDB.UnclaimActualLRPArgsForCall(0)
Expect(lrpKey.ProcessGuid).To(Equal(actual.ProcessGuid))
Expect(lrpKey.Index).To(Equal(actual.Index))

Expand Down
4 changes: 2 additions & 2 deletions controllers/lrp_convergence_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@ func (h *LRPConvergenceController) ConvergeLRPs(ctx context.Context) {
for _, lrpKey := range convergenceResult.UnstartedLRPKeys {
dereferencedKey := *lrpKey
works = append(works, func() {
before, after, err := h.lrpDB.UnclaimActualLRP(ctx, logger, dereferencedKey.Key)
before, after, err := h.lrpDB.UnclaimActualLRP(ctx, logger, true, dereferencedKey.Key)
if err != nil && err != models.ErrActualLRPCannotBeUnclaimed {
logger.Error("cannot-unclaim-lrp", err, lager.Data{"key": dereferencedKey})
return
Expand Down Expand Up @@ -218,7 +218,7 @@ func (h *LRPConvergenceController) ConvergeLRPs(ctx context.Context) {
// there is a Suspect LRP already, unclaim this previously created
// replacement and reauction it
logger.Debug("found-suspect-lrp-unclaiming", lager.Data{"key": dereferencedKey.Key})
before, after, err := h.lrpDB.UnclaimActualLRP(ctx, logger, dereferencedKey.Key)
before, after, err := h.lrpDB.UnclaimActualLRP(ctx, logger, false, dereferencedKey.Key)
if err != nil {
logger.Error("failed-unclaiming-lrp", err)
return
Expand Down
35 changes: 34 additions & 1 deletion controllers/lrp_convergence_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -262,8 +262,9 @@ var _ = Describe("LRP Convergence Controllers", func() {

It("transition the LRP to UNCLAIMED state", func() {
Eventually(fakeLRPDB.UnclaimActualLRPCallCount).Should(Equal(1))
_, _, actualKey := fakeLRPDB.UnclaimActualLRPArgsForCall(0)
_, _, isStale, actualKey := fakeLRPDB.UnclaimActualLRPArgsForCall(0)
Expect(actualKey).To(Equal(key))
Expect(isStale).To(BeTrue())
})

It("emits an LRPChanged event", func() {
Expand Down Expand Up @@ -320,6 +321,36 @@ var _ = Describe("LRP Convergence Controllers", func() {
Consistently(actualLRPInstanceHub.EmitCallCount).Should(BeZero())
})
})

Context("when the LRP cannot be unclaimed because it is stale and already claimed", func() {
BeforeEach(func() {
fakeLRPDB.UnclaimActualLRPReturns(nil, nil, models.ErrActualLRPCannotBeUnclaimed)
})

It("retains the claimed state and still auctions off the returned keys", func() {
Expect(fakeAuctioneerClient.RequestLRPAuctionsCallCount()).To(Equal(1))

_, actualTraceId, startAuctions := fakeAuctioneerClient.RequestLRPAuctionsArgsForCall(0)
Expect(startAuctions).To(HaveLen(1))
Expect(actualTraceId).To(Equal(traceId))
request := auctioneer.NewLRPStartRequestFromModel(model_helpers.NewValidDesiredLRP("some-guid"), 0)
Expect(startAuctions).To(ContainElement(&request))
})

It("calls UnclaimActualLRP with isStaleUnclaimed=true", func() {
Expect(fakeLRPDB.UnclaimActualLRPCallCount()).To(Equal(1))
_, _, isStaleUnclaimed, _ := fakeLRPDB.UnclaimActualLRPArgsForCall(0)
Expect(isStaleUnclaimed).To(BeTrue())
})

It("does not emit LRP changed event", func() {
Consistently(actualHub.EmitCallCount).Should(BeZero())
})

It("does not emit any instance events", func() {
Consistently(actualLRPInstanceHub.EmitCallCount).Should(BeZero())
})
})
})

Context("when there are missing ActualLRPs", func() {
Expand Down Expand Up @@ -511,6 +542,8 @@ var _ = Describe("LRP Convergence Controllers", func() {

It("unclaims the suspect replacement lrp", func() {
Expect(fakeLRPDB.UnclaimActualLRPCallCount()).To(Equal(1))
_, _, isStale, _ := fakeLRPDB.UnclaimActualLRPArgsForCall(0)
Expect(isStale).To(BeFalse())
})

It("does not emit change events", func() {
Expand Down
2 changes: 1 addition & 1 deletion db/actual_lrp_db.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ type ActualLRPDB interface {
ActualLRPs(ctx context.Context, logger lager.Logger, filter models.ActualLRPFilter) ([]*models.ActualLRP, error)
ActualLRPsByProcessGuids(ctx context.Context, logger lager.Logger, filter models.ActualLRPsByProcessGuidsFilter) ([]*models.ActualLRP, error)
CreateUnclaimedActualLRP(ctx context.Context, logger lager.Logger, key *models.ActualLRPKey) (after *models.ActualLRP, err error)
UnclaimActualLRP(ctx context.Context, logger lager.Logger, key *models.ActualLRPKey) (before *models.ActualLRP, after *models.ActualLRP, err error)
UnclaimActualLRP(ctx context.Context, logger lager.Logger, isStale bool, key *models.ActualLRPKey) (before *models.ActualLRP, after *models.ActualLRP, err error)
ClaimActualLRP(ctx context.Context, logger lager.Logger, processGuid string, index int32, instanceKey *models.ActualLRPInstanceKey) (before *models.ActualLRP, after *models.ActualLRP, err error)
StartActualLRP(ctx context.Context,
logger lager.Logger,
Expand Down
46 changes: 12 additions & 34 deletions db/dbfakes/fake_actual_lrpdb.go

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

108 changes: 12 additions & 96 deletions db/dbfakes/fake_db.go

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

16 changes: 0 additions & 16 deletions db/dbfakes/fake_desired_lrpdb.go

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

Loading