Cleanup volume GC

- add gc/Destroyer
- add RemoveDestroyingVolumes func to VolumeRepository
- refactor code to handle renaming of objects

Signed-off-by: Shash Reddy <sreddy@pivotal.io>

api/api_suite_test.go

@@ -39,9 +39,9 @@ var (
 	fakeEngine              *enginefakes.FakeEngine
 	fakeWorkerClient        *workerfakes.FakeClient
 	fakeWorkerProvider      *workerfakes.FakeWorkerProvider
-	fakeVolumeFactory       *dbfakes.FakeVolumeFactory
+	fakeVolumeRepository    *dbfakes.FakeVolumeRepository
 	fakeContainerRepository *dbfakes.FakeContainerRepository
-	fakeContainerDestroyer  *gcfakes.FakeContainerDestroyer
+	fakeDestroyer           *gcfakes.FakeDestroyer
 	dbTeamFactory           *dbfakes.FakeTeamFactory
 	dbPipelineFactory       *dbfakes.FakePipelineFactory
 	dbJobFactory            *dbfakes.FakeJobFactory
@@ -123,9 +123,9 @@ var _ = BeforeEach(func() {
 	fakeSchedulerFactory = new(jobserverfakes.FakeSchedulerFactory)
 	fakeScannerFactory = new(resourceserverfakes.FakeScannerFactory)
 
-	fakeVolumeFactory = new(dbfakes.FakeVolumeFactory)
+	fakeVolumeRepository = new(dbfakes.FakeVolumeRepository)
 	fakeContainerRepository = new(dbfakes.FakeContainerRepository)
-	fakeContainerDestroyer = new(gcfakes.FakeContainerDestroyer)
+	fakeDestroyer = new(gcfakes.FakeDestroyer)
 
 	fakeVariablesFactory = new(credsfakes.FakeVariablesFactory)
 
@@ -172,9 +172,9 @@ var _ = BeforeEach(func() {
 		dbPipelineFactory,
 		dbJobFactory,
 		dbWorkerFactory,
-		fakeVolumeFactory,
+		fakeVolumeRepository,
 		fakeContainerRepository,
-		fakeContainerDestroyer,
+		fakeDestroyer,
 		dbBuildFactory,
 
 		peerURL,

api/containers_test.go

@@ -1013,7 +1013,7 @@ var _ = Describe("Containers API", func() {
 				It("returns 404", func() {
 					response, err = client.Do(req)
 					Expect(err).NotTo(HaveOccurred())
-					Expect(fakeContainerDestroyer.DestroyCallCount()).To(Equal(0))
+					Expect(fakeDestroyer.DestroyContainersCallCount()).To(Equal(0))
 					Expect(response.StatusCode).To(Equal(http.StatusNotFound))
 				})
 
@@ -1046,7 +1046,7 @@ var _ = Describe("Containers API", func() {
 
 				Context("when there is an error", func() {
 					BeforeEach(func() {
-						fakeContainerDestroyer.DestroyReturns(errors.New("some error"))
+						fakeDestroyer.DestroyContainersReturns(errors.New("some error"))
 					})
 
 					It("returns 500", func() {
@@ -1058,7 +1058,7 @@ var _ = Describe("Containers API", func() {
 
 				Context("when containers are destroyed", func() {
 					BeforeEach(func() {
-						fakeContainerDestroyer.DestroyReturns(nil)
+						fakeDestroyer.DestroyContainersReturns(nil)
 					})
 
 					It("returns 204", func() {
@@ -1071,9 +1071,9 @@ var _ = Describe("Containers API", func() {
 				It("queries with it in the worker name", func() {
 					_, err = client.Do(req)
 					Expect(err).NotTo(HaveOccurred())
-					Expect(fakeContainerDestroyer.DestroyCallCount()).To(Equal(1))
+					Expect(fakeDestroyer.DestroyContainersCallCount()).To(Equal(1))
 
-					workerName, handles := fakeContainerDestroyer.DestroyArgsForCall(0)
+					workerName, handles := fakeDestroyer.DestroyContainersArgsForCall(0)
 					Expect(workerName).To(Equal("some-worker-name"))
 					Expect(handles).To(Equal([]string{"handle1", "handle2"}))
 				})

api/containerserver/report.go

@@ -35,7 +35,7 @@ func (s *Server) ReportWorkerContainers(w http.ResponseWriter, r *http.Request)
 			"handles": handles,
 		})
 
-		err = s.containerDestroyer.Destroy(workerName, handles)
+		err = s.destroyer.DestroyContainers(workerName, handles)
 		if err != nil {
 			logger.Error("failed-to-destroy-containers", err)
 			w.WriteHeader(http.StatusInternalServerError)

api/containerserver/server.go

@@ -15,7 +15,7 @@ type Server struct {
 	variablesFactory        creds.VariablesFactory
 	interceptTimeoutFactory InterceptTimeoutFactory
 	containerRepository     db.ContainerRepository
-	containerDestroyer      gc.ContainerDestroyer
+	destroyer               gc.Destroyer
 }
 
 func NewServer(
@@ -24,14 +24,14 @@ func NewServer(
 	variablesFactory creds.VariablesFactory,
 	interceptTimeoutFactory InterceptTimeoutFactory,
 	containerRepository db.ContainerRepository,
-	containerDestroyer gc.ContainerDestroyer,
+	destroyer gc.Destroyer,
 ) *Server {
 	return &Server{
 		logger:                  logger,
 		workerClient:            workerClient,
 		variablesFactory:        variablesFactory,
 		interceptTimeoutFactory: interceptTimeoutFactory,
 		containerRepository:     containerRepository,
-		containerDestroyer:      containerDestroyer,
+		destroyer:               destroyer,
 	}
 }

api/handler.go

@@ -45,9 +45,9 @@ func NewHandler(
 	dbPipelineFactory db.PipelineFactory,
 	dbJobFactory db.JobFactory,
 	dbWorkerFactory db.WorkerFactory,
-	volumeFactory db.VolumeFactory,
+	volumeRepository db.VolumeRepository,
 	containerRepository db.ContainerRepository,
-	containerDestroyer gc.ContainerDestroyer,
+	destroyer gc.Destroyer,
 	dbBuildFactory db.BuildFactory,
 
 	peerURL string,
@@ -92,8 +92,8 @@ func NewHandler(
 	workerServer := workerserver.NewServer(logger, dbTeamFactory, dbWorkerFactory, workerProvider)
 	logLevelServer := loglevelserver.NewServer(logger, sink)
 	cliServer := cliserver.NewServer(logger, absCLIDownloadsDir)
-	containerServer := containerserver.NewServer(logger, workerClient, variablesFactory, interceptTimeoutFactory, containerRepository, containerDestroyer)
-	volumesServer := volumeserver.NewServer(logger, volumeFactory)
+	containerServer := containerserver.NewServer(logger, workerClient, variablesFactory, interceptTimeoutFactory, containerRepository, destroyer)
+	volumesServer := volumeserver.NewServer(logger, volumeRepository, destroyer)
 	teamServer := teamserver.NewServer(logger, dbTeamFactory, externalURL)
 	infoServer := infoserver.NewServer(logger, version, workerVersion)
 	legacyServer := legacyserver.NewServer(logger)
@@ -178,6 +178,7 @@ func NewHandler(
 
 		atc.ListVolumes:           teamHandlerFactory.HandlerFor(volumesServer.ListVolumes),
 		atc.ListDestroyingVolumes: http.HandlerFunc(volumesServer.ListDestroyingVolumes),
+		atc.ReportWorkerVolumes:   http.HandlerFunc(volumesServer.ReportWorkerVolumes),
 
 		atc.LegacyListAuthMethods: http.HandlerFunc(legacyServer.ListAuthMethods),
 		atc.LegacyGetAuthToken:    http.HandlerFunc(legacyServer.GetAuthToken),

api/volumes_test.go

@@ -1,7 +1,9 @@
 package api_test
 
 import (
+	"bytes"
 	"errors"
+	"io"
 	"io/ioutil"
 	"net/http"
 	"net/url"
@@ -63,15 +65,15 @@ var _ = Describe("Volumes API", func() {
 				})
 
 				It("asks the factory for the volumes", func() {
-					Expect(fakeVolumeFactory.GetTeamVolumesCallCount()).To(Equal(1))
+					Expect(fakeVolumeRepository.GetTeamVolumesCallCount()).To(Equal(1))
 				})
 
 				Context("when getting all volumes succeeds", func() {
 					BeforeEach(func() {
 						someOtherFakeWorker := new(dbfakes.FakeWorker)
 						someOtherFakeWorker.NameReturns("some-other-worker")
 
-						fakeVolumeFactory.GetTeamVolumesStub = func(teamID int) ([]db.CreatedVolume, error) {
+						fakeVolumeRepository.GetTeamVolumesStub = func(teamID int) ([]db.CreatedVolume, error) {
 							if teamID != 1 {
 								return []db.CreatedVolume{}, nil
 							}
@@ -225,7 +227,7 @@ var _ = Describe("Volumes API", func() {
 
 				Context("when getting all volumes fails", func() {
 					BeforeEach(func() {
-						fakeVolumeFactory.GetTeamVolumesReturns([]db.CreatedVolume{}, errors.New("oh no!"))
+						fakeVolumeRepository.GetTeamVolumesReturns([]db.CreatedVolume{}, errors.New("oh no!"))
 					})
 
 					It("returns 500 Internal Server Error", func() {
@@ -235,7 +237,7 @@ var _ = Describe("Volumes API", func() {
 
 				Context("when a volume is deleted during the request", func() {
 					BeforeEach(func() {
-						fakeVolumeFactory.GetTeamVolumesStub = func(teamID int) ([]db.CreatedVolume, error) {
+						fakeVolumeRepository.GetTeamVolumesStub = func(teamID int) ([]db.CreatedVolume, error) {
 							volume1 := new(dbfakes.FakeCreatedVolume)
 							volume1.ResourceTypeReturns(nil, errors.New("Something"))
 
@@ -332,13 +334,13 @@ var _ = Describe("Volumes API", func() {
 				})
 
 				It("asks the factory for the detroying volumes", func() {
-					Expect(fakeVolumeFactory.GetDestroyingVolumesCallCount()).To(Equal(1))
-					Expect(fakeVolumeFactory.GetDestroyingVolumesArgsForCall(0)).To(Equal("some-worker-name"))
+					Expect(fakeVolumeRepository.GetDestroyingVolumesCallCount()).To(Equal(1))
+					Expect(fakeVolumeRepository.GetDestroyingVolumesArgsForCall(0)).To(Equal("some-worker-name"))
 				})
 
 				Context("when getting all destroying volumes succeeds", func() {
 					BeforeEach(func() {
-						fakeVolumeFactory.GetDestroyingVolumesReturns([]string{
+						fakeVolumeRepository.GetDestroyingVolumesReturns([]string{
 							"volume1",
 							"volume2",
 						}, nil)
@@ -361,7 +363,7 @@ var _ = Describe("Volumes API", func() {
 
 					Context("when getting all volumes fails", func() {
 						BeforeEach(func() {
-							fakeVolumeFactory.GetDestroyingVolumesReturns([]string{}, errors.New("oh no!"))
+							fakeVolumeRepository.GetDestroyingVolumesReturns([]string{}, errors.New("oh no!"))
 						})
 
 						It("returns 500 Internal Server Error", func() {
@@ -371,7 +373,7 @@ var _ = Describe("Volumes API", func() {
 
 					Context("when list of volume is empty", func() {
 						BeforeEach(func() {
-							fakeVolumeFactory.GetDestroyingVolumesReturns([]string{}, nil)
+							fakeVolumeRepository.GetDestroyingVolumesReturns([]string{}, nil)
 						})
 
 						It("returns empty list of volumes", func() {
@@ -394,4 +396,115 @@ var _ = Describe("Volumes API", func() {
 			})
 		})
 	})
+
+	Describe("GET /api/v1/volumes/report", func() {
+		var response *http.Response
+		var req *http.Request
+		var body io.Reader
+		var err error
+
+		BeforeEach(func() {
+			body = bytes.NewBufferString(`
+				[
+					"handle1",
+					"handle2"
+				]
+			`)
+		})
+		JustBeforeEach(func() {
+			fakeAccessor.CreateReturns(fakeaccess)
+			req, err = http.NewRequest("PUT", server.URL+"/api/v1/volumes/report", body)
+			Expect(err).NotTo(HaveOccurred())
+			req.Header.Set("Content-Type", "application/json")
+		})
+
+		Context("when not authenticated", func() {
+			BeforeEach(func() {
+				fakeaccess.IsAuthenticatedReturns(false)
+			})
+
+			It("returns 401 Unauthorized", func() {
+				response, err = client.Do(req)
+				Expect(err).NotTo(HaveOccurred())
+				Expect(response.StatusCode).To(Equal(http.StatusUnauthorized))
+			})
+		})
+
+		Context("when authenticated as system", func() {
+			BeforeEach(func() {
+				fakeaccess.IsAuthenticatedReturns(true)
+				fakeaccess.IsSystemReturns(true)
+			})
+
+			Context("with no params", func() {
+				It("returns 404", func() {
+					response, err = client.Do(req)
+					Expect(err).NotTo(HaveOccurred())
+					Expect(fakeDestroyer.DestroyVolumesCallCount()).To(Equal(0))
+					Expect(response.StatusCode).To(Equal(http.StatusNotFound))
+				})
+
+				It("returns Content-Type application/json", func() {
+					response, err = client.Do(req)
+					Expect(err).NotTo(HaveOccurred())
+					Expect(response.StatusCode).To(Equal(http.StatusNotFound))
+					Expect(response.Header.Get("Content-Type")).To(Equal("application/json"))
+				})
+			})
+
+			Context("querying with worker name", func() {
+				JustBeforeEach(func() {
+					req.URL.RawQuery = url.Values{
+						"worker_name": []string{"some-worker-name"},
+					}.Encode()
+				})
+
+				Context("with invalid json", func() {
+					BeforeEach(func() {
+						body = bytes.NewBufferString(`{}`)
+					})
+
+					It("returns 400", func() {
+						response, err = client.Do(req)
+						Expect(err).NotTo(HaveOccurred())
+						Expect(response.StatusCode).To(Equal(http.StatusBadRequest))
+					})
+				})
+
+				Context("when there is an error", func() {
+					BeforeEach(func() {
+						fakeDestroyer.DestroyVolumesReturns(errors.New("some error"))
+					})
+
+					It("returns 500", func() {
+						response, err = client.Do(req)
+						Expect(err).NotTo(HaveOccurred())
+						Expect(response.StatusCode).To(Equal(http.StatusInternalServerError))
+					})
+				})
+
+				Context("when volumes are destroyed", func() {
+					BeforeEach(func() {
+						fakeDestroyer.DestroyVolumesReturns(nil)
+					})
+
+					It("returns 204", func() {
+						response, err = client.Do(req)
+						Expect(err).NotTo(HaveOccurred())
+						Expect(response.StatusCode).To(Equal(http.StatusNoContent))
+					})
+				})
+
+				It("queries with it in the worker name", func() {
+					_, err = client.Do(req)
+					Expect(err).NotTo(HaveOccurred())
+					Expect(fakeDestroyer.DestroyVolumesCallCount()).To(Equal(1))
+
+					workerName, handles := fakeDestroyer.DestroyVolumesArgsForCall(0)
+					Expect(workerName).To(Equal("some-worker-name"))
+					Expect(handles).To(Equal([]string{"handle1", "handle2"}))
+				})
+			})
+		})
+	})
 })

api/volumeserver/destroying.go

@@ -14,7 +14,7 @@ func (s *Server) ListDestroyingVolumes(w http.ResponseWriter, r *http.Request) {
 	hLog := s.logger.Session("marked-volumes-for-worker", lager.Data{"worker_name": workerName})
 
 	if workerName != "" {
-		volumeHandles, err := s.factory.GetDestroyingVolumes(workerName)
+		volumeHandles, err := s.repository.GetDestroyingVolumes(workerName)
 		if err != nil {
 			hLog.Error("failed-to-find-destroying-volumes", err)
 			w.WriteHeader(http.StatusInternalServerError)

api/volumeserver/list.go

@@ -16,7 +16,7 @@ func (s *Server) ListVolumes(team db.Team) http.Handler {
 	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		hLog.Debug("listing")
 
-		volumes, err := s.factory.GetTeamVolumes(team.ID())
+		volumes, err := s.repository.GetTeamVolumes(team.ID())
 		if err != nil {
 			hLog.Error("failed-to-find-volumes", err)
 			w.WriteHeader(http.StatusInternalServerError)