Connect My Computer: Keeping compatibility promise #31951

gzdunek · 2023-09-15T14:25:55Z

Contributes to #27881

This PR adds a compatibility information when the agent is not compatible with the cluster or is on a lower version and can be upgraded.
You can read more about it in the RFD [Agent versioning].
Short example: when the cluster is on version v15 it supports components (clients, nodes) that are on versions v15 and v14.

Setup

Status

Not compatible

Upgrade suggestion (I'm not sure if the copy reads well here, any recommendations are welcome)

gzdunek · 2023-09-15T14:40:48Z

@roraback WDYT of the proposed designs?

ravicious

I was thinking if there's a way to simplify the logic by having an enum instead of a bool to answer the question "is the agent compatible?", like "compatible" | "agentTooOld" | "agentTooNew", but I don't think it'd help much here.

I'm still not sure about recommending the downgrade of the app, but I guess it makes sense in a scenario where the user uses our cloud and cannot downgrade the cluster.

In all other cases I'd assume that the person using Connect My Computer is also the cluster admin in some sense and has control over the version.

lib/teleterm/clusters/cluster.go

web/packages/teleterm/src/ui/Search/useSearch.test.tsx

web/packages/teleterm/src/ui/ConnectMyComputer/CompatibilityPromise.tsx

.../ConnectMyComputer/DocumentConnectMyComputerStatus/DocumentConnectMyComputerStatus.story.tsx

web/packages/teleterm/src/ui/ConnectMyComputer/connectMyComputerContext.tsx

...ui/ConnectMyComputer/DocumentConnectMyComputerSetup/DocumentConnectMyComputerSetup.story.tsx

web/packages/teleterm/src/ui/ConnectMyComputer/CompatibilityPromise.tsx

…eRootCluster`/`makeLeafCluster`

…text

gzdunek · 2023-09-20T10:09:06Z

I'm still not sure about recommending the downgrade of the app, but I guess it makes sense in a scenario where the user uses our cloud and cannot downgrade the cluster.

Yeah, I thought exactly about the cloud case.

ravicious · 2023-09-20T10:37:38Z

web/packages/teleterm/src/ui/ConnectMyComputer/UpgradeAgentSuggestion.tsx

+  runtimeSettings: Pick<RuntimeSettings, 'appVersion' | 'isLocalBuild'>
+): boolean {
+  const { appVersion, isLocalBuild } = runtimeSettings;
+  const isClusterOnOlderVersion =


Suggested change

const isClusterOnOlderVersion =

const isAgentOnOlderVersion =

web/packages/teleterm/src/ui/ConnectMyComputer/CompatibilityPromise.tsx

ravicious

This looks really solid now. I submitted one more suggestion to indicate incompatibility issues in the navigation menu icon.

ravicious · 2023-09-20T10:52:37Z

web/packages/teleterm/src/ui/ConnectMyComputer/connectMyComputerContext.tsx

@@ -283,7 +283,7 @@ export const ConnectMyComputerContextProvider: FC<{
    const shouldAutoStartAgent =
      isAgentConfigured &&
      canUse &&
-      !isNonCompatibleAgent &&
+      isAgentCompatible &&


This made me realize: if the agent has been configured but it wasn't started because it's no longer compatible with the proxy, maybe we could display a warning indicator in the icon?

It should be a matter of doing this but please double check this:

diff --git a/web/packages/teleterm/src/ui/ConnectMyComputer/NavigationMenu.tsx b/web/packages/teleterm/src/ui/ConnectMyComputer/NavigationMenu.tsx index ac8405ceda..25d92d7d55 100644 --- a/web/packages/teleterm/src/ui/ConnectMyComputer/NavigationMenu.tsx +++ b/web/packages/teleterm/src/ui/ConnectMyComputer/NavigationMenu.tsx @@ -112,15 +112,28 @@ export function NavigationMenu() { function getIndicatorStatus( currentAction: CurrentAction, - isAgentConfiguredAttempt: Attempt<boolean> + isAgentConfiguredAttempt: Attempt<boolean>, + isAgentCompatible: boolean ): IndicatorStatus { if (isAgentConfiguredAttempt.status === 'error') { return 'error'; } + const isAgentConfigured = + isAgentConfiguredAttempt.status === 'success' && + isAgentConfiguredAttempt.data; + + if (!isAgentConfigured) { + return ''; + } + if (currentAction.kind === 'observe-process') { switch (currentAction.agentProcessState.status) { case 'not-started': { + if (!isAgentCompatible) { + return 'error'; + } + return ''; } case 'error': {

Good catch! We should definitely show it on the icon.

I realized that in an ideal world, the error indicator would only show up if autostart was prevented due to incompatibility issues. Otherwise if you configure the agent, stop it and compatibility issues occur in the future, you're going to see the error indicator even though you're not actively using the agent.

Anyways, I think we can keep it as it is, at least it'll nudge those users towards an upgrade of Connect.

Yes, it would be better, but IMO it's still not bad, at least users will be more aware that they are using an incompatible version.

I ran into another issue with the indicator.

isAgentCompatible looks at rootCluster.proxyVersion, but this information is not immediately available. So if I configure my agent and then restart the app, the indicator will show a warning color until the app fetches detailed information about the cluster.

I think we should be able to fix this by replacing isAgentCompatible with agentCompatibility that can be one of three values: compatible, incompatible, unknown. I'll attempt to fix it next week.

Also, during the review I forgot that differentiating between success & error states with just color is usually not enough for users with red-green confusion.

Screen.Recording.2023-09-22.at.15.57.05.mov

Ah, I didn't think of it 😞

I think we should be able to fix this by replacing isAgentCompatible with agentCompatibility that can be one of three values: compatible, incompatible, unknown.

👍

As for colors, probably we should use an icon? I can take a look at it.

Yeah, an icon would be a good solution.

ravicious · 2023-09-20T10:54:58Z

web/packages/teleterm/src/ui/ConnectMyComputer/CompatibilityPromise.tsx

+        , clusters don't support clients that are on a newer major version. If
+        you wish to connect your computer,{' '}
+        {isAppDowngradePossible && (
+          <>downgrade the app to {downgradeAppTo} or </>


Suggested change

<>downgrade the app to {downgradeAppTo} or </>

<>downgrade the app to version {downgradeAppTo} or </>

I think it should say that, otherwise we say "version" for the cluster and just the number alone for the app.

Changed also in the upgrade suggestion.

I think we can also cut out "If you wish" and say To connect your computer, downgrade...

Done, now it says: To use Connect My Computer, downgrade... (we already changed the copy a bit).

roraback · 2023-09-20T13:20:43Z

@roraback WDYT of the proposed designs?

I think the designs work well for now. Thanks for pinging me!

FYI, longer term I'd love to move to a different error message style that Rishi has been working on, and that handles CTAs and two levels of text more gracefully, but the designs are quite new and aren't implemented, yet.

ravicious · 2023-09-21T09:37:01Z

web/packages/teleterm/src/ui/ConnectMyComputer/connectMyComputerContext.tsx

@@ -283,7 +283,7 @@ export const ConnectMyComputerContextProvider: FC<{
    const shouldAutoStartAgent =
      isAgentConfigured &&
      canUse &&
-      !isNonCompatibleAgent &&
+      isAgentCompatible &&


I realized that in an ideal world, the error indicator would only show up if autostart was prevented due to incompatibility issues. Otherwise if you configure the agent, stop it and compatibility issues occur in the future, you're going to see the error indicator even though you're not actively using the agent.

Anyways, I think we can keep it as it is, at least it'll nudge those users towards an upgrade of Connect.

# Conflicts: # web/packages/teleterm/src/ui/ConnectMyComputer/DocumentConnectMyComputerSetup/DocumentConnectMyComputerSetup.tsx # web/packages/teleterm/src/ui/ConnectMyComputer/DocumentConnectMyComputerStatus/DocumentConnectMyComputerStatus.tsx # web/packages/teleterm/src/ui/ConnectMyComputer/connectMyComputerContext.tsx

public-teleport-github-review-bot · 2023-09-21T14:14:40Z

@gzdunek See the table below for backport results.

Branch	Result
branch/v14	Failed

public-teleport-github-review-bot · 2023-09-22T15:33:05Z

@gzdunek See the table below for backport results.

Branch	Result
branch/v14	Failed

* Add server version to cluster * Add components to display compatibility promise * Show compatibility promise on status page * Show compatibility promise on setup page * Rename `serverVersion` -> `proxyVersion`, make all places to use `makeRootCluster`/`makeLeafCluster` * Move `UpgradeAgentSuggestion` to a new file, make it stateless * Return `isAgentCompatible` instead of `isNonCompatibleAgent` from context * Add // DELETE IN comments * Improve copies * Add a story for too old client in Setup * Extract CONNECT_MY_COMPUTER_RELEASE_MAJOR_VERSION * Run prettier * Fix license * Show an error on the CMC icon when the agent is not compatible * Always say "version" before the version number * Adjust tests * Drop "if you wish" from the copies (cherry picked from commit 81c352c)

gzdunek requested a review from avatus September 15, 2023 14:25

gzdunek force-pushed the gzdunek/cmc-compatibility-promise branch 2 times, most recently from bdeb65d to dd2bbab Compare September 15, 2023 14:34

gzdunek requested review from ravicious and roraback September 15, 2023 14:35

gzdunek marked this pull request as ready for review September 15, 2023 14:35

github-actions bot added size/md ui labels Sep 15, 2023

github-actions bot requested review from kimlisa and rudream September 15, 2023 14:36

gzdunek removed request for kimlisa and rudream September 15, 2023 14:42

gzdunek force-pushed the gzdunek/cmc-compatibility-promise branch from dd2bbab to fad2508 Compare September 15, 2023 14:52

gzdunek mentioned this pull request Sep 15, 2023

Connect My Computer #27881

Closed

8 tasks

Base automatically changed from gzdunek/do-not-omit-checking-permissions to master September 18, 2023 05:28

gzdunek added 4 commits September 18, 2023 10:09

Add server version to cluster

1d75e91

Add components to display compatibility promise

d90a3b6

Show compatibility promise on status page

f0a570b

Show compatibility promise on setup page

bfacfa0

gzdunek force-pushed the gzdunek/cmc-compatibility-promise branch from fad2508 to bfacfa0 Compare September 18, 2023 08:10

ravicious reviewed Sep 18, 2023

View reviewed changes

avatus reviewed Sep 18, 2023

View reviewed changes

web/packages/teleterm/src/ui/ConnectMyComputer/CompatibilityPromise.tsx Outdated Show resolved Hide resolved

gzdunek added 7 commits September 20, 2023 09:19

Rename serverVersion -> proxyVersion, make all places to use `mak…

7b50dad

…eRootCluster`/`makeLeafCluster`

Move UpgradeAgentSuggestion to a new file, make it stateless

29c16c5

Return isAgentCompatible instead of isNonCompatibleAgent from con…

c5c7755

…text

Add // DELETE IN comments

3c5823f

Improve copies

8f0b2d4

Add a story for too old client in Setup

c4d4744

Extract CONNECT_MY_COMPUTER_RELEASE_MAJOR_VERSION

1a8f4b1

Run prettier

5312c24

gzdunek requested a review from ravicious September 20, 2023 10:10

gzdunek added the backport/branch/v14 label Sep 20, 2023

Fix license

b97a21f

ravicious reviewed Sep 20, 2023

View reviewed changes

gzdunek added 3 commits September 20, 2023 13:12

Show an error on the CMC icon when the agent is not compatible

02d3218

Always say "version" before the version number

4983018

Adjust tests

a4d609b

gzdunek requested a review from ravicious September 20, 2023 11:20

Drop "if you wish" from the copies

3dd2c53

avatus approved these changes Sep 20, 2023

View reviewed changes

ravicious approved these changes Sep 21, 2023

View reviewed changes

gzdunek enabled auto-merge September 21, 2023 13:36

gzdunek added this pull request to the merge queue Sep 21, 2023

Merged via the queue into master with commit 81c352c Sep 21, 2023

gzdunek deleted the gzdunek/cmc-compatibility-promise branch September 21, 2023 14:13

gzdunek mentioned this pull request Sep 22, 2023

[v14] Connect My Computer: Keeping compatibility promise (#31951) #32394

Merged

gzdunek mentioned this pull request Sep 25, 2023

Connect My Computer: Agent compatibility fixes #32477

Merged

fheinecke mentioned this pull request Sep 26, 2023

Release 14.0.1 #32611

Merged

ravicious mentioned this pull request Sep 27, 2023

Check compatibility when autostarting, not before autostart #32638

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Connect My Computer: Keeping compatibility promise #31951

Connect My Computer: Keeping compatibility promise #31951

gzdunek commented Sep 15, 2023

gzdunek commented Sep 15, 2023

ravicious left a comment

gzdunek commented Sep 20, 2023

ravicious Sep 20, 2023

ravicious left a comment

ravicious Sep 20, 2023

gzdunek Sep 20, 2023

ravicious Sep 21, 2023

gzdunek Sep 21, 2023

ravicious Sep 22, 2023

gzdunek Sep 25, 2023

ravicious Sep 25, 2023

ravicious Sep 20, 2023

gzdunek Sep 20, 2023

roraback Sep 20, 2023

gzdunek Sep 20, 2023

roraback commented Sep 20, 2023

ravicious Sep 21, 2023

public-teleport-github-review-bot bot commented Sep 21, 2023

public-teleport-github-review-bot bot commented Sep 22, 2023

	const isClusterOnOlderVersion =
	const isAgentOnOlderVersion =

	<>downgrade the app to {downgradeAppTo} or </>
	<>downgrade the app to version {downgradeAppTo} or </>

Connect My Computer: Keeping compatibility promise #31951

Connect My Computer: Keeping compatibility promise #31951

Conversation

gzdunek commented Sep 15, 2023

Setup

Status

gzdunek commented Sep 15, 2023

ravicious left a comment

Choose a reason for hiding this comment

gzdunek commented Sep 20, 2023

Choose a reason for hiding this comment

ravicious left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

roraback commented Sep 20, 2023

Choose a reason for hiding this comment

public-teleport-github-review-bot bot commented Sep 21, 2023

public-teleport-github-review-bot bot commented Sep 22, 2023