Skip to content

Commit 94a2b10

Browse files
docs(upstream): complete affected bug handoffs
Record the tested apple/container#1757 launchd identity fix and the narrowed apple/containerization#792 implementation for apple/containerization#790 and apple/container#1895. Include exact source and shared-test-support commits so either change can be reviewed for Apple without pushing it there.
1 parent b45e235 commit 94a2b10

5 files changed

Lines changed: 132 additions & 8 deletions

File tree

docs/upstream/APPLE-UPSTREAM-REVIEW.md

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -37,17 +37,15 @@ six approved open pull requests.
3737
| [apple/container#1888](https://github.com/apple/container/issues/1888) | `container system start` status text used stdout. The focused change from [apple/container#1889](https://github.com/apple/container/pull/1889) is ported as `0fe7833`. |
3838
| [apple/container#1672](https://github.com/apple/container/issues/1672) | `container system stop --prefix` accepted path-like values. [apple/container#1717](https://github.com/apple/container/pull/1717) is ported as `7329f12`. |
3939
| [apple/container#1767](https://github.com/apple/container/issues/1767) | Image snapshots omitted ordered EXT4 journaling. Approved [apple/container#1818](https://github.com/apple/container/pull/1818) is ported as `6e525cc`. |
40+
| [apple/container#1757](https://github.com/apple/container/issues/1757) | `system start` discarded launchd failures and could adopt a daemon using another app root. Fixed in `stephenlclarke/container` `6ac1253`. |
41+
| [apple/containerization#790](https://github.com/apple/containerization/issues/790) and [apple/container#1895](https://github.com/apple/container/issues/1895) | Retrying an interrupted ECR blob PUT reused a stale upload session. The useful fresh-session design from [apple/containerization#792](https://github.com/apple/containerization/pull/792) is implemented with narrower retry semantics and local registry tests in `d388a15` and `c8043bb`. |
4042

4143
## Open Follow-up
4244

43-
- [apple/container#1895](https://github.com/apple/container/issues/1895) and
44-
[apple/containerization#790](https://github.com/apple/containerization/issues/790)
45-
affect registry push when ECR briefly returns `404` after upload completion.
46-
Open `apple/containerization#792` retries a broader set of failures and has no
47-
regression coverage, so it should be narrowed and tested before use.
48-
- [apple/container#1757](https://github.com/apple/container/issues/1757) affects
49-
service registration because a nonzero `launchctl bootstrap` result is not
50-
propagated. The local fork still needs a focused error-handling fix.
45+
No confirmed local impact from this review remains without either a fix or an
46+
existing stronger local implementation. Re-run the review before release or
47+
Apple submission because newly opened and newly approved work can change that
48+
result.
5149

5250
## Submission Boundary
5351

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
# System Start Can Adopt The Wrong App Root
2+
3+
## Upstream Reference
4+
5+
- Existing report: [apple/container#1757](https://github.com/apple/container/issues/1757)
6+
7+
Do not open a duplicate issue.
8+
9+
## Problem
10+
11+
`ServiceManager.register` discards the `launchctl bootstrap` termination
12+
status. A repeated `container system start --app-root B` can therefore appear
13+
successful while launchd continues running the singleton API service created
14+
for app root A. The health response already exposes the live daemon's app root,
15+
but `SystemStart` discards it.
16+
17+
## Expected Behavior
18+
19+
- A nonzero launchd bootstrap result is an error.
20+
- An already registered API service is not bootstrapped again.
21+
- The live health response must identify the requested canonical app root.
22+
- A mismatch explains that the existing system must be stopped before changing
23+
`--app-root`.
24+
25+
## Ownership
26+
27+
This is native `container` service lifecycle and identity handling. Compose
28+
must be able to trust a successful system start without probing private paths.
Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
# Pull Request: Verify Launchd Startup Identity
2+
3+
## Summary
4+
5+
- Surface nonzero `launchctl bootstrap` status.
6+
- Skip bootstrap when the API service label is already registered.
7+
- Compare the health response's canonical app root with the requested path.
8+
- Cover status validation and matching/mismatched app roots without invoking
9+
launchd in unit tests.
10+
11+
## Upstream Reference
12+
13+
- Fixes [apple/container#1757](https://github.com/apple/container/issues/1757).
14+
- No overlapping open pull request was found.
15+
16+
## Commit Tracking
17+
18+
- Fork commit: `6ac1253` in `stephenlclarke/container`.
19+
- The commit is intentionally separate from imported upstream changes.
20+
21+
## Validation
22+
23+
```sh
24+
swift test --disable-automatic-resolution \
25+
--filter 'ServiceManagerTests|SystemStartTests'
26+
make check
27+
make test
28+
```
Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
# Interrupted ECR Blob Uploads Reuse A Stale Session
2+
3+
## Upstream Reference
4+
5+
- Runtime issue: [apple/containerization#790](https://github.com/apple/containerization/issues/790)
6+
- CLI report: [apple/container#1895](https://github.com/apple/container/issues/1895)
7+
- Overlapping open pull request:
8+
[apple/containerization#792](https://github.com/apple/containerization/pull/792)
9+
10+
Do not open a competing issue or pull request. Use these notes to review or
11+
improve the existing proposal.
12+
13+
## Problem
14+
15+
The generic request retry recreates a blob body stream at byte zero while
16+
reusing the same registry upload URL. If ECR committed part of the first PUT,
17+
it rejects the restarted body with HTTP 416 and `BLOB_UPLOAD_INVALID` because
18+
the session expects the next byte offset.
19+
20+
The overlapping PR has the correct high-level idea but retries manifests and
21+
non-retryable semantic failures, and it invents three retries for clients whose
22+
retry policy is nil.
23+
24+
## Expected Behavior
25+
26+
- Every retried blob attempt starts with a new `POST /blobs/uploads/` session.
27+
- The PUT itself never retries against the same upload UUID.
28+
- Manifests retain the existing request behavior.
29+
- Nil or zero retry policy performs no fresh-session retry.
30+
- Only transport failures, server errors, and ECR's specific
31+
`416/BLOB_UPLOAD_INVALID` response restart the blob upload.
Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
# Pull Request: Restart Interrupted Blob Uploads In Fresh Sessions
2+
3+
## Summary
4+
5+
- Incorporate the fresh-session design from
6+
[apple/containerization#792](https://github.com/apple/containerization/pull/792).
7+
- Add an internal request policy that can truly disable generic retries.
8+
- Apply the outer retry loop only to blobs.
9+
- Retry only transport errors, 5xx responses, and ECR
10+
`416/BLOB_UPLOAD_INVALID` responses.
11+
- Share the local HTTP stub between registry and Cloud Hypervisor tests.
12+
- Prove that retry uses a second upload UUID and nil policy performs one POST
13+
and one PUT.
14+
15+
## Upstream Reference
16+
17+
- Fixes [apple/containerization#790](https://github.com/apple/containerization/issues/790).
18+
- Resolves the lower-layer cause of
19+
[apple/container#1895](https://github.com/apple/container/issues/1895).
20+
- Update the existing open
21+
[apple/containerization#792](https://github.com/apple/containerization/pull/792)
22+
rather than opening a competing pull request.
23+
24+
## Commit Tracking
25+
26+
- Shared test support: `d388a15` in `stephenlclarke/containerization`.
27+
- Source fix and registry regressions: `c8043bb` in
28+
`stephenlclarke/containerization`.
29+
- Keep the source fix as its own upstream-overlap commit.
30+
31+
## Validation
32+
33+
```sh
34+
swift test --disable-automatic-resolution --filter blobPush
35+
swift test --disable-automatic-resolution \
36+
--filter CloudHypervisorTests.ClientTests
37+
make check
38+
make test
39+
```

0 commit comments

Comments
 (0)