Skip to content

Commit

Permalink
shell: Navigate to a newly added host
Browse files Browse the repository at this point in the history 
After adding a host, the Shell will navigate to it immediately.
Previously, the host switcher would stay open with the new host in it
and the user would have to navigate to it explicitly.
  • Loading branch information
@mvollmer @martinpitt
mvollmer authored and martinpitt committed Nov 15, 2024
1 parent f06b13a commit a5c3cf2
Show file tree
Hide file tree
Showing 4 changed files with 72 additions and 53 deletions.
2 changes: 1 addition & 1 deletion pkg/shell/hosts.jsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ export class CockpitHosts extends React.Component {
}

async onAddNewHost() {
await add_host(this.props.host_modal_state);
await add_host(this.props.host_modal_state, this.props.state);
}

async onHostEdit(event, machine) {
Expand Down
28 changes: 20 additions & 8 deletions pkg/shell/hosts_dialog.jsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -78,20 +78,32 @@ export const HostModalState = () => {
return self;
};

export async function add_host(state) {
await state.show_modal({ });
/* Activate and jump to a newly added machine, identified by its
connection string.
*/
function jump_to_new_connection_string(shell_state, connection_string) {
// Get the address of the machine from its connection string
const addr = split_connection_string(connection_string).address;
// Tell the Loader that now is the time to start monitoring the
// manifests.
shell_state.loader.connect(addr);
// Navigate to it.
shell_state.jump(build_href({ host: addr }));
}

export async function add_host(state, shell_state) {
const connection_string = await state.show_modal({ });
if (connection_string)
jump_to_new_connection_string(shell_state, connection_string);
}

export async function edit_host(state, shell_state, machine) {
const { current_machine } = shell_state;
const connection_string = await state.show_modal({ address: machine.address });
if (connection_string) {
const parts = split_connection_string(connection_string);
const addr = build_href({ host: parts.address });
if (machine == current_machine && parts.address != machine.address) {
shell_state.loader.connect(parts.address);
shell_state.jump(addr);
}
const addr = split_connection_string(connection_string).address;
if (machine == current_machine && addr != machine.address)
jump_to_new_connection_string(shell_state, connection_string);
}
}

Expand Down
91 changes: 49 additions & 42 deletions test/verify/check-shell-host-switching
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,12 @@ class HostSwitcherHelpers:
for address in expected:
b._wait_present(f"#hosts_setup_server_dialog datalist option[value='{address}']")

def wait_host_addresses(self, b, expected):
def wait_host_addresses(self, b, expected, host_switcher_is_open=True):
if not host_switcher_is_open:
# wait for host switcher to close
b.wait_not_present("#nav-hosts.interact")
# open it again
b.click("#hosts-sel button")
b.wait_js_cond(f'ph_select("#nav-hosts .nav-item a").length == {len(expected)}')
for address in expected:
if address == "localhost":
Expand Down Expand Up @@ -91,6 +96,14 @@ class HostSwitcherHelpers:
with b.wait_timeout(30):
b.wait_not_present('#hosts_setup_server_dialog')

def wait_connected(self, b, address, expected_user=None):
b.wait_visible(f".connected a[href='/@{address}']")
if expected_user:
b.wait_text("#current-username", expected_user)
# Switch back to localhost, since the rest of the test expects that
b.click("a[href='/']")
b.click("#hosts-sel button")

def connect_and_wait(self, b, address, expected_user=None, expect_warning=False):
b.click(f"a[href='/@{address}']")
if expect_warning:
Expand All @@ -101,12 +114,7 @@ class HostSwitcherHelpers:
b.wait_not_present("#nav-hosts.interact")
# open it again
b.click("#hosts-sel button")
b.wait_visible(f".connected a[href='/@{address}']")
if expected_user:
b.wait_text("#current-username", expected_user)
# Switch back to localhost, since the rest of the test expects that
b.click("a[href='/']")
b.click("#hosts-sel button")
self.wait_connected(b, address, expected_user)


@testlib.skipBeiboot("host switching disabled in beiboot mode")
Expand Down Expand Up @@ -212,9 +220,9 @@ class TestHostSwitching(testlib.MachineCase, HostSwitcherHelpers):
b.click('#hosts_setup_server_dialog .pf-m-primary')
b.wait_not_present('#hosts_setup_server_dialog')

self.wait_host_addresses(b, ["localhost", "10.111.113.2"])
self.wait_host_addresses(b, ["localhost", "10.111.113.2"], host_switcher_is_open=False)
# defaults to current host user name "admin"
self.connect_and_wait(b, "10.111.113.2", "admin")
self.wait_connected(b, "10.111.113.2", "admin")

# Main host should have both buttons disabled, the second both enabled
b.click("button:contains('Edit hosts')")
Expand All @@ -230,8 +238,8 @@ class TestHostSwitching(testlib.MachineCase, HostSwitcherHelpers):
b.wait_not_present(".nav-item a[href='/@10.111.113.2'] .nav-status")

self.add_new_machine(b, "10.111.113.3")
self.wait_host_addresses(b, ["localhost", "10.111.113.3", "10.111.113.2"])
self.connect_and_wait(b, "10.111.113.3", "admin")
self.wait_host_addresses(b, ["localhost", "10.111.113.3", "10.111.113.2"], host_switcher_is_open=False)
self.wait_connected(b, "10.111.113.3", "admin")

b.assert_pixels("#nav-hosts", "nav-hosts-2-remotes")

Expand All @@ -247,16 +255,16 @@ class TestHostSwitching(testlib.MachineCase, HostSwitcherHelpers):

# Add one back, check addresses on both browsers
self.add_new_machine(b, "10.111.113.2", known_host=True)
self.wait_host_addresses(b, ["localhost", "10.111.113.2"])
self.connect_and_wait(b, "10.111.113.2")
self.wait_host_addresses(b, ["localhost", "10.111.113.2"], host_switcher_is_open=False)
self.wait_connected(b, "10.111.113.2")
self.check_discovered_addresses(b, ["10.111.113.3"])

b.wait_not_present(".nav-item a[href='/@10.111.113.2'] .nav-status")

# And the second one, check addresses
self.add_new_machine(b, "10.111.113.3", known_host=True)
self.wait_host_addresses(b, ["localhost", "10.111.113.2", "10.111.113.3"])
self.connect_and_wait(b, "10.111.113.3")
self.wait_host_addresses(b, ["localhost", "10.111.113.2", "10.111.113.3"], host_switcher_is_open=False)
self.wait_connected(b, "10.111.113.3")
self.check_discovered_addresses(b, [])

# Test change user, not doing in edit to reuse machines
Expand Down Expand Up @@ -324,14 +332,14 @@ class TestHostSwitching(testlib.MachineCase, HostSwitcherHelpers):

# plain address and separate "User name:" field
self.add_new_machine(b, "10.111.113.2", known_host=True, user="someone", expect_password_auth=True)
self.wait_host_addresses(b, ["localhost", "10.111.113.2"])
self.connect_and_wait(b, "10.111.113.2", "someone")
self.wait_host_addresses(b, ["localhost", "10.111.113.2"], host_switcher_is_open=False)
self.wait_connected(b, "10.111.113.2", "someone")
self.machine_remove(b, "10.111.113.2", second_to_last=True)

# address with user and different "User name:" field, latter wins
self.add_new_machine(b, "[email protected]", known_host=True, user="someone", expect_password_auth=True)
self.wait_host_addresses(b, ["localhost", "10.111.113.2"])
self.connect_and_wait(b, "10.111.113.2", "someone")
self.wait_host_addresses(b, ["localhost", "10.111.113.2"], host_switcher_is_open=False)
self.wait_connected(b, "10.111.113.2", "someone")
self.machine_remove(b, "10.111.113.2", second_to_last=True)

# switch off warnings for the rest of this test (nneds the
Expand All @@ -346,37 +354,37 @@ class TestHostSwitching(testlib.MachineCase, HostSwitcherHelpers):

# ssh:// prefix and implied user, no warning because we switched it off above
self.add_new_machine(b, "ssh://10.111.113.2", known_host=True, expect_warning=False)
self.wait_host_addresses(b, ["localhost", "10.111.113.2"])
self.connect_and_wait(b, "10.111.113.2", "admin")
self.wait_host_addresses(b, ["localhost", "10.111.113.2"], host_switcher_is_open=False)
self.wait_connected(b, "10.111.113.2", "admin")
self.machine_remove(b, "10.111.113.2", second_to_last=True)

# ssh:// prefix and separate "User name:" field
self.add_new_machine(b, "ssh://10.111.113.2", known_host=True, user="admin")
self.wait_host_addresses(b, ["localhost", "10.111.113.2"])
self.connect_and_wait(b, "10.111.113.2", "admin")
self.wait_host_addresses(b, ["localhost", "10.111.113.2"], host_switcher_is_open=False)
self.wait_connected(b, "10.111.113.2", "admin")
self.machine_remove(b, "10.111.113.2", second_to_last=True)

self.add_new_machine(b, "ssh://10.111.113.2", known_host=True, user="someone", expect_password_auth=True)
self.wait_host_addresses(b, ["localhost", "10.111.113.2"])
self.connect_and_wait(b, "10.111.113.2", "someone")
self.wait_host_addresses(b, ["localhost", "10.111.113.2"], host_switcher_is_open=False)
self.wait_connected(b, "10.111.113.2", "someone")
self.machine_remove(b, "10.111.113.2", second_to_last=True)

# ssh:// prefix with user name
self.add_new_machine(b, "ssh://[email protected]", known_host=True, expect_password_auth=True)
self.wait_host_addresses(b, ["localhost", "10.111.113.2"])
self.connect_and_wait(b, "10.111.113.2", "someone")
self.wait_host_addresses(b, ["localhost", "10.111.113.2"], host_switcher_is_open=False)
self.wait_connected(b, "10.111.113.2", "someone")
self.machine_remove(b, "10.111.113.2", second_to_last=True)

# ssh:// prefix with user and different "User name:" field, latter wins
self.add_new_machine(b, "ssh://[email protected]", known_host=True, user="someone", expect_password_auth=True)
self.wait_host_addresses(b, ["localhost", "10.111.113.2"])
self.connect_and_wait(b, "10.111.113.2", "someone")
self.wait_host_addresses(b, ["localhost", "10.111.113.2"], host_switcher_is_open=False)
self.wait_connected(b, "10.111.113.2", "someone")
self.machine_remove(b, "10.111.113.2", second_to_last=True)

# ssh:// prefix with user name and port in the connection target
self.add_new_machine(b, "ssh://[email protected]:22", known_host=True)
self.wait_host_addresses(b, ["localhost", "10.111.113.2"])
self.connect_and_wait(b, "10.111.113.2")
self.wait_host_addresses(b, ["localhost", "10.111.113.2"], host_switcher_is_open=False)
self.wait_connected(b, "10.111.113.2")
self.machine_remove(b, "10.111.113.2", second_to_last=True)

self.allow_journal_messages(".*server offered unsupported authentication methods: password public-key.*")
Expand Down Expand Up @@ -410,9 +418,9 @@ class TestHostSwitching(testlib.MachineCase, HostSwitcherHelpers):
self.wait_host_addresses(b2, ["localhost"])

self.add_new_machine(b, "10.111.113.2", expect_warning=True)
self.wait_host_addresses(b, ["localhost", "10.111.113.2"])
self.wait_host_addresses(b, ["localhost", "10.111.113.2"], host_switcher_is_open=False)
self.wait_host_addresses(b2, ["localhost", "10.111.113.2"])
self.connect_and_wait(b, "10.111.113.2")
self.wait_connected(b, "10.111.113.2")
self.connect_and_wait(b2, "10.111.113.2", expect_warning=True)

# Main host should have both buttons disabled, the second both enabled
Expand All @@ -428,9 +436,9 @@ class TestHostSwitching(testlib.MachineCase, HostSwitcherHelpers):
b.wait_not_present(".nav-item a[href='/@10.111.113.2'] .nav-status")

self.add_new_machine(b, "10.111.113.3")
self.wait_host_addresses(b, ["localhost", "10.111.113.3", "10.111.113.2"])
self.wait_host_addresses(b, ["localhost", "10.111.113.3", "10.111.113.2"], host_switcher_is_open=False)
self.wait_host_addresses(b2, ["localhost", "10.111.113.3", "10.111.113.2"])
self.connect_and_wait(b, "10.111.113.3")
self.wait_connected(b, "10.111.113.3")
self.connect_and_wait(b2, "10.111.113.3")

# Remove two
Expand All @@ -449,19 +457,19 @@ class TestHostSwitching(testlib.MachineCase, HostSwitcherHelpers):

# Add one back, check addresses on both browsers
self.add_new_machine(b, "10.111.113.2", known_host=True)
self.wait_host_addresses(b, ["localhost", "10.111.113.2"])
self.wait_host_addresses(b, ["localhost", "10.111.113.2"], host_switcher_is_open=False)
self.wait_host_addresses(b2, ["localhost", "10.111.113.2"])
self.connect_and_wait(b, "10.111.113.2")
self.wait_connected(b, "10.111.113.2")
self.check_discovered_addresses(b, ["10.111.113.3"])
self.check_discovered_addresses(b2, ["10.111.113.3"])

b.wait_not_present(".nav-item a[href='/@10.111.113.2'] .nav-status")

# And the second one, check addresses on both browsers
self.add_new_machine(b, "10.111.113.3", known_host=True)
self.wait_host_addresses(b, ["localhost", "10.111.113.2", "10.111.113.3"])
self.wait_host_addresses(b, ["localhost", "10.111.113.2", "10.111.113.3"], host_switcher_is_open=False)
self.wait_host_addresses(b2, ["localhost", "10.111.113.2", "10.111.113.3"])
self.connect_and_wait(b, "10.111.113.3")
self.wait_connected(b, "10.111.113.3")
self.check_discovered_addresses(b, [])
self.check_discovered_addresses(b2, [])

Expand Down Expand Up @@ -490,8 +498,8 @@ class TestHostSwitching(testlib.MachineCase, HostSwitcherHelpers):

b.click("#hosts-sel button")
self.add_new_machine(b, "10.111.113.3", expect_warning=True)
self.wait_host_addresses(b, ["localhost", "10.111.113.3"])
self.connect_and_wait(b, "10.111.113.3")
self.wait_host_addresses(b, ["localhost", "10.111.113.3"], host_switcher_is_open=False)
self.wait_connected(b, "10.111.113.3")

b.click("button:contains('Edit hosts')")
b.click("#nav-hosts .nav-item a[href='/@10.111.113.3'] + span button.nav-action.pf-m-secondary")
Expand Down Expand Up @@ -573,7 +581,6 @@ class TestHostSwitching(testlib.MachineCase, HostSwitcherHelpers):
# Add and connect to a second machine
b.click("#hosts-sel button")
self.add_new_machine(b, "10.111.113.2", expect_warning=True)
b.click("a[href='/@10.111.113.2']")
b.wait_visible("iframe.container-frame[name='cockpit1:10.111.113.2/system']")
self.assertIn("admin", m2.execute("loginctl"))
b.click("#hosts-sel button")
Expand Down
4 changes: 2 additions & 2 deletions test/verify/check-shell-multi-machine-key
View file
Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,7 @@ Host 10.111.113.2
b.wait_in_text("#hosts_setup_server_dialog", "/home/admin/.ssh/id_ed25519")
b.set_input_text("#locked-identity-password", "locked")
b.click("#hosts_setup_server_dialog .pf-m-primary")
b.wait_visible("a[href='/@10.111.113.2']")
b.enter_page("/system", "10.111.113.2")
self.allow_hostkey_messages()

def testLockedDefaultIdentity(self):
Expand Down Expand Up @@ -295,7 +295,7 @@ Host 10.111.113.2
b.wait_in_text("#hosts_setup_server_dialog", "/home/admin/.ssh/id_rsa")
b.set_input_text("#locked-identity-password", "foobarfoo")
b.click("#hosts_setup_server_dialog .pf-m-primary")
b.wait_visible("a[href='/@10.111.113.2']")
b.enter_page("/system", "10.111.113.2")
self.allow_hostkey_messages()


Expand Down

0 comments on commit a5c3cf2

Please sign in to comment.