Manageiq: enhance Provider Operations API

[durandom]

What are provider operations?

Well, all operations you do on a provider's CI (configuration item). E.g. power / command operations like Vm#stop or CRUD operations like CloudVolumeSnapshot.create

Current situation

Various clients / consumers of operations API. So, we don't _really_ know how we are called.

• Classic UI
• REST API
• Automate
• Models themselves?
• ...

Implementation

• Delegation def raw_operation is called by def operation
• Delegation run_command_via_parent will execute command on ExtManagementSystem.
• Inheritance reboot_guest is implemented in subclass, thus violating the raw_operation pattern
• Model does queuing via MiqTask

Invocation

• UI via process_tasks and MiqQueue
• REST API via MiqTask
• Automate via expose and expose :raw_

As you see implementation and invocation is not consistent and therefore its hard to programatically check e.g. if an operation is implemented.

High level goals

• have the same interface across providers
• have the same interface across CIs / objects
• hard to break for providers (vs. inheritance)
  
  
  
  • dont overwrite api methods like raw_stop
  
  
• decouple persistence layer (ActiveRecord) single responsibilty principle

from a provider author’s POV

• operations should be easy to implement
• just be concerned with external system

from platform POV

• easy to test if operation is implemented
  
  
  
  • supports_feature has a hard time with inheritance
  
  
• easy to add additional tasks
  
  
  
  • logging, queuing, rbac, perm check
  
  
• standard interface

Service Objects

as a possible solution:

On Tue, Mar 12, 2013 at 1:26 PM, Martin Fowler wrote:

The term pops up in some different places, so it's hard to know what it means without some context. In PoEAA I use the pattern Service Layer to represent a domain-oriented layer of behaviors that provide an API for the domain layer. This may or may not sit on top of a Domain Model. In DDD Eric Evans uses the term Service Object to refer to objects that represent processes (as opposed to Entities and Values). DDD Service Objects are often useful to factor out behavior that would otherwise bloat Entities, it's also a useful step to patterns like Strategy and Command.

maybe impose some constraints on our service objects:

from Kamil Lelonek

Service Objects must not have a return value. They are not queries, they do not fetch any data. They performs actions with side effects and they don’t respond to anything. They can send an event, execute a callback, but still only perform a particular command. They have to be completely separated from queries.

Here an example implementation from https://gist.github.com/durandom/82437a51d096575e274426bcbbf18ec7

# current implementation Amazon
def raw_stop
  with_provider_object(&:stop)
  # Temporarily update state for quick UI response until refresh comes along
  self.update_attributes!(:raw_power_state => "shutting_down")
end

# implementation with Service Object

# base class for api docs
class Vm::Operations::Stop
  def call(provider_object, ems)    
  end  
end

# amazon implementation
class Amazon::Vm::Operations::Stop < Vm::Operations::Stop
  def call(provider_object, ems)
    provider_object.stop
  end  
end

# calling it 
class Platform::PowerOperations
  def stop(vm)
    # check if vm is connected to ems
    raise "not connected" unless vm.ems.present?
    # do some perm checking
    check_policy_prevent(:request_vm_stop)
    vm.with_provider_object do |provider_object|
      vm.class::Operations::Stop.new.call(provider_object)
    vm.update_attributes!(:raw_power_state => vm.class::Operations::STOP_POWERSTATE)
  rescue => e
    # do some logging    
  end
end

or a more complex operation

# currently in Openstack::CloudManager::CloudVolumeSnapshot
def self.create_snapshot(cloud_volume, options = {})
  raise ArgumentError, _("cloud_volume cannot be nil") if cloud_volume.nil?
  ext_management_system = cloud_volume.try(:ext_management_system)
  raise ArgumentError, _("ext_management_system cannot be nil") if ext_management_system.nil?

  cloud_tenant = cloud_volume.cloud_tenant
  snapshot = nil
  options[:volume_id] = cloud_volume.ems_ref
  ext_management_system.with_provider_connection(connection_options(cloud_tenant)) do |service|
    snapshot = service.snapshots.create(options)
  end

  create(
    :name                  => snapshot.name,
    :description           => snapshot.description,
    :ems_ref               => snapshot.id,
    :status                => snapshot.status,
    :cloud_volume          => cloud_volume,
    :cloud_tenant          => cloud_tenant,
    :ext_management_system => ext_management_system,
  )
rescue => e
  _log.error "snapshot=[#{options[:name]}], error: #{e}"
  raise MiqException::MiqVolumeSnapshotCreateError, e.to_s, e.backtrace
end

# as a service object
class OpenStack::CloudVolumeSnapshot::Create
  def call(provider_connection, cloud_tenant, options)
    snapshot = provider_connection.snapshots.create(options)
    MiqEvent.put_on_queue("CLOUD_VOLUME_SNAPSHOT_CREATED", :payload => snapshot)
    # will do this
    # Dto::SaveInventory.save(snapshot)
  end
end

this is a rough idea and base for discussion started in the providers team.

cc @blomquisg @djberg96 @Ladas @agrare @bronaghs @jameswnl @juliancheal
cc @Fryguy @jrafanie @kbrock @chrisarcand @NickLaMuro @chessbyte
all comments welcome - feel free to cc more people :)

[djberg96]

Hm, it seems we could use an actual Interface object so that we not only have a common interface, but get some actual enforcement as well. There is a gem for this, simply called "interface" we could use if you want enforcement.

More to your point, I think this is too granular at one class per operation, especially if we have an enforceable and documented interface.

[miq-bot]

This issue has been automatically marked as stale because it has not been updated for at least 6 months.

If you can still reproduce this issue on the current release or on master, please reply with all of the information you have about it in order to keep the issue open.

Thank you for all your contributions!

[Ladas]

@blomquisg we should move this to the design docs? We should find some time to tackle this.

[juliancheal]

@Ladas @djberg96 has any of Marcel's ideas been implemented yet?

[juliancheal]

@miq-bot assign @juliancheal

[JPrause]

@juliancheal is this still a valid issue. If not can you close.
If there's no update by next week, I'll be closing this issue.

[juliancheal]

@JPrause we can close this.

[JPrause]

Thanks @juliancheal
@miq-bot close_issue