Make ironic timeouts customizable

[furkatgofurov7]

What this PR does / why we need it:
This commit adds some parameters for the ironic configuration

[furkatgofurov7]

/cc @maelk

[maelk]

I would rather not set the values at all in the config and use the ironic default, rather than adding defaults here that might not be in sync with Ironic's at some point. So if the environment variables are unset, we should probably not add the parameters in the configuration file. we'd do that only if they are set

[maelk]

I didn't see that it was set this way in ironic.conf, sorry, discard what I said, since we anyways set our own settings

[furkatgofurov7]

/test-integration

[furkatgofurov7]

/test-integration

[maelk]

/lgtm

[furkatgofurov7]

/test-integration

[furkatgofurov7]

/assign @bfournie
/cc @elfosardo
/cc @hardys

[furkatgofurov7]

/cc @dhellmann
/cc @dtantsur

[dhellmann]

I feel a bit like we're reimplementing confd. :-)

These are oslo.config settings, right? In other places we used the automatically checked environment variables to pass values right to the daemon. Would that approach work here, too? Could we document the relevant variables names instead of modifying the config file?

[maelk]

That's a very good point. This change could instead be a readme explaining how to override the settings in the ironic config.

[derekhiggins]

That's a very good point. This change could instead be a readme explaining how to override the settings in the ironic config.

Something like this could work
#165

[furkatgofurov7]

I feel a bit like we're reimplementing confd. :-)

These are oslo.config settings, right? In other places we used the automatically checked environment variables to pass values right to the daemon. Would that approach work here, too? Could we document the relevant variables names instead of modifying the config file?

@dhellmann thanks for the suggestion, I have addressed your comments and it is documented now in the readme by following the format from #165

[furkatgofurov7]

/test-integration

[furkatgofurov7]

/test-integration

[derekhiggins]

I feel a bit like we're reimplementing confd. :-)
These are oslo.config settings, right? In other places we used the automatically checked environment variables to pass values right to the daemon. Would that approach work here, too? Could we document the relevant variables names instead of modifying the config file?

@dhellmann thanks for the suggestion, I have addressed your comments and it is documented now in the readme by following the format from #165

I think we should just merge #165, I don't see any reason why these Timeout values should be special and mentioned in the config.

[dhellmann]

I don't have a problem with listing some variables that are commonly useful to override. Maybe we can also describe the general way to build the right name for other variables? That could be a separate PR.

This one needs a rebase.

[furkatgofurov7]

I feel a bit like we're reimplementing confd. :-)
These are oslo.config settings, right? In other places we used the automatically checked environment variables to pass values right to the daemon. Would that approach work here, too? Could we document the relevant variables names instead of modifying the config file?

@dhellmann thanks for the suggestion, I have addressed your comments and it is documented now in the readme by following the format from #165

I think we should just merge #165, I don't see any reason why these Timeout values should be special and mentioned in the config.

I don't have a problem with listing some variables that are commonly useful to override. Maybe we can also describe the general way to build the right name for other variables? That could be a separate PR.

This one needs a rebase.

These are examples of timeout values and how can they be overridden. And also we are not mentioning them in the config but README.md file.

/test-integration

[furkatgofurov7]

Hi @dhellmann, is there anything that needs to be changed/improved in this PR, or we can move forward with it?

[dhellmann]

/lgtm

[furkatgofurov7]

Hi @derekhiggins! is there anything that needs to be improved in this PR or we can proceed with it?

[derekhiggins]

/lgtm

[derekhiggins]

/approve

[metal3-io-bot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: derekhiggins, elfosardo, furkatgofurov7

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

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [derekhiggins]

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