support older ironic checksum expectations

[dhellmann]

Older versions of ironic that do not have
https://review.opendev.org/#/c/711816/ refuse to provision if the
image information does not include 'image_checksum', even if the other
hash value parameters are given. It seems safe to always pass the old
name, so go ahead and do so.

[metal3-io-bot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dhellmann

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 [dhellmann]

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

[dhellmann]

/test-integration

[zaneb]

Should we condition this on the algorithm being MD5? If it's a different type then we will end up reporting that the checksum is wrong when in fact we just can't support it at all (on older versions of Ironic).

[dhellmann]

Done

[kirankt]

There are other references to image_os_algo_{value,type} in the ironic.go file. Should we add the appropriate image_checksum request there as well?
e.g.
https://github.com/dhellmann/baremetal-operator/blob/ea64a310bc8f33359191239c20ffabd254401d6c/pkg/provisioner/ironic/ironic.go#L291
https://github.com/dhellmann/baremetal-operator/blob/ea64a310bc8f33359191239c20ffabd254401d6c/pkg/provisioner/ironic/ironic.go#L872

[dhellmann]

There are other references to image_os_algo_{value,type} in the ironic.go file. Should we add the appropriate image_checksum request there as well?
e.g.
https://github.com/dhellmann/baremetal-operator/blob/ea64a310bc8f33359191239c20ffabd254401d6c/pkg/provisioner/ironic/ironic.go#L291
https://github.com/dhellmann/baremetal-operator/blob/ea64a310bc8f33359191239c20ffabd254401d6c/pkg/provisioner/ironic/ironic.go#L872

I added it in the other place where we pass values to ironic on line 291.

The reference on 872 seems like it's OK because it's checking the new field name and we're still setting the new name. Let me know what you think.

[dhellmann]

/test-integration

[zaneb]

/lgtm

[dhellmann]

@maelk the the stdout log file for the baremetal-operator in the test job is empty and the stderr log file just says

Error from server (BadRequest): container "baremetal-operator" in pod "metal3-baremetal-operator-74986f48d6-l8b6d" is waiting to start: ContainerCreating

Does that mean anything to you?

[dhellmann]

ah, found another error in the job console log

+ make release-manifests
make[1]: Entering directory '/home/****/go/src/github.com/metal3-io/cluster-api-provider-metal3'
mkdir -p out/
kustomize build config > out/infrastructure-components.yaml
Error: accumulating resources: recursed accumulation of path 'webhook': accumulating resources: recursed accumulation of path '../manager': builtin PatchStrategicMergeTransformer config: [112 97 116 104 115 58 10 45 32 109 97 110 97 103 101 114 95 105 109 97 103 101 95 112 97 116 99 104 46 121 97 109 108 10 45 32 109 97 110 97 103 101 114 95 112 117 108 108 95 112 111 108 105 99 121 46 121 97 109 108 10 45 32 109 97 110 97 103 101 114 95 97 117 116 104 95 112 114 111 120 121 95 112 97 116 99 104 46 121 97 109 108 10]: YAML file [manager_image_patch.yaml] encounters a format error.
error converting YAML to JSON: yaml: line 11: mapping values are not allowed in this context

Makefile:326: recipe for target 'release-manifests' failed

So that looks like a bad kustomize input of some sort, maybe?

[dhellmann]

/test-integration

[maelk]

@dhellmann There was a bug introduced in the last commit of metal3-dev-env on Friday. I fixed it. I'll trigger the tests again, they should pass now.
/test-integration