17215: Merge branch 'master' into 17215-aws-roles-a-d-c
[arvados.git] / services / api / app / models / container_request.rb
index c412e4b8500c141617b18de64007078f7b715c4d..837f2cdc7010eaacc1182f868e9fb01084367ddb 100644 (file)
@@ -30,14 +30,16 @@ class ContainerRequest < ArvadosModel
   serialize :command, Array
   serialize :scheduling_parameters, Hash
 
+  after_find :fill_container_defaults_after_find
   before_validation :fill_field_defaults, :if => :new_record?
-  before_validation :validate_runtime_constraints
+  before_validation :fill_container_defaults
   before_validation :set_default_preemptible_scheduling_parameter
   before_validation :set_container
   validates :command, :container_image, :output_path, :cwd, :presence => true
   validates :output_ttl, numericality: { only_integer: true, greater_than_or_equal_to: 0 }
   validates :priority, numericality: { only_integer: true, greater_than_or_equal_to: 0, less_than_or_equal_to: 1000 }
   validate :validate_datatypes
+  validate :validate_runtime_constraints
   validate :validate_scheduling_parameters
   validate :validate_state_change
   validate :check_update_whitelist
@@ -125,10 +127,10 @@ class ContainerRequest < ArvadosModel
       # (same order as Container#handle_completed). Locking always
       # reloads the Container and ContainerRequest records.
       c = Container.find_by_uuid(container_uuid)
-      c.lock!
+      c.lock! if !c.nil?
       self.lock!
 
-      if container_uuid != c.uuid
+      if !c.nil? && container_uuid != c.uuid
         # After locking, we've noticed a race, the container_uuid is
         # different than the container record we just loaded.  This
         # can happen if Container#handle_completed scheduled a new
@@ -138,13 +140,18 @@ class ContainerRequest < ArvadosModel
         redo
       end
 
-      if state == Committed && c.final?
-        # The current container is
-        act_as_system_user do
-          leave_modified_by_user_alone do
-            finalize!
+      if !c.nil?
+        if state == Committed && c.final?
+          # The current container is
+          act_as_system_user do
+            leave_modified_by_user_alone do
+              finalize!
+            end
           end
         end
+      elsif state == Committed
+        # Behave as if the container is cancelled
+        update_attributes!(state: Final)
       end
       return true
     end
@@ -153,7 +160,28 @@ class ContainerRequest < ArvadosModel
   # Finalize the container request after the container has
   # finished/cancelled.
   def finalize!
-    update_collections(container: Container.find_by_uuid(container_uuid))
+    container = Container.find_by_uuid(container_uuid)
+    if !container.nil?
+      update_collections(container: container)
+
+      if container.state == Container::Complete
+        log_col = Collection.where(portable_data_hash: container.log).first
+        if log_col
+          # Need to save collection
+          completed_coll = Collection.new(
+            owner_uuid: self.owner_uuid,
+            name: "Container log for container #{container_uuid}",
+            properties: {
+              'type' => 'log',
+              'container_request' => self.uuid,
+              'container_uuid' => container_uuid,
+            },
+            portable_data_hash: log_col.portable_data_hash,
+            manifest_text: log_col.manifest_text)
+          completed_coll.save_with_unique_name!
+        end
+      end
+    end
     update_attributes!(state: Final)
   end
 
@@ -161,17 +189,20 @@ class ContainerRequest < ArvadosModel
     collections.each do |out_type|
       pdh = container.send(out_type)
       next if pdh.nil?
+      c = Collection.where(portable_data_hash: pdh).first
+      next if c.nil?
+      manifest = c.manifest_text
+
       coll_name = "Container #{out_type} for request #{uuid}"
       trash_at = nil
       if out_type == 'output'
-        if self.output_name
+        if self.output_name and self.output_name != ""
           coll_name = self.output_name
         end
         if self.output_ttl > 0
           trash_at = db_current_time + self.output_ttl
         end
       end
-      manifest = Collection.where(portable_data_hash: pdh).first.manifest_text
 
       coll_uuid = self.send(out_type + '_uuid')
       coll = coll_uuid.nil? ? nil : Collection.where(uuid: coll_uuid).first
@@ -187,6 +218,7 @@ class ContainerRequest < ArvadosModel
       end
 
       if out_type == "log"
+        # Copy the log into a merged collection
         src = Arv::Collection.new(manifest)
         dst = Arv::Collection.new(coll.manifest_text)
         dst.cp_r("./", ".", src)
@@ -248,60 +280,51 @@ class ContainerRequest < ArvadosModel
       if self.container_count_changed?
         errors.add :container_count, "cannot be updated directly."
         return false
-      else
-        self.container_count += 1
-        if self.container_uuid_was
-          old_container = Container.find_by_uuid(self.container_uuid_was)
-          old_logs = Collection.where(portable_data_hash: old_container.log).first
-          if old_logs
-            log_coll = self.log_uuid.nil? ? nil : Collection.where(uuid: self.log_uuid).first
-            if self.log_uuid.nil?
-              log_coll = Collection.new(
-                owner_uuid: self.owner_uuid,
-                name: coll_name = "Container log for request #{uuid}",
-                manifest_text: "")
-            end
+      end
 
-            # copy logs from old container into CR's log collection
-            src = Arv::Collection.new(old_logs.manifest_text)
-            dst = Arv::Collection.new(log_coll.manifest_text)
-            dst.cp_r("./", "log for container #{old_container.uuid}", src)
-            manifest = dst.manifest_text
-
-            log_coll.assign_attributes(
-              portable_data_hash: Digest::MD5.hexdigest(manifest) + '+' + manifest.bytesize.to_s,
-              manifest_text: manifest)
-            log_coll.save_with_unique_name!
-            self.log_uuid = log_coll.uuid
-          end
-        end
+      self.container_count += 1
+      return if self.container_uuid_was.nil?
+
+      old_container = Container.find_by_uuid(self.container_uuid_was)
+      return if old_container.nil?
+
+      old_logs = Collection.where(portable_data_hash: old_container.log).first
+      return if old_logs.nil?
+
+      log_coll = self.log_uuid.nil? ? nil : Collection.where(uuid: self.log_uuid).first
+      if self.log_uuid.nil?
+        log_coll = Collection.new(
+          owner_uuid: self.owner_uuid,
+          name: coll_name = "Container log for request #{uuid}",
+          manifest_text: "")
       end
+
+      # copy logs from old container into CR's log collection
+      src = Arv::Collection.new(old_logs.manifest_text)
+      dst = Arv::Collection.new(log_coll.manifest_text)
+      dst.cp_r("./", "log for container #{old_container.uuid}", src)
+      manifest = dst.manifest_text
+
+      log_coll.assign_attributes(
+        portable_data_hash: Digest::MD5.hexdigest(manifest) + '+' + manifest.bytesize.to_s,
+        manifest_text: manifest)
+      log_coll.save_with_unique_name!
+      self.log_uuid = log_coll.uuid
     end
   end
 
   def set_default_preemptible_scheduling_parameter
-    c = get_requesting_container()
-    if self.state == Committed
-      # If preemptible instances (eg: AWS Spot Instances) are allowed,
-      # ask them on child containers by default.
-      if Rails.configuration.Containers.UsePreemptibleInstances and !c.nil? and
-        self.scheduling_parameters['preemptible'].nil?
-          self.scheduling_parameters['preemptible'] = true
-      end
+    if Rails.configuration.Containers.UsePreemptibleInstances && state == Committed && get_requesting_container()
+      self.scheduling_parameters['preemptible'] = true
     end
   end
 
   def validate_runtime_constraints
     case self.state
     when Committed
-      [['vcpus', true],
-       ['ram', true],
-       ['keep_cache_ram', false]].each do |k, required|
-        if !required && !runtime_constraints.include?(k)
-          next
-        end
+      ['vcpus', 'ram'].each do |k|
         v = runtime_constraints[k]
-        unless (v.is_a?(Integer) && v > 0)
+        if !v.is_a?(Integer) || v <= 0
           errors.add(:runtime_constraints,
                      "[#{k}]=#{v.inspect} must be a positive integer")
         end
@@ -440,10 +463,10 @@ class ContainerRequest < ArvadosModel
   end
 
   def update_priority
-    return unless state_changed? || priority_changed? || container_uuid_changed?
+    return unless saved_change_to_state? || saved_change_to_priority? || saved_change_to_container_uuid?
     act_as_system_user do
       Container.
-        where('uuid in (?)', [self.container_uuid_was, self.container_uuid].compact).
+        where('uuid in (?)', [container_uuid_before_last_save, self.container_uuid].compact).
         map(&:update_priority!)
     end
   end