From d59c3ba5f14bbda110749a5c1fd6b480b43ded96 Mon Sep 17 00:00:00 2001 From: Martin Pitt Date: Tue, 11 Jul 2023 17:04:38 +0200 Subject: [PATCH] Eliminate redundant getContainers/inspectContainer calls MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- src/ContainerCheckpointModal.jsx | 4 +- src/ContainerCommitModal.jsx | 6 +- src/ContainerDeleteModal.jsx | 4 +- src/ContainerDetails.jsx | 29 ++++--- src/ContainerHealthLogs.jsx | 27 +++--- src/ContainerIntegration.jsx | 38 ++++----- src/ContainerRenameModal.jsx | 6 +- src/ContainerRestoreModal.jsx | 4 +- src/ContainerTerminal.jsx | 1 - src/Containers.jsx | 137 ++++++++++++++++--------------- src/ImageRunModal.jsx | 2 +- src/ImageUsedBy.jsx | 4 +- src/app.jsx | 61 ++++---------- src/client.js | 8 +- 14 files changed, 146 insertions(+), 185 deletions(-) diff --git a/src/ContainerCheckpointModal.jsx b/src/ContainerCheckpointModal.jsx index acb8173b0..45d58b18f 100644 --- a/src/ContainerCheckpointModal.jsx +++ b/src/ContainerCheckpointModal.jsx @@ -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); }) @@ -38,7 +38,7 @@ const ContainerCheckpointModal = ({ containerWillCheckpoint, onAddNotification } {' '} diff --git a/src/ContainerDetails.jsx b/src/ContainerDetails.jsx index e98c0ea28..d234a231f 100644 --- a/src/ContainerDetails.jsx +++ b/src/ContainerDetails.jsx @@ -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) ); @@ -34,27 +33,27 @@ const ContainerDetails = ({ container, containerDetail }) => { {_("Image")} - {container.Image} + {container.ImageName} {_("Command")} - {container.Command ? utils.quote_cmdline(container.Command) : ""} + {container.Config?.Cmd ? utils.quote_cmdline(container.Config.Cmd) : ""} {networkOptions && - {containerDetail && containerDetail.NetworkSettings.IPAddress && + {container.NetworkSettings?.IPAddress && {_("IP address")} - {containerDetail.NetworkSettings.IPAddress} + {container.NetworkSettings.IPAddress} } - {containerDetail && containerDetail.NetworkSettings.Gateway && + {container.NetworkSettings?.Gateway && {_("Gateway")} - {containerDetail.NetworkSettings.Gateway} + {container.NetworkSettings.Gateway} } - {containerDetail && containerDetail.NetworkSettings.MacAddress && + {container.NetworkSettings?.MacAddress && {_("MAC address")} - {containerDetail.NetworkSettings.MacAddress} + {container.NetworkSettings.MacAddress} } } diff --git a/src/ContainerHealthLogs.jsx b/src/ContainerHealthLogs.jsx index e2838bf6d..a2d64ec26 100644 --- a/src/ContainerHealthLogs.jsx +++ b/src/ContainerHealthLogs.jsx @@ -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 ( <> @@ -83,22 +76,22 @@ const ContainerHealthLogs = ({ container, containerDetail, onAddNotification, st {_("Timeout")} {format_nanoseconds(healthCheck.Timeout)} } - {healthCheck.HealthcheckOnFailureAction && + {container.Config?.HealthcheckOnFailureAction && {_("When unhealthy")} - {HealthcheckOnFailureActionText[healthCheck.HealthcheckOnFailureAction]} + {HealthcheckOnFailureActionText[container.Config.HealthcheckOnFailureAction]} } - {failingStreak !== 0 && + {healthState.FailingStreak && {_("Failing streak")} - {failingStreak} + {healthState.FailingStreak} } - { container.State === "running" && + { container.State.Status === "running" && } - {infraContainerDetails.Mounts && infraContainerDetails.Mounts.length !== 0 && + {infraContainer.Mounts && infraContainer.Mounts.length !== 0 && @@ -610,7 +614,7 @@ class Containers extends React.Component { emptyCaption = _("No running containers"); if (this.props.containers !== null && this.props.pods !== null) { - filtered = Object.keys(this.props.containers).filter(id => !(this.props.filter == "running") || ["running", "restarting"].includes(this.props.containers[id].State)); + filtered = Object.keys(this.props.containers).filter(id => !(this.props.filter == "running") || ["running", "restarting"].includes(this.props.containers[id].State.Status)); if (this.props.userServiceAvailable && this.props.systemServiceAvailable && this.props.ownerFilter !== "all") { filtered = filtered.filter(id => { @@ -624,10 +628,10 @@ class Containers extends React.Component { if (this.props.textFilter.length > 0) { const lcf = this.props.textFilter.toLowerCase(); - filtered = filtered.filter(id => this.props.containers[id].Names[0].toLowerCase().indexOf(lcf) >= 0 || + filtered = filtered.filter(id => this.props.containers[id].Name.toLowerCase().indexOf(lcf) >= 0 || (this.props.containers[id].Pod && this.props.pods[this.props.containers[id].Pod + this.props.containers[id].isSystem.toString()].Name.toLowerCase().indexOf(lcf) >= 0) || - this.props.containers[id].Image.toLowerCase().indexOf(lcf) >= 0 + this.props.containers[id].ImageName.toLowerCase().indexOf(lcf) >= 0 ); } @@ -636,9 +640,9 @@ class Containers extends React.Component { filtered.sort((a, b) => { // Show unhealthy containers first - if (this.props.containersDetails[a] && this.props.containersDetails[b]) { - const a_health = this.props.containersDetails[a].State.Health || this.props.containersDetails[a].State.Healthcheck; - const b_health = this.props.containersDetails[b].State.Health || this.props.containersDetails[b].State.Healthcheck; + if (this.props.containers[a] && this.props.containers[b]) { + const a_health = this.props.containers[a].State.Health || this.props.containers[a].State.Healthcheck; + const b_health = this.props.containers[b].State.Health || this.props.containers[b].State.Healthcheck; if (a_health.Status !== b_health.Status) { if (a_health.Status === "unhealthy") return -1; @@ -649,7 +653,7 @@ class Containers extends React.Component { // User containers are in front of system ones if (this.props.containers[a].isSystem !== this.props.containers[b].isSystem) return this.props.containers[a].isSystem ? 1 : -1; - return this.props.containers[a].Names > this.props.containers[b].Names ? 1 : -1; + return this.props.containers[a].Name > this.props.containers[b].Name ? 1 : -1; }); Object.keys(this.props.pods || {}).forEach(pod => { partitionedContainers[pod] = [] }); @@ -687,12 +691,12 @@ class Containers extends React.Component { for (const containerid of Object.keys(this.props.containers)) { const container = this.props.containers[containerid]; // Ignore pods and running containers - if (!prune_states.includes(container.State) || container.Pod) + if (!prune_states.includes(container.State.Status) || container.Pod) continue; unusedContainers.push({ id: container.Id + container.isSystem.toString(), - name: container.Names[0], + name: container.Name, created: container.Created, system: container.isSystem, }); @@ -828,7 +832,6 @@ class Containers extends React.Component { const tableProps = {}; const rows = partitionedContainers[section].map(container => { return this.renderRow(this.props.containersStats, container, - this.props.containersDetails[container.Id + container.isSystem.toString()] || null, localImages); }); let caption; diff --git a/src/ImageRunModal.jsx b/src/ImageRunModal.jsx index 65ef920bb..e4bd6b7ca 100644 --- a/src/ImageRunModal.jsx +++ b/src/ImageRunModal.jsx @@ -331,7 +331,7 @@ export class ImageRunModal extends React.Component { // Assign temporary properties to allow rendering tempImage.Id = tempImage.name; tempImage.isSystem = isSystem; - tempImage.State = _("downloading"); + tempImage.State = { Status: _("downloading") }; tempImage.Created = new Date(); tempImage.Names = [tempImage.name]; tempImage.Image = createConfig.image; diff --git a/src/ImageUsedBy.jsx b/src/ImageUsedBy.jsx index 4c73ffe94..6db307b2a 100644 --- a/src/ImageUsedBy.jsx +++ b/src/ImageUsedBy.jsx @@ -17,7 +17,7 @@ const ImageUsedBy = ({ containers, showAll }) => { {containers.map(c => { const container = c.container; - const isRunning = container.State == "running"; + const isRunning = container.State?.Status === "running"; return ( @@ -30,7 +30,7 @@ const ImageUsedBy = ({ containers, showAll }) => { if (!isRunning) showAll(); }}> - {container.Names} + {container.Name} {isRunning && {_("Running")}} diff --git a/src/app.jsx b/src/app.jsx index 2e272a929..fddd2451c 100644 --- a/src/app.jsx +++ b/src/app.jsx @@ -49,7 +49,6 @@ class Application extends React.Component { containers: null, containersFilter: "all", containersStats: {}, - containersDetails: {}, userContainersLoaded: null, systemContainersLoaded: null, userPodsLoaded: null, @@ -171,39 +170,31 @@ class Application extends React.Component { }); } - inspectContainerDetail(id, system) { - client.inspectContainer(system, id) - .then(reply => { - this.updateState("containersDetails", reply.Id + system.toString(), reply); - }) - .catch(e => console.log(e)); - } - initContainers(system) { return client.getContainers(system) - .then(reply => { + .then(containerList => Promise.all( + containerList.map(container => client.inspectContainer(system, container.Id)) + )) + .then(containerDetails => { this.setState(prevState => { - // Copy only containers that could not be deleted with this event - // So when event from system come, only copy user containers and vice versa + // keep/copy the containers for !system const copyContainers = {}; Object.entries(prevState.containers || {}).forEach(([id, container]) => { if (container.isSystem !== system) copyContainers[id] = container; }); - for (const container of reply) { - container.isSystem = system; - copyContainers[container.Id + system.toString()] = container; + for (const detail of containerDetails) { + detail.isSystem = system; + copyContainers[detail.Id + system.toString()] = detail; } + console.log("initContainers", system ? "system:" : "user:", copyContainers); return { containers: copyContainers, [system ? "systemContainersLoaded" : "userContainersLoaded"]: true, }; }); this.updateContainerStats(system); - for (const container of reply) { - this.inspectContainerDetail(container.Id, system); - } }) .catch(console.log); } @@ -263,29 +254,14 @@ class Application extends React.Component { } updateContainer(id, system, event) { - return client.getContainers(system, id) - .then(reply => { - if (reply && reply.length > 0) { - reply = reply[0]; - - reply.isSystem = system; - // HACK: during restart State never changes from "running" - // override it to reconnect console after restart - if (event && event.Action === "restart") - reply.State = "restarting"; - this.updateState("containers", reply.Id + system.toString(), reply); - if (["running", "created", "exited", "paused", "stopped"].find(containerState => containerState === reply.State)) { - this.inspectContainerDetail(reply.Id, system); - } else { - this.setState(prevState => { - const copyDetails = Object.assign({}, prevState.containersDetails); - const copyStats = Object.assign({}, prevState.containersStats); - delete copyDetails[reply.Id + system.toString()]; - delete copyStats[reply.Id + system.toString()]; - return { containersDetails: copyDetails, containersStats: copyStats }; - }); - } - } + return client.inspectContainer(system, id) + .then(details => { + details.isSystem = system; + // HACK: during restart State never changes from "running" + // override it to reconnect console after restart + if (event?.Action === "restart") + details.State.Status = "restarting"; + this.updateState("containers", details.Id + system.toString(), details); }) .catch(console.log); } @@ -673,7 +649,7 @@ class Application extends React.Component { if (this.state.containers !== null) { Object.keys(this.state.containers).forEach(c => { const container = this.state.containers[c]; - const image = container.ImageID + container.isSystem.toString(); + const image = container.Image + container.isSystem.toString(); if (imageContainerList[image]) { imageContainerList[image].push({ container, @@ -739,7 +715,6 @@ class Application extends React.Component { containers={this.state.systemContainersLoaded && this.state.userContainersLoaded ? this.state.containers : null} pods={this.state.systemPodsLoaded && this.state.userPodsLoaded ? this.state.pods : null} containersStats={this.state.containersStats} - containersDetails={this.state.containersDetails} filter={this.state.containersFilter} handleFilterChange={this.onContainerFilterChanged} textFilter={this.state.textFilter} diff --git a/src/client.js b/src/client.js index 1c460fb43..572056243 100644 --- a/src/client.js +++ b/src/client.js @@ -51,13 +51,7 @@ export function getInfo(system) { }); } -export function getContainers(system, id) { - const options = { all: true }; - if (id) - options.filters = JSON.stringify({ id: [id] }); - - return podmanJson("libpod/containers/json", "GET", options, system); -} +export const getContainers = system => podmanJson("libpod/containers/json", "GET", { all: true }, system); export const streamContainerStats = (system, callback) => podmanMonitor("libpod/containers/stats", "GET", { stream: true }, callback, system);