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.

Sign up for GitHub

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

Handle unreachable checks execution scenario #304

Merged
merged 14 commits into from
Apr 14, 2022
Merged

Handle unreachable checks execution scenario #304

Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
984d6f9
Parse reachable and msg from execution payload
arbulu89 Apr 6, 2022
73716a6
Add host unreachable event
arbulu89 Apr 6, 2022
0028e95
Add hosts executions state in the cluster domain
arbulu89 Apr 6, 2022
2191f58
Project hosts executions state data
arbulu89 Apr 6, 2022
e6a90d4
Rename hosts execution read model and associate data in usecases
arbulu89 Apr 6, 2022
eccfa05
Set requested and running icons in results view better
arbulu89 Apr 6, 2022
b5cf097
Remove the host unreachable event to make the code leaner
arbulu89 Apr 6, 2022
18989ae
Show unreachable state in frontend
arbulu89 Apr 6, 2022
0c8a0c9
Apply all linting fixes
arbulu89 Apr 11, 2022
8a029c3
Update unit tests
arbulu89 Apr 11, 2022
615045c
Fix frontend linting issues
arbulu89 Apr 12, 2022
fec5713
Rename to hosts_checks_executions
arbulu89 Apr 12, 2022
a4f931f
Remove guards and rename function to be meaningful
arbulu89 Apr 12, 2022
046bcd9
Add TODO reference to tech debts
arbulu89 Apr 12, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
163 changes: 89 additions & 74 deletions assets/js/components/ChecksResults.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,28 +2,12 @@ import React, { useEffect } from 'react';
import { useParams } from 'react-router-dom';
import { useSelector, useDispatch } from 'react-redux';

import { EOS_LENS_FILLED, EOS_ERROR } from 'eos-icons-react';
import { EOS_LENS_FILLED, EOS_ERROR, EOS_SCHEDULE } from 'eos-icons-react';
import Spinner from './Spinner';

import NotificationBox from './NotificationBox';
import LoadingBox from './LoadingBox';

const getChecksResults = (cluster) => {
if (cluster) {
return cluster.checks_results
.filter((check_result) => check_result.result != 'skipped') // Filter "skipped" results by now
.reduce((acc, checkResult) => {
acc[checkResult.host_id] = [
...(acc[checkResult.host_id] || []),
checkResult,
];

return acc;
}, {});
}
return {};
};

const getHostname =
(hosts = []) =>
(hostId) => {
Expand All @@ -36,16 +20,26 @@ const getHostname =
}, '');
};

const sortChecksResults = (checksResults = [], group) => {
const sortChecksResults = (checksResults = []) => {
return checksResults.sort((a, b) => {
if (a.check_id === b.check_id) {
return group(a.check_id) > group(b.check_id) ? 1 : -1;
}
return a.check_id > b.check_id ? 1 : -1;
});
};

const getResultIcon = (result) => {
const sortHosts = (hosts = []) => {
return hosts.sort((a, b) => {
return a.host_id > b.host_id ? 1 : -1;
});
};

const getResultIcon = (checks_execution, result) => {
switch (checks_execution) {
case 'requested':
return <EOS_SCHEDULE className="fill-gray-500" />;
case 'running':
return <Spinner></Spinner>;
}

switch (result) {
case 'passing':
return <EOS_LENS_FILLED className="fill-jungle-green-500" />;
Expand All @@ -54,7 +48,7 @@ const getResultIcon = (result) => {
case 'critical':
return <EOS_LENS_FILLED className="fill-red-500" />;
default:
return <Spinner></Spinner>;
return <EOS_LENS_FILLED className="fill-gray-500" />;
}
};

Expand All @@ -78,8 +72,6 @@ const ChecksResults = () => {
});
};

const checksResults = getChecksResults(cluster);

const hostname = getHostname(useSelector((state) => state.hostsList.hosts));

useEffect(() => {
Expand Down Expand Up @@ -107,60 +99,83 @@ const ChecksResults = () => {

return (
<div>
{Object.keys(checksResults).map((c) => (
<div key="c" className="flex flex-col">
<div className="-my-2 overflow-x-auto sm:-mx-6 lg:-mx-8">
<div className="py-2 align-middle inline-block min-w-full sm:px-6 lg:px-8">
<div className="shadow overflow-hidden border-b border-gray-200 sm:rounded-lg">
<div className="bg-white px-4 py-5 border-b border-gray-200 sm:px-6">
<h3 className="text-lg leading-6 font-medium text-gray-900">
{hostname(c)}
</h3>
</div>
<table className="min-w-full divide-y divide-gray-200">
<thead className="bg-gray-50">
<tr>
<th
scope="col"
className="px-6 py-3 text-left text-xs font-medium text-gray-500 uppercase tracking-wider"
{sortHosts(cluster?.hosts_executions.slice()).map(
({ _cluster_id, host_id, reachable, msg }) => (
<div key="c" className="flex flex-col">
<div className="-my-2 overflow-x-auto sm:-mx-6 lg:-mx-8">
<div className="py-2 align-middle inline-block min-w-full sm:px-6 lg:px-8">
<div className="shadow overflow-hidden border-b border-gray-200 sm:rounded-lg">
<div className="bg-white px-4 py-5 border-b border-gray-200 sm:px-6">
<h3 className="text-lg leading-6 font-medium text-gray-900">
{hostname(host_id)}
</h3>
{reachable == false && (
<div
className="bg-yellow-200 border-yellow-600 text-yellow-600 border-l-4 p-4"
role="alert"
>
ID
</th>
<th
scope="col"
className="px-6 py-3 text-left text-xs font-medium text-gray-500 uppercase tracking-wider"
>
Description
</th>
<th
scope="col"
className="px-6 py-3 text-left text-xs font-medium text-gray-500 uppercase tracking-wider"
>
Result
</th>
</tr>
</thead>
<tbody className="bg-white divide-y divide-gray-200">
{sortChecksResults(checksResults[c]).map((checkResult) => (
<tr key={checkResult.check_id} className="animate-fade">
<td className="px-6 py-4 whitespace-nowrap">
{checkResult.check_id}
</td>
<td className="px-6 py-4 whitespace-nowrap">
{description(checkResult.check_id)}
</td>
<td className="px-6 py-4 whitespace-nowrap content-center">
{getResultIcon(checkResult.result)}
</td>
<p>{msg}</p>
</div>
)}
</div>
<table className="min-w-full divide-y divide-gray-200">
<thead className="bg-gray-50">
<tr>
<th
scope="col"
className="px-6 py-3 text-left text-xs font-medium text-gray-500 uppercase tracking-wider"
>
ID
</th>
<th
scope="col"
className="px-6 py-3 text-left text-xs font-medium text-gray-500 uppercase tracking-wider"
>
Description
</th>
<th
scope="col"
className="px-6 py-3 text-left text-xs font-medium text-gray-500 uppercase tracking-wider"
>
Result
</th>
</tr>
))}
</tbody>
</table>
</thead>
<tbody className="bg-white divide-y divide-gray-200">
{sortChecksResults(cluster?.checks_results.slice())
.filter(
(check_result) => check_result.host_id == host_id
)
.filter(
(check_result) => check_result.result != 'skipped'
) // Filter "skipped" results by now
.map((checkResult) => (
<tr
key={checkResult.check_id}
className="animate-fade"
>
<td className="px-6 py-4 whitespace-nowrap">
{checkResult.check_id}
</td>
<td className="px-6 py-4 whitespace-nowrap">
{description(checkResult.check_id)}
</td>
<td className="px-6 py-4 whitespace-nowrap content-center">
{getResultIcon(
cluster.checks_execution,
checkResult.result
)}
</td>
</tr>
))}
</tbody>
</table>
</div>
</div>
</div>
</div>
</div>
))}
)
)}
</div>
);
};
Expand Down
7 changes: 7 additions & 0 deletions assets/js/state/clusters.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,13 @@ export const clustersListSlice = createSlice({
}),
...action.payload.checks_results,
];

cluster.hosts_executions = [
...cluster.hosts_executions.filter((host_execution) => {
return host_execution.host_id !== action.payload.host_id;
}),
...action.payload.hosts_executions,
];
}
return cluster;
});
Expand Down
4 changes: 3 additions & 1 deletion lib/trento/application/integration/checks/checks.ex
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,9 +99,11 @@ defmodule Trento.Integration.Checks do
CompleteChecksExecution.new(%{
cluster_id: cluster_id,
hosts_executions:
Enum.map(hosts, fn %{host_id: host_id, results: results} ->
Enum.map(hosts, fn %{host_id: host_id, reachable: reachable, msg: msg, results: results} ->
%{
host_id: host_id,
reachable: reachable,
msg: msg,
checks_results: Enum.map(results, &Map.from_struct/1)
}
end)
Expand Down
4 changes: 3 additions & 1 deletion lib/trento/application/integration/checks/dto/execution_completed_event_dto.ex
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ defmodule Trento.Integration.Checks.ExecutionCompletedEventDto do

embeds_many :hosts, Host do
field :host_id, Ecto.UUID
field :reachable, :boolean
field :msg, :string

embeds_many :results, Result do
field :check_id, :string
Expand All @@ -29,7 +31,7 @@ defmodule Trento.Integration.Checks.ExecutionCompletedEventDto do

defp host_changeset(host, attrs) do
host
|> cast(attrs, [:host_id])
|> cast(attrs, [:host_id, :reachable, :msg])
|> cast_embed(:results, with: &result_changeset/2)
|> validate_required([:host_id])
end
Expand Down
49 changes: 47 additions & 2 deletions lib/trento/application/projectors/check_result_projector.ex
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,10 @@ defmodule Trento.CheckResultProjector do
HostChecksExecutionCompleted
}

alias Trento.CheckResultReadModel
alias Trento.{
CheckResultReadModel,
HostChecksExecutionsReadModel
}

project(
%ChecksExecutionRequested{
Expand All @@ -24,14 +27,42 @@ defmodule Trento.CheckResultProjector do
checks: checks
},
fn multi ->
# Delete old hosts executions states
multi =
Ecto.Multi.delete_all(
multi,
:delete_old_hosts_executions,
from(c in CheckResultReadModel, where: c.cluster_id == ^cluster_id)
)

# Delete old results
multi =
Ecto.Multi.delete_all(
multi,
:delete_old_checks_results,
from(c in CheckResultReadModel, where: c.cluster_id == ^cluster_id)
from(h in HostChecksExecutionsReadModel, where: h.cluster_id == ^cluster_id)
)

multi =
hosts
|> Enum.map(fn host_id ->
HostChecksExecutionsReadModel.changeset(
%HostChecksExecutionsReadModel{},
%{
cluster_id: cluster_id,
host_id: host_id,
reachable: nil,
msg: nil
}
)
end)
|> List.flatten()
|> Enum.reduce(multi, fn %{changes: %{cluster_id: cluster_id, host_id: host_id}} =
changeset,
acc ->
Ecto.Multi.insert(acc, "#{cluster_id}_#{host_id}", changeset)
end)

hosts
|> Enum.map(fn host_id ->
Enum.map(checks, fn check_id ->
Expand All @@ -58,9 +89,17 @@ defmodule Trento.CheckResultProjector do
%HostChecksExecutionCompleted{
cluster_id: cluster_id,
host_id: host_id,
reachable: reachable,
msg: msg,
checks_results: checks_results
},
fn multi ->
hosts_executions_changeset =
%HostChecksExecutionsReadModel{cluster_id: cluster_id, host_id: host_id}
|> HostChecksExecutionsReadModel.changeset(%{reachable: reachable, msg: msg})

multi = Ecto.Multi.update(multi, :hosts_executions, hosts_executions_changeset)

checks_results
|> Enum.map(fn %{
check_id: check_id,
Expand Down Expand Up @@ -95,6 +134,7 @@ defmodule Trento.CheckResultProjector do
TrentoWeb.Endpoint.broadcast("monitoring:clusters", "checks_results_updated", %{
cluster_id: cluster_id,
host_id: host_id,
hosts_executions: [%{cluster_id: cluster_id, host_id: host_id, reachable: true, msg: ""}],
checks_results:
Enum.map(checks, fn check_id ->
%{host_id: host_id, check_id: check_id, result: :unknown}
Expand All @@ -108,6 +148,8 @@ defmodule Trento.CheckResultProjector do
%HostChecksExecutionCompleted{
cluster_id: cluster_id,
host_id: host_id,
reachable: reachable,
msg: msg,
checks_results: checks_results
},
_,
Expand All @@ -116,6 +158,9 @@ defmodule Trento.CheckResultProjector do
TrentoWeb.Endpoint.broadcast("monitoring:clusters", "checks_results_updated", %{
cluster_id: cluster_id,
host_id: host_id,
hosts_executions: [
%{cluster_id: cluster_id, host_id: host_id, reachable: reachable, msg: msg}
],
checks_results:
Enum.map(checks_results, fn %{check_id: check_id, result: result} ->
%{host_id: host_id, check_id: check_id, result: result}
Expand Down
6 changes: 5 additions & 1 deletion lib/trento/application/read_models/cluster_read_model.ex
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,10 @@ defmodule Trento.ClusterReadModel do

import Ecto.Changeset

alias Trento.CheckResultReadModel
alias Trento.{
CheckResultReadModel,
HostChecksExecutionsReadModel
}

@type t :: %__MODULE__{}

Expand All @@ -24,6 +27,7 @@ defmodule Trento.ClusterReadModel do
field :details, :map
field :checks_execution, Ecto.Enum, values: [:not_running, :requested, :running]

has_many :hosts_executions, HostChecksExecutionsReadModel, foreign_key: :cluster_id
Copy link
Member

Choose a reason for hiding this comment

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

is there a reason why we are not nesting check results under checks execution?
is it more ergonomic on the frontend side?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It was much more hassle to do so, and without big advantages. It forces you to do more hierarchical operations to get the checks (loop through hosts, and them loop through the checks), and as we were already storing the host_id for each check, I thought that it was good enough. In fact, as you commented, it is easier to handle the 2 lists independently in the frontend

Copy link
Member

Choose a reason for hiding this comment

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

Perfect, i was just curious about the rationale. at the end read models are for, well, reading, so it make sense to make them closer to the usecase.

has_many :checks_results, CheckResultReadModel, foreign_key: :cluster_id
has_many :tags, Trento.Tag, foreign_key: :resource_id
end
Expand Down
Loading