18562: Auto-enable preemptible when changing state to Committed.

(As opposed to during any update.)

Avoids a situation where a client creates a child CR with
preemptible=false, preemptible is automatically changed to true, but
then preemptible instances are removed from config, and the client
cannot change the CR state to committed without also explicitly
clearing the preemptible flag, because the automatically-enabled
preemptible flag is not allowed.

Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom@curii.com>

diff --git a/services/api/app/models/container_request.rb b/services/api/app/models/container_request.rb
index 273664c5bc736c2b959cfb16951922c78d83e336..3186df0905e99e626b59e6a60e07dda58dbcd112 100644 (file)
--- a/services/api/app/models/container_request.rb
+++ b/services/api/app/models/container_request.rb
@@ -326,7 +326,9 @@ class ContainerRequest < ArvadosModel
   end
 
   def set_preemptible
-    if Rails.configuration.Containers.UsePreemptibleInstances &&
+    if (new_record? || state_changed?) &&
+       state == Committed &&
+       Rails.configuration.Containers.UsePreemptibleInstances &&
        get_requesting_container_uuid() &&
        self.class.any_preemptible_instances?
       self.scheduling_parameters['preemptible'] = true

diff --git a/services/api/test/unit/container_request_test.rb b/services/api/test/unit/container_request_test.rb
index bf12a3960e1537d589fcf95588693a94bbe2ab0a..b7a1a7636ea94c06bc224381e25552b9d099e98c 100644 (file)
--- a/services/api/test/unit/container_request_test.rb
+++ b/services/api/test/unit/container_request_test.rb
@@ -1053,10 +1053,17 @@ class ContainerRequestTest < ActiveSupport::TestCase
 
       expect[true].push create_minimal_req!(attrs_p)
       expect[true].push create_minimal_req!(attrs_nonp)
+      commit_later = create_minimal_req!()
 
       Rails.configuration.InstanceTypes = ConfigLoader.to_OrderedOptions({})
 
       expect[false].push create_minimal_req!(attrs_nonp)
+
+      # Even though preemptible is not allowed, we should be able to
+      # commit a CR that was created earlier when preemptible was the
+      # default.
+      commit_later.update_attributes!(priority: 1, state: "Committed")
+      expect[false].push commit_later
     end
 
     set_user_from_auth :active