bug fix: Schedules can not be removed in synchronization when an appl…

…ication is deleted directly cloudfoundry#553 (cloudfoundry#554)

* bug fix: Schedules can not be removed in synchronization when an application is deleted directly cloudfoundry#553

* modify log

scheduler/src/main/java/org/cloudfoundry/autoscaler/scheduler/service/ScheduleManager.java

@@ -479,15 +479,11 @@ private void notifyScalingEngineForDelete(String appId, long scheduleId) {
 		try {
 			restOperations.delete(scalingEnginePathActiveSchedule);
 		} catch (HttpStatusCodeException hce) {
-			if (hce.getStatusCode() == HttpStatus.NOT_FOUND) {
-				message = messageBundleResourceHelper
-						.lookupMessage("scalingengine.notification.activeschedule.notFound", appId, scheduleId);
-				logger.info(message, hce);
-			} else {
-				String errorMessage = messageBundleResourceHelper.lookupMessage("scalingengine.notification.error",
-						hce.getResponseBodyAsString(), appId, scheduleId, "delete");
-				throw new SchedulerInternalException(errorMessage, hce);
-			}
+
+			String errorMessage = messageBundleResourceHelper.lookupMessage("scalingengine.notification.error",
+					hce.getResponseBodyAsString(), appId, scheduleId, "delete");
+			throw new SchedulerInternalException(errorMessage, hce);
+
 		} catch (ResourceAccessException rae) {
 			String errorMessage = messageBundleResourceHelper.lookupMessage("scalingengine.notification.error",
 					rae.getMessage(), appId, scheduleId, "delete");

scheduler/src/test/java/org/cloudfoundry/autoscaler/scheduler/service/ScheduleManagerTest.java

@@ -535,7 +535,7 @@ public void testDeleteSchedules_when_activeSchedule_not_found_in_scalingEngine()
 		String scalingEnginePathActiveSchedule = scalingEngineUrl + "/v1/apps/" + appId + "/active_schedules/"
 				+ scheduleId;
 		mockServer.expect(ExpectedCount.times(1), requestTo(scalingEnginePathActiveSchedule))
-				.andExpect(method(HttpMethod.DELETE)).andRespond(withStatus(HttpStatus.NOT_FOUND));
+				.andExpect(method(HttpMethod.DELETE)).andRespond(withStatus(HttpStatus.OK));
 
 		scheduleManager.deleteSchedules(appId);
 
@@ -554,11 +554,6 @@ public void testDeleteSchedules_when_activeSchedule_not_found_in_scalingEngine()
 		Mockito.verify(activeScheduleDao, Mockito.times(1)).deleteActiveSchedulesByAppId(appId);
 
 		mockServer.verify();
-
-		String expectedMessage = messageBundleResourceHelper
-				.lookupMessage("scalingengine.notification.activeschedule.notFound", appId, scheduleId);
-		assertThat(logCaptor.getValue().getMessage().getFormattedMessage(), Is.is(expectedMessage));
-		assertThat("Log level should be INFO", logCaptor.getValue().getLevel(), Is.is(Level.INFO));
 	}
 
 	@Test

src/autoscaler/scalingengine/cmd/scalingengine/scalingengine_test.go

@@ -190,7 +190,7 @@ var _ = Describe("Main", func() {
 
 			rsp, err = httpClient.Do(req)
 			Expect(err).NotTo(HaveOccurred())
-			Expect(rsp.StatusCode).To(Equal(http.StatusNoContent))
+			Expect(rsp.StatusCode).To(Equal(http.StatusOK))
 			rsp.Body.Close()
 		})
 	})

src/autoscaler/scalingengine/scalingengine.go

@@ -33,6 +33,8 @@ type scalingEngine struct {
 
 type ActiveScheduleNotFoundError struct {
 }
+type AppNotFoundError struct {
+}
 
 func (ase *ActiveScheduleNotFoundError) Error() string {
 	return fmt.Sprintf("active schedule not found")
@@ -306,9 +308,8 @@ func (s *scalingEngine) RemoveActiveSchedule(appId string, scheduleId string) er
 	}
 
 	if (currentSchedule == nil) || (currentSchedule.ScheduleId != scheduleId) {
-		err = &ActiveScheduleNotFoundError{}
-		logger.Error("failed-to-remove-active-schedule", err)
-		return err
+		logger.Info("active-schedule-not-found", lager.Data{"appId": appId, "scheduleId": scheduleId})
+		return nil
 	}
 
 	err = s.scalingEngineDB.RemoveActiveSchedule(appId)
@@ -330,6 +331,11 @@ func (s *scalingEngine) RemoveActiveSchedule(appId string, scheduleId string) er
 
 	appEntity, err := s.cfClient.GetApp(appId)
 	if err != nil {
+		if _, ok := err.(*models.AppNotFoundErr); ok {
+			logger.Info("app-not-found", lager.Data{"appId": appId})
+			history.Status = models.ScalingStatusIgnored
+			return nil
+		}
 		logger.Error("failed-to-get-app-info", err)
 		history.Status = models.ScalingStatusFailed
 		history.Error = "failed to get app info: " + err.Error()

src/autoscaler/scalingengine/scalingengine_test.go

@@ -967,8 +967,8 @@ var _ = Describe("ScalingEngine", func() {
 			BeforeEach(func() {
 				scalingEngineDB.GetActiveScheduleReturns(nil, nil)
 			})
-			It("should error", func() {
-				Expect(err).To(BeAssignableToTypeOf(&ActiveScheduleNotFoundError{}))
+			It("should not have any error", func() {
+				Expect(err).NotTo(HaveOccurred())
 				Expect(scalingEngineDB.RemoveActiveScheduleCallCount()).To(BeZero())
 			})
 		})
@@ -977,8 +977,8 @@ var _ = Describe("ScalingEngine", func() {
 			BeforeEach(func() {
 				scalingEngineDB.GetActiveScheduleReturns(&models.ActiveSchedule{ScheduleId: "a-different-schedule-id"}, nil)
 			})
-			It("should error", func() {
-				Expect(err).To(BeAssignableToTypeOf(&ActiveScheduleNotFoundError{}))
+			It("should not have any error", func() {
+				Expect(err).NotTo(HaveOccurred())
 				Expect(scalingEngineDB.RemoveActiveScheduleCallCount()).To(BeZero())
 			})
 		})

src/autoscaler/scalingengine/server/scaling_handler.go

@@ -220,22 +220,13 @@ func (h *ScalingHandler) RemoveActiveSchedule(w http.ResponseWriter, r *http.Req
 
 	if err != nil {
 		logger.Error("failed-to-remove-active-schedule", err)
-		switch err.(type) {
-		case *scalingengine.ActiveScheduleNotFoundError:
-			handlers.WriteJSONResponse(w, http.StatusNotFound, models.ErrorResponse{
-				Code:    "Not-Found",
-				Message: "Active schedule not found",
-			})
-		default:
-			handlers.WriteJSONResponse(w, http.StatusInternalServerError, models.ErrorResponse{
-				Code:    "Interal-Server-Error",
-				Message: "Error removing active schedule"})
-
-		}
+		handlers.WriteJSONResponse(w, http.StatusInternalServerError, models.ErrorResponse{
+			Code:    "Interal-Server-Error",
+			Message: "Error removing active schedule"})
 		return
 	}
 
-	w.WriteHeader(http.StatusNoContent)
+	w.WriteHeader(http.StatusOK)
 }
 
 func (h *ScalingHandler) GetActiveSchedule(w http.ResponseWriter, r *http.Request, vars map[string]string) {

src/autoscaler/scalingengine/server/scaling_handler_test.go

@@ -4,7 +4,6 @@ import (
 	"autoscaler/db"
 	"autoscaler/fakes"
 	"autoscaler/models"
-	"autoscaler/scalingengine"
 	. "autoscaler/scalingengine/server"
 
 	"code.cloudfoundry.org/lager/lagertest"
@@ -514,26 +513,8 @@ var _ = Describe("ScalingHandler", func() {
 		})
 
 		Context("when removing active schedule succeeds", func() {
-			It("returns 204", func() {
-				Expect(resp.Code).To(Equal(http.StatusNoContent))
-			})
-		})
-
-		Context("when active schedule is not found", func() {
-			BeforeEach(func() {
-				scalingEngine.RemoveActiveScheduleReturns(&scalingengine.ActiveScheduleNotFoundError{})
-			})
-
-			It("returns 404", func() {
-				Expect(resp.Code).To(Equal(http.StatusNotFound))
-
-				errJson := &models.ErrorResponse{}
-				err = json.Unmarshal(resp.Body.Bytes(), errJson)
-				Expect(err).ToNot(HaveOccurred())
-				Expect(errJson).To(Equal(&models.ErrorResponse{
-					Code:    "Not-Found",
-					Message: "Active schedule not found",
-				}))
+			It("returns 200", func() {
+				Expect(resp.Code).To(Equal(http.StatusOK))
 			})
 		})
 

src/autoscaler/scalingengine/server/server_test.go

@@ -190,9 +190,9 @@ var _ = Describe("Server", func() {
 				method = http.MethodDelete
 			})
 			Context("when requesting correctly", func() {
-				It("should return 204", func() {
+				It("should return 200", func() {
 					Expect(err).ToNot(HaveOccurred())
-					Expect(rsp.StatusCode).To(Equal(http.StatusNoContent))
+					Expect(rsp.StatusCode).To(Equal(http.StatusOK))
 					rsp.Body.Close()
 				})
 			})