diff --git a/depot/containerstore/containerstore.go b/depot/containerstore/containerstore.go index 8de98bb0..382b835d 100644 --- a/depot/containerstore/containerstore.go +++ b/depot/containerstore/containerstore.go @@ -288,11 +288,12 @@ func (cs *containerStore) Destroy(logger lager.Logger, guid string) error { err = node.Destroy(logger) if err != nil { logger.Error("failed-to-destroy-container", err) + return err } cs.containers.Remove(guid) - return err + return nil } func (cs *containerStore) Get(logger lager.Logger, guid string) (executor.Container, error) { diff --git a/depot/containerstore/containerstore_test.go b/depot/containerstore/containerstore_test.go index 7adfc8f4..326c176d 100644 --- a/depot/containerstore/containerstore_test.go +++ b/depot/containerstore/containerstore_test.go @@ -2549,23 +2549,26 @@ var _ = Describe("Container Store", func() { Eventually(getMetrics).Should(HaveKey(containerstore.GardenContainerDestructionFailedDuration)) }) - It("does remove the container from the container store", func() { + It("does not remove the container from the container store", func() { err := containerStore.Destroy(logger, containerGuid) Expect(err).To(Equal(destroyErr)) Expect(gardenClient.DestroyCallCount()).To(Equal(1)) Expect(gardenClient.DestroyArgsForCall(0)).To(Equal(containerGuid)) - _, err = containerStore.Get(logger, containerGuid) - Expect(err).To(Equal(executor.ErrContainerNotFound)) + container, err := containerStore.Get(logger, containerGuid) + Expect(err).ToNot(HaveOccurred()) + Expect(container.Guid).To(Equal(containerGuid)) }) - It("frees the containers resources", func() { + It("does not free up the containers resources", func() { err := containerStore.Destroy(logger, containerGuid) Expect(err).To(Equal(destroyErr)) - remainingResources := containerStore.RemainingResources(logger) - Expect(remainingResources).To(Equal(totalCapacity)) + resources := containerStore.RemainingResources(logger) + expectedResources := totalCapacity.Copy() + expectedResources.Subtract(&resource) + Expect(resources).To(Equal(expectedResources)) }) }) }) @@ -2745,6 +2748,46 @@ var _ = Describe("Container Store", func() { }) }) }) + + Context("when there are no process associated with the container", func() { + Context("when container is in completed state", func() { + JustBeforeEach(func() { + destroyErr := errors.New("failed-to-destroy-container") + gardenClient.DestroyReturns(destroyErr) + eventEmitter.EmitStub = func(e executor.Event) { + // processing completed container + defer GinkgoRecover() + if e.EventType() == executor.EventTypeContainerComplete { + // sleeping to avoid atomic destroying state + time.Sleep(time.Second) + err := containerStore.Destroy(logger, containerGuid) + Expect(err).To(HaveOccurred()) + } + } + }) + + It("does not emit the completed event", func() { + // completed event will trigger Destroy for failed completed container and that will cause an infinite loop + Eventually(eventEmitter.EmitCallCount).Should(Equal(1)) + event1 := eventEmitter.EmitArgsForCall(0) + Expect(event1.EventType()).To(Equal(executor.EventTypeContainerReserved)) + + err := containerStore.Destroy(logger, containerGuid) + Expect(err).To(HaveOccurred()) + + container, err := containerStore.Get(logger, containerGuid) + Expect(err).NotTo(HaveOccurred()) + + Eventually(eventEmitter.EmitCallCount).Should(Equal(2)) + event2 := eventEmitter.EmitArgsForCall(1) + Expect(event2).To(Equal(executor.ContainerCompleteEvent{ + RawContainer: container, + })) + + Consistently(eventEmitter.EmitCallCount).Should(Equal(2)) + }) + }) + }) }) Describe("Get", func() { diff --git a/depot/containerstore/storenode.go b/depot/containerstore/storenode.go index 67023cdd..1a4aa75e 100644 --- a/depot/containerstore/storenode.go +++ b/depot/containerstore/storenode.go @@ -645,7 +645,9 @@ func (n *storeNode) stop(logger lager.Logger) { n.process.Signal(os.Interrupt) logger.Debug("signalled-process") } else { - n.complete(logger, true, "stopped-before-running", false) + if n.info.State != executor.StateCompleted { + n.complete(logger, true, "stopped-before-running", false) + } } } diff --git a/initializer/initializer.go b/initializer/initializer.go index 59aadb01..00777397 100644 --- a/initializer/initializer.go +++ b/initializer/initializer.go @@ -61,6 +61,7 @@ type executorContainers struct { func (containers *executorContainers) Containers() ([]garden.Container, error) { return containers.gardenClient.Containers(garden.Properties{ executor.ContainerOwnerProperty: containers.owner, + executor.ContainerStateProperty: "all", }) } diff --git a/initializer/initializer_test.go b/initializer/initializer_test.go index 03be0619..f4b19988 100644 --- a/initializer/initializer_test.go +++ b/initializer/initializer_test.go @@ -185,6 +185,9 @@ var _ = Describe("Initializer", func() { fakeGarden.RouteToHandler("GET", "/containers", func(w http.ResponseWriter, r *http.Request) { r.ParseForm() + gardenState := r.URL.Query()["garden.state"] + Expect(gardenState).To(HaveLen(1)) + Expect(gardenState[0]).To(Equal("all")) healthcheckTagQueryParam := gardenhealth.HealthcheckTag if r.FormValue(healthcheckTagQueryParam) == gardenhealth.HealthcheckTagValue { ghttp.RespondWithJSONEncoded(http.StatusOK, struct{}{})(w, r) diff --git a/resources.go b/resources.go index f9a7230a..a9b5dfe2 100644 --- a/resources.go +++ b/resources.go @@ -10,7 +10,10 @@ import ( "code.cloudfoundry.org/routing-info/internalroutes" ) -const ContainerOwnerProperty = "executor:owner" +const ( + ContainerOwnerProperty = "executor:owner" + ContainerStateProperty = "garden.state" +) type State string