-
Notifications
You must be signed in to change notification settings - Fork 114
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
Skip firmware reset on devices the operator doesn't control #734
Conversation
Signed-off-by: Sebastian Sch <[email protected]>
Signed-off-by: Sebastian Sch <[email protected]>
Thanks for your PR,
To skip the vendors CIs use one of:
|
Pull Request Test Coverage Report for Build 9938668005Details
💛 - Coveralls |
Thanks for your PR,
To skip the vendors CIs use one of:
|
_, exist, err := p.helpers.LoadPfsStatus(iface.PciAddress) | ||
if err != nil { | ||
log.Log.Error(err, "failed to load PF status from disk", "address", iface.PciAddress) | ||
continue |
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 fail here ?
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.
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.
fair enough :)
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.
perhaps we shoud consider failing in both places, LMK what you think i can submit a followup PR.
LoadPfStatus will fail if file exists but it failed reading from it or its corrupt and we werent able to unmarshall
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.
sounds good I agree that should failed in both functions :)
when you have time please open the PR I can review it
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.
Only one minor comment from my side. LGTM
Signed-off-by: Sebastian Sch <[email protected]>
Signed-off-by: Sebastian Sch <[email protected]>
Thanks for your PR,
To skip the vendors CIs use one of:
|
In case the user allocate vf or configure the mellanox card firmware and we don't have any policy in the operator we should not reset that configuration