From 7b070fc8458f4108d44d6bfb939e36d3cc76af84 Mon Sep 17 00:00:00 2001
From: Tom Clegg <tom@curii.com>
Date: Thu, 17 Mar 2022 15:55:11 -0400
Subject: [PATCH] 18691: Disallow freezing projects with active container
 requests.

Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom@curii.com>
---
 services/api/app/models/group.rb     | 12 ++++++++
 services/api/test/unit/group_test.rb | 41 ++++++++++++++++++++--------
 2 files changed, 41 insertions(+), 12 deletions(-)

diff --git a/services/api/app/models/group.rb b/services/api/app/models/group.rb
index f56c3281d7..b1b2e942c6 100644
--- a/services/api/app/models/group.rb
+++ b/services/api/app/models/group.rb
@@ -176,6 +176,18 @@ class Group < ArvadosModel
       "Group.update_frozen.select",
       [[nil, self.uuid],
        [nil, !self.frozen_by_uuid.nil?]])
+    if frozen_by_uuid
+      rows = ActiveRecord::Base.connection.exec_query(
+        "select cr.uuid, cr.state from container_requests cr, #{temptable} frozen " +
+        "where cr.owner_uuid = frozen.uuid and frozen.is_frozen " +
+        "and cr.state not in ($1, $2) limit 1",
+        "Group.update_frozen.check_container_requests",
+        [[nil, ContainerRequest::Uncommitted],
+         [nil, ContainerRequest::Final]])
+      if rows.any?
+        raise ArgumentError.new("cannot freeze project containing container request #{rows.first['uuid']} with state = #{rows.first['state']}")
+      end
+    end
     ActiveRecord::Base.connection.exec_delete(
       "delete from frozen_groups where uuid in (select uuid from #{temptable} where not is_frozen)",
       "Group.update_frozen.delete")
diff --git a/services/api/test/unit/group_test.rb b/services/api/test/unit/group_test.rb
index 1f1d379084..7a16962402 100644
--- a/services/api/test/unit/group_test.rb
+++ b/services/api/test/unit/group_test.rb
@@ -317,6 +317,18 @@ update links set tail_uuid='#{g5}' where uuid='#{l1.uuid}'
   test "freeze project" do
     act_as_user users(:active) do
       Rails.configuration.API.UnfreezeProjectRequiresAdmin = false
+
+      test_cr_attrs = {
+        command: ["echo", "foo"],
+        container_image: links(:docker_image_collection_tag).name,
+        cwd: "/tmp",
+        environment: {},
+        mounts: {"/out" => {"kind" => "tmp", "capacity" => 1000000}},
+        output_path: "/out",
+        runtime_constraints: {"vcpus" => 1, "ram" => 2},
+        name: "foo",
+        description: "bar",
+      }
       parent = Group.create!(group_class: 'project', name: 'freeze-test-parent', owner_uuid: users(:active).uuid)
       proj = Group.create!(group_class: 'project', name: 'freeze-test', owner_uuid: parent.uuid)
       proj_inner = Group.create!(group_class: 'project', name: 'freeze-test-inner', owner_uuid: proj.uuid)
@@ -398,18 +410,7 @@ update links set tail_uuid='#{g5}' where uuid='#{l1.uuid}'
         assert_raises do
           Group.create!(owner_uuid: frozen.uuid, group_class: 'project', name: 'inside-frozen-project')
         end
-        cr = ContainerRequest.new(
-          command: ["echo", "foo"],
-          container_image: links(:docker_image_collection_tag).name,
-          cwd: "/tmp",
-          environment: {},
-          mounts: {"/out" => {"kind" => "tmp", "capacity" => 1000000}},
-          output_path: "/out",
-          runtime_constraints: {"vcpus" => 1, "ram" => 2},
-          name: "foo",
-          description: "bar",
-          owner_uuid: frozen.uuid,
-        )
+        cr = ContainerRequest.new(test_cr_attrs.merge(owner_uuid: frozen.uuid))
         assert_raises ArvadosModel::PermissionDeniedError do
           cr.save
         end
@@ -509,6 +510,22 @@ update links set tail_uuid='#{g5}' where uuid='#{l1.uuid}'
         # Admin can unfreeze.
         assert proj.update_attributes(frozen_by_uuid: nil), proj.errors.messages
       end
+
+      # Cannot freeze a project if it contains container requests in
+      # Committed state (this would cause operations on the relevant
+      # Containers to fail when syncing container request state)
+      creq_uncommitted = ContainerRequest.create!(test_cr_attrs.merge(owner_uuid: proj_inner.uuid))
+      creq_committed = ContainerRequest.create!(test_cr_attrs.merge(owner_uuid: proj_inner.uuid, state: 'Committed'))
+      err = assert_raises do
+        proj.update_attributes!(frozen_by_uuid: users(:active).uuid)
+      end
+      assert_match /container request zzzzz-xvhdp-.* with state = Committed/, err.inspect
+      proj.reload
+
+      # Can freeze once all container requests are in Uncommitted or
+      # Final state
+      creq_committed.update_attributes!(state: ContainerRequest::Final)
+      assert proj.update_attributes(frozen_by_uuid: users(:active).uuid)
     end
   end
 end
-- 
2.30.2