Skip to content

Commit

Permalink
Eliminate redundant getContainers/inspectContainer calls
Browse files Browse the repository at this point in the history
Both the initial container scan and each update called getContainers()
once and inspectContainer() twice (!!). But this is very redundant: The
objects returned by getContainers() and inspectContainer() have a very
similar structure. The latter does not have `Pid` and `PodName`, but we
don't use these properties anyway. The others map in a straightforward
way:

 * `Names[]` → single `Name`
 * `Ports` list → `NetworkSettings.Ports` map
 * `Mounts` (simple target dir list) →  list of objects with lots of
    info; direct translation is `.Destination`
 * `Started/Exited...At` → `State.*`
 * `State` → `State.Status`
 * `Command` → `Config.Cmd`
 * `Labels` → `Config.Labels`
 * `ImageID` → `Image`
 * `Image` → `ImageName`

So change the state keeping to call getContainers() only once at
initialization, and from then on only a single inspectContainer() for
each state update. Adjust the code for the above property structure
changes.
  • Loading branch information
martinpitt committed Jul 12, 2023
1 parent b15258e commit d59c3ba
Show file tree
Hide file tree
Showing 14 changed files with 146 additions and 185 deletions.
4 changes: 2 additions & 2 deletions src/ContainerCheckpointModal.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ const ContainerCheckpointModal = ({ containerWillCheckpoint, onAddNotification }
tcpEstablished,
})
.catch(ex => {
const error = cockpit.format(_("Failed to checkpoint container $0"), containerWillCheckpoint.Names);
const error = cockpit.format(_("Failed to checkpoint container $0"), containerWillCheckpoint.Name);
onAddNotification({ type: 'danger', error, errorDetail: ex.message });
setProgress(false);
})
Expand All @@ -38,7 +38,7 @@ const ContainerCheckpointModal = ({ containerWillCheckpoint, onAddNotification }
<Modal isOpen
showClose={false}
position="top" variant="medium"
title={cockpit.format(_("Checkpoint container $0"), containerWillCheckpoint.Names)}
title={cockpit.format(_("Checkpoint container $0"), containerWillCheckpoint.Name)}
footer={<>
<Button variant="primary" isDisabled={inProgress}
isLoading={inProgress}
Expand Down
6 changes: 3 additions & 3 deletions src/ContainerCommitModal.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ const ContainerCommitModal = ({ container, localImages }) => {
const [imageName, setImageName] = useState("");
const [tag, setTag] = useState("");
const [author, setAuthor] = useState("");
const [command, setCommand] = useState(container.Command ? utils.quote_cmdline(container.Command) : "");
const [command, setCommand] = useState(container.Config.Cmd ? utils.quote_cmdline(container.Config.Cmd) : "");
const [pause, setPause] = useState(false);
const [useDocker, setUseDocker] = useState(false);

Expand Down Expand Up @@ -78,7 +78,7 @@ const ContainerCommitModal = ({ container, localImages }) => {
client.commitContainer(container.isSystem, commitData)
.then(() => Dialogs.close())
.catch(ex => {
setDialogError(cockpit.format(_("Failed to commit container $0"), container.Names));
setDialogError(cockpit.format(_("Failed to commit container $0"), container.Name));
setDialogErrorDetail(cockpit.format("$0: $1", ex.message, ex.reason));
setCommitInProgress(false);
});
Expand Down Expand Up @@ -134,7 +134,7 @@ const ContainerCommitModal = ({ container, localImages }) => {
showClose={false}
position="top" variant="medium"
title={_("Commit container")}
description={fmt_to_fragments(_("Create a new image based on the current state of the $0 container."), <b>{container.Names}</b>)}
description={fmt_to_fragments(_("Create a new image based on the current state of the $0 container."), <b>{container.Name}</b>)}
footer={<>
<Button variant="primary"
className="btn-ctr-commit"
Expand Down
4 changes: 2 additions & 2 deletions src/ContainerDeleteModal.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ const ContainerDeleteModal = ({ containerWillDelete, onAddNotification }) => {
Dialogs.close();
client.delContainer(container.isSystem, id, false)
.catch(ex => {
const error = cockpit.format(_("Failed to remove container $0"), container.Names);
const error = cockpit.format(_("Failed to remove container $0"), container.Name);
onAddNotification({ type: 'danger', error, errorDetail: ex.message });
});
};
Expand All @@ -28,7 +28,7 @@ const ContainerDeleteModal = ({ containerWillDelete, onAddNotification }) => {
position="top" variant="medium"
titleIconVariant="warning"
onClose={Dialogs.close}
title={cockpit.format(_("Delete $0?"), containerWillDelete.Names)}
title={cockpit.format(_("Delete $0?"), containerWillDelete.Name)}
footer={<>
<Button variant="danger" className="btn-ctr-delete" onClick={handleRemoveContainer}>{_("Delete")}</Button>{' '}
<Button variant="link" onClick={Dialogs.close}>{_("Cancel")}</Button>
Expand Down
29 changes: 14 additions & 15 deletions src/ContainerDetails.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,19 +8,18 @@ import { Flex, FlexItem } from "@patternfly/react-core/dist/esm/layouts/Flex";
const _ = cockpit.gettext;

const render_container_state = (container) => {
if (container.State === "running") {
return cockpit.format(_("Up since $0"), utils.localize_time(container.StartedAt));
if (container.State.Status === "running") {
return cockpit.format(_("Up since $0"), utils.localize_time(Date.parse(container.State.StartedAt) / 1000));
}
return cockpit.format(_("Exited"));
};

const ContainerDetails = ({ container, containerDetail }) => {
const ContainerDetails = ({ container }) => {
const networkOptions = (
containerDetail &&
[
containerDetail.NetworkSettings.IPAddress,
containerDetail.NetworkSettings.Gateway,
containerDetail.NetworkSettings.MacAddress,
container.NetworkSettings?.IPAddress,
container.NetworkSettings?.Gateway,
container.NetworkSettings?.MacAddress,
].some(itm => !!itm)
);

Expand All @@ -34,27 +33,27 @@ const ContainerDetails = ({ container, containerDetail }) => {
</DescriptionListGroup>
<DescriptionListGroup>
<DescriptionListTerm>{_("Image")}</DescriptionListTerm>
<DescriptionListDescription>{container.Image}</DescriptionListDescription>
<DescriptionListDescription>{container.ImageName}</DescriptionListDescription>
</DescriptionListGroup>
<DescriptionListGroup>
<DescriptionListTerm>{_("Command")}</DescriptionListTerm>
<DescriptionListDescription>{container.Command ? utils.quote_cmdline(container.Command) : ""}</DescriptionListDescription>
<DescriptionListDescription>{container.Config?.Cmd ? utils.quote_cmdline(container.Config.Cmd) : ""}</DescriptionListDescription>
</DescriptionListGroup>
</DescriptionList>
</FlexItem>
<FlexItem>
{networkOptions && <DescriptionList columnModifier={{ default: '2Col' }} className='container-details-networking'>
{containerDetail && containerDetail.NetworkSettings.IPAddress && <DescriptionListGroup>
{container.NetworkSettings?.IPAddress && <DescriptionListGroup>
<DescriptionListTerm>{_("IP address")}</DescriptionListTerm>
<DescriptionListDescription>{containerDetail.NetworkSettings.IPAddress}</DescriptionListDescription>
<DescriptionListDescription>{container.NetworkSettings.IPAddress}</DescriptionListDescription>
</DescriptionListGroup>}
{containerDetail && containerDetail.NetworkSettings.Gateway && <DescriptionListGroup>
{container.NetworkSettings?.Gateway && <DescriptionListGroup>
<DescriptionListTerm>{_("Gateway")}</DescriptionListTerm>
<DescriptionListDescription>{containerDetail.NetworkSettings.Gateway}</DescriptionListDescription>
<DescriptionListDescription>{container.NetworkSettings.Gateway}</DescriptionListDescription>
</DescriptionListGroup>}
{containerDetail && containerDetail.NetworkSettings.MacAddress && <DescriptionListGroup>
{container.NetworkSettings?.MacAddress && <DescriptionListGroup>
<DescriptionListTerm>{_("MAC address")}</DescriptionListTerm>
<DescriptionListDescription>{containerDetail.NetworkSettings.MacAddress}</DescriptionListDescription>
<DescriptionListDescription>{container.NetworkSettings.MacAddress}</DescriptionListDescription>
</DescriptionListGroup>}
</DescriptionList>}
</FlexItem>
Expand Down
27 changes: 10 additions & 17 deletions src/ContainerHealthLogs.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -42,17 +42,10 @@ const HealthcheckOnFailureActionText = {
kill: _("Force stop"),
};

const ContainerHealthLogs = ({ container, containerDetail, onAddNotification, state }) => {
let healthCheck = {};
let failingStreak = 0;
let logs = [];
if (containerDetail) {
healthCheck = containerDetail.Config.Healthcheck || containerDetail.Config.Health;
healthCheck.HealthcheckOnFailureAction = containerDetail.Config.HealthcheckOnFailureAction;
const healthState = containerDetail.State.Healthcheck || containerDetail.State.Health;
failingStreak = healthState.FailingStreak || 0;
logs = [...(healthState.Log || [])].reverse();
}
const ContainerHealthLogs = ({ container, onAddNotification, state }) => {
const healthCheck = container.Config?.Healthcheck ?? container.Config?.Health ?? {};
const healthState = container.State?.Healthcheck ?? container.State?.Health ?? {};
const logs = [...(healthState.Log || [])].reverse();

return (
<>
Expand Down Expand Up @@ -83,22 +76,22 @@ const ContainerHealthLogs = ({ container, containerDetail, onAddNotification, st
<DescriptionListTerm>{_("Timeout")}</DescriptionListTerm>
<DescriptionListDescription>{format_nanoseconds(healthCheck.Timeout)}</DescriptionListDescription>
</DescriptionListGroup>}
{healthCheck.HealthcheckOnFailureAction && <DescriptionListGroup>
{container.Config?.HealthcheckOnFailureAction && <DescriptionListGroup>
<DescriptionListTerm>{_("When unhealthy")}</DescriptionListTerm>
<DescriptionListDescription>{HealthcheckOnFailureActionText[healthCheck.HealthcheckOnFailureAction]}</DescriptionListDescription>
<DescriptionListDescription>{HealthcheckOnFailureActionText[container.Config.HealthcheckOnFailureAction]}</DescriptionListDescription>
</DescriptionListGroup>}
{failingStreak !== 0 && <DescriptionListGroup>
{healthState.FailingStreak && <DescriptionListGroup>
<DescriptionListTerm>{_("Failing streak")}</DescriptionListTerm>
<DescriptionListDescription>{failingStreak}</DescriptionListDescription>
<DescriptionListDescription>{healthState.FailingStreak}</DescriptionListDescription>
</DescriptionListGroup>}
</DescriptionList>
</FlexItem>
{ container.State === "running" &&
{ container.State.Status === "running" &&
<FlexItem>
<Button variant="secondary" onClick={() => {
client.runHealthcheck(container.isSystem, container.Id)
.catch(ex => {
const error = cockpit.format(_("Failed to run health check on container $0"), container.Names);
const error = cockpit.format(_("Failed to run health check on container $0"), container.Name);
onAddNotification({ type: 'danger', error, errorDetail: ex.message });
});
}}>
Expand Down
38 changes: 18 additions & 20 deletions src/ContainerIntegration.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,25 +10,23 @@ import { EmptyStatePanel } from "cockpit-components-empty-state.jsx";

const _ = cockpit.gettext;

export const renderContainerPublishedPorts = (ports) => {
// ports is a mapping like { "5000/tcp": [{"HostIp": "", "HostPort": "6000"}] }
export const renderContainerPublishedPorts = ports => {
if (!ports)
return null;

const result = ports.map(port => {
// podman v4 has different names than v3
const protocol = port.protocol;
const hostPort = port.hostPort || port.host_port;
const containerPort = port.containerPort || port.container_port;
const hostIp = port.hostIP || port.host_ip || '0.0.0.0';

return (
<ListItem key={ protocol + hostPort + containerPort }>
{ hostIp }:{ hostPort } &rarr; { containerPort }/{ protocol }
</ListItem>
);
const items = [];
Object.entries(ports).forEach(([containerPort, hostBindings]) => {
hostBindings.forEach(binding => {
items.push(
<ListItem key={ containerPort + binding.HostIp + binding.HostPort }>
{ binding.HostIp || "0.0.0.0" }:{ binding.HostPort } &rarr; { containerPort }
</ListItem>
);
});
});

return <List isPlain>{result}</List>;
return <List isPlain>{items}</List>;
};

export const renderContainerVolumes = (volumes) => {
Expand Down Expand Up @@ -89,18 +87,18 @@ const ContainerEnv = ({ containerEnv, imageEnv }) => {
return <List isPlain>{result}</List>;
};

const ContainerIntegration = ({ container, containerDetail, localImages }) => {
if (containerDetail === null || localImages === null) {
const ContainerIntegration = ({ container, localImages }) => {
if (localImages === null) {
return (
<EmptyStatePanel title={_("Loading details...")} loading />
);
}

const ports = renderContainerPublishedPorts(container.Ports);
const volumes = renderContainerVolumes(containerDetail.Mounts);
const ports = renderContainerPublishedPorts(container.NetworkSettings.Ports);
const volumes = renderContainerVolumes(container.Mounts);

const image = localImages.filter(img => img.Id === container.ImageID)[0];
const env = <ContainerEnv containerEnv={containerDetail.Config.Env} imageEnv={image.Env || []} />;
const image = localImages.filter(img => img.Id === container.Image)[0];
const env = <ContainerEnv containerEnv={container.Config.Env} imageEnv={image.Env || []} />;

return (
<DescriptionList isAutoColumnWidths columnModifier={{ md: '3Col' }} className='container-integration'>
Expand Down
6 changes: 3 additions & 3 deletions src/ContainerRenameModal.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ const _ = cockpit.gettext;

const ContainerRenameModal = ({ container, version, updateContainer }) => {
const Dialogs = useDialogs();
const [name, setName] = useState(container.Names[0]);
const [name, setName] = useState(container.Name);
const [nameError, setNameError] = useState(null);
const [dialogError, setDialogError] = useState(null);
const [dialogErrorDetail, setDialogErrorDetail] = useState(null);
Expand Down Expand Up @@ -50,7 +50,7 @@ const ContainerRenameModal = ({ container, version, updateContainer }) => {
}
})
.catch(ex => {
setDialogError(cockpit.format(_("Failed to rename container $0"), container.Names[0]));
setDialogError(cockpit.format(_("Failed to rename container $0"), container.Name));
setDialogErrorDetail(cockpit.format("$0: $1", ex.message, ex.reason));
});
};
Expand Down Expand Up @@ -81,7 +81,7 @@ const ContainerRenameModal = ({ container, version, updateContainer }) => {
position="top" variant="medium"
onClose={Dialogs.close}
onKeyDown={handleKeyDown}
title={cockpit.format(_("Rename container $0"), container.Names[0])}
title={cockpit.format(_("Rename container $0"), container.Name)}
footer={<>
<Button variant="primary"
className="btn-ctr-rename"
Expand Down
4 changes: 2 additions & 2 deletions src/ContainerRestoreModal.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ const ContainerRestoreModal = ({ containerWillRestore, onAddNotification }) => {
ignoreStaticMAC,
})
.catch(ex => {
const error = cockpit.format(_("Failed to restore container $0"), containerWillRestore.Names);
const error = cockpit.format(_("Failed to restore container $0"), containerWillRestore.Name);
onAddNotification({ type: 'danger', error, errorDetail: ex.message });
setInProgress(false);
})
Expand All @@ -41,7 +41,7 @@ const ContainerRestoreModal = ({ containerWillRestore, onAddNotification }) => {
<Modal isOpen
showClose={false}
position="top" variant="medium"
title={cockpit.format(_("Restore container $0"), containerWillRestore.Names)}
title={cockpit.format(_("Restore container $0"), containerWillRestore.Name)}
footer={<>
<Button variant="primary" isDisabled={inProgress}
isLoading={inProgress}
Expand Down
1 change: 0 additions & 1 deletion src/ContainerTerminal.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,6 @@ class ContainerTerminal extends React.Component {

componentDidUpdate(prevProps, prevState) {
// Connect channel when there is none and either container started or tty was resolved
// tty is being read as part of containerDetails and it is asynchronous so we need to wait for it
if (!this.state.channel && (
(this.props.containerStatus === "running" && prevProps.containerStatus !== "running") ||
(this.props.tty !== undefined && prevProps.tty === undefined)))
Expand Down
Loading

0 comments on commit d59c3ba

Please sign in to comment.