Skip to content
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

✨ download BM images via oci-registry. #1071

Merged
merged 1 commit into from
Dec 6, 2023

Conversation

guettli
Copy link
Collaborator

@guettli guettli commented Dec 4, 2023

What this PR does / why we need it:

Up to now, bare-metal machine images could only be downloaded from http/https URLs.

This PR make it possible to download the machine images from oci-registries.

You can use a tool like oras to upload the machine image in tgz format to an oci registry.

@janiskemper
Copy link
Contributor

does it make sense to have the shell script in the Go code? Shouldn't it be rather in hack/?

@guettli guettli force-pushed the tg/support-images-from-oci-registries branch 3 times, most recently from 1415a1b to 381caf0 Compare December 4, 2023 21:11
@guettli
Copy link
Collaborator Author

guettli commented Dec 4, 2023

does it make sense to have the shell script in the Go code? Shouldn't it be rather in hack/?

The shell script does not get executed like during build/compile time like the other scripts.

The shell scripts get copied from the controller intpkg/services/baremetal/client/ssh/download-from-oci.sho the rescue-system. I added a comment at the top.

I think hack/ is no good place for this script.

I could copy the script into the Go code like (instead of using embed):

var script=`#!/bin/bash
....

@janiskemper what do you think? I can copy it to hack/ if you prefer that.

@guettli guettli requested a review from batistein December 4, 2023 21:13
@guettli guettli force-pushed the tg/support-images-from-oci-registries branch from 381caf0 to 801d46d Compare December 4, 2023 21:15
@guettli guettli requested a review from janiskemper December 4, 2023 21:16
@guettli guettli changed the title ✨ download BM images via ocr-registry. ✨ download BM images via oci-registry. Dec 4, 2023
@guettli guettli force-pushed the tg/support-images-from-oci-registries branch from 801d46d to d70a92d Compare December 5, 2023 07:34
@syself-bot syself-bot bot added size/M Denotes a PR that changes 50-200 lines, ignoring generated files. area/hack Changes made in the hack directory area/code Changes made in the code directory area/api Changes made in the api directory labels Dec 5, 2023
@janiskemper
Copy link
Contributor

I think I prefer having it hard-coded in the Go code!

@guettli guettli force-pushed the tg/support-images-from-oci-registries branch 2 times, most recently from 4ecdc8a to 4e11b82 Compare December 5, 2023 09:23
@syself-bot syself-bot bot added size/L Denotes a PR that changes 200-800 lines, ignoring generated files. and removed size/M Denotes a PR that changes 50-200 lines, ignoring generated files. labels Dec 5, 2023
@guettli
Copy link
Collaborator Author

guettli commented Dec 5, 2023

I think I prefer having it hard-coded in the Go code!

ok, I put the shell script into the Go code.

@guettli guettli force-pushed the tg/support-images-from-oci-registries branch 5 times, most recently from 91fd462 to a859a03 Compare December 5, 2023 16:41
batistein
batistein previously approved these changes Dec 6, 2023
@guettli guettli force-pushed the tg/support-images-from-oci-registries branch from a859a03 to f484ce4 Compare December 6, 2023 09:17
@guettli guettli self-assigned this Dec 6, 2023
@guettli guettli merged commit 0b8a308 into main Dec 6, 2023
9 checks passed
@guettli guettli deleted the tg/support-images-from-oci-registries branch December 6, 2023 10:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/api Changes made in the api directory area/code Changes made in the code directory area/hack Changes made in the hack directory size/L Denotes a PR that changes 200-800 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants