Skip to content

Commit

Permalink
[v14] Improve Connect My Computer UI & logout experience (#32791)
Browse files Browse the repository at this point in the history 
* Kill agent after removing workspace on logout

Quick shutdown of an agent now induces a 3 second timeout if there are
active connections (#31869). I changed the logout procedure so that we
first remove the workspace (and thus close all tabs) and only then kill
the agent. This makes it so that if there were any open connections from
the app, we'll close them before killing the agent, which means the app
will close without that 3 second timeout.

* Make Start Agent button's handler ignore errors

* Avoid rendering labels if there's no node

This could happen if someone started and stopped the agent and then clicked
Start Agent with expired certs. agentNode would go from undefined to defined
back to undefined, but it seems that <Transition> doesn't unmount immediately.

* clusters.Cluster.Logout: Make error handling more explicit

!trace.IsNotFound(err) returns true both when the error is nil and when
it's not nil but the error is not NotFound. Meaning that code after the
conditional would run only when err is NotFound.

I added a bogus error to test something after the conditional and then
spent 30 minutes wondering what's going on.

* Display an empty ring if agent is set up but not running

This makes it consistent with the Connections list which also displays
an empty ring if there are no connections.

* Expand NavigationMenu stories, add story for agent starting

* Avoid calculating indicator status if user cannot use feature

* Copy progress icon from Cloud's ProgressBar

* Return 'not-configured' early if agent is not configured

* Add story for when agent is configured but not started, reorder stories
  • Loading branch information
@ravicious
ravicious authored Sep 29, 2023
1 parent 0d2b682 commit 987c78e
Show file tree
Hide file tree
Showing 6 changed files with 201 additions and 94 deletions.
2 changes: 1 addition & 1 deletion lib/teleterm/clusters/cluster_auth.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ func (c *Cluster) Logout(ctx context.Context) error {
}

// Remove keys for this user from disk and running agent.
if err := c.clusterClient.Logout(); !trace.IsNotFound(err) {
if err := c.clusterClient.Logout(); err != nil && !trace.IsNotFound(err) {
return trace.Wrap(err)
}

Expand Down
16 changes: 12 additions & 4 deletions web/packages/teleterm/src/ui/ClusterLogout/useClusterLogout.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ export function useClusterLogout({
}) {
const ctx = useAppContext();
const [{ status, statusText }, removeCluster] = useAsync(async () => {
await ctx.connectMyComputerService.killAgentAndRemoveData(clusterUri);
await ctx.clustersService.logout(clusterUri);

if (ctx.workspacesService.getRootClusterUri() === clusterUri) {
Expand All @@ -40,11 +39,20 @@ export function useClusterLogout({
}
}

// remove connections first, they depend both on the cluster and the workspace
// Remove connections first, they depend both on the cluster and the workspace.
ctx.connectionTracker.removeItemsBelongingToRootCluster(clusterUri);
// remove the workspace next, because it depends on the cluster

// Remove the workspace next, because it depends on the cluster.
ctx.workspacesService.removeWorkspace(clusterUri);
// remove the cluster, it does not depend on anything

// If there are active ssh connections to the agent, killing it will take a few seconds. To work
// around this, kill the agent only after removing the workspace. Removing the workspace closes
// ssh tabs, so it should terminate connections to the cluster from the app.
//
// If ClustersService.logout above fails, the user should still be able to manage the agent.
await ctx.connectMyComputerService.killAgentAndRemoveData(clusterUri);

// Remove the cluster, it does not depend on anything.
await ctx.clustersService.removeClusterAndResources(clusterUri);
});

Expand Down
52 changes: 34 additions & 18 deletions ...packages/teleterm/src/ui/ConnectMyComputer/DocumentConnectMyComputerSetup/ProgressBar.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
import React from 'react';
import styled, { useTheme } from 'styled-components';
import { Flex, Box } from 'design';
import { Check, Warning, Refresh } from 'design/Icon';
import * as icons from 'design/Icon';
import { decomposeColor, emphasize } from 'design/theme/utils/colorManipulator';
import { AttemptStatus } from 'shared/hooks/useAsync';

Expand Down Expand Up @@ -74,7 +74,13 @@ function Phase({

return (
<>
<StyledPhase mr="3" bg={bg}>
<StyledPhase
mr="3"
bg={bg}
css={`
position: relative;
`}
>
<PhaseIcon status={status} />
</StyledPhase>
{!isLast && (
Expand Down Expand Up @@ -108,29 +114,19 @@ const StyledLine = styled(Box)`

function PhaseIcon({ status }: { status: AttemptStatus }): JSX.Element {
if (status === 'success') {
return <Check size="small" color="white" />;
return <icons.Check size="small" color="white" />;
}

if (status === 'error') {
return <Warning size="small" color="white" />;
return <icons.Warning size="small" color="white" />;
}

if (status === 'processing') {
return (
<Refresh
size="extraLarge"
color="success"
css={`
animation: anim-rotate 1.5s infinite linear;
@keyframes anim-rotate {
0% {
transform: rotate(0);
}
100% {
transform: rotate(360deg);
}
`}
/>
<>
<Spinner />
<icons.Restore size="small" color="buttons.text" />
</>
);
}

Expand All @@ -151,3 +147,23 @@ function getPhaseSolidColor(theme: any): string {
const alpha = decomposeColor(theme.colors.spotBackground[1]).values[3] || 0;
return emphasize(theme.colors.levels.surface, alpha);
}

const Spinner = styled.div`
opacity: 1;
color: ${props => props.theme.colors.spotBackground[2]};
border: 3px solid ${props => props.theme.colors.success};
border-radius: 50%;
border-top: 3px solid ${props => props.theme.colors.spotBackground[0]};
width: 24px;
height: 24px;
position: absolute;
animation: spinner 4s linear infinite;
@keyframes spinner {
0% {
transform: rotate(0deg);
}
100% {
transform: rotate(360deg);
}
}
`;
15 changes: 12 additions & 3 deletions .../ui/ConnectMyComputer/DocumentConnectMyComputerStatus/DocumentConnectMyComputerStatus.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
limitations under the License.
*/

import React from 'react';
import React, { useCallback } from 'react';
import {
Alert,
Box,
Expand Down Expand Up @@ -82,6 +82,13 @@ export function DocumentConnectMyComputerStatus(
const isAgentIncompatible = agentCompatibility === 'incompatible';
const isAgentIncompatibleOrUnknown =
agentCompatibility === 'incompatible' || agentCompatibility === 'unknown';
const downloadAndStartAgentAndIgnoreErrors = useCallback(async () => {
try {
await downloadAndStartAgent();
} catch (error) {
// Ignore the error, it'll be shown in the UI by inspecting the attempts.
}
}, [downloadAndStartAgent]);

const prettyCurrentAction = prettifyCurrentAction(currentAction);

Expand Down Expand Up @@ -222,7 +229,9 @@ export function DocumentConnectMyComputerStatus(
>
{state => (
<LabelsContainer gap={1} className={state}>
{renderLabels(agentNode.labelsList)}
{/* Explicitly check for existence of agentNode because Transition doesn't seem to
unmount immediately when `in` becomes falsy. */}
{agentNode?.labelsList && renderLabels(agentNode.labelsList)}
</LabelsContainer>
)}
</Transition>
Expand Down Expand Up @@ -298,7 +307,7 @@ export function DocumentConnectMyComputerStatus(
<ButtonPrimary
block
disabled={disableStartAgentButton}
onClick={downloadAndStartAgent}
onClick={downloadAndStartAgentAndIgnoreErrors}
size="large"
data-testid="start-agent"
>
Expand Down
94 changes: 75 additions & 19 deletions web/packages/teleterm/src/ui/ConnectMyComputer/NavigationMenu.story.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,7 @@
* limitations under the License.
*/

import React from 'react';

import { wait } from 'shared/utils/wait';
import React, { useEffect, useRef, useLayoutEffect } from 'react';

import { MockWorkspaceContextProvider } from 'teleterm/ui/fixtures/MockWorkspaceContextProvider';
import { makeRootCluster } from 'teleterm/services/tshd/testHelpers';
Expand All @@ -34,6 +32,46 @@ export default {
title: 'Teleterm/ConnectMyComputer/NavigationMenu',
};

export function AgenNotConfigured() {
return (
<ShowState
agentProcessState={{ status: 'not-started' }}
isAgentConfigFileCreated={async () => {
return false;
}}
/>
);
}

export function AgentConfiguredButNotStarted() {
return <ShowState agentProcessState={{ status: 'not-started' }} />;
}

export function AgentStarting() {
const abortControllerRef = useRef(new AbortController());

useEffect(() => {
return () => {
abortControllerRef.current.abort();
};
}, []);

const appContext = new MockAppContext({ appVersion: '17.0.0' });

appContext.connectMyComputerService.downloadAgent = () =>
new Promise((resolve, reject) => {
abortControllerRef.current.signal.addEventListener('abort', () => reject);
});

return (
<ShowState
appContext={appContext}
agentProcessState={{ status: 'not-started' }}
autoStart={true}
/>
);
}

export function AgentRunning() {
return <ShowState agentProcessState={{ status: 'running' }} />;
}
Expand Down Expand Up @@ -76,25 +114,24 @@ export function AgentExitedUnsuccessfully() {
);
}

export function AgentSetupNotDone() {
return (
<ShowState
agentProcessState={{ status: 'not-started' }}
isAgentConfigFileCreated={async () => {
return false;
}}
/>
);
}

export function LoadingAgentConfigFile() {
const abortControllerRef = useRef(new AbortController());

useEffect(() => {
return () => {
abortControllerRef.current.abort();
};
}, []);

const getPromiseRejectedOnUnmount = () =>
new Promise<boolean>((resolve, reject) => {
abortControllerRef.current.signal.addEventListener('abort', () => reject);
});

return (
<ShowState
agentProcessState={{ status: 'not-started' }}
isAgentConfigFileCreated={async () => {
await wait(60_000);
return true;
}}
isAgentConfigFileCreated={getPromiseRejectedOnUnmount}
/>
);
}
Expand All @@ -113,14 +150,18 @@ export function FailedToLoadAgentConfigFile() {
function ShowState({
isAgentConfigFileCreated = async () => true,
agentProcessState,
appContext = new MockAppContext(),
autoStart = false,
}: {
agentProcessState: AgentProcessState;
isAgentConfigFileCreated?: () => Promise<boolean>;
appContext?: MockAppContext;
autoStart?: boolean;
}) {
const cluster = makeRootCluster({
features: { isUsageBasedBilling: true, advancedAccessWorkflows: false },
proxyVersion: '17.0.0',
});
const appContext = new MockAppContext();
appContext.clustersService.state.clusters.set(cluster.uri, cluster);
appContext.configService = createMockConfigService({
'feature.connectMyComputer': true,
Expand All @@ -139,6 +180,21 @@ function ShowState({
appContext.connectMyComputerService.isAgentConfigFileCreated =
isAgentConfigFileCreated;

if (autoStart) {
appContext.workspacesService.setConnectMyComputerAutoStart(
cluster.uri,
true
);
}

useLayoutEffect(() => {
(
document.querySelector(
'[data-testid=connect-my-computer-icon]'
) as HTMLButtonElement
)?.click();
});

return (
<MockAppContextProvider appContext={appContext}>
<MockWorkspaceContextProvider rootClusterUri={cluster.uri}>
Expand Down
Loading

0 comments on commit 987c78e

Please sign in to comment.