kubevirt / kubevirt.github.io

KubeVirt website repo, documentation at https://kubevirt.io/user-guide/
https://kubevirt.io
MIT License
29 stars 109 forks source link

Fix localnet blog #912

Closed maiqueb closed 11 months ago

maiqueb commented 11 months ago

What this PR does / why we need it: This PR reacts to changes in the ovn-kubernetes repo, where the _br-localnet suffix was removed from the mapping configuration.

Thus, we need to adjust the existing blog post, otherwise the provided instructions will not provide a working scenario.

We also clarify the existing command to retrieve the name of the bridge where the "gateway" IP must be set only works for podman.

Does this PR fix any issue? _(optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged)_:

Fixes #

Special notes for your reviewer:

maiqueb commented 11 months ago

/cc @oshoval @phoracek

kubevirt-bot commented 11 months ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: phoracek

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files: - ~~[OWNERS](https://github.com/kubevirt/kubevirt.github.io/blob/main/OWNERS)~~ [phoracek] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment
maiqueb commented 11 months ago

Hm, the podman change doesn't render correctly: image

I guess in the meantime people can just look at the src code ...

I'll push a follow up PR.

oshoval commented 11 months ago

only method found so far that works for netlify is html code (but it aint nice as code segment of markdown)

<code>
podman network inspect underlay --format '&#123;&#123; .NetworkInterface &#125;&#125;'
podman3

ip addr add 10.128.0.1/24 dev podman3
</code>
oshoval commented 11 months ago

https://github.com/kubevirt/kubevirt.github.io/pull/912/files#diff-31ef61c45b96bec86589e47d8e03989143b97093e43ec093f8d6575738f2d859R348-R350

need to be fixed as well please (missing {)

about docker and the rest of the little fixes created https://github.com/kubevirt/kubevirt.github.io/pull/913

oshoval commented 11 months ago

We saw that the docker command works for podman as well (the fact the new test works for you and uses docker command) Therefor can consider using this command in the doc for both podman and docker, wdyt ? (it is more complex but robust)

Thanks for checking with podman

maiqueb commented 11 months ago

We saw that the docker command works for podman as well (the fact the new test works for you and uses docker command) Therefor can consider using this command in the doc for both podman and docker, wdyt ? (it is more complex but robust)

Thanks for checking with podman

Let's think this through for a moment: you need to know what's the name of the bridge that implements a network. That's literally what the podman command is doing. You propose to change that to return which interface is holding the GW IP of the network, because both runtimes provide IPAM (for now) on their implementations.

I think the docker API just return the name of the bridge as part of its data, not that we should adapt to the fact they fail to report the bridge name.

And furthermore, your proposed approach relies on IPAM - something which is not technically required for any of our use cases.

I would rather not replace the podman command.