Add Ubuntu 18.04 QEMU image for Cluster API#169
Add Ubuntu 18.04 QEMU image for Cluster API#169k8s-ci-robot merged 1 commit intokubernetes-sigs:masterfrom
Conversation
detiber
left a comment
There was a problem hiding this comment.
Overall, these seems pretty reasonable. Can you add some documentation for this as well?
0a24b50 to
a8325b7
Compare
Sure, I added docs/book/src/capi/providers/openstack.md. |
sbueringer
left a comment
There was a problem hiding this comment.
@hidekazuna A lot of nitpicking I compared it with my version. I hope these are useful suggestions :)
|
@sbueringer All comments addressed. Thanks a lot! |
|
@detiber @ncdc I'm not a reviewer/approver in this repo. I know you're probably busy with the CAPI release, but can one of you please take a look at this PR and approve/lgtm it if looks good to you? P.S. I rebased my own repo on top of this PR. And apart from some minor differences this should also verify that the build works: https://github.com/sbueringer/image-builder/runs/499696434?check_suite_focus=true |
|
@hidekazuna looks perfect now. Thx for working on this! :) |
|
/lgtm |
|
/approve |
|
@codenrhoden said he would have a quick look and approve this week still |
Okay perfect thx :) |
|
@hidekazuna for info- if you're already using QEMU for the builder, you could also look at using args = [
'vmware-vdiskmanager',
'-r', infile,
'-t', '5',
outfile
]to: args = [
'qemu-img',
'convert',
'-o',
'compat6,subformat=streamOptimized',
'-f',
'qcow2',
'-O',
'vmdk',
infile,
outfile
] |
@hidekazuna Similar if you want. There is also a command like this to create a vmdk Not sure if that's wanted upstream but with one additional conversion we can create also a vmdk. In our use case we also need this (because we're using VMWare Integrated OpenStack which doesn't support qcow2), but for me it's okay to have this on a fork. But probably would be a nice feature for somebody. |
|
Note that there are known issues when using |
codenrhoden
left a comment
There was a problem hiding this comment.
Hi @hidekazuna!
Thanks for putting this together. It looks really thorough. I just have a couple requests:
- a header file that needs updating for copyright
- removing
capi_versionfrom your packer.json
The rest are questions about whether we can symlink the preseed files instead of duplicating them, and a couple of other simple ones.
Thanks again!
| state: link | ||
| when: ansible_os_family == "RedHat" | ||
|
|
||
| - name: Disable Hyper-V KVP protocol daemon on Ubuntu |
There was a problem hiding this comment.
I also meant to ask if this was something you wanted to keep. In the VMW provider, it made sense because it 1) worked around an Ubuntu kernel bug that made startup times increase by a bunch and 2) wasn't necessary since it would be running on an ESXi hypervisor rather than Hyper-V.
For a QEMU image, do you know that you'd never be running on Hyper-V?
There was a problem hiding this comment.
Thanks for explaining why Hyper-V KVP protocol daemon is disabled. I am not sure QEMU image is "NEVER" used on Hyper-V, but typically if we want to use existing image, convert QEMU image to VHD and use it on Hyper-V.
|
@codenrhoden @hidekazuna Would be cool if could get this merged soon because I would like to refer to the qemu build in the ClusterAPI book: kubernetes-sigs/cluster-api#2683 Thx :) |
|
@hidekazuna brand new to this , can you help on confirm that this will provide a way to build an image from somewhere (maybe iso or cloud image) into something can be used by cluster api e.g add openstack unique things along with kubeadm related stuffs so that we can use the image as glance source to boot from for controller pane and compute pane ? (I used to deploy a VM then install kubeadm manually and capture that image) |
@jichenjc (the version is more or less this PR + github actions build + ubuntu 1910) The conformance tests are using a local devstack with kvm. |
@jichenjc Yes, you can use pre-kubeadm installed image for cluster-api-provider-openstack by this PR. No need to install kubeadm manually! |
|
@moshloop @sbueringer Could you teach me what I have to address to merge this PR? |
|
@hidekazuna pr looks good to me. I'm just not a reviewer/approver in this repo 😅 |
|
@sbueringer Thanks for quick answer. kubernetes-sigs/cluster-api#2683 remains the link to this PR and has been merged so I wondered if something I have to fix. |
|
@hidekazuna nope everything alright. I just didn't want to wait with the Capo release. I'll update the link in the book after this pr is merged |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: detiber, hidekazuna, moshloop The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
@detiber @codenrhoden Could you please review this PR again and add lgtm label if you do not mind? |
|
I think a /lgtm from @sbueringer should work as well |
|
/lgtm |
|
@moshloop So what are the requirements that a lgtm works? I though I have to be reviewer in the OWNERS file? |
|
I think you only need to be a member of kubernetes-sigs for a lgtm - it might be different for k/k |
|
@sbueringer: @moshloop is correct any member of kubernetes-sigs can lgtm, reviewers in owners files are automatically assigned to PRs for review. I believe it is possible to override the default policy, but we haven't seen a need for it as of yet. |
This PR adds Ubuntu 18.04 QEMU image for Cluster API.
Fixes #96