Restart Policies directory with features - #433
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a new Python-based HTTP server example intended to demonstrate Unikraft Cloud restart policies by providing endpoints that can exit with configurable codes, plus deployment/API helper scripts and documentation.
Changes:
- Introduces a minimal Python HTTP server with
/,/health, and/exit?code=N. - Adds a walkthrough README for deploying instances with
never,always, andon-failurerestart policies. - Adds a Kraftfile + Dockerfile and simple bash scripts to create/list/interact/delete instances via the API.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| httpserver-python-restartpolicie/server.py | New HTTP server implementation used to trigger exits and observe restart behavior. |
| httpserver-python-restartpolicie/README.md | Usage docs for deploying/testing restart policies via CLI and API scripts. |
| httpserver-python-restartpolicie/Kraftfile.yml | New build/runtime configuration for the example. |
| httpserver-python-restartpolicie/Dockerfile | Rootfs definition for packaging the example payload. |
| httpserver-python-restartpolicie/api/create.sh | Creates an instance via the Unikraft Cloud API. |
| httpserver-python-restartpolicie/api/list.sh | Lists instances via the Unikraft Cloud API. |
| httpserver-python-restartpolicie/api/interact.sh | Calls the example’s endpoints via HTTPS. |
| httpserver-python-restartpolicie/api/delete.sh | Deletes an instance via the Unikraft Cloud API. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Suppressed comments (6)
httpserver-python-restartpolicie/server.py:22
/exitparses thecodequery parameter withint(...)without validation; a non-integer value (e.g.?code=foo) raisesValueErrorand results in an unhandled exception instead of a controlled response.
elif parsed.path == "/exit":
code = int(params.get("code", ["0"])[0])
self._respond(200, f"Exiting with code {code}...\n")
self.wfile.flush()
sys.exit(code)
httpserver-python-restartpolicie/README.md:35
- The README lists server endpoints but omits
GET /health, even though the server implements it (returnsOK). This can confuse readers trying to verify liveness behavior.
| Endpoint | Description |
|----------|-------------|
| `GET /` | Returns `Hello from restart-demo!` |
| `GET /exit?code=N` | Calls `sys.exit(N)` — simulates crash or clean exit |
httpserver-python-restartpolicie/Kraftfile.yml:5
- This Kraftfile uses
spec: v0.6and the short-formrootfs: ./Dockerfile, but the rest of the repo’s Kraftfiles consistently usespec: v0.7with an explicitrootfs.source+rootfs.format(e.g.httpserver-bun/Kraftfile:1-11). Using the consistent format reduces tooling surprises across examples.
spec: v0.6
name: restart-demo
runtime: python:3.12
rootfs: ./Dockerfile
cmd: ["/usr/bin/python3", "/app/server.py"]
httpserver-python-restartpolicie/api/list.sh:8
- The API endpoint is hardcoded to the
frametro. Since the README instructs users to setUKC_METRO, hardcoding the region makes the script fail for other metros.
curl -s \
-H "Authorization: Bearer $UKC_TOKEN" \
https://api.fra.unikraft.cloud/v1/instances \
| jq .
httpserver-python-restartpolicie/api/create.sh:23
- The instance-create script hardcodes the
frametro in the API URL, which contradicts the README’sUKC_METROsetup and breaks usage in other regions.
curl -s \
-X POST \
-H "Authorization: Bearer $UKC_TOKEN" \
-H "Content-Type: application/json" \
-d "{
\"name\": \"$NAME\",
\"image\": \"$IMAGE\",
\"memory_mb\": $MEMORY,
\"restart_policy\": \"$POLICY\",
\"autostart\": true
}" \
https://api.fra.unikraft.cloud/v1/instances \
| jq .
httpserver-python-restartpolicie/api/delete.sh:12
- The delete script hardcodes the
frametro in the API URL. UsingUKC_METRO(with a sensible default) makes it consistent with the README and usable across regions.
curl -s \
-X DELETE \
-H "Authorization: Bearer $UKC_TOKEN" \
https://api.fra.unikraft.cloud/v1/instances/$UUID \
| jq .
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Signed-off-by: grobeert <grobeert@gmail.com>
grobeert
left a comment
There was a problem hiding this comment.
Change metro from hardcoded FRA to user's metro
There was a problem hiding this comment.
🟡 Changes recommended
There are correctness/convention issues that can impact usability (mixed \r characters in server.py, incomplete endpoint docs, and a non-standard Kraftfile name/spec compared to the rest of the repo).
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (6)
Previously missed (1) — in code that hasn't changed since the last review.
httpserver-python-restartpolicie/server.py:37
address_string()can trigger reverse-DNS lookups per request, which can add latency. Logging the raw client IP avoids that overhead.
httpserver-python-restartpolicie/README.md:35
- The README’s endpoint table is missing
GET /health, but the server implements it (server.py:15-16). This makes the documented interface incomplete.
| Endpoint | Description |
|----------|-------------|
| `GET /` | Returns `Hello from restart-demo!` |
| `GET /exit?code=N` | Calls `sys.exit(N)` — simulates crash or clean exit |
httpserver-python-restartpolicie/Kraftfile.yml:5
- This repository consistently uses
Kraftfile(no extension) withspec: v0.7(e.g., httpserver-python3.12/Kraftfile:1). UsingKraftfile.ymlwithspec: v0.6is a convention deviation and may not be picked up by tooling expectingKraftfile. Consider renaming toKraftfileand updating to the v0.7 schema used elsewhere.
spec: v0.6
name: restart-demo
runtime: python:3.12
rootfs: ./Dockerfile
cmd: ["/usr/bin/python3", "/app/server.py"]
httpserver-python-restartpolicie/api/list.sh:7
- The setup section asks users to set
UKC_METRO, but this script always calls the FRA API endpoint. UsingUKC_METROhere makes the script work across metros without edits.
-H "Authorization: Bearer $UKC_TOKEN" \
https://api.fra.unikraft.cloud/v1/instances \
httpserver-python-restartpolicie/api/create.sh:22
- This script hard-codes the FRA API endpoint, so it won’t work if the user’s metro differs from
fra. Parameterize the base URL usingUKC_METROto match the README setup.
}" \
https://api.fra.unikraft.cloud/v1/instances \
httpserver-python-restartpolicie/api/delete.sh:11
- This script hard-codes the FRA API endpoint, so it won’t work if the user’s metro differs from
fra. Parameterize the base URL usingUKC_METROto match the README setup.
-H "Authorization: Bearer $UKC_TOKEN" \
https://api.fra.unikraft.cloud/v1/instances/$UUID \
- Files reviewed: 8/8 changed files
- Comments generated: 1
- Review effort level: Lite
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Signed-off-by: grobeert <grobeert@gmail.com>
There was a problem hiding this comment.
🟡 Changes recommended
The current changes include a server handler with redundant/unreachable code and mixed line endings, plus a nonstandard Kraftfile naming/schema choice that may break or be ignored by existing tooling.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (6)
Previously missed (2) — in code that hasn't changed since the last review.
httpserver-python-restartpolicie/api/create.sh:24
- Like the other API scripts, this hard-codes the
fraAPI endpoint and does not validate required positional arguments; using theUKC_METROenv var (defaulting tofra) and basic arg checks improves reliability.
httpserver-python-restartpolicie/api/delete.sh:12 - This script hard-codes the
fraAPI endpoint and does not validate required arguments, which can lead to requests being sent to the wrong region or to malformed URLs (empty UUID).
httpserver-python-restartpolicie/README.md:35
- The server implements
GET /health(server.py:15-16) but the endpoint is missing from the documented endpoint table here.
| Endpoint | Description |
|----------|-------------|
| `GET /` | Returns `Hello from restart-demo!` |
| `GET /exit?code=N` | Calls `sys.exit(N)` — simulates crash or clean exit |
httpserver-python-restartpolicie/api/list.sh:8
- The API endpoint is hard-coded to
fra, soUKC_METROfrom the README setup is ignored. Parameterizing the metro (with a sensible default) makes the scripts reusable across regions and avoids needing to edit source.
curl -s \
-H "Authorization: Bearer $UKC_TOKEN" \
https://api.fra.unikraft.cloud/v1/instances \
| jq .
httpserver-python-restartpolicie/api/interact.sh:17
- The script silently does nothing for unknown commands or missing args (e.g., empty FQDN), which makes failures hard to diagnose during demos.
FQDN=$1
CMD=$2
CODE=${3:-1}
if [ "$CMD" = "hello" ]; then
curl -s https://$FQDN/
elif [ "$CMD" = "exit" ]; then
curl -s https://$FQDN/exit?code=$CODE
fi
httpserver-python-restartpolicie/Kraftfile.yml:5
- This repo consistently uses a
Kraftfile(no extension) withspec: v0.7and a structuredrootfs:block (e.g. basic-ops/Kraftfile:1-10). UsingKraftfile.ymlwithspec: v0.6androotfs: ./Dockerfilerisks being ignored or parsed differently by the tooling.
spec: v0.6
name: restart-demo
runtime: python:3.12
rootfs: ./Dockerfile
cmd: ["/usr/bin/python3", "/app/server.py"]
- Files reviewed: 8/8 changed files
- Comments generated: 1
- Review effort level: Lite
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Signed-off-by: grobeert <grobeert@gmail.com>
…lement dynamic metro selection
There was a problem hiding this comment.
🟡 Changes recommended
The API create script currently fails due to an unset MEMORY variable, and the scratch Dockerfile diverges from established repo patterns in a way that risks a broken Python runtime.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 8/10 changed files
- Comments generated: 3
- Review effort level: Lite
| NAME=$1 | ||
| IMAGE=$2 | ||
| POLICY=$3 | ||
| METRO=${UKC_METRO:-fra} | ||
|
|
||
| payload=$(jq -n \ | ||
| --arg name "$NAME" \ | ||
| --arg image "$IMAGE" \ | ||
| --arg policy "$POLICY" \ | ||
| --argjson memory_mb "$MEMORY" \ | ||
| '{name: $name, image: $image, memory_mb: $memory_mb, restart_policy: $policy, autostart: true}' | ||
| ) |
| | Endpoint | Description | | ||
| |----------|-------------| | ||
| | `GET /` | Returns `Hello from restart-demo!` | | ||
| | `GET /exit?code=N` | Calls `sys.exit(N)` — simulates crash or clean exit | |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Signed-off-by: grobeert <grobeert@gmail.com>
There was a problem hiding this comment.
🟡 Changes recommended
The Dockerfile uses CRLF line endings that can break builds, and the API scripts/docs have concrete correctness gaps (e.g., undefined memory argument handling, missing endpoint in README, missing UUID validation).
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (3)
Previously missed (3) — in code that hasn't changed since the last review.
httpserver-python-restartpolicie/api/create.sh:17
MEMORYis referenced when building the JSON payload but is never set from the optional[memory_mb]argument, which will causejq --argjsonto fail (and the request payload to be invalid). Parse the 4th arg (with a sensible default) and validate required args before invokingjq.
httpserver-python-restartpolicie/api/delete.sh:8- The script does not validate that a UUID was provided; when
$1is empty it will call the API with an incomplete URL, which can lead to confusing failures. Add a simple argument check and usage message.
httpserver-python-restartpolicie/README.md:36 - The README’s endpoint table omits
GET /health, but the server implements it. This can confuse users when testing/monitoring the demo.
- Files reviewed: 8/10 changed files
- Comments generated: 1
- Review effort level: Lite
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Signed-off-by: grobeert <grobeert@gmail.com>
There was a problem hiding this comment.
🟡 Changes recommended
There are concrete functional/config issues (e.g., Kraftfile rootfs key mismatch and create.sh undefined memory value) plus Dockerfile inconsistencies/duplication that should be corrected before merging.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (3)
Previously missed (2) — in code that hasn't changed since the last review.
httpserver-python-restartpolicie/api/create.sh:17
MEMORYis referenced formemory_mbbut never set, sojq --argjson memory_mb "$MEMORY"will fail (empty/undefined is not valid JSON) and instance creation will break. Also the optional[memory_mb]argument documented in the header isn’t currently read.
httpserver-python-restartpolicie/Dockerfile:9- This Dockerfile defines only a single stage but then uses
COPY --from=build ...within that same stage; across this repo’s Python examples the established pattern is a multi-stage build that switches toFROM scratchfor the final rootfs (e.g. httpserver-python3.12/Dockerfile:6). Adding the scratch stage also avoids carrying the full base image into the exported rootfs.
httpserver-python-restartpolicie/README.md:35
- The server implements
GET /health(server.py) but the endpoint table doesn’t document it, which makes the README incomplete for users verifying liveness/readiness.
| Endpoint | Description |
|----------|-------------|
| `GET /` | Returns `Hello from restart-demo!` |
| `GET /exit?code=N` | Calls `sys.exit(N)` — simulates crash or clean exit |
- Files reviewed: 8/10 changed files
- Comments generated: 2
- Review effort level: Lite
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Signed-off-by: grobeert <grobeert@gmail.com>
No description provided.