Skip to content
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

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

stefanhipfel
Copy link
Contributor

  • adds wait polling for retrieving redfish storages. Some vendors redfish resources are not immediately available after a powerOn.
  • moves waitForPower function to bmc client

@github-actions github-actions bot added the size/M label Nov 8, 2024
@stefanhipfel stefanhipfel linked an issue Nov 8, 2024 that may be closed by this pull request
bmc/redfish.go Outdated
err = wait.PollUntilContextTimeout(
ctx,
10*time.Second,
5*time.Minute,
Copy link
Contributor

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?

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 was thinking about it. We just have so many config options already :)

@github-actions github-actions bot added size/L and removed size/M labels Nov 8, 2024
@stefanhipfel stefanhipfel changed the title adds wait polling for retrieving redfish storages adds wait polling for retrieving redfish storages and systems Nov 11, 2024
bmc/redfish.go Outdated
Comment on lines 49 to 50

options PollingOptions,
Copy link
Contributor

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.

Copy link
Contributor Author

@stefanhipfel stefanhipfel Nov 12, 2024

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.

Copy link
Member

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.

bmc/redfish.go Outdated Show resolved Hide resolved
bmc/redfish.go Outdated Show resolved Hide resolved
@afritzler afritzler changed the title adds wait polling for retrieving redfish storages and systems Add wait polling for retrieving redfish storages and systems Nov 12, 2024
cmd/manager/main.go Show resolved Hide resolved
bmc/redfish.go Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Storage discovery: handle ResourceNotReady
4 participants