refactor deployment create

Author: sethboyles

app/actions/deployment_create.rb

@@ -13,6 +13,9 @@ def create(app:, user_audit_info:, message:)
         DeploymentModel.db.transaction do
           app.lock!
 
+          # Stopped apps will have quota validated since we their process up immediately
+          validate_quota!(message, app) unless app.stopped?
+
           message.strategy ||= DeploymentModel::ROLLING_STRATEGY
 
           target_state = DeploymentTargetState.new(app, message)
@@ -29,46 +32,43 @@ def create(app:, user_audit_info:, message:)
 
           previous_deployment = DeploymentModel.find(app: app, status_value: DeploymentModel::ACTIVE_STATUS_VALUE)
 
+          deployment = create_deployment(
+            app,
+            message,
+            previous_deployment,
+            previous_droplet,
+            revision,
+            target_state,
+            user_audit_info
+          )
+
           if app.stopped?
-            return deployment_for_stopped_app(
-              app,
-              message,
-              previous_deployment,
-              previous_droplet,
-              revision,
-              target_state,
-              user_audit_info
-            )
+            process = app.newest_web_process
+          else
+            process_instances = starting_process_instances(deployment, desired_instances(app.oldest_web_process, previous_deployment))
+            process = create_deployment_process(app, deployment.guid, revision, process_instances)
           end
 
-          desired_instances = desired_instances(app.oldest_web_process, previous_deployment)
-
-          validate_quota!(message, app)
-
-          deployment = DeploymentModel.create(
-            app: app,
-            state: starting_state(message),
-            status_value: DeploymentModel::ACTIVE_STATUS_VALUE,
-            status_reason: DeploymentModel::DEPLOYING_STATUS_REASON,
-            droplet: target_state.droplet,
-            previous_droplet: previous_droplet,
-            original_web_process_instance_count: desired_instances,
-            revision_guid: revision&.guid,
-            revision_version: revision&.version,
-            strategy: message.strategy,
-            memory_in_mb: message.memory_in_mb,
-            max_in_flight: message.max_in_flight,
-            canary_steps: message.options&.dig(:canary, :steps),
-            web_instances: message.web_instances || desired_instances
-          )
+          process.memory = message.memory_in_mb if message.memory_in_mb
 
-          MetadataUpdate.update(deployment, message)
+          if app.stopped?
+            process.instances = message.web_instances if message.web_instances
 
-          supersede_deployment(previous_deployment)
+            process.save_changes
+
+            # Do not create a revision here because AppStart will not handle the rollback case
+            AppStart.start(app: app, user_audit_info: user_audit_info, create_revision: false)
+            deployment.update(state: DeploymentModel::DEPLOYED_STATE,
+                              status_value: DeploymentModel::FINALIZED_STATUS_VALUE,
+                              status_reason: DeploymentModel::DEPLOYED_STATUS_REASON)
+            record_audit_event(deployment, target_state.droplet, user_audit_info, message)
+            return deployment
+          end
 
-          process_instances = starting_process_instances(deployment, desired_instances)
+          process.save
+
+          supersede_deployment(previous_deployment)
 
-          process = create_deployment_process(app, message, deployment.guid, revision, process_instances)
           # Need to transition from STOPPED to STARTED to engage the ProcessObserver to desire the LRP.
           # It'd be better to do this via Diego::Runner.new(process, config).start,
           # but it is nontrivial to get that working in test.
@@ -86,8 +86,8 @@ def create(app:, user_audit_info:, message:)
         raise error
       end
 
-      def create_deployment_process(app, message, deployment_guid, revision, process_instances)
-        process = clone_existing_web_process(app, message, revision, process_instances)
+      def create_deployment_process(app, deployment_guid, revision, process_instances)
+        process = clone_existing_web_process(app, revision, process_instances)
 
         DeploymentProcessModel.create(
           deployment_guid: deployment_guid,
@@ -98,7 +98,7 @@ def create_deployment_process(app, message, deployment_guid, revision, process_i
         process
       end
 
-      def clone_existing_web_process(app, message, revision, process_instances)
+      def clone_existing_web_process(app, revision, process_instances)
         web_process = app.newest_web_process
         command = if revision
                     revision.commands_by_process_type[ProcessTypes::WEB]
@@ -111,7 +111,7 @@ def clone_existing_web_process(app, message, revision, process_instances)
           state: ProcessModel::STOPPED,
           instances: process_instances,
           command: command,
-          memory: message.memory_in_mb || web_process.memory,
+          memory: web_process.memory,
           file_descriptors: web_process.file_descriptors,
           disk_quota: web_process.disk_quota,
           log_rate_limit: web_process.log_rate_limit,
@@ -181,33 +181,24 @@ def validate_quota!(message, app)
         current_web_process.reload
       end
 
-      def deployment_for_stopped_app(app, message, previous_deployment, previous_droplet, revision, target_state, user_audit_info)
-        app.newest_web_process.update(instances: message.web_instances) if message.web_instances
-        app.newest_web_process.update(memory: message.memory_in_mb) if message.memory_in_mb
-        # Do not create a revision here because AppStart will not handle the rollback case
-        AppStart.start(app: app, user_audit_info: user_audit_info, create_revision: false)
-
+      def create_deployment(app, message, previous_deployment, previous_droplet, revision, target_state, user_audit_info)
         deployment = DeploymentModel.create(
           app: app,
-          state: DeploymentModel::DEPLOYED_STATE,
-          status_value: DeploymentModel::FINALIZED_STATUS_VALUE,
-          status_reason: DeploymentModel::DEPLOYED_STATUS_REASON,
+          state: starting_state(message),
+          status_value: DeploymentModel::ACTIVE_STATUS_VALUE,
+          status_reason: DeploymentModel::DEPLOYING_STATUS_REASON,
           droplet: target_state.droplet,
           previous_droplet: previous_droplet,
           original_web_process_instance_count: desired_instances(app.oldest_web_process, previous_deployment),
           revision_guid: revision&.guid,
           revision_version: revision&.version,
           strategy: message.strategy,
+          memory_in_mb: message.memory_in_mb,
           max_in_flight: message.max_in_flight,
           canary_steps: message.options&.dig(:canary, :steps),
-          memory_in_mb: message.memory_in_mb,
           web_instances: message.web_instances || desired_instances(app.oldest_web_process, previous_deployment)
         )
-
         MetadataUpdate.update(deployment, message)
-
-        record_audit_event(deployment, target_state.droplet, user_audit_info, message)
-
         deployment
       end