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

Handle unnamed cluster event #497

Merged
merged 6 commits into from
May 10, 2022
Merged
Show file tree
Hide file tree
Changes from 5 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
4 changes: 4 additions & 0 deletions .photofinish.toml
Original file line number Diff line number Diff line change
Expand Up @@ -83,3 +83,7 @@ files = ["./test/fixtures/scenarios/clusters-overview/13e8c25c-3180-5a9a-95c8-51
[cluster-1-SFAIL]

files = ["./test/fixtures/scenarios/clusters-overview/13e8c25c-3180-5a9a-95c8-51ec38e50cfc_ha_cluster_discovery_1_SFAIL.json"]

[cluster-unnamed]

files = ["./test/fixtures/scenarios/clusters-overview/13e8c25c-3180-5a9a-95c8-51ec38e50cfc_ha_cluster_discovery_unnamed.json"]
10 changes: 8 additions & 2 deletions assets/js/components/ClusterDetails/ChecksResults.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,12 @@ import LoadingBox from '../LoadingBox';
import Button from '@components/Button';
import { getCluster } from '@state/selectors';
import TrentoLogo from '../../../static/trento-icon.png';
import { TriggerChecksExecutionRequest } from './ClusterDetails';
import {
TriggerChecksExecutionRequest,
truncatedClusterNameClasses,
} from './ClusterDetails';
import { ExecutionIcon } from './ExecutionIcon';
import { getClusterName } from '@components/ClusterLink';

import ReactMarkdown from 'react-markdown';
import remarkGfm from 'remark-gfm';
Expand Down Expand Up @@ -209,7 +213,9 @@ export const ChecksResults = () => {
<div className="flex mb-4">
<h1 className="text-3xl w-3/5">
<span className="font-medium">Checks Results for cluster</span>{' '}
<span className="font-bold">{cluster && cluster.name}</span>
<span className={`font-bold ${truncatedClusterNameClasses}`}>
{getClusterName(cluster)}
</span>
</h1>
<div className="flex w-2/5 justify-end text-white">
<Button
Expand Down
11 changes: 10 additions & 1 deletion assets/js/components/ClusterDetails/ClusterDetails.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,16 @@ import { groupBy } from '@lib/lists';

import SiteDetails from './SiteDetails';

import { getClusterName } from '@components/ClusterLink';

import { EOS_SETTINGS, EOS_CLEAR_ALL, EOS_PLAY_CIRCLE } from 'eos-icons-react';
import { getCluster } from '@state/selectors';
import classNames from 'classnames';

export const truncatedClusterNameClasses = classNames(
'font-bold truncate w-60 inline-block align-top'
);

const siteDetailsConfig = {
usePadding: false,
columns: [
Expand Down Expand Up @@ -80,7 +86,10 @@ const ClusterDetails = () => {
<div>
<div className="flex">
<h1 className="text-3xl font-bold w-1/2">
Pacemaker cluster details: {cluster.name}
Pacemaker cluster details:{' '}
<span className={truncatedClusterNameClasses}>
{getClusterName(cluster)}
</span>
</h1>
<div className="flex w-1/2 justify-end">
<Button
Expand Down
10 changes: 8 additions & 2 deletions assets/js/components/ClusterDetails/ClusterSettings.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,11 @@ import { Tab } from '@headlessui/react';
import { ChecksSelection } from './ChecksSelection';
import { ConnectionSettings } from './ConnectionSettings';
import { getCluster } from '@state/selectors';
import { TriggerChecksExecutionRequest } from './ClusterDetails';
import {
TriggerChecksExecutionRequest,
truncatedClusterNameClasses,
} from './ClusterDetails';
import { getClusterName } from '@components/ClusterLink';

export const ClusterSettings = () => {
const { clusterID } = useParams();
Expand All @@ -32,7 +36,9 @@ export const ClusterSettings = () => {
<div className="flex mb-2">
<h1 className="text-3xl w-1/2">
<span className="font-medium">Cluster Settings for</span>{' '}
<span className="font-bold">{cluster && cluster.name}</span>
<span className={`font-bold ${truncatedClusterNameClasses}`}>
{getClusterName(cluster)}
</span>
</h1>
<div className="flex w-1/2 justify-end text-white">
<Button
Expand Down
16 changes: 13 additions & 3 deletions assets/js/components/ClusterLink.jsx
Original file line number Diff line number Diff line change
@@ -1,20 +1,30 @@
import React from 'react';

import { Link } from 'react-router-dom';
import classNames from 'classnames';

export const getClusterName = (cluster) => {
return cluster?.name || cluster?.id;
};

const ClusterLink = ({ cluster }) => {
const clusterName = getClusterName(cluster);
const truncatedClasses = classNames(
'truncate w-32 inline-block align-middle'
);

const ClusterLink = ({ cluster, children }) => {
if (cluster?.type == 'hana_scale_up' || cluster?.type == 'hana_scale_out') {
return (
<Link
className="text-jungle-green-500 hover:opacity-75"
to={`/clusters/${cluster.id}`}
>
{children}
<span className={truncatedClasses}>{clusterName}</span>
</Link>
);
}

return <span>{children}</span>;
return <span className={truncatedClasses}>{clusterName}</span>;
};

export default ClusterLink;
6 changes: 3 additions & 3 deletions assets/js/components/ClustersList.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -54,9 +54,9 @@ const ClustersList = () => {
key: 'name',
filter: true,
render: (content, item) => (
<ClusterLink cluster={item}>
<span className="tn-clustername">{content}</span>
</ClusterLink>
<span className="tn-clustername">
<ClusterLink cluster={item} />
</span>
),
},
{
Expand Down
4 changes: 1 addition & 3 deletions assets/js/components/HostDetails/HostDetails.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -50,9 +50,7 @@ const HostDetails = () => {
{ title: 'Name', content: host.hostname },
{
title: 'Cluster',
content: (
<ClusterLink cluster={cluster}>{cluster?.name}</ClusterLink>
),
content: <ClusterLink cluster={cluster} />,
},
{ title: 'Agent version', content: host.agent_version },
]}
Expand Down
2 changes: 1 addition & 1 deletion assets/js/components/HostsList.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ const HostsList = () => {
key: 'cluster',
className: 'w-40',
render: (cluster) => {
return <ClusterLink cluster={cluster}>{cluster?.name}</ClusterLink>;
return <ClusterLink cluster={cluster} />;
},
},
{
Expand Down
2 changes: 1 addition & 1 deletion assets/js/components/InstanceOverview/InstanceOverview.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ const InstanceOverview = ({
)}
<div className="table-cell p-2">
{cluster ? (
<ClusterLink cluster={cluster}>{cluster.name}</ClusterLink>
<ClusterLink cluster={cluster} />
) : (
<p className="text-gray-500 dark:text-gray-300 text-sm">
not available
Expand Down
2 changes: 1 addition & 1 deletion lib/trento/application/read_models/cluster_read_model.ex
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ defmodule Trento.ClusterReadModel do
@derive {Jason.Encoder, except: [:__meta__, :__struct__]}
@primary_key {:id, :binary_id, autogenerate: false}
schema "clusters" do
field :name, :string
field :name, :string, default: ""
Copy link
Member

Choose a reason for hiding this comment

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

My ocd calls for a nil here. 😄
Would that make sense? And would it be possible or does it have to be an empty string because of ecto?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If we don't set the default value, when we get a nil value in the name field (which is the current scenario), it doesn't apply any change in the persistent storage, I guess because ecto works 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.

@nelsonkopliku I will add some default value in the name field by now. Let's postpone the addition of the ID, as it would requires to change the nice 3x3 grid we have now, and I don't want to play with that by now hehe

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here we go (a434de3):
image

Copy link
Member

Choose a reason for hiding this comment

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

ok makes sense! Thanks!

field :sid, :string
field :provider, Ecto.Enum, values: [:azure, :aws, :gcp, :unknown]
field :type, Ecto.Enum, values: [:hana_scale_up, :hana_scale_out, :unknown]
Expand Down
2 changes: 1 addition & 1 deletion lib/trento/domain/cluster/cluster.ex
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ defmodule Trento.Domain.Cluster do

@type t :: %__MODULE__{
cluster_id: String.t(),
name: [String.t()],
name: String.t(),
type: :hana_scale_up | :hana_scale_out | :unknown,
provider: :azure | :aws | :gcp | :unknown,
discovered_health: nil | :passing | :warning | :critical | :unknown,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ defmodule Trento.Domain.Commands.RegisterClusterHost do
@required_fields [
:cluster_id,
:host_id,
:name,
:type,
:designated_controller,
:discovered_health,
Expand Down
21 changes: 10 additions & 11 deletions test/e2e/cypress/fixtures/clusters-overview/available_clusters.js
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
const availableClusters = [
['04a81f89c847e82390e35bece2e25c9b', 'drbd_cluster'],
['238a4de1239aae2aa87433eed788b3ad', ' drbd_cluster'],
['a034a158905404befe08775682910ee1', ' drbd_cluster'],
['04b8f8c21f9fd8991224478e8c4362f8', 'hana_cluster_1'],
['4e905d706da85f5be14f85fa947c1e39', 'hana_cluster_2'],
['9c832998801e28cd70ad77380e82a5c0', 'hana_cluster_3'],
['057f083c3be591f4398eed816d4c8cd7', 'netweaver_cluster'],
['8bca366a6cb7816555538092a1ddd5aa', 'netweaver_cluster'],
['acf59e7a5338f76f55d5055af3273480', 'netweaver_cluster'],
['8a66f8fb-5fe9-51b3-a34c-24321271a4e3', 'drbd_cluster'],
['6bd7ec60-8cb1-5c6b-a892-29e1fd2f8380', 'drbd_cluster'],
['c7a1e943-bf46-590b-bd26-bfc7c78def97', 'drbd_cluster'],
['7965f822-0254-5858-abca-f6e8b4c27714', 'hana_cluster_1'],
['fa0d74a3-9240-5d9e-99fa-61c4137acf81', 'hana_cluster_2'],
['469e7be5-4e20-5007-b044-c6f540a87493', 'hana_cluster_3'],
['5284f376-c1f4-5178-8966-d490df3dab4f', 'netweaver_cluster'],
['fb861bce-d212-56b5-8786-74afd6eb58cb', 'netweaver_cluster'],
['0eac831a-aa66-5f45-89a4-007fbd2c5714', 'netweaver_cluster'],
];

export const allClusterNames = () =>
availableClusters.map(([_, clusterName]) => clusterName);
export const allClusterIds = () =>
Expand All @@ -18,4 +18,3 @@ const availableClusters = [
availableClusters.find(([, name]) => name === clusterName)[0];
export const clusterNameById = (clusterId) =>
availableClusters.find(([id]) => id === clusterId)[1];

10 changes: 10 additions & 0 deletions test/e2e/cypress/integration/clusters_overview.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,5 +33,15 @@ import {
});
});
});
describe('Unnamed cluster', () => {
before(() => {
cy.loadScenario('cluster-unnamed');
});

it('Unnamed clusters should use the ID as details page link', () => {
const clusterID = clusterIdByName('hana_cluster_1')
cy.get(`a:contains(${clusterID})`).should('be.visible')
});
});
});
});