Support uploading to OpenStack#337
Conversation
FrostyX
requested review from
achilleas-k,
croissanne and
thozza
and removed request for
a team
|
Sorry, I thought the |
|
Hey @FrostyX. Now that the images PR has been merged, should we get this rebased and fixed up? We can take it over if you don't have time. |
No need, I was monitoring the images repo releases and wanted to rebase today. |
supakeen
left a comment
supakeen
left a comment
There was a problem hiding this comment.
I have some minor remarks on wording but not really blockers.
| uploadCmd.Flags().String("libvirt-connection", "", "connection URI (only for type=libvirt)") | ||
| uploadCmd.Flags().String("libvirt-pool", "", "pool name (only for type=libvirt)") | ||
| uploadCmd.Flags().String("libvirt-volume", "", "volume name (only for type=libvirt)") | ||
| uploadCmd.Flags().String("openstack-image", "", "name for the uploaded image (only for type=openstack)") |
There was a problem hiding this comment.
What do these type= things here (and above) actually mean? I never quite looked at the upload command in depth.
There was a problem hiding this comment.
My understanding is that it can be only used for image-builder upload --to openstack or --to aws, etc. Not sure why the descriptions say type= though. I did the same that the AWS uploader did.
There was a problem hiding this comment.
Isn't it just documenting what image type the uploader applies to? You can't (or shouldn't) upload a vhd to AWS, only ami.
There was a problem hiding this comment.
@achilleas-k That was my first thought as well but only ami is an image type?
There was a problem hiding this comment.
I guess for aws it would be more correct to either list everything (e.g. ec2) or nothing. But for this one it's actually correct. The only openstack image type we have is openstack it's all good.
achilleas-k
left a comment
achilleas-k
left a comment
There was a problem hiding this comment.
LGTM but needs to resolve conflicts :/
3 participants
See osbuild/images#1921