Resource Request Feature#332
Conversation
Issue: #258 Introduce a full resource request feature: adds ResourceRequest model and migration, API routes for creating/listing/counting/effective/approving/denying requests, and server logic to auto-approve requests that are within defaults or created by admins. New behavior applies approved changes to existing running containers by creating reconfigure jobs. Client changes: add types and queries for resource requests/effective resources, a Resources section in the container form (requesting resources), an admin Resource Requests page to review/approve/deny, and a badge/link in the sidebar showing pending request count. Also register the new router under /api/v1/resource-requests. Daemon/CLI updates: create-container now consults approved resources when provisioning (cores/memory/swap/rootfs) and reconfigure-container accepts resource flags and will apply them via the Proxmox client. Misc: adjust .dockerignore to use **/node_modules. Indexes added for efficient lookups (siteId/hostname/username/status) and request defaults/validation are defined on the model.
Introduce a full resource-requests workflow: add a new MyRequestsPage and enhance ResourceRequestsPage with tabs for pending/closed requests, improved tables, and admin actions. Update the sidebar and router to expose /my-requests for non-admin users and rename labels for clarity. Backend: extend API to handle a 'closed' query (approved|denied), validate that a provisioned container exists before approving a request, and send non-blocking notification emails when a request is reviewed. Add sendResourceRequestStatusEmail to utils/email and wire up emailing (with HTML/text templates). Other changes: make reconfigure-container only restart containers when necessary and only update MAC/IP when a restart occurred; debounce hostname input in the container form; tweak some UI styles and layout widths. Includes Sequelize Op/User imports and small refactors to support these features.
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
| ); | ||
| const cores = approvedResources.cpus || 4; | ||
| const memory = approvedResources.memory || 4096; | ||
| const swap = approvedResources.swap !== undefined ? approvedResources.swap : 0; |
There was a problem hiding this comment.
Why isn't this using the same <resource || default> pattern as the previous and following assignments?
There was a problem hiding this comment.
I've modified to keep the consistency
| type: Sequelize.STRING(255), | ||
| allowNull: false, | ||
| }, | ||
| requestedBy: { |
There was a problem hiding this comment.
Is username and requestedBy not the same thing?
There was a problem hiding this comment.
They have the same value, but kept it seperated in the migration for ease of identification. I am creating a new migration to drop this in favor username.
|
|
||
| // Verify at least one provisioned container exists for this request before approving. | ||
| // A container is considered provisioned when it has a Proxmox VMID (containerId is not null). | ||
| const provisionedContainer = await Container.findOne({ |
There was a problem hiding this comment.
I think this check should be moved after setting the request to approved in the database but before applying it to any running container. A user should be able to pre-load resource requests if they know they're going to make a container that will need them.
There was a problem hiding this comment.
The reason why this check was included was of a bug I found where if a user requests extra resources during the build form, but does NOT proceed to submit to the container creator and backs out, a request will be received and if approved, the page will break.
| ); | ||
|
|
||
| /** | ||
| * Send an approval/denial email to the user who submitted the request. |
There was a problem hiding this comment.
This scares me a little bit. I don't want to spam people with emails since we don't yet have a way to opt-out. Our domain reputation is still building I don't want to risk spam reports for non-critical notifications.
There was a problem hiding this comment.
I'll remove the email feature
…tion or class' Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com>
…tion or class' Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com>
Remove the requestedBy column and related notification logic across the app. Adds a migration to drop requestedBy (with a down migration to re-add it), removes the field from the Sequelize model and client types, and updates the frontend to display req.username instead of req.requestedBy. API routes were updated to use username from session/creation and to remove User lookup and notifyRequestReviewed/sendResourceRequestStatusEmail usage; the email helper implementation/export was also removed. Also includes a minor CLI tweak to default swap with a simpler expression in create-container/bin/create-container.js.
2 participants
Resolves: #258
This pull request introduces a new resource request system for container resources, allowing users to request changes to resource allocations (such as RAM, CPUs, etc.) for their containers, with an approval workflow for changes above default values. It adds new UI components for managing and requesting resources, updates navigation and routing to support new resource request pages, and expands the API query and type definitions to support these features.
Resource Request System Implementation:
ResourcesSectionReact component (ResourcesSection.tsx) that displays and allows editing of container resources, with a workflow for submitting resource requests and handling admin approval. This component is integrated into the container form page. [1] [2]ResourceRequest,EffectiveResources, andResourceTypeintypes.tsfor strong typing of resource requests and allocations.API and Query Enhancements:
queries.tsto support listing resource requests, counting pending requests, and fetching effective resources for a container. [1] [2]Navigation and Routing Updates:
ResourceRequestsPageandMyRequestsPage). [1] [2]Miscellaneous Improvements:
.dockerignoreto exclude allnode_modulesdirectories recursively.These changes lay the groundwork for a more robust and user-friendly resource management workflow in the application.