Skip to content

Commit

Permalink
Generalize client.js API for multiple users
Browse files Browse the repository at this point in the history 
Similarly to the previous commit for rest.js, change the API to accept an user
id instead of an `isSystem` boolean. Change the Container, Pod, and Image
properties accordingly.

However, keep the UI logic and higher-level `isSystem` flags/helpers as-is for
now, as this is intrusive enough.

Clean up the often repeated "id + isSystem" index key calculation for
the app's containers/images/pods states, and centralize it into a
`makeKey()` helper. Put the uid in front, so that they have more natural
sorting (in the UI we sort by owner first), and add a `-` in between so
that they are easier to read.
  • Loading branch information
@martinpitt
martinpitt committed Feb 6, 2025
1 parent cb89f48 commit bac14d1
Show file tree
Hide file tree
Showing 22 changed files with 277 additions and 276 deletions.
2 changes: 1 addition & 1 deletion src/ContainerCheckpointModal.jsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ const ContainerCheckpointModal = ({ containerWillCheckpoint, onAddNotification }

const handleCheckpointContainer = () => {
setProgress(true);
client.postContainer(containerWillCheckpoint.isSystem, "checkpoint", containerWillCheckpoint.Id, {
client.postContainer(containerWillCheckpoint.uid, "checkpoint", containerWillCheckpoint.Id, {
keep,
leaveRunning,
tcpEstablished,
Expand Down
4 changes: 2 additions & 2 deletions src/ContainerCommitModal.jsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ const ContainerCommitModal = ({ container, localImages }) => {
if (full_name.indexOf("/") < 0)
full_name = "localhost/" + full_name;

if (!force && localImages.some(image => image.isSystem === container.isSystem && image.Name === full_name)) {
if (!force && localImages.some(image => image.uid === container.uid && image.Name === full_name)) {
setNameError(_("Image name is not unique"));
return;
}
Expand Down Expand Up @@ -77,7 +77,7 @@ const ContainerCommitModal = ({ container, localImages }) => {
setNameError("");
setDialogError("");
setDialogErrorDetail("");
client.commitContainer(container.isSystem, commitData)
client.commitContainer(container.uid, commitData)
.then(() => Dialogs.close())
.catch(ex => {
setDialogError(cockpit.format(_("Failed to commit container $0"), container.Name));
Expand Down
2 changes: 1 addition & 1 deletion src/ContainerDeleteModal.jsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ const ContainerDeleteModal = ({ containerWillDelete, onAddNotification }) => {
const id = container ? container.Id : "";

Dialogs.close();
client.delContainer(container.isSystem, id, false)
client.delContainer(container.uid, id, false)
.catch(ex => {
const error = cockpit.format(_("Failed to remove container $0"), container.Name); // not-covered: OS error
onAddNotification({ type: 'danger', error, errorDetail: ex.message });
Expand Down
2 changes: 1 addition & 1 deletion src/ContainerHealthLogs.jsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ const ContainerHealthLogs = ({ container, onAddNotification, state }) => {
{ container.State.Status === "running" &&
<FlexItem>
<Button variant="secondary" onClick={() => {
client.runHealthcheck(container.isSystem, container.Id)
client.runHealthcheck(container.uid, container.Id)
.catch(ex => {
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
4 changes: 2 additions & 2 deletions src/ContainerLogs.jsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ class ContainerLogs extends React.Component {
}
this.resize(this.props.width);

const connection = rest.connect(this.props.system ? 0 : null);
const connection = rest.connect(this.props.uid);
const options = {
method: "GET",
path: client.VERSION + "libpod/containers/" + this.props.containerId + "/logs",
Expand Down Expand Up @@ -173,7 +173,7 @@ class ContainerLogs extends React.Component {

ContainerLogs.propTypes = {
containerId: PropTypes.string.isRequired,
system: PropTypes.bool.isRequired,
uid: PropTypes.number,
width: PropTypes.number.isRequired
};

Expand Down
4 changes: 2 additions & 2 deletions src/ContainerRenameModal.jsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,12 +44,12 @@ const ContainerRenameModal = ({ container, updateContainer }) => {

setNameError(null);
setDialogError(null);
client.renameContainer(container.isSystem, container.Id, { name })
client.renameContainer(container.uid, container.Id, { name })
.then(() => {
Dialogs.close();
// HACK: This is a workaround for missing API rename event in Podman versions less than 4.1.
if (version.localeCompare("4.1", undefined, { numeric: true, sensitivity: 'base' }) < 0) {
updateContainer(container.Id, container.isSystem); // not-covered: only on old version
updateContainer(container.Id, container.uid); // not-covered: only on old version
}
})
.catch(ex => {
Expand Down
2 changes: 1 addition & 1 deletion src/ContainerRestoreModal.jsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ const ContainerRestoreModal = ({ containerWillRestore, onAddNotification }) => {

const handleRestoreContainer = () => {
setInProgress(true);
client.postContainer(containerWillRestore.isSystem, "restore", containerWillRestore.Id, {
client.postContainer(containerWillRestore.uid, "restore", containerWillRestore.Id, {
keep,
tcpEstablished,
ignoreStaticIP,
Expand Down
10 changes: 5 additions & 5 deletions src/ContainerTerminal.jsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ class ContainerTerminal extends React.Component {
const realWidth = this.term._core._renderService.dimensions.css.cell.width;
const cols = Math.floor((width - padding) / realWidth);
this.term.resize(cols, 24);
client.resizeContainersTTY(this.props.system, this.state.sessionId, this.props.tty, cols, 24)
client.resizeContainersTTY(this.props.uid, this.state.sessionId, this.props.tty, cols, 24)
.catch(e => this.setState({ errorMessage: e.message }));
}

Expand Down Expand Up @@ -190,9 +190,9 @@ class ContainerTerminal extends React.Component {
}

execAndConnect() {
client.execContainer(this.props.system, this.state.container)
client.execContainer(this.props.uid, this.state.container)
.then(r => {
const address = rest.getAddress(this.props.system ? 0 : null);
const address = rest.getAddress(this.props.uid);
const channel = cockpit.channel({
payload: "stream",
unix: address.path,
Expand All @@ -212,7 +212,7 @@ class ContainerTerminal extends React.Component {
}

connectToTty() {
const address = rest.getAddress(this.props.system ? 0 : null);
const address = rest.getAddress(this.props.uid);
const channel = cockpit.channel({
payload: "stream",
unix: address.path,
Expand Down Expand Up @@ -276,7 +276,7 @@ ContainerTerminal.propTypes = {
containerId: PropTypes.string.isRequired,
containerStatus: PropTypes.string.isRequired,
width: PropTypes.number.isRequired,
system: PropTypes.bool.isRequired,
uid: PropTypes.number,
tty: PropTypes.bool,
};

Expand Down
60 changes: 30 additions & 30 deletions src/Containers.jsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ const ContainerActions = ({ container, onAddNotification, localImages, updateCon
const handleForceRemoveContainer = () => {
const id = container ? container.Id : "";

return client.delContainer(container.isSystem, id, true)
return client.delContainer(container.uid, id, true)
.catch(ex => {
const error = cockpit.format(_("Failed to force remove container $0"), container.Name); // not-covered: OS error
onAddNotification({ type: 'danger', error, errorDetail: ex.message });
Expand All @@ -79,31 +79,31 @@ const ContainerActions = ({ container, onAddNotification, localImages, updateCon

if (force)
args.t = 0;
client.postContainer(container.isSystem, "stop", container.Id, args)
client.postContainer(container.uid, "stop", container.Id, args)
.catch(ex => {
const error = cockpit.format(_("Failed to stop container $0"), container.Name); // not-covered: OS error
onAddNotification({ type: 'danger', error, errorDetail: ex.message });
});
};

const startContainer = () => {
client.postContainer(container.isSystem, "start", container.Id, {})
client.postContainer(container.uid, "start", container.Id, {})
.catch(ex => {
const error = cockpit.format(_("Failed to start container $0"), container.Name); // not-covered: OS error
onAddNotification({ type: 'danger', error, errorDetail: ex.message });
});
};

const resumeContainer = () => {
client.postContainer(container.isSystem, "unpause", container.Id, {})
client.postContainer(container.uid, "unpause", container.Id, {})
.catch(ex => {
const error = cockpit.format(_("Failed to resume container $0"), container.Name); // not-covered: OS error
onAddNotification({ type: 'danger', error, errorDetail: ex.message });
});
};

const pauseContainer = () => {
client.postContainer(container.isSystem, "pause", container.Id, {})
client.postContainer(container.uid, "pause", container.Id, {})
.catch(ex => {
const error = cockpit.format(_("Failed to pause container $0"), container.Name); // not-covered: OS error
onAddNotification({ type: 'danger', error, errorDetail: ex.message });
Expand All @@ -120,7 +120,7 @@ const ContainerActions = ({ container, onAddNotification, localImages, updateCon

if (force)
args.t = 0;
client.postContainer(container.isSystem, "restart", container.Id, args)
client.postContainer(container.uid, "restart", container.Id, args)
.catch(ex => {
const error = cockpit.format(_("Failed to restart container $0"), container.Name); // not-covered: OS error
onAddNotification({ type: 'danger', error, errorDetail: ex.message });
Expand Down Expand Up @@ -190,7 +190,7 @@ const ContainerActions = ({ container, onAddNotification, localImages, updateCon
);
}

if (container.isSystem && !isPaused) {
if (container.uid == 0 && !isPaused) {
actions.push(
<Divider key="separator-0" />,
<DropdownItem key="checkpoint"
Expand All @@ -211,7 +211,7 @@ const ContainerActions = ({ container, onAddNotification, localImages, updateCon
if (!isSystemdService) {
addRenameAction();
}
if (container.isSystem && container.State?.CheckpointPath) {
if (container.uid == 0 && container.State?.CheckpointPath) {
actions.push(
<Divider key="separator-0" />,
<DropdownItem key="restore"
Expand Down Expand Up @@ -320,7 +320,7 @@ class Containers extends React.Component {
}

renderRow(containersStats, container, localImages) {
const containerStats = containersStats[container.Id + container.isSystem.toString()];
const containerStats = containersStats[container.key];
const image = container.ImageName;
const isToolboxContainer = container.Config?.Labels?.["com.github.containers.toolbox"] === "true";
const isDistroboxContainer = container.Config?.Labels?.manager === "distrobox";
Expand All @@ -335,7 +335,7 @@ class Containers extends React.Component {

let proc = "";
let mem = "";
if (this.props.cgroupVersion == 'v1' && !container.isSystem && status == 'running') { // not-covered: only on old version
if (this.props.cgroupVersion == 'v1' && container.uid !== 0 && status == 'running') { // not-covered: only on old version
proc = <div><abbr title={_("not available")}>{_("n/a")}</abbr></div>;
mem = <div><abbr title={_("not available")}>{_("n/a")}</abbr></div>;
}
Expand Down Expand Up @@ -396,9 +396,9 @@ class Containers extends React.Component {
const columns = [
{ title: info_block, sortKey: container.Name ?? container.Id },
{
title: container.isSystem ? _("system") : <div><span className="ct-grey-text">{_("user:")} </span>{this.props.user}</div>,
title: (container.uid === 0) ? _("system") : <div><span className="ct-grey-text">{_("user:")} </span>{this.props.user}</div>,
props: { modifier: "nowrap" },
sortKey: container.isSystem.toString()
sortKey: container.key,
},
{ title: proc, props: { modifier: "nowrap" }, sortKey: containerState === "Running" ? containerStats?.CPU ?? -1 : -1 },
{ title: mem, props: { modifier: "nowrap" }, sortKey: containerStats?.MemUsage ?? -1 },
Expand Down Expand Up @@ -435,12 +435,12 @@ class Containers extends React.Component {
tabs.push({
name: _("Logs"),
renderer: ContainerLogs,
data: { containerId: container.Id, containerStatus: container.State.Status, width: this.state.width, system: container.isSystem }
data: { containerId: container.Id, containerStatus: container.State.Status, width: this.state.width, uid: container.uid }
});
tabs.push({
name: _("Console"),
renderer: ContainerTerminal,
data: { containerId: container.Id, containerStatus: container.State.Status, width: this.state.width, system: container.isSystem, tty }
data: { containerId: container.Id, containerStatus: container.State.Status, width: this.state.width, uid: container.uid, tty }
});
}
}
Expand All @@ -458,8 +458,8 @@ class Containers extends React.Component {
columns,
initiallyExpanded: document.location.hash.substring(1) === container.Id,
props: {
key: container.Id + container.isSystem.toString(),
"data-row-id": container.Id + container.isSystem.toString(),
key: container.key,
"data-row-id": container.key,
"data-started-at": container.State?.StartedAt,
},
};
Expand All @@ -482,7 +482,7 @@ class Containers extends React.Component {
let cpu = 0;
let mem = 0;
for (const container of pod.Containers) {
const containerStats = containersStats[container.Id + pod.isSystem.toString()];
const containerStats = containersStats[utils.makeKey(pod.uid, container.Id)];
if (!containerStats)
continue;

Expand All @@ -502,7 +502,7 @@ class Containers extends React.Component {

renderPodDetails(pod, podStatus) {
const podStats = this.podStats(pod);
const infraContainer = this.props.containers[pod.InfraId + pod.isSystem.toString()];
const infraContainer = this.props.containers[utils.makeKey(pod.uid, pod.InfraId)];
const numPorts = Object.keys(infraContainer?.NetworkSettings?.Ports ?? {}).length;

return (
Expand Down Expand Up @@ -595,9 +595,9 @@ class Containers extends React.Component {

if (this.props.userServiceAvailable && this.props.systemServiceAvailable && this.props.ownerFilter !== "all") {
filtered = filtered.filter(id => {
if (this.props.ownerFilter === "system" && !this.props.containers[id].isSystem)
if (this.props.ownerFilter === "system" && this.props.containers[id].uid !== 0)
return false;
if (this.props.ownerFilter !== "system" && this.props.containers[id].isSystem)
if (this.props.ownerFilter !== "system" && this.props.containers[id].uid === 0)
return false;
return true;
});
Expand All @@ -607,7 +607,7 @@ class Containers extends React.Component {
const lcf = this.props.textFilter.toLowerCase();
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.pods[utils.makeKey(this.props.containers[id].uid, this.props.containers[id].Pod)].Name.toLowerCase().indexOf(lcf) >= 0) ||
this.props.containers[id].ImageName.toLowerCase().indexOf(lcf) >= 0
);
}
Expand All @@ -631,8 +631,8 @@ class Containers extends React.Component {
return 1;
}
// 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;
if (this.props.containers[a].uid !== this.props.containers[b].uid)
return (this.props.containers[a].uid === 0) ? 1 : -1;
return this.props.containers[a].Name > this.props.containers[b].Name ? 1 : -1;
});

Expand All @@ -641,7 +641,7 @@ class Containers extends React.Component {
filtered.forEach(id => {
const container = this.props.containers[id];
if (container)
(partitionedContainers[container.Pod ? (container.Pod + container.isSystem.toString()) : 'no-pod'] || []).push(container);
(partitionedContainers[container.Pod ? utils.makeKey(container.uid, container.Pod) : 'no-pod'] || []).push(container);
});

// Append downloading containers
Expand All @@ -658,8 +658,8 @@ class Containers extends React.Component {
// If nor the pod name nor any container inside the pod fit the filter, hide the whole pod
(!partitionedContainers[section].length && pod.Name.toLowerCase().indexOf(lcf) < 0) ||
((this.props.userServiceAvailable && this.props.systemServiceAvailable && this.props.ownerFilter !== "all") &&
((this.props.ownerFilter === "system" && !pod.isSystem) ||
(this.props.ownerFilter !== "system" && pod.isSystem))))
((this.props.ownerFilter === "system" && pod.uid !== 0) ||
(this.props.ownerFilter !== "system" && pod.uid === 0))))
delete partitionedContainers[section];
}
});
Expand All @@ -675,10 +675,10 @@ class Containers extends React.Component {
continue;

unusedContainers.push({
id: container.Id + container.isSystem.toString(),
id: container.Id,
name: container.Name,
created: container.Created,
system: container.isSystem,
uid: container.uid,
});
}
}
Expand Down Expand Up @@ -806,8 +806,8 @@ class Containers extends React.Component {
else if (b == "no-pod") return 1;

// User pods are in front of system ones
if (this.props.pods[a].isSystem !== this.props.pods[b].isSystem)
return this.props.pods[a].isSystem ? 1 : -1;
if (this.props.pods[a].uid !== this.props.pods[b].uid)
return this.props.pods[a].uid === 0 ? 1 : -1;
return this.props.pods[a].Name > this.props.pods[b].Name ? 1 : -1;
})
.map(section => {
Expand Down
Loading

0 comments on commit bac14d1

Please sign in to comment.