-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
baremetal: incorrect checksum used for bootstrap vm with bootstrapOSImage override #2845
Comments
/label platform/baremetal |
@kirankt FYI I found this while testing with openshift-metal3/dev-scripts#867 I'm not yet sure why this isn't playing nicely wrt #2657 |
@hardys I left a comment earlier in openshift-metal3/dev-scripts#867 that we need to use the uncompressed sha256 hash. Its confusing, but that's what the cache algorithm expects. |
Ok thanks for confirming @kirankt IMHO that is probably a bug, I see it appends the uncompressed hash here: Line 32 in a50b3ac
It's a counterintuitive interface though, when you see a URL like this I expect the checksum to relate to the file actually downloaded:
@RobertKrawitz - hi do you have any thoughts on this - would it be reasonable to rework the cache code to checksum, uncompress then provide the uncompressed checksum for input e.g to terraform? Perhaps you already looked into that and it'd be good to save some time if we can share any experiences you have there, thanks! :) |
We discussed doing that, but decided that it's best to not have to store (temporarily) two copies of the image on disk. If a .gz file is explicitly requested, perhaps the code should decompress it on the fly (perhaps calculating the compressed checksum on passthrough), but only land the uncompressed file on disk. |
@RobertKrawitz thanks for the additional context, it does seem like the interface would be more intuitive/consistent if we calculated the checksum for the compressed file. As you say we could do it on the fly via streaming, e.g locally I've been doing I'm creating a local cache for disconnected installs that includes images used by the installer and other provisioning components deployed via IPI baremetal, and in that case I don't really want to store the uncompressed files, it's better (and faster) to validate the compressed data. |
Last time I tried it, the machine would not boot the compressed image, so we stored it uncompressed and calculated the checksum on that basis. If you do make this kind of change, please ensure that libvirt and OpenStack function correctly. |
I don't want to store compressed data, because it's not useful to any of the consumers, libvirt, openstack can't use the compressed format... so it's most useful to store and cache the uncompressed files. as for faster, caching means you don't have to do it always, so i don't see the benefit there. |
Issues go stale after 90d of inactivity. Mark the issue as fresh by commenting If this issue is safe to close now please do so with /lifecycle stale |
Stale issues rot after 30d of inactivity. Mark the issue as fresh by commenting If this issue is safe to close now please do so with /lifecycle rotten |
Rotten issues close after 30d of inactivity. Reopen the issue by commenting /close |
@openshift-bot: Closing this issue. In response to this:
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/test-infra repository. |
Since #2757 landed it is possible to override the bootstrap image for disconnected installs, and I tried that like this:
However we see that although the installer cache code downloads from the expected location, the checksum is not recalculated before passing to terraform, so the validation fails:
Here we can see the compressed and uncompressed checksum - I think in the case where bootstrapOSImage is specified the sha256 should be used to validate the downloaded image, then a new checksum calculated for the gunzipped file?
The text was updated successfully, but these errors were encountered: