Scale disk, memory, and log rate with deployments (#4264)

sethboyles · web-flow · commit 47446d341fb7 · 2025-03-28T09:59:36.000-07:00
* also dont set web_instances unless explicitly set
diff --git a/app/actions/deployment_create.rb b/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 scale their process up immediately later
+          validate_quota!(message, app) unless app.stopped?
+
           message.strategy ||= DeploymentModel::ROLLING_STRATEGY
 
           target_state = DeploymentTargetState.new(app, message)
@@ -29,45 +32,45 @@ 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,
-            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
+          process.disk_quota = message.disk_in_mb if message.disk_in_mb
+          process.log_rate_limit = message.log_rate_limit_in_bytes_per_second if message.log_rate_limit_in_bytes_per_second
 
-          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, 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.
@@ -104,7 +107,6 @@ def clone_existing_web_process(app, revision, process_instances)
                   else
                     web_process.command
                   end
-
         ProcessModel.create(
           app: app,
           type: ProcessTypes::WEB,
@@ -167,43 +169,42 @@ def record_audit_event(deployment, droplet, user_audit_info, message)
       private
 
       def validate_quota!(message, app)
-        return if message.web_instances.blank?
+        return if message.web_instances.blank? && message.memory_in_mb.blank? && message.log_rate_limit_in_bytes_per_second.blank?
 
         current_web_process = app.newest_web_process
-        current_web_process.instances = message.web_instances
-
+        current_web_process.instances = message.web_instances if message.web_instances
+        current_web_process.memory = message.memory_in_mb if message.memory_in_mb
+        current_web_process.disk_quota = message.disk_in_mb if message.disk_in_mb
+        current_web_process.log_rate_limit = message.log_rate_limit_in_bytes_per_second if message.log_rate_limit_in_bytes_per_second
+        # Quotas wont get checked unless the process is started
+        current_web_process.state = ProcessModel::STARTED
         current_web_process.validate
 
         raise Sequel::ValidationFailed.new(current_web_process) unless current_web_process.valid?
 
         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
-        # 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,
           max_in_flight: message.max_in_flight,
+          memory_in_mb: message.memory_in_mb,
+          disk_in_mb: message.disk_in_mb,
+          log_rate_limit_in_bytes_per_second: message.log_rate_limit_in_bytes_per_second,
           canary_steps: message.options&.dig(:canary, :steps),
-          web_instances: message.web_instances || desired_instances(app.oldest_web_process, previous_deployment)
+          web_instances: message.web_instances
         )
-
         MetadataUpdate.update(deployment, message)
-
-        record_audit_event(deployment, target_state.droplet, user_audit_info, message)
-
         deployment
       end
 
diff --git a/app/messages/deployment_create_message.rb b/app/messages/deployment_create_message.rb
@@ -1,4 +1,5 @@
 require 'messages/metadata_base_message'
+require 'messages/process_scale_message'
 
 module VCAP::CloudController
   class DeploymentCreateMessage < MetadataBaseMessage
@@ -14,6 +15,9 @@ class DeploymentCreateMessage < MetadataBaseMessage
       canary
       max_in_flight
       web_instances
+      memory_in_mb
+      disk_in_mb
+      log_rate_limit_in_bytes_per_second
     ].freeze
 
     ALLOWED_STEP_KEYS = [
@@ -48,6 +52,18 @@ def web_instances
       options&.dig(:web_instances)
     end
 
+    def memory_in_mb
+      options&.dig(:memory_in_mb)
+    end
+
+    def disk_in_mb
+      options&.dig(:disk_in_mb)
+    end
+
+    def log_rate_limit_in_bytes_per_second
+      options&.dig(:log_rate_limit_in_bytes_per_second)
+    end
+
     private
 
     def mutually_exclusive_droplet_sources
@@ -66,12 +82,31 @@ def validate_options
 
       disallowed_keys = options.keys - ALLOWED_OPTION_KEYS
       errors.add(:options, "has unsupported key(s): #{disallowed_keys.join(', ')}") if disallowed_keys.present?
-
-      validate_web_instances if options[:web_instances]
+      validate_scaling_options
       validate_max_in_flight if options[:max_in_flight]
       validate_canary if options[:canary]
     end
 
+    def validate_scaling_options
+      scaling_options = {
+        instances: options[:web_instances],
+        memory_in_mb: options[:memory_in_mb],
+        disk_in_mb: options[:disk_in_mb],
+        log_rate_limit_in_bytes_per_second: options[:log_rate_limit_in_bytes_per_second]
+      }
+
+      message = ProcessScaleMessage.new(scaling_options)
+      message.valid?
+      if message.errors[:instances].present?
+        message.errors.select { |e| e.attribute == :instances }.each do |error|
+          errors.import(error, { attribute: :web_instances })
+        end
+        message.errors.delete(:instances)
+      end
+
+      errors.merge!(message.errors)
+    end
+
     def validate_max_in_flight
       max_in_flight = options[:max_in_flight]
 
diff --git a/app/models/runtime/constraints/max_log_rate_limit_policy.rb b/app/models/runtime/constraints/max_log_rate_limit_policy.rb
@@ -50,7 +50,7 @@ class AppMaxLogRateLimitPolicy < BaseMaxLogRateLimitPolicy
 
   def additional_checks
     resource.started? &&
-      (resource.column_changed?(:state) || resource.column_changed?(:instances))
+      (resource.column_changed?(:state) || resource.column_changed?(:instances) || resource.column_changed?(:log_rate_limit))
   end
 
   def requested_log_rate_limit
diff --git a/app/models/runtime/deployment_model.rb b/app/models/runtime/deployment_model.rb
@@ -114,10 +114,6 @@ def continuable?
     end
 
     def desired_web_instances
-      # It seems redundant to have method since web_instances defaults to original_web_process_instance_count,
-      # (in deployment create action)
-      # but this should handle deployments created on old API vms mid bosh deployment
-      # we can probably clean this up in the future
       web_instances || original_web_process_instance_count
     end
 
diff --git a/app/presenters/v3/deployment_presenter.rb b/app/presenters/v3/deployment_presenter.rb
@@ -57,9 +57,12 @@ def new_processes
 
     def options(deployment)
       options = {
-        max_in_flight: deployment.max_in_flight,
-        web_instances: deployment.desired_web_instances
+        max_in_flight: deployment.max_in_flight
       }
+      options[:web_instances] = deployment.web_instances if deployment.web_instances
+      options[:memory_in_mb] = deployment.memory_in_mb if deployment.memory_in_mb
+      options[:disk_in_mb] = deployment.disk_in_mb if deployment.disk_in_mb
+      options[:log_rate_limit_in_bytes_per_second] = deployment.log_rate_limit_in_bytes_per_second if deployment.log_rate_limit_in_bytes_per_second
 
       if deployment.strategy == VCAP::CloudController::DeploymentModel::CANARY_STRATEGY && deployment.canary_steps
         options[:canary] = {
diff --git a/config/cloud_controller.yml b/config/cloud_controller.yml
@@ -11,7 +11,7 @@ readiness_port:
 
 external_protocol: http
 external_domain: api2.vcap.me
-temporary_disable_deployments: true
+temporary_disable_deployments: false
 deployment_updater:
   update_frequency_in_seconds: 30
   lock_key: 'cf-deployment-updater'
diff --git a/db/migrations/20250312233355_add_scaling_columns_to_deployments.rb b/db/migrations/20250312233355_add_scaling_columns_to_deployments.rb
@@ -0,0 +1,16 @@
+Sequel.migration do
+  up do
+    alter_table(:deployments) do
+      add_column :memory_in_mb, :integer, null: true
+      add_column :disk_in_mb, :integer, null: true
+      add_column :log_rate_limit_in_bytes_per_second, :integer, null: true
+    end
+  end
+  down do
+    alter_table(:deployments) do
+      drop_column :memory_in_mb
+      drop_column :disk_in_mb
+      drop_column :log_rate_limit_in_bytes_per_second
+    end
+  end
+end
diff --git a/docs/v3/source/includes/api_resources/_deployments.erb b/docs/v3/source/includes/api_resources/_deployments.erb
@@ -20,6 +20,9 @@
   "options" : {
     "max_in_flight": 3,
     "web_instances": 5,
+    "memory_in_mb": 1024,
+    "disk_in_mb": 1024,
+    "log_rate_limit_in_bytes_per_second": -1,
     "canary": {
       "steps": [
         {
diff --git a/docs/v3/source/includes/resources/deployments/_create.md.erb b/docs/v3/source/includes/resources/deployments/_create.md.erb
@@ -79,6 +79,9 @@ Name | Type | Description | Default
 **strategy** | _string_ | The strategy to use for the deployment | `rolling`
 **options.max_in_flight** | _integer_ | The maximum number of new instances to deploy simultaneously | 1
 **options.web_instances** | _integer_ | The number of web instances the deployment will scale to | The current web process's instance count
+**options.memory_in_mb** |  _integer_ | The amount of memory in megabytes to allocate per web process instance.  If `null`, the amount allocated will be taken from the previous web process. | `null`
+**options.disk_in_mb** | _integer_ | The amount of disk in megabytes to allocate per web process instance.  If `null`, the amount allocated will be taken from the previous web process. | `null`
+**options.log_rate_limit_in_bytes_per_second** | _integer_ | Log rate limit in bytes per second to allocate per web process instance.  If `null`, the amount allocated will be taken from the previous web process. | `null`
 **options.canary.steps** | _array of [canary step objects](#canary-steps-object)_ | An array of canary steps to use for the deployment
 **metadata.labels** | [_label object_](#labels) | Labels applied to the deployment
 **metadata.annotations**  | [_annotation object_](#annotations) | Annotations applied to the deployment
diff --git a/docs/v3/source/includes/resources/deployments/_object.md.erb b/docs/v3/source/includes/resources/deployments/_object.md.erb
@@ -21,6 +21,9 @@ Name | Type | Description
 **strategy** | _string_ | Strategy used for the deployment; supported strategies are `rolling` and `canary` (experimental)
 **options.max_in_flight** | _integer_ | The maximum number of new instances to deploy simultaneously
 **options.web_instances** | _integer_ | The number of web instances the deployment will scale to
+**options.memory_in_mb** |  _integer_ | The amount of memory in megabytes to allocate per web process instance.  If `null`, the amount allocated will be taken from the previous web process.
+**options.disk_in_mb** | _integer_ | The amount of disk in megabytes to allocate per web process instance.  If `null`, the amount allocated will be taken from the previous web process.
+**options.log_rate_limit_in_bytes_per_second** | _integer_ | Log rate limit in bytes per second to allocate per web process instance.  If `null`, the amount allocated will be taken from the previous web process.
 **options.canary.steps** | _array of [canary step objects](#canary-steps-object)_ | Canary steps to use for the deployment. Only available for deployments with strategy 'canary'. (experimental)
 **droplet.guid** | _string_ | The droplet guid that the deployment is transitioning the app to
 **previous_droplet.guid** | _string_ | The app's [current droplet guid](#get-current-droplet-association-for-an-app) before the deployment was created
diff --git a/spec/request/deployments_spec.rb b/spec/request/deployments_spec.rb
diff --git a/spec/unit/actions/deployment_create_spec.rb b/spec/unit/actions/deployment_create_spec.rb
diff --git a/spec/unit/messages/deployment_create_message_spec.rb b/spec/unit/messages/deployment_create_message_spec.rb
diff --git a/spec/unit/models/runtime/constraints/max_log_rate_limit_policy_spec.rb b/spec/unit/models/runtime/constraints/max_log_rate_limit_policy_spec.rb
diff --git a/spec/unit/presenters/v3/deployment_presenter_spec.rb b/spec/unit/presenters/v3/deployment_presenter_spec.rb
