Save operation

[mkasztelnik]

save (no save as, that we have already) operation is requested by our partners. In this issue I will try to describe how I imagine to implement it and it is the place for the rest of the team to submit comments, improvements. I'm planning to have an agreement on how it should be done till 2nd of December.

The simplest solution to implement mentioned problem is as follows:

• Api request on appliance_type resource, with information about appliance, which should be used to save new VMT, is performed. Such request may look like this:

PUT /api/v1/appliance_types/{at_id}

{"appliance_type": { "appliance_id": 123 }}

• We will check if AT used to save appliance still exists. If no, an exception is returned to the user.
• If AT exists, a new delay action is created (e.g. SaveApplianceType.perform_async(appliance_type_id, appliance_id))
• Inside SaveApplianceType action new VMT will be created using appliance VM. Relation between ApplianceType and VMT will be established and the most important part follow_action VMT field will be filled with value replace_all.
• Inside VmTemplateMonitoringWorker we will have a generic solution for follow_action, which will have a mapping between follow_action value and real action, which needs to be performed after VMT state changes from "something" into active. It can look as follow:

perform_follow_acton if statuch_changed_into_active?

def perform_follow_action
  switch vmt.follow_action
  case :replace_all then DestroyATTemplatesExcept.perform_async(vmt)
  case ...
  end
end

• DestroyATTemplatesExcept responsibility will be to do something like:

ats = Atmosphere::VirtualMachineTemplate.where(appliance_type_id: vmt.appliance_type_id).not(id: vmt.id)
ats.each(&:destroy)

Questions to be answered:

• Do you think it is a good solution? If not, please describe an alternative.
• I'm not sure if proposed API request is good enough. Maybe api/v1/appliances/action {save: null} should be used instead. Any other remarks, propositions?
• Are there any other side effects?

/cc @nowakowski, @Nuanda, @tbartynski, @bwilk, @paoolo I'm waiting for your comments, remarks.

[nowakowski]

Thanks for kickstarting this discussion. Couple of comments for your perusal:

1. I'm not sure I understand the syntax {"appliance_type": { "appliance_id": 123 }}. Do you mean that PUT /api/v1/apppliance_types should admit an appliance_type parameter with a hash value? Why not simply accept an appliance_id parameter with a value of 123?
2. The devmode appliance needs to be locked pending the save operation (particularly if the save is to be performed asynchronously).
3. Conceptually speaking, I'm not sure I like follow_action as an attribute of VMT. This violates a fundamental rule of object-oriented design in that each attribute should reflect a property of its class. "Action to be executed on ApplianceType" is not a property of VirtualMachineTemplate. Rather, it is a property of ApplianceType. Yes, I know I'm opening a can of worms here; feel free to ignore the ramblings of an old geezer - but I would instead propose a VMT->VMT relation called superseded_by (declare old_vmt.superseded_by = new_vmt and have the monitoring worker check whether new_vmt is active; if so, it should simply delete old_vmt). Much more elegant IMO. :)
4. How exactly are you going to implement statuch_changed_into_active? (sweet typo ;) in VMTemplateMonitoringWorker? You'd need some way to remember the old status of the VMT. Such information is indeed available in ActiveRecord callbacks as <attribute_name>_was but you won't have access to it in a worker class. Note that this problem disappears if you accept the design change suggested in (3) above.
5. The method you are proposing will destroy all VMTs assigned to the template except the new VMT, including any VMTs registered on external ComputeSites. I can see why one would want to do this but if so, we should also automatically schedule the new VMT to be propagated to all sites which held copies of the old VMT.

[mkasztelnik]

Ad 1. I'm trying to follow http://jsonapi.org/ standard (more/less :smiling_imp:). That is why resource prefix at the beginning. The main + of such approach is when you want to create resource with some subresources (e.g. AT with port mappings ) during one POST request

Ad 2. I believe that we already have such mechanism implemented for save as action

Ad 3. I was trying to find something that is generic and can be used in similar cases. Relation between AT and VMT is very specific and as a conclusion we will finish with a lot of relations between objects for every specific situation.

Ad 4. We have access to <attribute_name>_was inside worker. What is more important we have access to <attribute_name>_changed?. Take a look here for example.

Ad 5. Currently I was not planning to schedule migration to other CSes, because this feature is not yet merged into master.

[nowakowski]

I agree on all counts except #3 - perhaps because I simply don't understand the point you're trying to get across. :)

• What do you mean by "Relation between AT and VMT is very specific."?
• "As a conclusion (...)" - a conclusion (consequence?) of what? Your approach or my approach?
• What is a "specific situation"?
• Could you quantify "a lot of relations"? ^_^

[mkasztelnik]

• By very specific I mean that we are duplicating relation we already have (appliance belongs_to appliance_type). As a conclusion we already have all the information needed to perform save operation and we don't need to duplicate it.
• If we will follow your solution than we will have relation between appliance and appliance_type which brings us only one information - remove all other templates after state changed into active (and I mean this as "specific situation"). IMHO this has the same meaning as follow_action but follow_action is much more generic and can be used in the feature to implement similar actions (and additionally we don't duplicate relations between elements, where most probably we would need to add additional guard checking if this relation is the same as appliance belongs_to appliance_type.
• by "a lot of relations" I mean at least one more than needed :smile:

[bwilk]

I sopport REST interface design. When it comes to save operation logic yet another possible solution comes to my mind. Let me explain.

Let's start with introducing field version to VirtualMachineTemplate. When we start Appliance we simply select VMT with the largest version and state active. When we save Appliance we create new VirtualMachineTemplate with incremented version counter. After the save process is completed (VMT state changed to active) we remove all VMTs that have version lower than the largest version of active VMTs. To omit avoid computing the largest version we may store current version in ApplianceType.

Let me know what you think about such solution.

[mkasztelnik]

It was agreed that solution with VMT version will be used.