feat: bmc fw update, passwd and reset roles

[glimchb]

Signed-off-by: Boris Glimcher [email protected]

[sachin-apa]

vars defined here cannot be given as a role input, So only role vars that are used in the role as a constant can go here.

[sachin-apa]

Printing the values will not help the user to get this at later stage, good to add under the facts ex: role_name_facts.Taskid , so that in case the job is not finished even after the retries/delay. user has an hold of the task id to take up next action.

[glimchb]

@sachin-apa see the code above I just added ansible.builtin.set_fact . Is that what you had in mind ?

[sachin-apa]

We can add changed_when and failed_when condition for users to understand if the firmware was updated. it helps to set the meta fields changed, failed etc that can be used to check later in the playbook.

[sachin-apa]

1. Define defaults for the roles if applicable. the default vars are not mandatory fields that can be changed as part of the role input.
2. Add meta/argument_specs.yml to that helps in validation, documentation, changelog of a role.
3. Add meta/main.yml with the details of the author and other data.
4. Lets add the lint, sanity checks as it becomes hectic to fix it.
5. Role vars, register and fact vars need to follow naming convention that prefix role_name_

[glimchb]

this should not be in vars...

[jcastanos2]

so, I took a cursory look at the pull request. Just a few comments to get started:

• I do not think that downloading the file from a repository into /tmp should be part of this task, but something it should be done separately. The update_bmc_fw should receive a filename/location of the image to install and that's it. Think you're updating a lot of dpus; you want to download it once. The datacenter might not have external connectivity, and will probably have a repository where all the valid fw are located, signed, etc. We should point to that location
• we're delegating to localhost to run those commands. This should probably a {{ delegate_to }} so we can have multiple nodes pushing images
• we're hardcoding MultipartHTTPPushUpdate. We do not support that yet in Bluefield. We might also want SimpleUpdate
• retries/delay should also be variables. Different vendors might have different requirements
• I see you have the cec in vars. In BF, a fw update implies posting two files: the bmc fw and the cec (eROT) and then reboot. This second step is missing in the pull request (it is also a same post with a different file), but it is also something very particular to us. Maybe the update/wait can be done on a list of files?

[jcastanos2]

One more comment: before I run any command on the DPU BMC, I need to check the server is up and running, by calling a redfish command on the server BMC. Otherwise, all my DPU BMCs will fail as the DPU has no power. I tend to leave all these external dependencies out the individual roles, rather than including them in my roles.

[Gal-Zaidman]

get_bmc_facts sounds general, not specific to FW (UpdateService/FirmwareInventory/BMC_Firmware), I assume that later it will just grow which will make it retrieve unnecessary information and would result in more and more redfish calls.
I would either:

1. add options to it, specifying what we want to retrieve
2. not separate it to a different role and call FW directly in this role

I prefer [2] since it this is not something that will change in the future we care about a specific FW version

[glimchb]

• we should separate if we want to re-use it
• we can rename to fw facts to be more specific

[Gal-Zaidman]

I think that the role needs to recive a firmware location as a parameter, if it is a valid url then download it, if it is a file then we expect it to be present on the machine running (this makes more sense when installing a large cluster)

[glimchb]

agree, I can open an issue on this, so somebody can also contribute that part ...
or if i have more time I can also do it myself...

[glimchb]

opened #16