Project

General

Profile

Actions

Feature #15736

closed

[WB2] add fallback mechanism to site manager for adding ephemeral clusters

Added by Ward Vandewege over 2 years ago. Updated over 2 years ago.

Status:
Resolved
Priority:
Normal
Assigned To:
Category:
-
Target version:
Start date:
11/04/2019
Due date:
% Done:

100%

Estimated time:
(Total: 0.00 h)
Story points:
2.0
Release relationship:
Auto

Description

In workbench, it is possible to add a cluster to the site manager and log in to a remote user, even if that cluster is not defined in the federation and there is no remote user account that WB can access: the user gets redirected to the appropriate SSO, and then eventually returned to the requesting WB, having a logged in session with the remote cluster.

For WB2, this doesn't work anymore: the user simply sees "Could not validate cluster" when trying to do this. E.g. when trying to add qr1hi.arvadosapi.com from wb2.4xphq's site manager.

We want to keep supporting this feature to allow for multisite search to an ephemeral cluster.


Subtasks 1 (0 open1 closed)

Task #15787: Review 15736-site-mgrResolvedPeter Amstutz11/04/2019

Actions
Actions #1

Updated by Ward Vandewege over 2 years ago

  • Status changed from New to In Progress
Actions #2

Updated by Ward Vandewege over 2 years ago

  • Description updated (diff)
  • Status changed from In Progress to New
Actions #3

Updated by Ward Vandewege over 2 years ago

  • Target version set to To Be Groomed
Actions #4

Updated by Peter Amstutz over 2 years ago

  • Assigned To set to Peter Amstutz
  • Target version changed from To Be Groomed to 2019-11-06 Sprint
Actions #5

Updated by Peter Amstutz over 2 years ago

  • Story points set to 2.0
Actions #6

Updated by Peter Amstutz over 2 years ago

15736-site-mgr @ commit:d6538e3ec3a545258ff9073ef88501fe2c75a4f0

Can now add other clusters that are not part of the federation to multisite search by going through login process.

To test: run wb2 with c97qk as the local cluster (config.json has "API_HOST": "c97qk.arvadosapi.com"). Go to site manager and add qr1hi.arvadosapi.com. It will take you to log in, then return to wb2 "add-session" endpoint, record the session, and return to site manager.

Also improves error handling for searches. Errors in session validation are reported. Also, if one search fails in multisite search, it does not prevent other searches from being displayed.

Actions #7

Updated by Lucas Di Pentima over 2 years ago

  • There’s a test that needs updating at src/store/auth/auth-action.test.ts
  • On wb1 there’s the option to remove a remote cluster session from outside the federation, maybe it should be added on wb2?
  • When logging off some remote clusters (either from the federation or not), immediate searches seem to be done on the remaining logged in remotes, but if the app is reloaded, the remote sessions get logged in back.
Actions #8

Updated by Peter Amstutz over 2 years ago

Lucas Di Pentima wrote:

  • There’s a test that needs updating at src/store/auth/auth-action.test.ts

Fixed.

  • On wb1 there’s the option to remove a remote cluster session from outside the federation, maybe it should be added on wb2?

Sure. Added.

  • When logging off some remote clusters (either from the federation or not), immediate searches seem to be done on the remaining logged in remotes, but if the app is reloaded, the remote sessions get logged in back.

The new default behavior is to always try to log in when the app loads. Right now, the "not logged in" state can mean either it previously tried to connect and failed, or that the user manually pressed log out. The user can also now delete it from the list. Although, members of the federation are always re-added to the sessions list, so they will come back on the next app refresh.

We could add a new "disabled" state but actually I think the current behavior performing federation log in by default is actually better for the user, it reduces failure modes where sites that should have been search don't get searched (compare the earlier WB2 multi-site search behavior prior to my recent branches, it has a sticky log-in flag that isn't re-validated by default, we've already had support requests about search not working because it didn't log in by default.)

15736-site-mgr @ commit:ceb4f50aeca2bb6b0354a7bd96a599b4a14147fe

Actions #9

Updated by Lucas Di Pentima over 2 years ago

On wb1, the removable cluster sessions are the ones not part of the federation. Here, all sessions are removable, even the local one. When removing all sessions and then trying to add one, I get an error like:

Objects are not valid as a React child (found: Error: Could not validate cluster). If you meant to render a collection of children, use an array instead.
    in p (created by FormHelperText)
    in FormHelperText (created by WithStyles(FormHelperText))
    in WithStyles(FormHelperText) (created by TextField)
    in div (created by FormControl)
[...]

If not possible to match wb1's functionality, I think at least we shouldn't allow removing the local cluster's session.

With that, it LGTM.

Actions #10

Updated by Peter Amstutz over 2 years ago

Lucas Di Pentima wrote:

On wb1, the removable cluster sessions are the ones not part of the federation. Here, all sessions are removable, even the local one. When removing all sessions and then trying to add one, I get an error like:

[...]

If not possible to match wb1's functionality, I think at least we shouldn't allow removing the local cluster's session.

With that, it LGTM.

Thanks, the delete button doesn't show for those.

In the process, found and fixed a config loading bug as well.

15736-site-mgr @ commit:a1cfba8c2407131a8c5d6c21dc741d6319f8648a

Actions #11

Updated by Lucas Di Pentima over 2 years ago

The updates LGTM, but I think I catched another issue (sorry for not testing it earlier!): When searching, if I try to click on a search result from a non-federated remote cluster, nothing happens. I've checked the JS console & Dev network tab and couldn't see any kind of activity/error message.

Actions #12

Updated by Peter Amstutz over 2 years ago

Lucas Di Pentima wrote:

The updates LGTM, but I think I catched another issue (sorry for not testing it earlier!): When searching, if I try to click on a search result from a non-federated remote cluster, nothing happens. I've checked the JS console & Dev network tab and couldn't see any kind of activity/error message.

So the issue was no config entry for the non-federated remote cluster, so it didn't have a workbench address to use.

I did some more refactoring, and now it saves the config from every cluster that it fetches. Also eliminated some redundant config fetching.

To assist navigation, I added the cluster id to the title bar as well.

15736-site-mgr @ commit:087d49d5c43866c8a20e8ac830ccc9b12188408f

Actions #13

Updated by Lucas Di Pentima over 2 years ago

Clicking now works for me, thanks!

One more issue I've just found: when adding a remote cluster, it isn't being added to the localStorage.sessions array, so it dissapears once the page is reloaded. I thought I checked this but I tried it even on commit:ceb4f50a and it wasn't working then.

Actions #14

Updated by Peter Amstutz over 2 years ago

Lucas Di Pentima wrote:

Clicking now works for me, thanks!

One more issue I've just found: when adding a remote cluster, it isn't being added to the localStorage.sessions array, so it dissapears once the page is reloaded. I thought I checked this but I tried it even on commit:ceb4f50a and it wasn't working then.

Argh! I swear that was working before.

Actions #15

Updated by Lucas Di Pentima over 2 years ago

So after some more testing, we've found that this happens when using 4xphq as a local cluster. Using c97qk does work correctly.

Actions #16

Updated by Peter Amstutz over 2 years ago

Lucas Di Pentima wrote:

So after some more testing, we've found that this happens when using 4xphq as a local cluster. Using c97qk does work correctly.

Ok, I don't know how the cluster itself factors into this, but I found the bug. It was saving the session with the newly added cluster, but then a pending callback was saving the session again with an older value, that overwrote and forgot the newly added cluster. Whoops.

I suspect that "sync sessions to local storage" ought to be hooked up as some kind of listener or middleware instead of the current ad hoc method (do some stuff, then save), but I don't want to get into it.

15736-site-mgr @ commit:dec81d42222814723ff0386665928045ff41eac8

Actions #17

Updated by Lucas Di Pentima over 2 years ago

commit:dec81d42222814723ff0386665928045ff41eac8 worked great, LGTM. Thanks!

Actions #18

Updated by Peter Amstutz over 2 years ago

  • Status changed from New to Resolved
Actions #19

Updated by Tom Morris over 2 years ago

  • Release set to 22
Actions

Also available in: Atom PDF