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

Add Guest Agent transport setting #3387

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

arixmkii
Copy link
Contributor

Fixes #3177

Closes #3255

Adds setting, which could influence, which will then decide, which transport will be in use for connection to GA

Fixes:

  • QEMU always creating ga.sock even when SSH forwarder will be used
  • Fallback mode to SSH forwarder was unavailable for VZ

If the unsupported option is detected then a warning is printed and fallback will be used instead. The default is VMType and Host OS sensitive (due to limited support of options for some platforms).

Signed-off-by: Arthur Sengileyev <[email protected]>
@arixmkii
Copy link
Contributor Author

I manually tested it

  • QEMU on Windows
  • QEMU on M4 Mac
  • VZ on M4 Mac

PropagateProxyEnv *bool `yaml:"propagateProxyEnv,omitempty" json:"propagateProxyEnv,omitempty" jsonschema:"nullable"`
CACertificates CACertificates `yaml:"caCerts,omitempty" json:"caCerts,omitempty"`
Rosetta Rosetta `yaml:"rosetta,omitempty" json:"rosetta,omitempty"`
GuestAgentTransportType *GATransportType `yaml:"guestAgentTransportType,omitempty" json:"guestAgentTransportType,omitempty" jsonschema:"nullable"`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we should add a new struct GuestAgent?

GuestInstallPrefix can be moved there too. (The old form should be supported too for a while)

@AkihiroSuda AkihiroSuda requested a review from balajiv113 March 27, 2025 23:22
@balajiv113
Copy link
Member

@arixmkii
I understand the intent behind this PR.

But i noticed lots of issues related to qemu serial port. At this point of time I don't think it will work out for all of our cases. Major problem was they don't support multiple instances of sock.

With that being said, do you think having this option helps ??

@balajiv113
Copy link
Member

This option confuses people more because it has drawbacks based on selection in long run.
For instance, we tried with supporting serialport so that we can support pause resume. Ssh forward cannot be paused and resumed properly.

I think we should derive best mode of transport and use it as default always. Giving these config will confuse folks using it also this transport is internal to lima guest agent

@arixmkii
Copy link
Contributor Author

Major problem was they don't support multiple instances of sock.

This is serial specifics, somewhat expected.

With that being said, do you think having this option helps ??

It the only option usable on Windows hosts. Unix socket forwarding with SSH alone is not possible, all additional workarounds on top of SSH will have overhead and be more fragile.

It's having something, which works with limitations vs not having any working solution.

I think we should derive best mode of transport and use it as default always.

Sounds reasonable, until one steps on platform specific limitations, having unconditional non-configurable different choices on different platforms might be confusing as well.

Using "hacky" looking if runtime.GOOS == "windows" {} else {} here is possible, but it didn't feel right for me.

Giving these config will confuse folks using it also this transport is internal to lima guest agent

Just the discussion, I would disagree here. The setting is purely HostAgent specific, where it later communicates it to both GA and VM driver and makes decision how to establish its own connection.

@balajiv113
Copy link
Member

Using "hacky" looking if runtime.GOOS == "windows" {} else {} here is possible, but it didn't feel right for me.

I agree. Can we think of doing something like below

We can have this config but let do these config population as part of default windows itself and this can be a internal config. Not exposed to user

This way user doesn't require to configure but as you said we will have meaningful way of deciding on transport medium

@arixmkii
Copy link
Contributor Author

Not exposed to user

So, the main concern is exposing this to limayaml? Am I getting this right?

We can extend Driver API with SupportedTransports, where the priority based list is returned, this would become platform specific, and then HA would find the topmost suitable from this list, which it can use to configure both GA and the driver. This is a rough idea and needs polishing obviously. WDYT?

@balajiv113
Copy link
Member

So, the main concern is exposing this to limayaml? Am I getting this right?

Yes atleast for me this is the concern

extend Driver API with SupportedTransports

True, if we can do it with existing data sets it would be great (like config). But happy to see new approach as well

@arixmkii arixmkii mentioned this pull request Mar 30, 2025
@arixmkii arixmkii marked this pull request as draft March 31, 2025 05:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

QEMU: Support feature toggle for GA via serial port
3 participants