V3: update managed service instance

Initial implementation for sync brokers. Async brokers to follow.

[#171670120]

Co-authored-by: Felisia Martini <fmartini@pivotal.io>
Co-authored-by: Derik Evangelista <devangelista@pivotal.io>
Co-authored-by: George Blue <gblue@pivotal.io>

Author: 3 people

app/actions/service_instance_update.rb

@@ -0,0 +1,93 @@
+require 'jobs/v3/update_service_instance_job'
+require 'repositories/service_event_repository'
+
+module VCAP::CloudController
+  class ServiceInstanceUpdateManaged
+    class InvalidServiceInstance < StandardError
+    end
+
+    class NameTakenForServiceInstance < CloudController::Errors::ApiError
+    end
+
+    def initialize(service_event_repository)
+      @service_event_repository = service_event_repository
+    end
+
+    def update(service_instance, message)
+      raise_if_name_already_taken!(service_instance, message)
+
+      begin
+        lock = UpdaterLock.new(service_instance)
+        lock.lock!
+
+        if update_broker_needed?(service_instance, message)
+          job = update_async(service_instance, message)
+          lock.asynchronous_unlock!
+          return nil, job
+        else
+          si = update_sync(service_instance, message)
+          lock.synchronous_unlock!
+          return si, nil
+        end
+      ensure
+        lock.unlock_and_fail! if lock.needs_unlock?
+      end
+    end
+
+    private
+
+    attr_reader :service_event_repository
+
+    def update_broker_needed?(service_instance, message)
+      service_name_changed = message.requested?(:name) && service_instance.service.allow_context_updates
+      parameters_changed = message.requested?(:parameters)
+      service_plan_changed = message.service_plan_guid &&
+        message.service_plan_guid != service_instance.service_plan.guid
+
+      service_name_changed || parameters_changed || service_plan_changed
+    end
+
+    def update_sync(service_instance, message)
+      logger = Steno.logger('cc.action.service_instance_update')
+
+      updates = {}
+      updates[:name] = message.name if message.requested?(:name)
+      updates[:tags] = message.tags if message.requested?(:tags)
+
+      service_instance.db.transaction do
+        service_instance.update(updates) if updates.any?
+        MetadataUpdate.update(service_instance, message)
+        service_event_repository.record_service_instance_event(:update, service_instance, message.audit_hash)
+      end
+
+      logger.info("Finished updating service_instance #{service_instance.guid}")
+      return service_instance
+    rescue Sequel::ValidationFailed => e
+      raise InvalidServiceInstance.new(e.message)
+    end
+
+    def update_async(service_instance, message)
+      logger = Steno.logger('cc.action.service_instance_update')
+
+      update_job = V3::UpdateServiceInstanceJob.new(
+        service_instance.guid,
+        message: message,
+        user_audit_info: service_event_repository.user_audit_info
+      )
+      pollable_job = Jobs::Enqueuer.new(update_job, queue: Jobs::Queues.generic).enqueue_pollable
+
+      logger.info("Queued job #{pollable_job.guid} to update service_instance #{service_instance.guid}")
+      service_event_repository.record_service_instance_event(:start_update, service_instance, message.audit_hash)
+
+      return pollable_job
+    end
+
+    def raise_if_name_already_taken!(service_instance, message)
+      return unless message.requested?(:name)
+      return unless service_instance.name != message.name
+      return unless ServiceInstance.first(name: message.name, space: service_instance.space)
+
+      raise NameTakenForServiceInstance.new_from_details('ServiceInstanceNameTaken', message.name)
+    end
+  end
+end

app/actions/service_instance_update_managed.rb

@@ -43,6 +43,10 @@ def synchronous_unlock!
       @needs_unlock = false
     end
 
+    def asynchronous_unlock!
+      @needs_unlock = false
+    end
+
     def enqueue_unlock!(job)
       enqueuer = Jobs::Enqueuer.new(job, queue: Jobs::Queues.generic)
       enqueuer.enqueue

app/actions/services/locks/updater_lock.rb

@@ -12,7 +12,7 @@
 require 'presenters/v3/service_instance_presenter'
 require 'actions/service_instance_share'
 require 'actions/service_instance_unshare'
-require 'actions/service_instance_update'
+require 'actions/service_instance_update_managed'
 require 'actions/service_instance_update_user_provided'
 require 'actions/service_instance_create_user_provided'
 require 'actions/service_instance_create_managed'
@@ -96,18 +96,9 @@ def update
 
     case service_instance
     when ManagedServiceInstance
-      message = ServiceInstanceUpdateManagedMessage.new(hashed_params[:body])
-      unprocessable!(message.errors.full_messages) unless message.valid?
-
-      service_instance = ServiceInstanceUpdate.update(service_instance, message)
-      render status: :ok, json: Presenters::V3::ServiceInstancePresenter.new(service_instance)
+      update_managed(service_instance)
     when UserProvidedServiceInstance
-      message = ServiceInstanceUpdateUserProvidedMessage.new(hashed_params[:body])
-      unprocessable!(message.errors.full_messages) unless message.valid?
-
-      service_event_repository = VCAP::CloudController::Repositories::ServiceEventRepository::WithUserActor.new(user_audit_info)
-      service_instance = ServiceInstanceUpdateUserProvided.new(service_event_repository).update(service_instance, message)
-      render status: :ok, json: Presenters::V3::ServiceInstancePresenter.new(service_instance)
+      update_user_provided(service_instance)
     end
   end
 
@@ -187,7 +178,9 @@ def parameters
 
   def create_user_provided(message)
     service_event_repository = VCAP::CloudController::Repositories::ServiceEventRepository::WithUserActor.new(user_audit_info)
+
     instance = ServiceInstanceCreateUserProvided.new(service_event_repository).create(message)
+
     render status: :created, json: Presenters::V3::ServiceInstancePresenter.new(instance)
   rescue ServiceInstanceCreateUserProvided::InvalidUserProvidedServiceInstance => e
     unprocessable!(e.message)
@@ -196,9 +189,7 @@ def create_user_provided(message)
   def create_managed(message, space:)
     service_event_repository = VCAP::CloudController::Repositories::ServiceEventRepository.new(user_audit_info)
     service_plan = ServicePlan.first(guid: message.service_plan_guid)
-    unprocessable_service_plan! unless service_plan &&
-      visible_to_current_user?(plan: service_plan) &&
-      service_plan.visible_in_space?(space)
+    unprocessable_service_plan! unless service_plan_valid?(service_plan, space)
 
     broker_unavailable! unless service_plan.service_broker.available?
 
@@ -209,6 +200,36 @@ def create_managed(message, space:)
     unprocessable!(e.message)
   end
 
+  def update_user_provided(service_instance)
+    message = ServiceInstanceUpdateUserProvidedMessage.new(hashed_params[:body])
+    unprocessable!(message.errors.full_messages) unless message.valid?
+
+    service_event_repository = VCAP::CloudController::Repositories::ServiceEventRepository::WithUserActor.new(user_audit_info)
+    service_instance = ServiceInstanceUpdateUserProvided.new(service_event_repository).update(service_instance, message)
+    render status: :ok, json: Presenters::V3::ServiceInstancePresenter.new(service_instance)
+  end
+
+  def update_managed(service_instance)
+    message = ServiceInstanceUpdateManagedMessage.new(hashed_params[:body])
+    unprocessable!(message.errors.full_messages) unless message.valid?
+
+    if message.service_plan_guid
+      service_plan = ServicePlan.first(guid: message.service_plan_guid)
+      unprocessable_service_plan! unless service_plan_valid?(service_plan, service_instance.space)
+    end
+
+    service_event_repository = VCAP::CloudController::Repositories::ServiceEventRepository.new(user_audit_info)
+    service_instance, job = ServiceInstanceUpdateManaged.new(service_event_repository).update(service_instance, message)
+
+    if job.nil?
+      render status: :ok, json: Presenters::V3::ServiceInstancePresenter.new(service_instance)
+    else
+      head :accepted, 'Location' => url_builder.build_url(path: "/v3/jobs/#{job.guid}")
+    end
+  rescue ServiceInstanceUpdateManaged::NameTakenForServiceInstance => api_err
+    unprocessable!(api_err.message)
+  end
+
   def admin?
     permission_queryer.can_write_globally?
   end
@@ -282,6 +303,12 @@ def service_instance_not_found!
     resource_not_found!(:service_instance)
   end
 
+  def service_plan_valid?(service_plan, space)
+    service_plan &&
+      visible_to_current_user?(plan: service_plan) &&
+      service_plan.visible_in_space?(space)
+  end
+
   def unprocessable_space!
     unprocessable!('Invalid space. Ensure that the space exists and you have access to it.')
   end

app/controllers/v3/service_instances_controller.rb

@@ -0,0 +1,117 @@
+# require 'messages/service_instance_update_managed_message'
+
+module VCAP::CloudController
+  module V3
+    class UpdateServiceInstanceJob < VCAP::CloudController::Jobs::CCJob
+      def initialize(service_instance_guid, message:, user_audit_info:)
+        super()
+        @service_instance_guid = service_instance_guid
+        @message = message
+        @user_audit_info = user_audit_info
+      end
+
+      def perform
+        logger = Steno.logger('cc.background')
+        logger.info("Updating service instance #{service_instance_guid}")
+
+        gone! if service_instance.nil?
+
+        operation_in_progress = service_instance.last_operation.type
+        aborted! if operation_in_progress != 'update'
+
+        client = VCAP::Services::ServiceClientProvider.provide({ instance: service_instance })
+        broker_response, err = client.update(
+          service_instance,
+          service_plan,
+          accepts_incomplete: false,
+          arbitrary_parameters: message.parameters || {},
+          previous_values: previous_values,
+          name: message.requested?(:name) ? message.name : service_instance.name,
+        )
+
+        if err
+          service_instance.save_with_new_operation({}, broker_response[:last_operation])
+          raise err
+        end
+
+        updates = { service_plan: service_plan }
+        updates['name'] = message.name if message.requested?(:name)
+        updates['tags'] = message.tags if message.requested?(:tags)
+        updates['dashboard_url'] = broker_response[:dashboard_url] if broker_response.key?(:dashboard_url)
+
+        ServiceInstance.db.transaction do
+          service_instance.save_with_new_operation(updates, broker_response[:last_operation])
+          MetadataUpdate.update(service_instance, message)
+          record_event(service_instance, message.audit_hash)
+        end
+
+        logger.info("Service instance update complete #{service_instance_guid}")
+      end
+
+      def job_name_in_configuration
+        :service_instance_update
+      end
+
+      def max_attempts
+        1
+      end
+
+      def resource_type
+        'service_instances'
+      end
+
+      def resource_guid
+        service_instance_guid
+      end
+
+      def display_name
+        'service_instance.update'
+      end
+
+      private
+
+      attr_reader :service_instance_guid, :message, :user_audit_info
+
+      def service_instance
+        ManagedServiceInstance.first(guid: service_instance_guid)
+      end
+
+      def service_plan
+        plan = if message.service_plan_guid
+                 ServicePlan.first(guid: message.service_plan_guid)
+               else
+                 service_instance.service_plan
+               end
+
+        service_plan_gone! unless plan
+        plan
+      end
+
+      def previous_values
+        {
+          plan_id: service_instance.service_plan.broker_provided_id,
+          service_id: service_instance.service.broker_provided_id,
+          organization_id: service_instance.organization.guid,
+          space_id: service_instance.space.guid,
+        }
+      end
+
+      def record_event(service_instance, request_attrs)
+        Repositories::ServiceEventRepository.new(@user_audit_info).
+          record_service_instance_event(:update, service_instance, request_attrs)
+      end
+
+      def service_plan_gone!
+        raise CloudController::Errors::ApiError.new_from_details('ServicePlanNotFound', service_instance_guid)
+      end
+
+      def gone!
+        raise CloudController::Errors::ApiError.new_from_details('ServiceInstanceNotFound', service_instance_guid)
+      end
+
+      def aborted!
+        raise CloudController::Errors::ApiError.new_from_details('UnableToPerform', 'Update', 'delete in progress')
+      end
+    end
+  end
+end

app/jobs/v3/update_service_instance_job.rb

@@ -1,9 +1,53 @@
 require 'messages/metadata_base_message'
+require 'messages/validators'
 
 module VCAP::CloudController
   class ServiceInstanceUpdateManagedMessage < MetadataBaseMessage
-    register_allowed_keys []
+    def self.relationships_requested?
+      @relationships_requested ||= proc { |a| a.requested?(:relationships) }
+    end
+
+    register_allowed_keys [
+      :name,
+      :tags,
+      :parameters,
+      :relationships,
+    ]
 
     validates_with NoAdditionalKeysValidator
+    validates_with RelationshipValidator, if: relationships_requested?
+
+    validates :name, string: true, allow_nil: true
+    validates :tags, array: true, allow_blank: true
+    validate :tags_must_be_strings
+
+    validates :parameters, hash: true, allow_nil: true
+    validates :relationships, hash: true, allow_nil: true
+
+    def relationships_message
+      @relationships_message ||= Relationships.new(relationships&.deep_symbolize_keys)
+    end
+
+    delegate :service_plan_guid, to: :relationships_message
+
+    private
+
+    def tags_must_be_strings
+      if tags.present? && tags.is_a?(Array) && tags.any? { |i| !i.is_a?(String) }
+        errors.add(:tags, 'must be a list of strings')
+      end
+    end
+
+    class Relationships < BaseMessage
+      register_allowed_keys [:service_plan]
+
+      validates_with NoAdditionalKeysValidator
+
+      validates :service_plan, presence: true, allow_nil: false, to_one_relationship: true
+
+      def service_plan_guid
+        HashUtils.dig(service_plan, :data, :guid)
+      end
+    end
   end
 end