[CRDB-8488] ui: add alert banner on overview list page for staggered node versions

[Santamaura]

Users wish to see more info related to how the progress of a cluster upgrade
is going. Since we do not have a cluster upgrade status to use we are
instead showing an alert banner on the overview page when there are more
than one node versions detected. This alert banner lists the node versions
detected and how many nodes are on each version. This is a non-critical
alert and can be dismissed.

Release note (ui change): add alert banner on overview list page for
staggered node versions

Related issue: #67330

Release justification: Low risk ui changes with improved QoL results

[cockroach-teamcity]

This change is 

[dhartunian]

Reviewed all commit messages.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @koorosh, @Santamaura, and @zachlite)

---

pkg/ui/workspaces/db-console/src/redux/alerts.ts, line 159 at r1 (raw file):

      should be investigated if this is unexpected.`,
      dismiss: (dispatch: AppDispatch) => {
        dispatch(staggeredVersionDismissedSetting.set(true));

I'm a bit confused about the two warning selectors. does dismissing one dismiss both? should we just make both alerts show the same message?

---

pkg/ui/workspaces/db-console/src/redux/nodes.ts, line 496 at r1 (raw file):

  nodes => {
    if (!nodes) {
      return undefined;

why not return an empty map?

---

pkg/ui/workspaces/db-console/src/redux/nodes.ts, line 500 at r1 (raw file):

    const versionsMap = new Map();
    nodes.forEach(node => {
      if (!node?.build_info?.tag) {

if you want, you can use _.countBy as a shortcut

[Santamaura]

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @Annebirzin, @dhartunian, @koorosh, @thtruo, and @zachlite)

---

pkg/ui/workspaces/db-console/src/redux/alerts.ts, line 159 at r1 (raw file):

Previously, dhartunian (David Hartunian) wrote…

I'm a bit confused about the two warning selectors. does dismissing one dismiss both? should we just make both alerts show the same message?

Yeah since the warning banners information is about similar things I thought it made sense when a user dismisses one of them to also dismiss the other. @thtruo @Annebirzin What are your thoughts on if we should make both of them show the same message?

---

pkg/ui/workspaces/db-console/src/redux/nodes.ts, line 496 at r1 (raw file):

Previously, dhartunian (David Hartunian) wrote…

why not return an empty map?

Agree, it makes more sense to return empty map.

---

pkg/ui/workspaces/db-console/src/redux/nodes.ts, line 500 at r1 (raw file):

Previously, dhartunian (David Hartunian) wrote…

if you want, you can use _.countBy as a shortcut

Well TIL, very useful. Thanks!

[Annebirzin]

@Santamaura Good call, I think that does make sense to apply the same message to the warning on the metrics page. fyi @thtruo

[koorosh]

Reviewed all commit messages.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @Annebirzin, @dhartunian, @Santamaura, @thtruo, and @zachlite)

---

pkg/ui/workspaces/db-console/src/views/cluster/containers/clusterOverview/index.tsx, line 315 at r3 (raw file):

        <Helmet title="Cluster Overview" />
        <EmailSubscription />
        <OverviewListAlerts />

@Santamaura , should it be inline warning message or as floating alert banner (top-right corner)?
See pkg/ui/workspaces/db-console/src/views/app/containers/alertBanner/index.tsx component

[Santamaura]

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @Annebirzin, @dhartunian, @koorosh, @thtruo, and @zachlite)

---

pkg/ui/workspaces/db-console/src/views/cluster/containers/clusterOverview/index.tsx, line 315 at r3 (raw file):

Previously, koorosh (Andrii Vorobiov) wrote…

@Santamaura , should it be inline warning message or as floating alert banner (top-right corner)?
See pkg/ui/workspaces/db-console/src/views/app/containers/alertBanner/index.tsx component

Based on the selectors used with that component I believe it is used for more critical level alerts.

[koorosh]

Reviewed 3 of 6 files at r1, 1 of 1 files at r2, 1 of 2 files at r3.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @Annebirzin, @dhartunian, @Santamaura, @thtruo, and @zachlite)

---

pkg/ui/workspaces/db-console/src/redux/alerts.ts, line 139 at r3 (raw file):

 * This excludes decommissioned nodes.
 */
export const staggeredVersionWarningCountSelector = createSelector(

What is the reason of renaming this function to staggeredVersionWarningCountSelector? It doesn't return number of staggered versions as I understand.

---

pkg/ui/workspaces/db-console/src/redux/alerts.ts, line 152 at r3 (raw file):

    let versionsText = "";
    versionsMap.forEach((value, key) => {

This forEach construction can be simplified in more functional and readable way:

let versionsText = Array.from(versionsMap)
      .map(([k, v]) => `${v} nodes are running on ${k}`)
      .join(" and ")
      .concat(". ");

---

pkg/ui/workspaces/db-console/src/views/cluster/containers/clusterOverview/index.tsx, line 315 at r3 (raw file):

Previously, Santamaura (Alex Santamaura) wrote…

Based on the selectors used with that component I believe it is used for more critical level alerts.

I meant to incorporate your new alerts with existing panelAlertsSelector to display non-critical alerts:

/**
 * Selector which returns an array of all active alerts which should be
 * displayed in the alerts panel, which is embedded within the cluster overview
 * page; currently, this includes all non-critical alerts.
 */
export const panelAlertsSelector = createSelector(
...

[Santamaura]

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @Annebirzin, @dhartunian, @koorosh, @thtruo, and @zachlite)

---

pkg/ui/workspaces/db-console/src/redux/alerts.ts, line 139 at r3 (raw file):

Previously, koorosh (Andrii Vorobiov) wrote…

What is the reason of renaming this function to staggeredVersionWarningCountSelector? It doesn't return number of staggered versions as I understand.

I guess I was thinking, it's a selector for showing the versions by node count so added count. I reverted to the old name (I think that one should be ok).

---

pkg/ui/workspaces/db-console/src/redux/alerts.ts, line 152 at r3 (raw file):

Previously, koorosh (Andrii Vorobiov) wrote…

This forEach construction can be simplified in more functional and readable way:

let versionsText = Array.from(versionsMap)
      .map(([k, v]) => `${v} nodes are running on ${k}`)
      .join(" and ")
      .concat(". ");

Thanks for the optimization 😄

---

pkg/ui/workspaces/db-console/src/views/cluster/containers/clusterOverview/index.tsx, line 315 at r3 (raw file):

Previously, koorosh (Andrii Vorobiov) wrote…

I meant to incorporate your new alerts with existing panelAlertsSelector to display non-critical alerts:

/**
 * Selector which returns an array of all active alerts which should be
 * displayed in the alerts panel, which is embedded within the cluster overview
 * page; currently, this includes all non-critical alerts.
 */
export const panelAlertsSelector = createSelector(
...

So if we use this selector we might have a scenario where the new version banner shows up which I don't think we want in that area of the overview list page.

[koorosh]

Reviewed 2 of 2 files at r4, all commit messages.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @Annebirzin, @dhartunian, @thtruo, and @zachlite)

---

pkg/ui/workspaces/db-console/src/views/cluster/containers/clusterOverview/index.tsx, line 315 at r3 (raw file):

Previously, Santamaura (Alex Santamaura) wrote…

So if we use this selector we might have a scenario where the new version banner shows up which I don't think we want in that area of the overview list page.

I thought these banners can stack up... in this case you're right!

[Santamaura]

bors r+

[craig]

Build succeeded:

• GitHub CI (Cockroach)

[jocrl]

Hello! I'd like to reuse this to address #77535, but noticed that this only adds the banner to DB Console, and not CC Console.

Does this issue not apply to CC Console/is there a separate PR for that? It seems like customers would want cluster upgrade information in CC Console as well.

@Santamaura @thtruo

[Annebirzin]

@jocrl Just for context, on ticket #77535 we decided to go with a more generic message for the SQL Activity tab since in CC console users don't have much context about specific nodes and their versions. (Dedicated has a very simple node list but does not list out versions, Serverless does not show anything about nodes). The users of CC console wouldn't be able to do much with the specific knowledge of nodes/versions unless they went to DB console.