Project

General

Profile

Actions

Bug #18301

closed

WB1 cancel button not working

Added by Peter Amstutz over 2 years ago. Updated over 2 years ago.

Status:
Resolved
Priority:
Normal
Assigned To:
Category:
Workbench
Target version:
Story points:
-
Release relationship:
Auto

Subtasks 1 (0 open1 closed)

Task #18477: Review 18301-wb1-cancel-button-fixResolvedPeter Amstutz12/07/2021Actions
Actions #1

Updated by Peter Amstutz over 2 years ago

  • Target version changed from 2021-11-10 sprint to 2021-11-24 sprint
Actions #2

Updated by Peter Amstutz over 2 years ago

  • Target version changed from 2021-11-24 sprint to 2021-12-08 sprint
Actions #3

Updated by Peter Amstutz over 2 years ago

  • Category set to Workbench
Actions #4

Updated by Tom Clegg over 2 years ago

Not sure if this is the same problem, but the error I ran into was "can't update mounts when state is Final" (not exact wording) when trying to cancel a container that was in Locked state.

Actions #5

Updated by Lucas Di Pentima over 2 years ago

  • Assigned To set to Lucas Di Pentima
Actions #6

Updated by Lucas Di Pentima over 2 years ago

  • Status changed from New to In Progress
Actions #7

Updated by Lucas Di Pentima over 2 years ago

I'm not able to reproduce the issue, please provide a specific case.

Actions #8

Updated by Ward Vandewege over 2 years ago

On Pirca, trying to move https://workbench.pirca.arvadosapi.com/container_requests/pirca-xvhdp-om4zxfsae65h29f#Log into a project, I get:

//railsapi.internal/arvados/v1/container_requests/pirca-xvhdp-om4zxfsae65h29f: 422 Unprocessable Entity: Mounts cannot be modified in state 'Final' ({"/keep/bcca5ee5b64d1f30296f204be49fc050+270"=>{"kind"=>"collection", "portable_data_hash"=>"bcca5ee5b64d1f30296f204be49fc050+270"}, "/tmp"=>{"capacity"=>1073741824, "kind"=>"tmp"}, "/var/spool/cwl"=>{"capacity"=>1073741824, "kind"=>"tmp"}}, {"/keep/bcca5ee5b64d1f30296f204be49fc050+270"=>{"capacity"=>0, "commit"=>"", "content"=>nil, "device_type"=>"", "exclude_from_output"=>false, "git_url"=>"", "kind"=>"collection", "path"=>"", "portable_data_hash"=>"bcca5ee5b64d1f30296f204be49fc050+270", "repository_name"=>"", "uuid"=>"", "writable"=>false}, "/tmp"=>{"capacity"=>1073741824, "commit"=>"", "content"=>nil, "device_type"=>"", "exclude_from_output"=>false, "git_url"=>"", "kind"=>"tmp", "path"=>"", "portable_data_hash"=>"", "repository_name"=>"", "uuid"=>"", "writable"=>false}, "/var/spool/cwl"=>{"capacity"=>1073741824, "commit"=>"", "content"=>nil, "device_type"=>"", "exclude_from_output"=>false, "git_url"=>"", "kind"=>"tmp", "path"=>"", "portable_data_hash"=>"", "repository_name"=>"", "uuid"=>"", "writable"=>false}}) (req-evc322zkn8hjjscktltz)
Actions #9

Updated by Lucas Di Pentima over 2 years ago

It seems that controller is passing empty (zero-value) fields and that's why the validation fails:

Previous state

{
    "/keep/bcca5ee5b64d1f30296f204be49fc050+270"=>{
        "kind"=>"collection", 
        "portable_data_hash"=>"bcca5ee5b64d1f30296f204be49fc050+270" 
    }, 
    "/tmp"=>{
        "capacity"=>1073741824, 
        "kind"=>"tmp" 
    }, 
    "/var/spool/cwl"=>{
        "capacity"=>1073741824, 
        "kind"=>"tmp" 
    }
} 

New state

{
    "/keep/bcca5ee5b64d1f30296f204be49fc050+270"=>{
        "capacity"=>0, 
        "commit"=>"", 
        "content"=>nil, 
        "device_type"=>"", 
        "exclude_from_output"=>false, 
        "git_url"=>"", 
        "kind"=>"collection", 
        "path"=>"", 
        "portable_data_hash"=>"bcca5ee5b64d1f30296f204be49fc050+270", 
        "repository_name"=>"", 
        "uuid"=>"", 
        "writable"=>false
    }, 
    "/tmp"=>{
        "capacity"=>1073741824, 
        "commit"=>"", 
        "content"=>nil, 
        "device_type"=>"", 
        "exclude_from_output"=>false, 
        "git_url"=>"", 
        "kind"=>"tmp", 
        "path"=>"", 
        "portable_data_hash"=>"", 
        "repository_name"=>"", 
        "uuid"=>"", 
        "writable"=>false
    }, 
    "/var/spool/cwl"=>{
        "capacity"=>1073741824, 
        "commit"=>"", 
        "content"=>nil, 
        "device_type"=>"", 
        "exclude_from_output"=>false, 
        "git_url"=>"", 
        "kind"=>"tmp", 
        "path"=>"", 
        "portable_data_hash"=>"", 
        "repository_name"=>"", 
        "uuid"=>"", 
        "writable"=>false
    }
}
Actions #10

Updated by Lucas Di Pentima over 2 years ago

Update: I've kept trying to reproduce the issue and found something interesting: I was able to get the error when attempting to move a CR on pirca (v2.3.1~rc3), but when doing so on ce8i5 or tordo (both v2.4.0~dev) or even jutro (v2.2.1), they worked just fine.

Actions #11

Updated by Lucas Di Pentima over 2 years ago

Status update: While trying to reproduce the issue on local arvbox instances running different arvados versions, I realized that the failure isn't related to the version as previously thought, but the bug is triggered by some other reason. Example:

Both CRs have a read-only collection-type mount and several temp mounts.

Actions #12

Updated by Tom Clegg over 2 years ago

services/api/app/models/container_request.rb

  def check_update_whitelist
    ...
    elsif mounts_changed? && mounts_was.keys == mounts.keys
      ...

This looks like it's unintentionally sensitive to key order. Perhaps the outcome is different when, while reformatting the get/list response, controller sorts the keys differently than the program that created (or most recently updated) the record?

Actions #13

Updated by Lucas Di Pentima over 2 years ago

The ordering didn't seem to be a problem (although I think it won't hurt to .sort() the keys list for comparison), but I think I've found what's the problem: The code from check_update_whitelist doesn't take 0 (zero) as one of the zero-values.

Actions #14

Updated by Lucas Di Pentima over 2 years ago

Updates at 1f626bc34 - branch 18301-wb1-cancel-button-fix
Test run: developer-run-tests: #2831

  • Adds 0 (zero) as one of the default values that needs to be ignored when validating changes on mounts.
  • Adds test case.
Actions #15

Updated by Lucas Di Pentima over 2 years ago

  • Target version changed from 2021-12-08 sprint to 2022-01-05 sprint
Actions #16

Updated by Peter Amstutz over 2 years ago

Lucas Di Pentima wrote:

Updates at 1f626bc34 - branch 18301-wb1-cancel-button-fix
Test run: developer-run-tests: #2831

  • Adds 0 (zero) as one of the default values that needs to be ignored when validating changes on mounts.
  • Adds test case.

This LGTM

Actions #17

Updated by Lucas Di Pentima over 2 years ago

  • Status changed from In Progress to Resolved

Applied in changeset arvados-private:commit:arvados|2f344e8b8dde661e74307ed7e561a758809382e1.

Actions #18

Updated by Peter Amstutz over 2 years ago

  • Release set to 46
Actions

Also available in: Atom PDF