Skip to content

🌱 customize some firmware update configs#892

Open
iurygregory wants to merge 1 commit intometal3-io:mainfrom
iurygregory:fwup-configs
Open

🌱 customize some firmware update configs#892
iurygregory wants to merge 1 commit intometal3-io:mainfrom
iurygregory:fwup-configs

Conversation

@iurygregory
Copy link
Copy Markdown
Member

While testing firmware updates in different hardware models, we noticed it would require to change some of the default values we have in ironic.
In the redfish section we changed the following configs:

  • increase firmware_update_resource_validation_timeout to 480
  • set firmware_update_wait_unresponsive_bmc to 0

While testing firmware updates in different hardware models,
we noticed it would require to change some of the default values
we have in ironic.
In the redfish section we changed the following configs:
- increase firmware_update_resource_validation_timeout to 480
- set firmware_update_wait_unresponsive_bmc to 0

Signed-off-by: Iury Gregory Melo Ferreira <imelofer@redhat.com>
@metal3-io-bot
Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign rozzii for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@metal3-io-bot metal3-io-bot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Jan 26, 2026
@dtantsur
Copy link
Copy Markdown
Member

@iurygregory what's the reason of not bringing these values upstream into Ironic? After all, we're the only people who have tested this feature so extensively.

@jacob-anders
Copy link
Copy Markdown
Member

/lgtm

@metal3-io-bot
Copy link
Copy Markdown
Contributor

@jacob-anders: adding LGTM is restricted to approvers and reviewers in OWNERS files.

Details

In response to this:

/lgtm

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@elfosardo
Copy link
Copy Markdown
Member

/hold
wondering if we can jsut change default upstream as @dtantsur mentioned

@metal3-io-bot metal3-io-bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 26, 2026
@jacob-anders
Copy link
Copy Markdown
Member

@iurygregory what's the reason of not bringing these values upstream into Ironic? After all, we're the only people who have tested this feature so extensively.

this is a good point, I wonder if we should consider a dedicated patch for consolidating and cleaning up config parameters in upstream Ironic? @dtantsur do you think that would be a good approach?

@dtantsur
Copy link
Copy Markdown
Member

@jacob-anders yes.

(and if we need a quick patch downstream, we can always do it and later revert)

@jacob-anders
Copy link
Copy Markdown
Member

@jacob-anders yes.

(and if we need a quick patch downstream, we can always do it and later revert)

Thanks @dtantsur .

W/r/t quick patch I sneaked this into https://github.com/openshift/ironic-image/pull/769/files#diff-765b5bdc34dee809918ab0952e181ceda44d4c56af6d3b3957b970047a5d2a0b - this should do for testing and before we're in position to remove the hold, I will remove that commit from the PR.

I'll think through refactoring fwupd knobs and dials. It's got a feel of a plane that needs a flight engineer, cause it's too many knobs for two pilots to manage. We need glass cockpit and automation :) Thanks for great inputs here.

@iurygregory
Copy link
Copy Markdown
Member Author

Thanks @dtantsur and @elfosardo , I've pushed https://review.opendev.org/c/openstack/ironic/+/974800 let me know what you think!

@dtantsur
Copy link
Copy Markdown
Member

dtantsur commented Feb 2, 2026

The upstream patch got merged, should we abandon this one?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants