-
Notifications
You must be signed in to change notification settings - Fork 6
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
Add wait polling for retrieving redfish storages and systems #174
base: main
Are you sure you want to change the base?
Conversation
stefanhipfel
commented
Nov 8, 2024
- adds wait polling for retrieving redfish storages. Some vendors redfish resources are not immediately available after a powerOn.
- moves waitForPower function to bmc client
bmc/redfish.go
Outdated
err = wait.PollUntilContextTimeout( | ||
ctx, | ||
10*time.Second, | ||
5*time.Minute, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we make interval and timeout configurable like in bmcClient.WaitForServerPowerState
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was thinking about it. We just have so many config options already :)
bmc/redfish.go
Outdated
|
||
options PollingOptions, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Superfluos newline. Given the amount of optiones, it may be worth introducing a BMCClientOptions
struct to hold all of them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had that already implemented in the las commit (c7bf550).
Some of the settings are mandatory like user/pw, so I wanted to separate them.
Especially since we have some functions where we only need to provide these polling values:
func GetBMCClientFromBMC(ctx context.Context, c client.Client, bmcObj *metalv1alpha1.BMC, insecure bool, polling bmc.PollingOptions)
Username and password are retrieved via the k8s secrets.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would also favor for a generic BMCOptions
struct which we can extend as we go. That way we don't need to change the method signature too often.