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

-o kubevirt changes proposed for RHEL-45992 #56

Merged
merged 2 commits into from
Jul 11, 2024

Conversation

rwmjones
Copy link
Member

@rwmjones rwmjones commented Jul 4, 2024

@rwmjones
Copy link
Member Author

rwmjones commented Jul 4, 2024

Example output after this change:

---
# generated by virt-v2v 2.5.4local,libvirt
apiVersion: kubevirt.io/v1
kind: VirtualMachineInstance
metadata:
  name: fedora-39
  labels:
    libguestfs.org/virt-v2v-version: "2.5.4"
    libguestfs.org/osinfo: fedora39
spec:
  domain:
    os:
      firmware: bios
    resources:
      requests:
        memory: 2048Mi
      features:
        acpi: {}
        apic: {}
        pae: {}
    cpu:
      cores: 1
    devices:
      rng: {}
      disks:
      - disk:
          bus: virtio
        name: disk-0
      interfaces:
      - name: net_default
        masquerade: {}
  volumes:
  - hostDisk:
      path: /home/rjones/d/virt-v2v/./fedora-39-sda
      type: Disk
    name: disk-0
  networks:
  - networkName: default
    name: net_default
  terminationGracePeriodSeconds: 0
  preference:
    name: fedora

@bkhizgiy
Copy link
Contributor

bkhizgiy commented Jul 4, 2024

@rwmjones looking good, does it covers all the cases of CNV options?

@bkhizgiy
Copy link
Contributor

bkhizgiy commented Jul 4, 2024

btw, can there be a case where the reference field remains empty (from the virt-v2v perspective)?

Copy link
Contributor

@lyarwood lyarwood left a comment

Choose a reason for hiding this comment

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

I'm not entirely sure if v2v really needs to use preferences tbh if the VM/VMI output already has the required tuneables such as disk bus, interface model etc configured.

That said I can align the preference names provided by common-instancetypes with libosinfo in the next release if we do decided to use them.

output/create_kubevirt_yaml.ml Outdated Show resolved Hide resolved
output/create_kubevirt_yaml.ml Outdated Show resolved Hide resolved
@rwmjones
Copy link
Member Author

rwmjones commented Jul 4, 2024

@rwmjones looking good, does it covers all the cases of CNV options?

btw, can there be a case where the reference field remains empty (from the virt-v2v perspective)?

I have tried to include every guest type that I know about, but virt-v2v knows about more guest types than can be expressed in the spec -> preference -> name field, and if it's one of those guests that CNV doesn't know about then we omit that field.

See the new kubevirt_instancetype function. If that returns None then the field is omitted.

@rwmjones
Copy link
Member Author

rwmjones commented Jul 4, 2024

(Update just fixes the test suite)

@rwmjones
Copy link
Member Author

rwmjones commented Jul 4, 2024

New approach! Let's generate a VirtualMachine (instead of VirtualMachineInstance).

Example output can be found in tests/test-v2v-o-kubevirt-windows.yaml.expected and tests/test-v2v-o-kubevirt-fedora.yaml.expected (after the change is applied).

Add various information which we have but cannot add to the Kubevirt
yaml in any other place.

Example output:

...
metadata:
  name: fedora-39
  labels:
    libguestfs.org/virt-v2v-version: "2.5.4"
    libguestfs.org/osinfo: fedora39
...

Thanks: Bella Khizgiyaev, Liran Rotenberg
Fixes: https://issues.redhat.com/browse/RHEL-45992
VMs converted by virt-v2v are pets not cattle so we want to create
permanent non-running machines.  (They may later or even soon
afterwards be started up by some other tool, at which point they also
become Instances.)  Therefore use a different kind and slightly
restructure the yaml around this.

Thanks: Lee Yarwood
Copy link
Contributor

@lyarwood lyarwood left a comment

Choose a reason for hiding this comment

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

IMHO this is valid from a KubeVirt point of view.

Copy link
Contributor

@bkhizgiy bkhizgiy left a comment

Choose a reason for hiding this comment

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

Looking good. From MTV's perspective, I think it covers all that is needed.

@rwmjones rwmjones merged commit 2b00025 into libguestfs:master Jul 11, 2024
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.

3 participants