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

Use MultiListPage for BaremetalHostsPage #1698

Merged
merged 5 commits into from
Jun 18, 2019
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions frontend/packages/console-shared/src/selectors/common.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import * as _ from 'lodash-es';
Copy link
Member

Choose a reason for hiding this comment

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

We should consider a different name for this folder. "selectors" has a specific meaning in k8s (label selectors)


import { K8sResourceKind } from '@console/internal/module/k8s';

export const getName = (value: K8sResourceKind): string => _.get(value, 'metadata.name');
export const getNamespace = (value: K8sResourceKind): string => _.get(value, 'metadata.namespace');
8 changes: 2 additions & 6 deletions frontend/packages/console-shared/src/selectors/index.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,2 @@
import * as _ from 'lodash-es';

import { K8sResourceKind } from '@console/internal/module/k8s';

export const getName = (value: K8sResourceKind): string => _.get(value, 'metadata.name');
export const getNamespace = (value: K8sResourceKind): string => _.get(value, 'metadata.namespace');
export * from './common';
export * from './machine';
18 changes: 18 additions & 0 deletions frontend/packages/console-shared/src/selectors/machine.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
import * as _ from 'lodash-es';

import {
MachineKind,
MachineSetKind,
MachineDeploymentKind,
NodeKind,
} from '@console/internal/module/k8s';
import { getName } from './common';

export const getMachineRole = (obj: MachineKind | MachineSetKind | MachineDeploymentKind) =>
_.get(obj, ['metadata', 'labels', 'machine.openshift.io/cluster-api-machine-role']);
export const getMachineNodeName = (obj: MachineKind) => _.get(obj, 'status.nodeRef.name');
export const getMachineAWSPlacement = (machine: MachineKind) =>
_.get(machine, 'spec.providerSpec.value.placement') || {};

export const getMachineNode = (machine: MachineKind, nodes: NodeKind[]) =>
nodes.find((node) => getMachineNodeName(machine) === getName(node));
Original file line number Diff line number Diff line change
Expand Up @@ -11,4 +11,8 @@ module.exports = {

// One JSX element Per line
'react/jsx-one-expression-per-line': 'off',

// Prevent missing props validation in a React component definition
// Off due to false positives in typescript
'react/prop-types': 'off',
Copy link
Member

Choose a reason for hiding this comment

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

@christianvogt fyi, looks like this will conflict with #1721

Copy link
Contributor

Choose a reason for hiding this comment

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

I gave the suggestion. That's fine. Whichever comes first.

};
8 changes: 8 additions & 0 deletions frontend/packages/metal3-plugin/src/components/host-role.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import { getMachineRole, DASH } from '@console/shared';
import { MachineKind } from '@console/internal/module/k8s';

type BaremetalHostRoleProps = {
machine: MachineKind;
};
export const BaremetalHostRole: React.FC<BaremetalHostRoleProps> = ({ machine }) =>
getMachineRole(machine) || DASH;
208 changes: 138 additions & 70 deletions frontend/packages/metal3-plugin/src/components/host.tsx
Original file line number Diff line number Diff line change
@@ -1,57 +1,78 @@
import * as React from 'react';
import * as _ from 'lodash-es';
import * as classNames from 'classnames';
import { sortable } from '@patternfly/react-table';

import { getName, getNamespace } from '@console/shared';
// TODO(jtomasek): update import once models are moved to console-shared package
// import { MachineModel, NodeModel } from '@console/internal/models';
import { referenceForModel } from '@console/internal/module/k8s';
import { getName, getNamespace, getMachineNode } from '@console/shared';
import { MachineModel, NodeModel } from '@console/internal/models';

import { MultiListPage, Table, TableRow, TableData } from '@console/internal/components/factory';
import { ResourceLink, Kebab } from '@console/internal/components/utils';
import {
ListHeader,
ColHead,
List,
ListPage,
ResourceRow,
} from '@console/internal/components/factory';

import { ResourceLink } from '@console/internal/components/utils';
// import { WithResources } from '@console/shared';
referenceForModel,
K8sResourceKind,
MachineKind,
NodeKind,
} from '@console/internal/module/k8s';

import { BaremetalHostModel } from '../models';
import { getHostBMCAddress } from '../selectors';
import { getHostBMCAddress, getHostMachine } from '../selectors';
import { BaremetalHostRole } from './host-role';
import MachineCell from './machine-cell';

// const nameColumnClasses = 'col-lg-2 col-md-4 col-sm-6 col-xs-6';
// const statusColumnClasses = 'col-lg-2 col-md-4 hidden-sm hidden-xs';
// const machineColumnClasses = 'col-lg-3 visible-lg';
// const roleColumnClasses = 'col-lg-2 visible-lg';
// const addressColumnClasses = 'col-lg-2 visible-lg';
const columnClasses = 'col-sm-4';

const HostHeader = (props: React.ComponentProps<typeof ColHead>) => (
<ListHeader>
<ColHead {...props} className={columnClasses} sortField="metadata.name">
Name
</ColHead>
{/* <ColHead {...props} className={statusColumnClasses}>
Status
</ColHead> */}
<ColHead {...props} className={columnClasses}>
Machine
</ColHead>
{/* <ColHead {...props} className={roleColumnClasses}>
Role
</ColHead> */}
<ColHead {...props} className={columnClasses} sortField="spec.bmc.address">
Management Address
</ColHead>
</ListHeader>
);
const tableColumnClasses = [
classNames('col-lg-5', 'col-md-8', 'col-sm-12', 'col-xs-12'),
classNames('col-lg-2', 'col-md-4', 'col-sm-6', 'hidden-xs'),
classNames('col-lg-3', 'col-md-4', 'hidden-sm', 'hidden-xs'),
classNames('col-lg-2', 'hidden-md', 'hidden-sm', 'hidden-xs'),
classNames('col-lg-2', 'hidden-md', 'hidden-sm', 'hidden-xs'),
Kebab.columnClass,
];

const HostsTableHeader = () => [
{
title: 'Name',
sortField: 'metadata.name',
transforms: [sortable],
props: { className: tableColumnClasses[0] },
},
{
title: 'Machine',
sortField: 'spec.machineRef.name',
transforms: [sortable],
props: { className: tableColumnClasses[2] },
Copy link
Member

Choose a reason for hiding this comment

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

We're skipping a column class. Is that intentional?

Copy link
Author

Choose a reason for hiding this comment

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

Yes, that one is going to be used by Host Status. I've updated first column to span its space for the time being.

},
{
title: 'Role',
sortField: 'machine.metadata.labels["machine.openshift.io/cluster-api-machine-role"]',
transforms: [sortable],
props: { className: tableColumnClasses[3] },
},
{
title: 'Management Address',
sortField: 'spec.machineRef.name',
transforms: [sortable],
props: { className: tableColumnClasses[4] },
},
// {
// title: '',
// props: { className: tableColumnClasses[5] },
// },
];

type HostsTableRowProps = {
obj: K8sResourceKind & { machine: MachineKind; node: NodeKind };
index: number;
key?: string;
style: React.StyleHTMLAttributes<any>;
};

const HostRow = ({ obj: host }: React.ComponentProps<typeof ResourceRow>) => {
const HostsTableRow: React.FC<HostsTableRowProps> = ({ obj: host, index, key, style }) => {
const name = getName(host);
const namespace = getNamespace(host);
// const machineName = getHostMachineName(host);
const address = getHostBMCAddress(host);
const { machine } = host;

// TODO(jtomasek): other resource references will be updated as a subsequent change
// const machineResource = {
Expand All @@ -73,34 +94,39 @@ const HostRow = ({ obj: host }: React.ComponentProps<typeof ResourceRow>) => {
// };

return (
<ResourceRow obj={host}>
<div className={columnClasses}>
<TableRow id={host.metadata.uid} index={index} trKey={key} style={style}>
<TableData className={tableColumnClasses[0]}>
<ResourceLink
kind={referenceForModel(BaremetalHostModel)}
name={name}
namespace={namespace}
/>
</div>
{/* <div className={statusColumnClasses}>
<WithResources resourceMap={machineName ? hostResourceMap : {}}>
<BaremetalHostStatus host={host} />
</WithResources>
</div> */}
<div className={columnClasses}>
</TableData>
{/* <TableData className={tableColumnClasses[0]}>
<BaremetalHostStatus host={host} />
</TableData> */}
<TableData className={tableColumnClasses[2]}>
<MachineCell host={host} />
</div>
{/* <div className={roleColumnClasses}>
<WithResources resourceMap={machineName ? hostResourceMap : {}}>
<BaremetalHostRole />
</WithResources>
</div> */}
<div className={columnClasses}>{address}</div>
</ResourceRow>
</TableData>
<TableData className={tableColumnClasses[3]}>
<BaremetalHostRole machine={machine} />
</TableData>
<TableData className={tableColumnClasses[4]}>{address}</TableData>
{/* <TableData className={tableColumnClasses[5]}>
TODO(jtomasek): Add host actions here
</TableData> */}
</TableRow>
);
};

const HostList = (props: React.ComponentProps<typeof List>) => (
<List {...props} Header={HostHeader} Row={HostRow} />
const HostList: React.FC<React.ComponentProps<typeof Table>> = (props) => (
<Table
{...props}
aria-label="Baremetal Hosts"
Header={HostsTableHeader}
Row={HostsTableRow}
virtualize
/>
);

// TODO(jtomasek): re-enable filters once the extension point for list.tsx is in place
Expand All @@ -118,13 +144,55 @@ type BaremetalHostsPageProps = {
namespace: string;
};

export const BaremetalHostsPage = (props: BaremetalHostsPageProps) => (
<ListPage
{...props}
canCreate
rowFilters={filters}
createButtonText="Add Host"
kind={referenceForModel(BaremetalHostModel)}
ListComponent={HostList}
/>
);
export const BaremetalHostsPage: React.FC<BaremetalHostsPageProps> = (props) => {
const hostsResource = {
kind: referenceForModel(BaremetalHostModel),
namespaced: true,
prop: 'hosts',
};
const machinesResource = {
kind: referenceForModel(MachineModel),
namespaced: true,
prop: 'machines',
};
const nodesResource = {
kind: NodeModel.kind,
namespaced: false,
prop: 'nodes',
};

const flatten = (resources) => {
// TODO(jtomasek): Remove loaded check once ListPageWrapper_ is updated to call flatten only
// when resources are loaded
Copy link
Member

Choose a reason for hiding this comment

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

Are we sure we want to make that change in ListPageWrapper_? I'd think we'd want to show the data as it comes in. Otherwise we'd block rendering the table.

Copy link
Author

Choose a reason for hiding this comment

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

I'll create a separate issue for this and try to elaborate on the issue I am seeing.

const loaded = _.every(resources, (resource) =>
resource.optional ? resource.loaded || !_.isEmpty(resource.loadError) : resource.loaded,
);
const {
hosts: { data: hostsData },
machines: { data: machinesData },
nodes: { data: nodesData },
} = resources;

if (loaded) {
return hostsData.map((host) => {
const machine = getHostMachine(host, machinesData);
const node = getMachineNode(machine, nodesData);
return { ...host, machine, node };
});
}
return [];
};

return (
<MultiListPage
{...props}
canCreate
rowFilters={filters}
createButtonText="Add Host"
namespace={props.namespace}
resources={[hostsResource, machinesResource, nodesResource]}
flatten={flatten}
ListComponent={HostList}
/>
);
};
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ interface MachineCellProps {
host: K8sResourceKind;
}

const MachineCell = ({ host }: MachineCellProps) => {
const MachineCell: React.FC<MachineCellProps> = ({ host }) => {
const machineName = getHostMachineName(host);

const {
Expand Down
2 changes: 1 addition & 1 deletion frontend/packages/metal3-plugin/src/models.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ export const BaremetalHostModel: K8sKind = {
labelPlural: 'Bare Metal Hosts',
apiVersion: 'v1alpha1',
path: 'baremetalhosts',
apiGroup: 'metalkube.org',
apiGroup: 'metal3.io',
plural: 'baremetalhosts',
abbr: 'BMH',
namespaced: true,
Expand Down
5 changes: 5 additions & 0 deletions frontend/packages/metal3-plugin/src/selectors/index.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
import * as _ from 'lodash-es';

import { K8sResourceKind, MachineKind } from '@console/internal/module/k8s';
import { getName } from '@console/shared';

export const getOperationalStatus = (host) => _.get(host, 'status.operationalStatus');
export const getProvisioningState = (host) => _.get(host, 'status.provisioning.state');
export const getHostMachineName = (host) => _.get(host, 'spec.machineRef.name');
Expand All @@ -14,3 +17,5 @@ export const getHostDescription = (host) => _.get(host, 'spec.description', '');
export const isHostPoweredOn = (host) => _.get(host, 'status.poweredOn', false);
export const getHostTotalStorageCapacity = (host) =>
_.reduce(getHostStorage(host), (sum, disk) => sum + disk.sizeGiB, 0);
export const getHostMachine = (host: K8sResourceKind, machines: MachineKind[]) =>
machines.find((machine) => getHostMachineName(host) === getName(machine));
2 changes: 1 addition & 1 deletion frontend/public/components/machine-deployment.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,9 @@ import * as _ from 'lodash-es';
import { Link } from 'react-router-dom';
import { sortable } from '@patternfly/react-table';
import * as classNames from 'classnames';
import { getMachineRole } from '@console/shared';
import { MachineModel, MachineDeploymentModel } from '../models';
import { MachineDeploymentKind, referenceForModel } from '../module/k8s';
import { getMachineRole } from './machine';
import {
editCountAction,
getAWSPlacement,
Expand Down
3 changes: 2 additions & 1 deletion frontend/public/components/machine-set.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,10 @@ import * as _ from 'lodash-es';
import { Link } from 'react-router-dom';
import { sortable } from '@patternfly/react-table';
import * as classNames from 'classnames';
import { getMachineRole } from '@console/shared';
import { MachineAutoscalerModel, MachineModel, MachineSetModel } from '../models';
import { K8sKind, MachineDeploymentKind, MachineSetKind, referenceForModel } from '../module/k8s';
import { getMachineRole, MachinePage } from './machine';
import { MachinePage } from './machine';
import { configureMachineAutoscalerModal, configureReplicaCountModal } from './modals';
import { DetailsPage, ListPage, Table, TableRow, TableData } from './factory';
import {
Expand Down
16 changes: 6 additions & 10 deletions frontend/public/components/machine.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,9 @@ import * as React from 'react';
import * as _ from 'lodash-es';
import { sortable } from '@patternfly/react-table';
import * as classNames from 'classnames';
import { getMachineRole, getMachineNodeName, getMachineAWSPlacement } from '@console/shared';
import { MachineModel } from '../models';
import { MachineDeploymentKind, MachineKind, MachineSetKind, referenceForModel } from '../module/k8s';
import { MachineKind, referenceForModel } from '../module/k8s';
import { Conditions } from './conditions';
import { NodeIPList } from './node';
import { DetailsPage, ListPage, Table, TableRow, TableData } from './factory';
Expand All @@ -21,11 +22,6 @@ import { breadcrumbsForOwnerRefs } from './utils/breadcrumbs';
const { common } = Kebab.factory;
const menuActions = [...common];
export const machineReference = referenceForModel(MachineModel);
const getAWSPlacement = (machine: MachineKind) => _.get(machine, 'spec.providerSpec.value.placement') || {};

export const getMachineRole = (obj: MachineKind | MachineSetKind | MachineDeploymentKind) => _.get(obj, ['metadata', 'labels', 'sigs.k8s.io/cluster-api-machine-role']);

const getNodeName = (obj) => _.get(obj, 'status.nodeRef.name');

const tableColumnClasses = [
classNames('pf-m-3-col-on-xl', 'pf-m-4-col-on-lg', 'pf-m-4-col-on-md', 'pf-m-6-col-on-sm'),
Expand Down Expand Up @@ -66,8 +62,8 @@ const MachineTableHeader = () => {
MachineTableHeader.displayName = 'MachineTableHeader';

const MachineTableRow: React.FC<MachineTableRowProps> = ({obj, index, key, style}) => {
const { availabilityZone, region } = getAWSPlacement(obj);
const nodeName = getNodeName(obj);
const { availabilityZone, region } = getMachineAWSPlacement(obj);
const nodeName = getMachineNodeName(obj);
return (
<TableRow id={obj.metadata.uid} index={index} trKey={key} style={style}>
<TableData className={classNames(tableColumnClasses[0], 'co-break-word')}>
Expand Down Expand Up @@ -100,9 +96,9 @@ type MachineTableRowProps = {
};

const MachineDetails: React.SFC<MachineDetailsProps> = ({obj}: {obj: MachineKind}) => {
const nodeName = getNodeName(obj);
const nodeName = getMachineNodeName(obj);
const machineRole = getMachineRole(obj);
const { availabilityZone, region } = getAWSPlacement(obj);
const { availabilityZone, region } = getMachineAWSPlacement(obj);
return <React.Fragment>
<div className="co-m-pane__body">
<SectionHeading text="Machine Overview" />
Expand Down