Arvados

I'm not finding any special treatment on the storage_classes_confirmed (and neither on replication_confirmed) field on workbench1 that indicates it gets dropped on a copy operation, so my current hypothesis is that the following RailsAPI callback at the Collection class model needs the proper fields reset:

[...]
  before_validation :strip_signatures_and_update_replication_confirmed
[...]
  def strip_signatures_and_update_replication_confirmed
    if self.manifest_text_changed?
      in_old_manifest = {}
      # manifest_text_was could be nil when dealing with a freshly created snapshot,
      # so we skip this case because there was no real manifest change. (Bug #18005)
      if (not self.replication_confirmed.nil?) and (not self.manifest_text_was.nil?)
        self.class.each_manifest_locator(manifest_text_was) do |match|
          in_old_manifest[match[1]] = true
        end
      end

      stripped_manifest = self.class.munge_manifest_locators(manifest_text) do |match|
        if not self.replication_confirmed.nil? and not in_old_manifest[match[1]]
          # If the new manifest_text contains locators whose hashes
          # weren't in the old manifest_text, storage replication is no
          # longer confirmed.
          self.replication_confirmed_at = nil
          self.replication_confirmed = nil
      end
      [...]
  end

Upon followup inspection, it seems that workbench1 doesn't include the replication_* fields on its create request, but it does include the storage_classes_* fields. These fields should be treated the same way.

Here's a formatted copy of the params that RailsAPI receives on the create call when attempting to copy a collection from Workbench1

"params":{
    "cluster_id":"",
    "collection":"{
        \"manifest_text\":<removed>,
        \"name\":\"Copy of Source coll 6\",
        \"owner_uuid\":\"ce8i5-tpzed-f7960quf4ivgbxb\",
        \"properties\":{
            \"a\":\"b\",
            \"responsible_person_uuid\":\"ce8i5-tpzed-f7960quf4ivgbxb\" 
        },
        \"storage_classes_confirmed\":[\"default\"],
        \"storage_classes_desired\":[\"default\"],
        \"uuid\":null
    }" 
}

...and here's a copy of the params when copying the same collection from Workbench2:

"params":{
    "cluster_id":"",
    "collection":"{
        \"created_at\":\"2021-12-15T21:54:04.125582000Z\",
        \"current_version_uuid\":\"ce8i5-4zz18-cxxvreo5jstntpg\",
        \"delete_at\":null,
        \"description\":null,
        \"is_trashed\":false,
        \"manifest_text\":<removed>,
        \"modified_at\":\"2022-02-11T19:29:22.555003000Z\",
        \"modified_by_client_uuid\":\"ce8i5-ozdt8-qfoh901onn5potx\",
        \"modified_by_user_uuid\":\"ce8i5-tpzed-f7960quf4ivgbxb\",
        \"name\":\"Copy of: Source coll 6\",
        \"owner_uuid\":\"ce8i5-tpzed-f7960quf4ivgbxb\",
        \"portable_data_hash\":\"4ec1f24c071f9a7dd1e7237a3b0cf214+205\",
        \"preserve_version\":true,
        \"properties\":{
            \"a\":\"b\",
            \"responsible_person_uuid\":\"ce8i5-tpzed-f7960quf4ivgbxb\" 
        },
        \"replication_desired\":null,
        \"storage_classes_desired\":[\"default\"],
        \"trash_at\":null
    }" 
}

Workbench2's code explicitly avoids passing replication_confirmed and storage_classes_confirmed fields, but oddly enough I haven't been able to find code that avoids passing replication_confirmed on Workbench1.

Regarding why the replication_desired field isn't included on WB1's request, I think it is because for some reason, the collection being copied (ce8i5-4zz18-cxxvreo5jstntpg) has it set to null as it is evident on WB2's params.

Creating a new collection with arv-put with replication_desired set to 2, and then copying it with Workbench1 produced at request that on RailsAPI's side was seen with these params:

"params":{
    "cluster_id":"",
    "collection":"{
        \"manifest_text\":<removed>,
        \"owner_uuid\":\"ce8i5-j7d0g-hcfsyzcwl5bqy4b\",
        \"properties\":{
            \"responsible_person_uuid\":\"ce8i5-tpzed-f7960quf4ivgbxb\" 
        },
        \"storage_classes_confirmed\":[],
        \"storage_classes_desired\":[\"default\"],
        \"uuid\":null
    }" 
}

No replication_desired param on sight. The fact that WB2 request includes this parameter makes me think that there should be code that either whitelist some params for collections or deletes others that aren't allowed, but I haven't been able to find neither of those cases.

I suspect I'm missing something super obvious here...

Update at 12440f46c - branch 19144-wb-copy-collections-fix
Test run: developer-run-tests: #3161
Failed test re-run: developer-run-tests-remainder: #3311

Quick & dirty fix on WB1 by resetting the affected fields on the actions controller's copy method.
To test against our dev cluster: Use a non-admin user & attempt to copy a collection that already has a storage_classes_confirmed field set to something other than [].

Lucas Di Pentima wrote:

Update at 12440f46c - branch 19144-wb-copy-collections-fix
Test run: developer-run-tests: #3161
Failed test re-run: developer-run-tests-remainder: #3311

Quick & dirty fix on WB1 by resetting the affected fields on the actions controller's copy method.
To test against our dev cluster: Use a non-admin user & attempt to copy a collection that already has a storage_classes_confirmed field set to something other than [].

This LGTM