-
Notifications
You must be signed in to change notification settings - Fork 410
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
[WIP] Bug 1872238: oVirt: Introduce qemu-guest-agent container #2042
Conversation
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: eslutsky The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
17a8ffc
to
9047487
Compare
/test ? |
@Gal-Zaidman: The following commands are available to trigger jobs:
Use
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. |
/test e2e-ovirt |
1 similar comment
/test e2e-ovirt |
Hmm did you see coreos/afterburn#458 ? We are actively working on implementing the protocol in Afterburn. cc @lucab |
Slight correction to the above: we are exploring whether we can re-use the oVirt protocol/virtio-port in our CI to signal that a node booted into initramfs (for the sake of not reinveinting a wheel if that protocol is already good). |
b2cc0e7
to
3a70234
Compare
/bugzilla refresh |
@kikisdeliveryservice: Bugzilla bug 1764804 is in a bug group that is not in the allowed groups for this repo.
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. |
/test e2e-ovirt |
3a70234
to
c78392c
Compare
/retest e2e-ovirt |
@eslutsky: The
Use
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. |
/test e2e-ovirt |
c78392c
to
c7ffe89
Compare
/retest e2e-ovirt |
@eslutsky: The
Use
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. |
Is there a reason this is running via podman instead of as a daemonset? |
Also are you really aiming this at 4.6? |
--volume /usr/share/zoneinfo:/usr/share/zoneinfo \ | ||
--net=host \ | ||
--pull missing \ | ||
docker://registry.svc.ci.openshift.org/ovirt/qemu-guest-agent:4.2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We cannot pull arbitrary container images with OpenShift 4 by default. Anything we run by default must be part of the release image.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And once it's part of the release image, this would need to have its value substituted by templating, same as other containers the MCO references.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
agreed, we build this container in ovirt NS just for testing purposes, we want to see it running and document the gaps .
/hold |
@eslutsky: This pull request references Bugzilla bug 1872238, which is invalid:
Comment 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. |
/bugzilla refresh |
@eslutsky: This pull request references Bugzilla bug 1872238, which is invalid:
Comment 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. |
c7ffe89
to
f92d430
Compare
/test e2e-ovirt |
f92d430
to
9e63361
Compare
/test e2e-ovirt |
ocp VMs needs to run oVirt QEMU agent container. Bug: 1764804 Signed-off-by: Evgeny Slutsky <[email protected]>
9e63361
to
e452a3a
Compare
/test e2e-ovirt |
@cgwalters @runcom any feedback? |
@eslutsky: The following tests failed, say
Full PR test history. Your PR dashboard. 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. I understand the commands that are listed here. |
@eslutsky: The following tests failed, say
Full PR test history. Your PR dashboard. 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. I understand the commands that are listed here. |
@cgwalters , can we a get a feedback on this ,
@rgolangh , I've checked your CSI drivers reference , and it appears to be also be part of the release image:
so i don't think it's a valid example, |
I think the MCO should take care of this generically across platforms. How would qemu-guest-agent help with that anyways? Stated more strongly, exactly what functionality do you want from the agent? We have some work on implementing boot checkins in afterburn here: coreos/afterburn#458 It sounds like the guest agent has some protocol for the guest to report its IP address - what are you using that for? |
on each oVirt host we have a vdsm component which use libvirt to communicate with the agent over the virtio serial port . |
There's of course actually three options for running this:
They all have tradeoffs. (I'd actually like to switch the If the oVirt team feels strongly that it'd significantly help things now to just add the guest agent into the set of extensions and change the MCO to do exactly the above (dynamically layer just on oVirt platforms) - well...okay. But the thing is every time we do this it hinders our long term goals to get people to move to containers - and also to minimize the "agent footprint" on the host. |
On other IPI virt platforms (AWS/GCP/Azure/OpenStack) we get by without this. Why does oVirt need it but those don't? The hypervisor is in a position to know what DHCP address it served to an instance.
Again I strongly think this one is a generic problem; see related discussion in coreos/ignition#585 |
well, the hypervisor doesn't act as a DHCP server,
|
oVirt needs the guest agent running inside each VM that it manages (ocp or not ocp). Currently, we need this container to be available for our RHCOS VMs those are the only VMs that doesn't have the guest agent.
As you can see this PR is old and we have been dragging this for a while now, and we need it for 4.7. |
shipping the daemonset would still be backing this within the MCO repo (which doesn't help us with code that the MCO team isn't an expert about). Hina is setting up a meeting to discuss this as I also think it'll help. |
The list above missed the 4th option, which is:
So for clarity, all the options:
Now for option 3 - personally I am a bit skeptical that this needs to run before kubelet - we already have too many of these "special before kubelet" containers and they make things a lot more complicated. My preferred order looks like:
|
Can you clarify, is the desired functionality in the |
You don't want oVirt Guest Agent as that one is deprecated. What you really want is QEMU Guest Agent. |
OK this turned into https://bugzilla.redhat.com/show_bug.cgi?id=1900759 |
@cgwalters: Closed this PR. 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. |
@eslutsky: An error was encountered getting external tracker bugs for bug 1872238 on the Bugzilla server at https://bugzilla.redhat.com:
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. |
Though after that meeting I did think of yet another option - stick the code inside https://github.com/openshift/cluster-api-provider-ovirt Anyways though, fine continuing the angle of including in the OS by default, but leaving this here in case you guys are like "ah that sounds great!". |
OpenStack would also benefit from having the qemu-guest-agent available somehow. If we were to go down the "daemonset in the platform specific cluster API providers" path, that would mean duplicating the code from the ovirt provider to the openstack one. I can see it be rather hard to maintain on the long run. |
ocp VMs needs to run oVirt QEMU agent container.
Bug: 1872238
Signed-off-by: Evgeny Slutsky [email protected]