[HWORKS-483] Add creating status for deployments (#1344) (#1332) (#1333)

Co-authored-by: Javier de la Rúa Martínez <[email protected]>

Author: robzor92

hopsworks-IT/src/test/ruby/spec/helpers/serving_helper.rb

@@ -124,6 +124,8 @@ def delete_all_servings(project_id)
   end
 
   def start_serving(project, serving)
+    #Stopped status can happen if the deployment is reused, which is the case in some of the tests
+    wait_for_serving_status(project, serving[:name], ["Created", "Stopped"])
     post "#{ENV['HOPSWORKS_API']}/project/#{project[:id]}/serving/#{serving[:id]}?action=start"
     expect_status_details(200)
   end
@@ -201,18 +203,18 @@ def parse_inference_logging(value)
            end
   end
 
-  def get_serving(serving_name)
-    serving_list = get "#{ENV['HOPSWORKS_API']}/project/#{@project[:id]}/serving/"
+  def get_serving(project, serving_name)
+    serving_list = get "#{ENV['HOPSWORKS_API']}/project/#{project[:id]}/serving/"
     expect_status_details(200)
     servings = JSON.parse(serving_list)
     servings.select { |serving| serving['name'] == serving_name}[0]
   end
 
   def wait_for_serving_status(serving_name, status, timeout: 180, delay: 10)
     wait_result = wait_for_me_time(timeout, delay) do
-      result = get_serving(serving_name)
-      { 'success' => result['status'].eql?(status), 'status' => result['status'] }
+      result = get_serving(project, serving_name)
+      { 'success' => accepted_status.include?(result['status']), 'status' => result['status'] }
     end
-    expect(wait_result["status"]).to eql(status)
+    expect(accepted_status.include?(wait_result["status"]))
   end
 end

hopsworks-IT/src/test/ruby/spec/serving_spec.rb

@@ -926,6 +926,16 @@
         delete_all_servings(@project[:id])
       end
 
+      it "should fail to start a deployment that is being creating" do
+        result = get_serving(@project, @serving[:name])
+        if result['status'] == "Creating"
+          post "#{ENV['HOPSWORKS_API']}/project/#{@project[:id]}/serving/#{@serving[:id]}?action=start"
+          expect_status_details(400, error_code: 240003)
+        else
+          skip "Serving already created"
+        end
+      end
+
       it "should be able to start a serving instance" do
         start_serving(@project, @serving)
         wait_for_type(@serving[:name])
@@ -1006,22 +1016,32 @@
         with_tensorflow_serving(@project[:id], @project[:projectname], @user[:username])
 
         start_serving(@project, @serving)
-        wait_for_serving_status(@serving[:name], "Running")
+        wait_for_serving_status(@project, @serving[:name], ["Running"])
       end
 
       after :all do
         purge_all_tf_serving_instances
         delete_all_servings(@project[:id])
       end
 
-      it "should be able to kill a running serving instance" do
+      it "should fail to stop a deployment that is being creating" do
+        result = get_serving(@project, @serving[:name])
+        if result['status'] == "Creating"
+          post "#{ENV['HOPSWORKS_API']}/project/#{@project[:id]}/serving/#{@serving[:id]}?action=stop"
+          expect_status_details(400, error_code: 240003)
+        else
+          skip "Serving already created"
+        end
+      end
+
+      it "should be able to stop a running serving instance" do
         post "#{ENV['HOPSWORKS_API']}/project/#{@project[:id]}/serving/#{@serving[:id]}?action=stop"
         expect_status_details(200)
 
-        wait_for_serving_status(@serving[:name], "Stopped")
+        wait_for_serving_status(@project, @serving[:name], ["Stopped"])
       end
 
-      it "should fail to kill a non running instance" do
+      it "should fail to stop a non running instance" do
         # serving is already stopped
 
         post "#{ENV['HOPSWORKS_API']}/project/#{@project[:id]}/serving/#{@serving[:id]}?action=stop"
@@ -1034,7 +1054,7 @@
         end
 
         start_serving(@project, @serving)
-        wait_for_serving_status(@serving[:name], "Running")
+        wait_for_serving_status(@project, @serving[:name], ["Running"])
 
         # Simulate the process dying by its own
         system "pgrep -f tensorflow_model_server | xargs kill -9"
@@ -1093,6 +1113,15 @@
         delete_all_servings(@project[:id])
       end
 
+      it "should delete a deployment that is being creating" do
+        result = get_serving(@project, @serving[:name])
+        delete "#{ENV['HOPSWORKS_API']}/project/#{@project[:id]}/serving/#{@serving[:id]}"
+        expect_status_details(200)
+        if result['status'] != "Creating"
+          skip "Serving already created"
+        end
+      end
+
       it "should delete a serving instance" do
         delete "#{ENV['HOPSWORKS_API']}/project/#{@project[:id]}/serving/#{@serving[:id]}"
         expect_status_details(200)
@@ -1149,20 +1178,23 @@
       before :all do
         with_valid_project
         copy_mnist_files(@project[:projectname], @user[:username])
-        create_tensorflow_serving(@project[:id], @project[:projectname])
-        create_tensorflow_serving(@project[:id], @project[:projectname])
-        @tf_serving = create_tensorflow_serving(@project[:id], @project[:projectname])
+        @tf_serving1 = create_tensorflow_serving(@project[:id], @project[:projectname])
+        @tf_serving2 = create_tensorflow_serving(@project[:id], @project[:projectname])
+        @tf_serving3 = create_tensorflow_serving(@project[:id], @project[:projectname])
       end
 
       it "should return all servings" do
         get_servings(@project, nil)
         expect_status_details(200)
-        json_body.each {|model| expect(model[:status]).to eq "Created"}
+        json_body.each {|model| expect(["Creating", "Created"].find { |s| s == model[:status] }).to_not be_nil }
         expect(json_body.length).to eq 3
       end
 
       describe "#status" do
         it "should return all servings in created state" do
+          wait_for_serving_status(@project, @tf_serving1[:name], ["Created"])
+          wait_for_serving_status(@project, @tf_serving2[:name], ["Created"])
+          wait_for_serving_status(@project, @tf_serving3[:name], ["Created"])
           get_servings(@project, "?status=Created")
           expect_status_details(200)
           json_body.each {|model| expect(model[:status]).to eq "Created"}
@@ -1176,8 +1208,8 @@
         end
 
         it "should return single running serving" do
-          start_serving(@project, @tf_serving)
-          wait_for_serving_status(@tf_serving[:name], "Running")
+          start_serving(@project, @tf_serving3)
+          wait_for_serving_status(@project, @tf_serving3[:name], ["Running"])
 
           get_servings(@project, "?status=Running")
           expect_status_details(200)

hopsworks-common/src/main/java/io/hops/hopsworks/common/security/QuotasEnforcement.java

@@ -180,11 +180,10 @@ private List<ServingWrapper> getAllServings(Project project) throws ServingExcep
   }
 
   private boolean isDeploymentRunning(ServingWrapper serving) {
-    return serving.getStatus().equals(ServingStatusEnum.STARTING)
-            || serving.getStatus().equals(ServingStatusEnum.FAILED)
-            || serving.getStatus().equals(ServingStatusEnum.RUNNING)
-            || serving.getStatus().equals(ServingStatusEnum.IDLE)
-            || serving.getStatus().equals(ServingStatusEnum.UPDATING);
+    return !serving.getStatus().equals(ServingStatusEnum.CREATING)
+        && !serving.getStatus().equals(ServingStatusEnum.CREATED)
+        && !serving.getStatus().equals(ServingStatusEnum.STOPPING)
+        && !serving.getStatus().equals(ServingStatusEnum.STOPPED);
   }
 
   private void enforceFeaturegroupsQuotaInternal(Featurestore featurestore, List<Featuregroup> featuregroups,

hopsworks-common/src/main/java/io/hops/hopsworks/common/serving/ServingStatusCondition.java

@@ -37,6 +37,7 @@ public class ServingStatusCondition {
   public static final String UnscheduledInProgress = "Waiting for being unscheduled";
   public static final String StoppedInProgress = "Stopping deployment";
   public static final String StoppedSuccess = "Deployment is not running";
+  public static final String StoppedCreating = "Deployment is being prepared";
 
   public ServingStatusCondition() {
   }
@@ -130,4 +131,8 @@ public static ServingStatusCondition getStoppedInProgressCondition() {
   public static ServingStatusCondition getStoppedSuccessCondition() {
     return new ServingStatusCondition(ServingStatusConditionEnum.STOPPED, true, StoppedSuccess);
   }
+  
+  public static ServingStatusCondition getStoppedCreatingCondition() {
+    return new ServingStatusCondition(ServingStatusConditionEnum.STOPPED, StoppedCreating);
+  }
 }

hopsworks-common/src/main/java/io/hops/hopsworks/common/serving/ServingStatusEnum.java

@@ -43,6 +43,7 @@
 import com.fasterxml.jackson.annotation.JsonValue;
 
 public enum ServingStatusEnum {
+  CREATING("Creating"),
   CREATED("Created"),
   STARTING("Starting"),
   FAILED("Failed"),

hopsworks-common/src/test/io/hops/hopsworks/common/security/TestQuotasEnforcement.java

@@ -222,36 +222,40 @@ public void testQuotasRunningModelDeployments() throws Exception {
     List<ServingWrapper> mockServings = new ArrayList<>();
 
     ServingWrapper sw0 = new ServingWrapper(new Serving());
-    sw0.setStatus(ServingStatusEnum.CREATED);
+    sw0.setStatus(ServingStatusEnum.CREATING);
     mockServings.add(sw0);
 
     ServingWrapper sw1 = new ServingWrapper(new Serving());
-    sw1.setStatus(ServingStatusEnum.STARTING);
+    sw1.setStatus(ServingStatusEnum.CREATED);
     mockServings.add(sw1);
 
     ServingWrapper sw2 = new ServingWrapper(new Serving());
-    sw2.setStatus(ServingStatusEnum.FAILED);
+    sw2.setStatus(ServingStatusEnum.STARTING);
     mockServings.add(sw2);
 
     ServingWrapper sw3 = new ServingWrapper(new Serving());
-    sw3.setStatus(ServingStatusEnum.RUNNING);
+    sw3.setStatus(ServingStatusEnum.FAILED);
     mockServings.add(sw3);
-  
+
     ServingWrapper sw4 = new ServingWrapper(new Serving());
-    sw4.setStatus(ServingStatusEnum.IDLE);
+    sw4.setStatus(ServingStatusEnum.RUNNING);
     mockServings.add(sw4);
-    
+  
     ServingWrapper sw5 = new ServingWrapper(new Serving());
-    sw5.setStatus(ServingStatusEnum.UPDATING);
+    sw5.setStatus(ServingStatusEnum.IDLE);
     mockServings.add(sw5);
 
     ServingWrapper sw6 = new ServingWrapper(new Serving());
-    sw6.setStatus(ServingStatusEnum.STOPPING);
+    sw6.setStatus(ServingStatusEnum.UPDATING);
     mockServings.add(sw6);
-  
+
     ServingWrapper sw7 = new ServingWrapper(new Serving());
-    sw7.setStatus(ServingStatusEnum.STOPPED);
+    sw7.setStatus(ServingStatusEnum.STOPPING);
     mockServings.add(sw7);
+  
+    ServingWrapper sw8 = new ServingWrapper(new Serving());
+    sw8.setStatus(ServingStatusEnum.STOPPED);
+    mockServings.add(sw8);
 
     ServingController servingController = Mockito.mock(ServingController.class);
     Mockito.when(servingController.getAll(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any())).thenReturn(mockServings);