Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Checks results coming from Wanda #987

Merged
merged 15 commits into from
Nov 23, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions assets/.storybook/main.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ module.exports = {
...config.resolve.alias,
'@components': path.resolve(__dirname, '../js/components'),
'@lib': path.resolve(__dirname, '../js/lib'),
'@hooks': path.resolve(__dirname, '../js/hooks'),
'@state': path.resolve(__dirname, '../js/state'),
};
return config;
},
Expand Down
4 changes: 3 additions & 1 deletion assets/js/components/ChecksResults/CheckResult.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ const CheckResult = ({
description,
executionState,
health,
label,
onClick,
}) => (
<tr
Expand All @@ -23,8 +24,9 @@ const CheckResult = ({
{description}
</ReactMarkdown>
</td>
<td className="px-6 py-4 whitespace-nowrap content-center">
<td className="px-6 py-4 whitespace-nowrap content-center grid grid-flow-col items-center">
<ExecutionIcon executionState={executionState} health={health} />
{label}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As this is used in the old runner code as well, let's be sure that it doesn't impact the output

</td>
</tr>
);
Expand Down
50 changes: 49 additions & 1 deletion assets/js/components/ChecksResults/checksUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,5 +27,53 @@ export const getHostname =
};

export const findCheck = (catalog, checkID) => {
return catalog.find((check) => check.id === checkID);
return catalog?.find((check) => check.id === checkID);
};

export const getCheckResults = (executionData) => {
if (!executionData) {
return [];
}
if (!executionData.check_results) {
return [];
}
return executionData.check_results;
};

export const getHosts = (checkResults) => {
return checkResults.flatMap(({ agents_check_results }) =>
agents_check_results.map(({ agent_id }) => agent_id)
);
};

export const getChecks = (checkResults) => {
return checkResults.map(({ check_id }) => check_id);
};

export const getHealth = (checkResults, checkID, agentID) => {
arbulu89 marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe rename this to getCheckHealthByAgent or something ?
By the way, he I hate that we should the check state by agent...
It breaks all the logic of our execution payload.
Waiting to update this, we wouldn't need this kind of evaluation functions this way...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I hate that too 😄 looking forward to a new UX!

const checkResult = checkResults.find(({ check_id }) => check_id === checkID);
if (!checkResult) {
return;
}

const agentCheckResult = checkResult.agents_check_results.find(
({ agent_id }) => agent_id === agentID
);

const failedExpectationEvaluations = agentCheckResult?.expectation_evaluations
.filter((expectationEvaluation) => 'message' in expectationEvaluation)
.filter(({ type }) => type !== 'expect');

return {
expectations: checkResult.expectation_results.length,
failedExpectations: failedExpectationEvaluations.length,
health: failedExpectationEvaluations.length > 0 ? 'critical' : 'passing',
};
};

export const getCheckDescription = (catalog, checkID) => {
const check = findCheck(catalog, checkID);
if (check) {
return check.description;
}
};
83 changes: 83 additions & 0 deletions assets/js/components/ChecksResults/checksUtils.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
import { faker } from '@faker-js/faker';
import {
checksExecutionFactory,
catalogCheckFactory,
} from '@lib/test-utils/factories';

import {
getCheckDescription,
getCheckResults,
getChecks,
getHealth,
getHosts,
} from './checksUtils';

describe('checksUtils', () => {
it('getChecksResults returns a list of checks results', () => {
const agentID = faker.datatype.uuid;
const checksExecution = checksExecutionFactory.build({ agentID });
const checksResult = getCheckResults(checksExecution);

expect(checksResult[0].agents_check_results[0].agent_id).toBe(agentID);
expect(
checksResult[0].agents_check_results[0].expectation_evaluations.length
).toBe(1);
});

it('getChecksResults returns an empty list when there are no checks results', () => {
expect(getCheckResults({})).toStrictEqual([]);
});

it('getHosts returns hostnames', () => {
const agentID = faker.datatype.uuid;
const { check_results: checkResults } = checksExecutionFactory.build({
agentID,
});

expect(getHosts(checkResults)).toStrictEqual([agentID]);
});

it('getHealth should return health', () => {
const agentID = faker.datatype.uuid();
const checkID = faker.datatype.uuid();
const { check_results: checkResults } = checksExecutionFactory.build({
agentID,
checkID,
});
const { health, expectations, failedExpectations } = getHealth(
checkResults,
checkID,
agentID
);

expect(health).toBe('passing');
expect(expectations).toBe(1);
expect(failedExpectations).toBe(0);
});

it('getHealth should return undefined when check is not found', () => {
const agentID = faker.datatype.uuid();
const { check_results: checkResults } = checksExecutionFactory.build({
agentID,
});
const healthInfo = getHealth(checkResults, 'carbonara', agentID);

expect(healthInfo).toBe(undefined);
});

it('getChecks should return a list of the checks', () => {
const checkID = faker.datatype.uuid();
const { check_results: checkResults } = checksExecutionFactory.build({
checkID,
});

expect(getChecks(checkResults)).toStrictEqual([checkID]);
});

it('getDescription should return a check description', () => {
const catalog = catalogCheckFactory.buildList(2);
const [{ id, description }] = catalog;

expect(getCheckDescription(catalog, id)).toBe(description);
});
});
21 changes: 21 additions & 0 deletions assets/js/components/ChecksResults/index.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,24 @@
import ChecksResults from './ChecksResults';
import ResultsContainer from './ResultsContainer';
import HostResultsWrapper from './HostResultsWrapper';
import CheckResult from './CheckResult';
import {
getHosts,
getChecks,
getHealth,
getCheckResults,
getCheckDescription,
} from './checksUtils';

export {
ResultsContainer,
HostResultsWrapper,
CheckResult,
getHosts,
getChecks,
getHealth,
getCheckResults,
getCheckDescription,
};

export default ChecksResults;
159 changes: 159 additions & 0 deletions assets/js/components/ExecutionResults/ExecutionResults.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,159 @@
import React, { useEffect, useState } from 'react';
import classNames from 'classnames';

import ReactMarkdown from 'react-markdown';
import remarkGfm from 'remark-gfm';

import { logError } from '@lib/log';
import { getExecutionResult, getCatalog } from '@lib/api/wanda';

import Modal from '@components/Modal';
import BackButton from '@components/BackButton';
import WarningBanner from '@components/Banners/WarningBanner';
import LoadingBox from '@components/LoadingBox';
import {
ResultsContainer,
HostResultsWrapper,
CheckResult,
getHosts,
getChecks,
getHealth,
getCheckResults,
getCheckDescription,
} from '@components/ChecksResults';
import { UNKNOWN_PROVIDER } from '@components/ClusterDetails/ClusterSettings';

const truncatedClusterNameClasses =
'font-bold truncate w-60 inline-block align-top';

const getLabel = (health, expectations, failedExpectations) =>
health === 'passing'
? `${expectations}/${expectations} expectations passed`
: `${failedExpectations}/${expectations} failed`;

const ExecutionResults = ({
clusterID,
executionID,
clusterName,
cloudProvider,
arbulu89 marked this conversation as resolved.
Show resolved Hide resolved
hostnames = [],
onExecutionFetch = getExecutionResult,
onCatalogFetch = getCatalog,
onCatalogRefresh = () => {},
}) => {
const [loading, setLoading] = useState(false);
arbulu89 marked this conversation as resolved.
Show resolved Hide resolved
const [executionData, setExecutionData] = useState(null);
const [catalog, setCatalog] = useState(null);
const [selectedCheck, setSelectedCheck] = useState(null);
const [modalOpen, setModalOpen] = useState(false);

useEffect(() => {
setLoading(true);
Promise.all([onExecutionFetch(executionID), onCatalogFetch()])
.then(
([{ data: fetchedExecutionData }, { data: fetchedCatalogData }]) => {
setLoading(false);
arbulu89 marked this conversation as resolved.
Show resolved Hide resolved
setExecutionData(fetchedExecutionData);
setCatalog(fetchedCatalogData.items);
}
)
.catch((error) => {
setLoading(false);
logError(error);
});
}, [onExecutionFetch, onCatalogFetch, setExecutionData, setCatalog]);
arbulu89 marked this conversation as resolved.
Show resolved Hide resolved

if (loading) {
return <LoadingBox text="Loading checks execution..." />;
}

if (executionData?.status === 'running') {
return <LoadingBox text="Check execution currently running..." />;
arbulu89 marked this conversation as resolved.
Show resolved Hide resolved
}

const checkResults = getCheckResults(executionData);
const hosts = getHosts(checkResults);
const checks = getChecks(checkResults);

return (
<div>
<Modal
title={getCheckDescription(catalog, selectedCheck)}
open={modalOpen}
onClose={() => setModalOpen(false)}
>
<ReactMarkdown className="markdown" remarkPlugins={[remarkGfm]}>
{getCheckDescription(catalog, selectedCheck)}
</ReactMarkdown>
</Modal>
<BackButton url={`/clusters/${clusterID}`}>
Back to Cluster Details
</BackButton>
<div className="flex mb-4 justify-between">
<h1 className="text-3xl w-3/5">
<span className="font-medium">Checks Results for cluster</span>{' '}
<span
className={classNames('font-bold', truncatedClusterNameClasses)}
>
{clusterName}
</span>
</h1>
</div>
{cloudProvider == UNKNOWN_PROVIDER && (
<WarningBanner>
The following results are valid for on-premise bare metal platforms.
<br />
If you are running your HANA cluster on a different platform, please
use results with caution
</WarningBanner>
)}
<ResultsContainer
catalogError={false}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if the catalog has an error?

clusterID={clusterID}
hasAlreadyChecksResults
arbulu89 marked this conversation as resolved.
Show resolved Hide resolved
selectedChecks={checks}
onCatalogRefresh={onCatalogRefresh}
>
{hosts &&
hosts.map((hostID, idx) => (
<HostResultsWrapper
key={idx}
hostname={hostnames.find(({ id }) => hostID === id)?.hostname}
reachable
unreachableMessage=""
>
{checks.map((checkID) => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we can extract that logic, inside the arrow function, inside another component? Seems a little bit convoluted to follow

const { health, expectations, failedExpectations } = getHealth(
checkResults,
checkID,
hostID
);
const label = getLabel(
health,
expectations,
failedExpectations
);

return (
<CheckResult
key={checkID}
checkId={checkID}
description={getCheckDescription(catalog, checkID)}
executionState={executionData?.status}
health={health}
label={label}
onClick={() => {
setModalOpen(true);
setSelectedCheck(checkID);
}}
/>
);
})}
</HostResultsWrapper>
))}
</ResultsContainer>
</div>
);
};

export default ExecutionResults;