Skip to content

Prevent orphaned containers and duplicate hosts#318

Open
cmyers-mieweb wants to merge 2 commits into
mainfrom
cmyers_issue317
Open

Prevent orphaned containers and duplicate hosts#318
cmyers-mieweb wants to merge 2 commits into
mainfrom
cmyers_issue317

Conversation

@cmyers-mieweb

@cmyers-mieweb cmyers-mieweb commented May 18, 2026

Copy link
Copy Markdown
Collaborator

Issue: #317

  • Add safeguards to container creation and deletion to avoid orphaned Proxmox containers and duplicate hostnames.
  • Import Job model and check for cancelled creation jobs during provisioning; if cancelled, abort creation.
  • After provisioning but before applying config, verify the DB record still exists and if it was deleted, destroy the orphaned Proxmox container and abort.
  • In the API v1 containers router, enforce a unique hostname-per-site check (with transaction lock) and return 409 for duplicate hostnames.
  • Cancel any pending/running creation job when a container is deleted.
  • Resolve merge conflicts with main and preserve these safeguards in the updated router layout.

Changes affect

  1. create-a-container/bin/create-container.js
  2. create-a-container/routers/api/v1/containers.js

Add safeguards to container creation and deletion to avoid orphaned Proxmox containers and duplicate hostnames. Import Job model and check for cancelled creation jobs during provisioning; if cancelled, abort creation. After provisioning but before applying config, verify the DB record still exists and if it was deleted, destroy the orphaned Proxmox container and abort. In the containers router, enforce a unique hostname-per-site check (with transaction lock) and return 409 for API requests or flash an error for web UI to prevent duplicates. Also cancel any pending/running creation job when a container is deleted. Changes affect create-a-container/bin/create-container.js and create-a-container/routers/containers.js.
@cmyers-mieweb

Copy link
Copy Markdown
Collaborator Author

@copilot resolve the merge conflicts in this pull request

Copilot AI commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

@copilot resolve the merge conflicts in this pull request

Resolved in commit 7fc8f93. I merged main, resolved the conflicts, and preserved the container-creation/deletion safeguards in the new API router layout.

Comment on lines +416 to +427
const existingContainer = await Container.findOne({
where: { siteId: site.id, hostname },
transaction: t,
lock: t.LOCK.UPDATE,
});
if (existingContainer) {
throw new ApiError(
409,
'duplicate_hostname',
`Container with hostname "${hostname}" already exists in this site`,
);
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Container should have a unique condition in the database on siteId + hostname already. The create statement on line 466 below should throw in this case. This preflight check adds an extra DB roundtrip for something that should be caught there.

Comment on lines +237 to +240
if (job && job.status === 'cancelled') {
console.error('Job was cancelled — aborting container creation.');
process.exit(1);
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doing this would probably completely rewrite this PR but could we instead have job-runner watch for cancelled jobs and send SIGTERM to them? Then this job would have a SIGTERM handler to rollback it's changes before exitting.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants