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

Eliminate redundant getContainers/inspectContainer calls #1350

Merged
merged 5 commits into from
Jul 13, 2023
Merged

Eliminate redundant getContainers/inspectContainer calls #1350

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
94e4912
Eliminate redundant getContainers/inspectContainer calls
martinpitt Jul 11, 2023
c4f5b0a
Drop redundant falsy checks for utils.quote_cmdline()
martinpitt Jul 13, 2023
551d69f
Add not-covered annotations to podman API failures
martinpitt Jul 13, 2023
7878199
Drop unnecessary null fallback
martinpitt Jul 13, 2023
c21ae95
Add not-covered annotations
martinpitt Jul 13, 2023
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
4 changes: 2 additions & 2 deletions src/ContainerCheckpointModal.jsx
View file
Open in desktop
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); // not-covered: OS error
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
View file
Open in desktop
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(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
View file
Open in desktop
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); // not-covered: OS error
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
View file
Open in desktop
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>{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
View file
Open in desktop
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 ?? {}; // not-covered: only on old version
const healthState = container.State?.Healthcheck ?? container.State?.Health ?? {}; // not-covered: only on old version
const logs = [...(healthState.Log || [])].reverse(); // not-covered: Log should always exist, belt-and-suspenders

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); // not-covered: OS error
onAddNotification({ type: 'danger', error, errorDetail: ex.message });
});
}}>
Expand Down
38 changes: 18 additions & 20 deletions src/ContainerIntegration.jsx
View file
Open in desktop
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 => { // not-covered: null was observed in the wild, but unknown how to reproduce
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) { // not-covered: not a stable UI state
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
View file
Open in desktop
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)); // not-covered: OS error
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
View file
Open in desktop
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); // not-covered: OS error
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
View file
Open in desktop
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