Skip to content

Commit 3f5181a

Browse files
guettliclaude
guettli
and
claude
committed
PR feedback: clarify output.json docs and surface missing-status error
- Restructure output.json section: lead with what CAPH reads (status, message), explain why JSON over plain logs, show minimal examples, demote extended example to its own subsection - ParseAndApply now returns error when JSON is invalid or status field is missing; callers log it at StateFinishedSuccessfully so operators can see when output.json is present but has no status field Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 9f475aa commit 3f5181a

4 files changed

Lines changed: 64 additions & 33 deletions

File tree

‎docs/caph/04-developers/06-image-url-command.md‎

Lines changed: 46 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -80,9 +80,48 @@ provisioning.
8080

8181
## output.json (optional)
8282

83-
The command may write `/root/output.json` at any point during execution. CAPH reads it
84-
continuously to monitor provisioning progress. If the file does not exist, provisioning
85-
still succeeds based on `IMAGE_URL_DONE` alone.
83+
The command may write `/root/output.json` at any point during execution. If the file does not
84+
exist, provisioning still succeeds based on `IMAGE_URL_DONE` alone.
85+
86+
CAPH reads the `status` field from this file to set the `NodeProvisioningSucceeded` condition on the
87+
machine (HCloudMachine or HetznerBareMetalHost). The `message` field is forwarded verbatim into the
88+
condition message.
89+
90+
### Fields CAPH reads
91+
92+
CAPH only reads two top-level fields:
93+
94+
| Field | Required | Values | Purpose |
95+
|-----------|------------------------|------------------------------------------------|-----------------------------------------------|
96+
| `status` | yes (to set condition) | `"Succeeded"`, `"Failed"`, or any other string | Sets `NodeProvisioningSucceeded` condition |
97+
| `message` | no | free-form string | Included in the condition message |
98+
99+
Minimal success example:
100+
101+
```json
102+
{"status": "Succeeded"}
103+
```
104+
105+
Minimal failure example:
106+
107+
```json
108+
{"status": "Failed", "message": "failed to pull image: disk full"}
109+
```
110+
111+
Any other fields in the JSON are **ignored by CAPH** but are forwarded as-is via the Kubernetes
112+
event (see below). You can use them for your own structured debugging output.
113+
114+
### Kubernetes event on completion
115+
116+
When the command finishes (success or failure), CAPH emits a Kubernetes event with reason
117+
`ImageURLCommandOutputJSON` containing the **full JSON content** of the file. If the command
118+
failed, the event type is `Warning`; otherwise it is `Normal`. The content is also written to
119+
the controller log at key `outputJSON`.
120+
121+
### Extended example
122+
123+
Your command can include arbitrary extra fields for its own structured debug output. CAPH
124+
passes the whole JSON through untouched:
86125

87126
```json
88127
{
@@ -92,40 +131,22 @@ still succeeds based on `IMAGE_URL_DONE` alone.
92131
"status": "Succeeded",
93132
"duration": "45.2s",
94133
"steps": [
95-
{"name": "VerifyTools", "status": "Succeeded", "duration": "0.3s", "percentOfTimeout": 1, "message": ""},
96-
{"name": "CheckDeviceExists", "status": "Succeeded", "duration": "0.1s", "percentOfTimeout": 2, "message": ""}
134+
{"name": "VerifyTools", "status": "Succeeded", "duration": "0.3s"},
135+
{"name": "CheckDeviceExists", "status": "Succeeded", "duration": "0.1s"}
97136
]
98137
},
99138
"ImageDeployment": {
100139
"status": "Succeeded",
101140
"duration": "62.1s",
102141
"steps": [
103-
{"name": "PullImage", "status": "Succeeded", "duration": "58.4s", "percentOfTimeout": 13, "message": ""},
104-
{"name": "WriteImage", "status": "Succeeded", "duration": "3.5s", "percentOfTimeout": 2, "message": ""}
105-
]
106-
},
107-
"BootstrapDelivery": {
108-
"status": "Succeeded",
109-
"duration": "1.2s",
110-
"steps": [
111-
{"name": "ConfigureCloudInit", "status": "Succeeded", "duration": "0.8s", "percentOfTimeout": 4, "message": ""}
112-
]
113-
},
114-
"Handover": {
115-
"status": "Succeeded",
116-
"duration": "0.5s",
117-
"steps": [
118-
{"name": "UnmountDisk", "status": "Succeeded", "duration": "0.5s", "percentOfTimeout": 2, "message": ""}
142+
{"name": "PullImage", "status": "Succeeded", "duration": "58.4s"},
143+
{"name": "WriteImage", "status": "Succeeded", "duration": "3.5s"}
119144
]
120145
}
121146
}
122147
}
123148
```
124149

125-
When the command finishes (success or failure), CAPH emits a Kubernetes event with reason `ImageURLCommandOutputJSON` containing the full JSON content. If the command failed, the event type is `Warning`; otherwise it is `Normal`. The content is also written to the controller log at key `outputJSON`.
126-
127-
When present, CAPH sets the `NodeProvisioningSucceeded` condition on the machine (HCloudMachine or HetznerBareMetalHost) based on the top-level `status` field.
128-
129150
## Measured durations for hcloud
130151

131152
| oldState | newState | avg(s) | min(s) | max(s) |

‎pkg/services/baremetal/host/host.go‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1326,7 +1326,7 @@ func (s *Service) actionImageInstallingImageURLCommand(ctx context.Context, sshC
13261326
switch state {
13271327
case sshclient.ImageURLCommandStateRunning:
13281328
if outputJSON, readErr := sshClient.ReadOutputJSON(ctx); readErr == nil {
1329-
imageurlcommand.ParseAndApply(host, outputJSON)
1329+
_ = imageurlcommand.ParseAndApply(host, outputJSON)
13301330
}
13311331
return actionContinue{delay: 10 * time.Second}
13321332

@@ -1335,7 +1335,9 @@ func (s *Service) actionImageInstallingImageURLCommand(ctx context.Context, sshC
13351335
s.scope.Info("ImageURLCommandOutput", "logFile", logFile)
13361336

13371337
if outputJSON, readErr := sshClient.ReadOutputJSON(ctx); readErr == nil {
1338-
imageurlcommand.ParseAndApply(host, outputJSON)
1338+
if err := imageurlcommand.ParseAndApply(host, outputJSON); err != nil {
1339+
s.scope.Info("output.json: could not set NodeProvisioningSucceeded condition", "err", err)
1340+
}
13391341
record.Event(s.scope.HetznerBareMetalHost, "ImageURLCommandOutputJSON", outputJSON)
13401342
s.scope.Info("ImageURLCommandOutputJSON", "outputJSON", outputJSON)
13411343
}

‎pkg/services/hcloud/server/server.go‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1019,13 +1019,15 @@ func (s *Service) handleBootStateRunningImageCommand(ctx context.Context, server
10191019
Message: "imageURLCommand running",
10201020
})
10211021
if outputJSON, readErr := hcloudSSHClient.ReadOutputJSON(ctx); readErr == nil {
1022-
imageurlcommand.ParseAndApply(hm, outputJSON)
1022+
_ = imageurlcommand.ParseAndApply(hm, outputJSON)
10231023
}
10241024
return reconcile.Result{RequeueAfter: 5 * time.Second}, nil
10251025

10261026
case sshclient.ImageURLCommandStateFinishedSuccessfully:
10271027
if outputJSON, readErr := hcloudSSHClient.ReadOutputJSON(ctx); readErr == nil {
1028-
imageurlcommand.ParseAndApply(hm, outputJSON)
1028+
if err := imageurlcommand.ParseAndApply(hm, outputJSON); err != nil {
1029+
s.scope.Info("output.json: could not set NodeProvisioningSucceeded condition", "err", err)
1030+
}
10291031
record.Event(hm, "ImageURLCommandOutputJSON", outputJSON)
10301032
s.scope.Info("ImageURLCommandOutputJSON", "outputJSON", outputJSON)
10311033
}

‎pkg/services/imageurlcommand/conditions.go‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ package imageurlcommand
1919

2020
import (
2121
"encoding/json"
22+
"fmt"
2223

2324
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2425
clusterv1beta1 "sigs.k8s.io/cluster-api/api/core/v1beta1"
@@ -68,10 +69,15 @@ func ApplyNodeProvisioningConditions(obj conditionSetter, output Output) {
6869
}
6970

7071
// ParseAndApply unmarshals content into Output and updates conditions on obj.
71-
// It is a no-op if content is empty, not valid JSON, or has no status field.
72-
func ParseAndApply(obj conditionSetter, content string) {
72+
// Returns an error if content is not valid JSON or has no status field.
73+
func ParseAndApply(obj conditionSetter, content string) error {
7374
var output Output
74-
if err := json.Unmarshal([]byte(content), &output); err == nil && output.Status != "" {
75-
ApplyNodeProvisioningConditions(obj, output)
75+
if err := json.Unmarshal([]byte(content), &output); err != nil {
76+
return fmt.Errorf("output.json: %w", err)
7677
}
78+
if output.Status == "" {
79+
return fmt.Errorf("output.json: no status field")
80+
}
81+
ApplyNodeProvisioningConditions(obj, output)
82+
return nil
7783
}

0 commit comments

Comments
 (0)