Add Status card by rawagner · Pull Request #2806 · openshift/console

rawagner · 2019-09-21T09:56:33Z

andybraren

Looking great! Just one minor change to the update button label to make it clearer that the update won't happen immediately when clicked (sorry, the design doc for this flow is a little out of date).

rawagner · 2019-09-27T13:20:10Z

Added Control Plane health

andybraren · 2019-09-27T14:12:13Z

(Still) looking great! Good call on "API Request Success Rate" - I see that's the label that the old Cluster Status page used too. Does etcd have a health endpoint that we'd be able to expose or calculate (assuming it's important)?

rawagner · 2019-09-27T17:26:10Z

(Still) looking great! Good call on "API Request Success Rate" - I see that's the label that the old Cluster Status page used too. Does etcd have a health endpoint that we'd be able to expose or calculate (assuming it's important)?

not sure yet, I will be exploring that next week

rawagner · 2019-09-30T11:05:28Z

@kyoto any idea on a metric which could be used to get health of etcd ? #2806 (comment)

vojtechszocs · 2019-10-01T14:24:41Z


  const cephCluster = _.get(resources, 'ceph') as FirehoseResult;
-  const cephHealthState = getCephHealthState(queryResult, !!queryResultError, cephCluster);
+  const cephHealthState = getCephHealthState([queryResult], queryResultError, cephCluster);


Please check if we should use [queryResultError].

vojtechszocs · 2019-10-01T14:30:26Z

@@ -0,0 +1,68 @@
+import * as React from 'react';


Please update according to DevTools convention.

vojtechszocs · 2019-10-01T14:39:07Z

+      healthHandler: getControlPlaneHealth,
+      popupComponent: () =>
+        import(
+          './components/dashboards-page/control-plane-status' /* webpackChunkName: "app-control-plane-status" */


Suggested change

'./components/dashboards-page/control-plane-status' /* webpackChunkName: "app-control-plane-status" */

'./components/dashboards-page/control-plane-status' /* webpackChunkName: "console-app" */

vojtechszocs · 2019-10-01T14:48:22Z


  export interface DashboardsOverviewHealthPrometheusSubsystem
    extends DashboardsOverviewHealthSubsystem {
    /** The Prometheus query */


Suggested change

/** The Prometheus query */

/** The Prometheus queries */

vojtechszocs · 2019-10-01T14:49:18Z

-    ) => SubsystemHealth;
+    healthHandler: HealthHandler<PrometheusResponse[]>;
+
+    popupComponent?: LazyLoader<any>;


Please add brief comment on newly added properties.

vojtechszocs · 2019-10-01T14:50:26Z

-    query: string;
+    queries: string[];

    /** Resource which will be fetched and passed to healthHandler  */


Suggested change

/** Resource which will be fetched and passed to healthHandler */

/** Additional resource which will be fetched and passed to healthHandler */

vojtechszocs · 2019-10-01T14:50:45Z

     */
    fetch?: (url: string) => Promise<R>;

+    /** Resource which will be fetched and passed to healthHandler  */


Suggested change

/** Resource which will be fetched and passed to healthHandler */

/** Additional resource which will be fetched and passed to healthHandler */

vojtechszocs · 2019-10-01T14:51:14Z

+export type HealthHandler<R> = (
+  response: R,
+  error: any,
+  resource?: FirehoseResult<K8sResourceKind | K8sResourceKind[]>,


Suggested change

resource?: FirehoseResult<K8sResourceKind | K8sResourceKind[]>,

additionalResource?: FirehoseResult<K8sResourceKind | K8sResourceKind[]>,

vojtechszocs · 2019-10-01T15:00:43Z

+import { EmptyState, EmptyStateVariant, EmptyStateIcon, EmptyStateBody } from '@patternfly/react-core';
+import { CheckCircleIcon, QuestionIcon } from '@patternfly/react-icons';
+
+export const StatusBody = ({ children }) => (


Please inline.

vojtechszocs · 2019-10-01T15:05:26Z

 import { LazyLoader } from './types';

-namespace ExtensionProperties {
+export namespace ExtensionProperties {


Can't we use DashboardsOverviewHealthSubsystem['properties'] ?

kyoto · 2019-10-02T06:17:51Z

@kyoto any idea on a metric which could be used to get health of etcd ? #2806 (comment)

@rawagner Sorry, not sure. Probably best to ask someone on the etcd team.

vojtechszocs · 2019-10-02T14:02:04Z

/lgtm

vojtechszocs · 2019-10-02T14:03:24Z

/lgtm

openshift-ci-robot · 2019-10-02T14:03:40Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: rawagner, vojtechszocs

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~frontend/OWNERS~~ [vojtechszocs]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

openshift-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Sep 21, 2019

openshift-ci-robot requested review from andybraren and cloudbehl September 21, 2019 09:56

rawagner force-pushed the 4.3-status-card branch 3 times, most recently from 859cfe1 to dfad7f1 Compare September 23, 2019 11:19

openshift-ci-robot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. component/kubevirt Related to kubevirt-plugin labels Sep 23, 2019

andybraren reviewed Sep 23, 2019

View reviewed changes

Comment thread frontend/public/components/dashboards-page/overview-dashboard/status-card.tsx Outdated

rawagner force-pushed the 4.3-status-card branch 5 times, most recently from ace9793 to b71c9bf Compare September 25, 2019 08:22

rawagner mentioned this pull request Sep 25, 2019

Firehose - store firehoses in state to trigger rerender when changed #2822

Merged

rawagner force-pushed the 4.3-status-card branch from b71c9bf to b110e94 Compare September 26, 2019 07:29

openshift-ci-robot added component/monitoring Related to monitoring component/olm Related to OLM and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Sep 26, 2019

spadgett changed the base branch from master-4.3 to master September 26, 2019 14:36

rawagner force-pushed the 4.3-status-card branch from b110e94 to 14bd61c Compare September 27, 2019 12:34

openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 27, 2019

rawagner force-pushed the 4.3-status-card branch from 14bd61c to 3ab5f56 Compare September 27, 2019 12:49

rawagner force-pushed the 4.3-status-card branch from 3ab5f56 to 2cbfeda Compare September 27, 2019 13:39

openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 27, 2019

rawagner force-pushed the 4.3-status-card branch from 2cbfeda to b04e87f Compare September 27, 2019 13:53

rawagner force-pushed the 4.3-status-card branch from b04e87f to 8973421 Compare September 27, 2019 14:20

rawagner force-pushed the 4.3-status-card branch 3 times, most recently from 4694059 to 24ada75 Compare October 1, 2019 10:41

vojtechszocs suggested changes Oct 1, 2019

View reviewed changes

rawagner force-pushed the 4.3-status-card branch from 24ada75 to 750c54f Compare October 2, 2019 10:05

rawagner changed the title ~~[WIP] Add Status card~~ Add Status card Oct 2, 2019

openshift-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Oct 2, 2019

rawagner force-pushed the 4.3-status-card branch 2 times, most recently from c261794 to 1ed0007 Compare October 2, 2019 12:16

Add Status card

44828e5

rawagner force-pushed the 4.3-status-card branch from 1ed0007 to 44828e5 Compare October 2, 2019 12:59

openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 2, 2019

openshift-ci-robot assigned vojtechszocs Oct 2, 2019

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Oct 2, 2019

openshift-merge-robot merged commit dec402f into openshift:master Oct 2, 2019

spadgett added this to the v4.3 milestone Oct 4, 2019

bparees mentioned this pull request Dec 16, 2019

Bug 1781055: Fix new/old alerts comparison in withDashboardResources #3638

Merged

	'./components/dashboards-page/control-plane-status' /* webpackChunkName: "app-control-plane-status" */
	'./components/dashboards-page/control-plane-status' /* webpackChunkName: "console-app" */

	/** Resource which will be fetched and passed to healthHandler */
	/** Additional resource which will be fetched and passed to healthHandler */

	resource?: FirehoseResult<K8sResourceKind \| K8sResourceKind[]>,
	additionalResource?: FirehoseResult<K8sResourceKind \| K8sResourceKind[]>,

Uh oh!

Conversation

rawagner commented Sep 21, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

andybraren left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

rawagner commented Sep 27, 2019

Uh oh!

andybraren commented Sep 27, 2019

Uh oh!

rawagner commented Sep 27, 2019

Uh oh!

rawagner commented Sep 30, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kyoto commented Oct 2, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vojtechszocs commented Oct 2, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vojtechszocs commented Oct 2, 2019

Uh oh!

openshift-ci-robot commented Oct 2, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

rawagner commented Sep 21, 2019 •

edited

Loading

kyoto commented Oct 2, 2019 •

edited

Loading

vojtechszocs commented Oct 2, 2019 •

edited

Loading