Skip to content

feat(ibm): improve ibm vpc baremetal behaviour #375

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 23, 2024

Conversation

a-dubs
Copy link
Contributor

@a-dubs a-dubs commented May 15, 2024

Commit Description

Previously, if baremetal instances could not be launched due to
lack of capacity on the IBM Cloud, pycloudlib would fail to detect
this and just assume that the instance booted correctly.
Now, pycloudlib will return a capacity exception once this is
detected, allowing for consumers of pycloudlib to properly
implement retries for BM instances on IBM VPC.

Description of Testing:

This was tested both through an external program that leverages pycloudlib, and through a modified version of the ibm example script. The external program validated the use case of implementing retries for baremetal.

@a-dubs a-dubs force-pushed the fix-ibm-vpc-baremetal branch 2 times, most recently from ae2328b to 179790e Compare May 15, 2024 09:31
Copy link
Collaborator

@aciba90 aciba90 left a comment

Choose a reason for hiding this comment

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

Thanks, @a-dubs, for improving this!

LGTM, could you please fix the inline commnets, the pylint error and would you mind adding a bit of context in the commit message explaining what we are improving?

"""
if self._instance["status"] == _Status.FAILED.value:
if any(
"capacity" in reason["code"]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we have a link pointing for docs covering this? If so, would you mind adding it for reference?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't have any docs for this. I discovered this by manually launching and inspecting instance status via the API/SDK.

@a-dubs a-dubs force-pushed the fix-ibm-vpc-baremetal branch 2 times, most recently from da663a7 to a129908 Compare May 22, 2024 19:59
Previously, if baremetal instances could not be launched due to
lack of capacity on the IBM Cloud, pycloudlib would fail to detect
this and just assume that the instance booted correctly.
Now, pycloudlib will return a capacity exception once this is
detected, allowing for consumers of pycloudlib to properly
implement retries for BM instances on IBM VPC.
@a-dubs a-dubs force-pushed the fix-ibm-vpc-baremetal branch from a129908 to 85db81a Compare May 22, 2024 20:03
Copy link
Collaborator

@aciba90 aciba90 left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

# if self.b
self._wait_for_status(
_Status.RUNNING,
sleep_seconds=900,
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is good for now. But, the 900 should be configurable from a consumer POV, as it might be not flexible enough.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay I'll look at adding this sometime soon. Thanks!!

@aciba90 aciba90 merged commit d65b3af into canonical:main May 23, 2024
4 checks passed
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