Skip to content

Conversation

@thunderboltsid
Copy link
Contributor

@thunderboltsid thunderboltsid commented Jun 24, 2025

  • Handle nil responses of get storage container and get cluster API calls.
  • Improve error wording.
  • Correctly construct error responses of list API calls.
  • Test error response of list clusters API call.
  • Treat a missing storageContainer as a failed check, but not an internal error.

@github-actions github-actions bot added the fix label Jun 24, 2025
@thunderboltsid thunderboltsid requested a review from dkoshkin June 24, 2025 20:36
@github-actions github-actions bot removed the fix label Jun 24, 2025
@github-actions github-actions bot added the fix label Jun 24, 2025
@dlipovetsky
Copy link
Contributor

Should this handling of the List call be similar to what we have in getVMImages?

resp, err := client.ListImages(nil, nil, &filter_, nil, nil)
if err != nil {
return nil, err
}
if resp == nil || resp.GetData() == nil {
// No images were returned.
return []vmmv4.Image{}, nil
}
images, ok := resp.GetData().([]vmmv4.Image)
if !ok {
return nil, fmt.Errorf("failed to get data returned by ListImages")
}
return images, nil

@thunderboltsid
Copy link
Contributor Author

Should this handling of the List call be similar to what we have in getVMImages?

resp, err := client.ListImages(nil, nil, &filter_, nil, nil)
if err != nil {
return nil, err
}
if resp == nil || resp.GetData() == nil {
// No images were returned.
return []vmmv4.Image{}, nil
}
images, ok := resp.GetData().([]vmmv4.Image)
if !ok {
return nil, fmt.Errorf("failed to get data returned by ListImages")
}
return images, nil

you mean using if conditionals instead of switch conditionals?

@dlipovetsky
Copy link
Contributor

Should this handling of the List call be similar to what we have in getVMImages?

resp, err := client.ListImages(nil, nil, &filter_, nil, nil)
if err != nil {
return nil, err
}
if resp == nil || resp.GetData() == nil {
// No images were returned.
return []vmmv4.Image{}, nil
}
images, ok := resp.GetData().([]vmmv4.Image)
if !ok {
return nil, fmt.Errorf("failed to get data returned by ListImages")
}
return images, nil

you mean using if conditionals instead of switch conditionals?

Yes, but either is fine.

… checks

- Correctly construct error responses of list API calls.
- Test error response of list clusters API call.
- Treat a missing storageContainer as a failed check, but not an internal error.
@dlipovetsky dlipovetsky force-pushed the issue/storagecontainer-errors branch from 7b06600 to c24088b Compare June 25, 2025 19:45
@dlipovetsky dlipovetsky changed the title fix(preflight): improved error reporting for storage container checks fix(preflight): improved error reporting for storage container and VM image checks Jun 25, 2025
@github-actions github-actions bot added fix and removed fix labels Jun 25, 2025
… checks

Handle error response for list images API call
@dlipovetsky dlipovetsky force-pushed the issue/storagecontainer-errors branch from c24088b to da1e63b Compare June 25, 2025 19:55
@dlipovetsky dlipovetsky merged commit d752315 into main Jun 30, 2025
22 checks passed
@dlipovetsky dlipovetsky deleted the issue/storagecontainer-errors branch June 30, 2025 23:28
This was referenced Jul 3, 2025
dlipovetsky pushed a commit that referenced this pull request Jul 4, 2025
🤖 I have created a release *beep* *boop*
---


## 0.31.0 (2025-07-03)

<!-- Release notes generated using configuration in .github/release.yaml
at main -->

## What's Changed
### Exciting New Features 🎉
* feat: Allow configuration of kube-proxy mode on cluster creation by
@jimmidyson in
#1163
* feat: auto enable registry addon in workload clusters by @dkoshkin in
#1175
* feat(ntp): Configure NTP for clusters by @thunderboltsid in
#1185
* feat: adds a generic checker package with registry and mirror checks
by @faiq in
#1186
* feat: deploy registry syncer for workload clusters by @dkoshkin in
#1189
* feat(preflight): Add VM Image kubernetes version check by
@thunderboltsid in
#1172
* feat: CAREN support for NutanixFailureDomain by @yanhua121 in
#1192
### Fixes 🔧
* fix: Do not run preflight checks if Cluster is paused by @dlipovetsky
in
#1181
* fix: misc fixes to the preflight framework by @dlipovetsky in
#1188
* fix(preflight): improved error reporting for storage container and VM
image checks by @thunderboltsid in
#1180
* fix(file): rename test/request/capa.go and test/request/capx.go by
@thunderboltsid in
#1193
* fix: Move preflight skip annotation constants to api module by
@dlipovetsky in
#1187
* fix: Include correct field name in registry preflight check results by
@dlipovetsky in
#1194
* fix: add tolerations and nodeAffinity overrides for registry addon by
@supershal in
#1183
* fix: include GenericNodeSpec in aggregate type by @dkoshkin in
#1182
* fix: Do not treat expected preflight check failures as internal errors
by @dlipovetsky in
#1195

## New Contributors
* @yanhua121 made their first contribution in
#1192

**Full Changelog**:
v0.30.0...v0.31.0

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants