-
Notifications
You must be signed in to change notification settings - Fork 0
Operational logs over API: hypeman.log, vmm.log #34
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.
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
Conversation
✱ Stainless preview buildsThis PR will update the ✅ hypeman-go studio · code
⚡ hypeman-cli studio · conflict
This comment is auto-generated by GitHub Actions and is automatically kept up to date as you push. |
2169a4d to
713620e
Compare
1. Exec endpoint now has logger injection: 2. Simplified InstanceLogHandler - no more file caching: - Removed `sharedState` struct with mutex and fileCache - Each write now opens, writes, closes the file - No cleanup methods needed = no leak possible - Simpler code, no shared state complexity The performance impact is negligible since instance operations (start, stop, standby, etc.) are infrequent. The tradeoff of slightly more I/O vs. guaranteed no leaks and simpler code is worth it.
| // Get instance IDs that might have a running VMM for TAP cleanup safety. | ||
| // Include Unknown state: we couldn't confirm their state, but they might still | ||
| // have a running VMM. Better to leave a stale TAP than crash a running VM. | ||
| allInstances, _ := app.InstanceManager.ListInstances(app.Ctx) | ||
| var preserveTAPs []string | ||
| for _, inst := range allInstances { | ||
| if inst.State == instances.StateRunning || inst.State == instances.StateUnknown { | ||
| preserveTAPs = append(preserveTAPs, inst.Id) | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change is the fix for stopping a possibly running instance, the rest of the change is VMM and Hypeman logs feature, which was inspired by how this problem was debugged (find issue in CH logs)
1. Fixed `ErrAmbiguousName` to return 409 Conflict (instead of 404) Changed the HTTP status code from `http.StatusNotFound` to `http.StatusConflict` for ambiguous name errors, restoring the previous ingress behavior. 2. Added nil checks with 500 error responses for all `GetResolved*` calls Added defensive nil checks to all 14 handlers that use the resolved resource from middleware. If the middleware didn't set the resource (which shouldn't happen in production but could in tests), the handler now returns a 500 error with `"resource not resolved"` message instead of panicking with a nil pointer dereference.
$ hypeman ingress create q --hostname 'nginx-test' --port 80 --host-port 8081 Creating ingress nginx-test-wmbi... e5hbxzwc6cq0tnjchw861exg $ hypeman ingress list ID NAME HOSTNAME TARGET TLS CREATED e5hbxzwc6cq0 nginx-test-wmbi nginx-test nginx-gi7w:80 no 4 seconds ago Fix is shows "nginx-gi7w:80" instead of "g:80"
| required: false | ||
| schema: | ||
| type: string | ||
| enum: [app, vmm, hypeman] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
at first i thought maybe this endpoint should let you select more than one of these sources but i think for simplicity's sake it's fine to push this problem to clients to perform separate requests to get more than one source
| logPath = m.paths.InstanceHypemanLog(id) | ||
| default: | ||
| // Default to app log for backwards compatibility | ||
| logPath = m.paths.InstanceAppLog(id) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
enjoy the freedom of not having to care about backwards compatability right now! :)
Note
Introduce per-instance hypeman and VMM logs with API streaming (app/vmm/hypeman), resource-resolving middleware, and updated logging/ingress/network behaviors.
logs/hypeman.logand consolidate VMM output tologs/vmm.log; rename serial log tologs/app.log.InstanceLogHandlerto auto-route logs withinstance_idtohypeman.log; wire into global logger.RotateLogs.ResolveResourcemiddleware; handlers now consume resolvedinstance/volume/ingress/imagefrom context and simplify error paths./instances/{id}/logsto supportsourcequery (app|vmm|hypeman) and return specific errors (tail missing, log not found).instance_idlogging and new log paths.ingress_idand targetinstance_idfor per-instance audit logs.logs/vmm.log; tests updated accordingly.sourceparam for log streaming and updated docs.Written by Cursor Bugbot for commit 228f6b1. This will update automatically on new commits. Configure here.