Improve container tracking when containers failing to be created/destroyed in garden

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) {

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() {

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)
+		}
 	}
 }
 

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",
 	})
 }
 

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)

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