Skip to content

Address uncovered simplyblock API errors#580

Merged
boddumanohar merged 10 commits intomainfrom
sbapi-errors
Feb 7, 2026
Merged

Address uncovered simplyblock API errors#580
boddumanohar merged 10 commits intomainfrom
sbapi-errors

Conversation

@mxsrc
Copy link
Collaborator

@mxsrc mxsrc commented Feb 6, 2026

During the recent outage, failures accessing the simplyblock API flooded the controller log. The exception handling was inconsistent and relied on call-sites covering and translating errors. This changeset modifies the simplyblock API to always raise one specific error that can be caught.

Implementing this I got a bit carried away and addressed a few other concerns that popped up. Most notably

  • improved typing (using UUID where possible instead of string)
  • simplified pool ID lookup

Reviewing this is probably most sensible to do commit-wise, they should be self-contained and descriptive.

mxsrc added 9 commits February 6, 2026 11:17
We only every use the API for interaction below the pool level. The type
name is adapted to reflect that.
This will never change so we can do this once to assemble the base-URL.
The recent changes in the API propagate all errors as
VelaSimplyblockAPIError, this simplifies the call-sites.
@mxsrc mxsrc marked this pull request as ready for review February 6, 2026 12:55
@mxsrc mxsrc requested a review from boddumanohar February 6, 2026 12:55
cluster_secret=cluster_secret,
client=client,
)
api = SimplyblockPoolApi(*(await load_simplyblock_credentials()), "testing1")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

might be better to put the simplyblock pool name in a constant variable in rather than hardcode.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I kept it here because its the only occurence. Ideally we wouldnt even use this but request it from CSI, I couldnt figure out quickly where it resides though. Let me try that again.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be much better if you could get it from CSI. It's available here.

kubectl -n simplyblock-csi get sc simplyblock-csi-sc -ojson | jq -r .parameters.pool_name

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the info! Added a fix for this in bd0801e.

@mxsrc mxsrc requested a review from boddumanohar February 6, 2026 16:39
@boddumanohar boddumanohar merged commit 18de93d into main Feb 7, 2026
5 checks passed
@boddumanohar boddumanohar deleted the sbapi-errors branch February 7, 2026 01:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants