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

Fix race condition in testLifecycleOperations, Serialize updateContainer() calls, Ignore irrelevant container events #1353

Merged
merged 3 commits into from
Jul 14, 2023
Merged

Fix race condition in testLifecycleOperations, Serialize updateContainer() calls, Ignore irrelevant container events #1353

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
17846e6
test: Fix race condition in testLifecycleOperations
martinpitt Jul 13, 2023
eed34d9
Serialize updateContainer() calls
martinpitt Jul 13, 2023
7fcf4d1
Ignore irrelevant container events
martinpitt Jul 5, 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
32 changes: 22 additions & 10 deletions src/app.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,8 @@ class Application extends React.Component {
this.goToServicePage = this.goToServicePage.bind(this);
this.checkUserService = this.checkUserService.bind(this);
this.onNavigate = this.onNavigate.bind(this);

this.pendingUpdateContainer = {}; // id+system → promise
}

onAddNotification(notification) {
Expand Down Expand Up @@ -253,16 +255,25 @@ class Application extends React.Component {
}

updateContainer(id, system, event) {
return client.inspectContainer(system, id)
/* when firing off multiple calls in parallel, podman can return them in a random order.
* This messes up the state. So we need to serialize them for a particular container. */
const idx = id + system.toString();
const wait = this.pendingUpdateContainer[idx] ?? Promise.resolve();

const new_wait = wait.then(() => 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);
this.updateState("containers", idx, details);
})
.catch(console.log);
this.pendingUpdateContainer[idx] = new_wait;
new_wait.finally(() => { delete this.pendingUpdateContainer[idx] });

return new_wait;
}

updateImage(id, system) {
Expand Down Expand Up @@ -323,6 +334,13 @@ class Application extends React.Component {
case 'export':
case 'import':
case 'init':
case 'health_status': // HACK: broken, https://github.com/containers/podman/issues/19237; see exec_died
case 'kill':
case 'mount':
case 'prune':
martinpitt marked this conversation as resolved.
Show resolved Hide resolved
case 'restart':
case 'sync':
case 'unmount':
case 'wait':
break;
/* The following events need only to update the Container list
Expand All @@ -338,19 +356,13 @@ class Application extends React.Component {
).then(() => this.updateContainer(id, system, event));
break;
case 'checkpoint':
case 'cleanup':
case 'create':
case 'died':
case 'exec_died':
case 'kill':
case 'cleanup':
case 'mount':
case 'exec_died': // HACK: pick up health check runs, see https://github.com/containers/podman/issues/19237
case 'pause':
case 'prune':
case 'restart':
case 'restore':
case 'stop':
case 'sync':
case 'unmount':
case 'unpause':
case 'rename': // rename event is available starting podman v4.1; until then the container does not get refreshed after renaming
this.updateContainer(id, system, event);
Expand Down
5 changes: 3 additions & 2 deletions test/check-application
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1180,7 +1180,9 @@ class TestApplication(testlib.MachineCase):
b.wait(lambda: self.execute(auth, "podman ps --all | grep -e swamped-crate -e Exited"))

b.wait_visible("#containers-containers")
self.waitContainerRow("swamped-crate")
container_sha = self.execute(auth, "podman inspect --format '{{.Id}}' swamped-crate").strip()
self.waitContainer(container_sha, auth, name='swamped-crate', image=IMG_BUSYBOX,
state='Exited', owner="system" if auth else "admin")
b.click("#containers-containers tbody tr:contains('swamped-crate') .pf-v5-c-dropdown__toggle")

if not auth:
Expand All @@ -1196,7 +1198,6 @@ class TestApplication(testlib.MachineCase):
# Start the container
self.performContainerAction(IMG_BUSYBOX, "Start")

container_sha = self.execute(auth, "podman inspect --format '{{.Id}}' swamped-crate").strip()
self.waitContainer(container_sha, auth, name='swamped-crate', image=IMG_BUSYBOX,
state='Running', owner="system" if auth else "admin")

Expand Down