Git Product home page Git Product logo

Comments (12)

JM1 avatar JM1 commented on July 19, 2024 1

Oh wait, I now see what is causing issues for you.

hardware has no meaningful value in role defaults of jm1.libvirt.server although in fact it should add the storage volumes created in that role to the libvirt domain.

from ansible-collection-jm1-libvirt.

JM1 avatar JM1 commented on July 19, 2024

Use parameter hardware of module jm1.libvirt.domain for --os-variant and --disk as shown here:

- jm1.libvirt.domain:
    name: '{{ libvirt_domain }}'
    hardware:
    - cpu: 'host'
    - vcpus: '2'
    - memory: '1024'
    - virt_type: 'kvm'
    - graphics: 'spice,listen=none'
    - network: 'network=lan-bridge,model=virtio'
    - disk: "vol='{{ libvirt_pool }}/{{ libvirt_volumes[0]['name'] }}',device=disk,bus=virtio,serial='root'"
    - disk: "vol='{{ libvirt_pool }}/{{ libvirt_configdrive }}',device=disk,bus=virtio,serial='cidata'"
    # Specifying os variant is HIGHLY RECOMMENDED, as it can greatly increase performance by specifying virtio
    # among other guest tweaks. It also enables support for QEMU Guest Agent by adding a virtio-serial channel.
    # Ref.: man virt-install
    - os_variant: 'debian10'
    state: 'present'

from ansible-collection-jm1-libvirt.

JM1 avatar JM1 commented on July 19, 2024

Also it makes more sense for the volume variable to default to {{ domain }}.{{ volume_format }} instead of {{ inventory_hostname }}.{{ volume_format }}

The whole role jm1.libvirt.server is meant more like an example of how to use the modules in collection jm1.libvirt. Writing generic Ansible roles which fit "all" use cases is hard. Often its easier to keep it simple, use Ansible modules as generic building blocks (instead of roles) and write "new" Ansible roles for your specific use case. I tried to develop a set of highly reusable roles in collection jm1.cloudy which minimize assumptions and use Ansible host_vars and group_vars for specifying the customizable bits aka configuration.

from ansible-collection-jm1-libvirt.

LecrisUT avatar LecrisUT commented on July 19, 2024

Use parameter hardware of module jm1.libvirt.domain for --os-variant and --disk as shown here:

- jm1.libvirt.domain:
    name: '{{ libvirt_domain }}'
    hardware:
    - cpu: 'host'
    - vcpus: '2'
    - memory: '1024'
    - virt_type: 'kvm'
    - graphics: 'spice,listen=none'
    - network: 'network=lan-bridge,model=virtio'
    - disk: "vol='{{ libvirt_pool }}/{{ libvirt_volumes[0]['name'] }}',device=disk,bus=virtio,serial='root'"
    - disk: "vol='{{ libvirt_pool }}/{{ libvirt_configdrive }}',device=disk,bus=virtio,serial='cidata'"
    # Specifying os variant is HIGHLY RECOMMENDED, as it can greatly increase performance by specifying virtio
    # among other guest tweaks. It also enables support for QEMU Guest Agent by adding a virtio-serial channel.
    # Ref.: man virt-install
    - os_variant: 'debian10'
    state: 'present'

Since both os_variant and disk are required parameters, they should be included in the role's vars instead of having to input them manually as such.

About the change from inventory_hostname to domain as the defaults, this is because if we build upon the existing role of jm1.libvirt.server, these would be linked to the virtual host, so if we deploy multiple virtual clients, the naming will clash by default with inventory_hostname. Using domain instead makes more sense because that is the default of virt-install and when one wants to create a new machine, it has to be unique. Maybe in your setup, the inventory host is the future virtual client, thus why you are defaulting for inventory_hostname, but is that applicable when connecting to the virtual server remotely?

from ansible-collection-jm1-libvirt.

LecrisUT avatar LecrisUT commented on July 19, 2024

Looking at jm1.cloudy, it is a bit too complex to navigate compare to the specific components like this one. It has most of the tasks that I would be doing in shell otherwise so I will still look into using it. But right off-the-bat, I think it is better to rewrite jm1.cloudy.libvirt_domain role so that it searches for a variable of libvirt_domains instead of the individual parameters so that the role can be attached to a virtual host instead of the client machines.

from ansible-collection-jm1-libvirt.

JM1 avatar JM1 commented on July 19, 2024

Since both os_variant and disk are required parameters, they should be included in the role's vars instead of having to input them manually as such.

There already is a role variable hardware which can be used to add os_variant and disk in jm1.libvirt.server.

About the change from inventory_hostname to domain as the defaults, this is because if we build upon the existing role of jm1.libvirt.server, these would be linked to the virtual host, so if we deploy multiple virtual clients, the naming will clash by default with inventory_hostname. Using domain instead makes more sense because that is the default of virt-install and when one wants to create a new machine, it has to be unique. Maybe in your setup, the inventory host is the future virtual client, thus why you are defaulting for inventory_hostname, but is that applicable when connecting to the virtual server remotely?

Simply override role default variable configdrive with your desired value in your inventory or playbook or whatever fits your use case best.

from ansible-collection-jm1-libvirt.

LecrisUT avatar LecrisUT commented on July 19, 2024

There already is a role variable hardware which can be used to add os_variant and disk in jm1.libvirt.server.

That is not ideal. One must first know that defining volume, etc. is incomplete to get the example playbooks e.g. in the Readme. But with regards to the code, two new role variables/facts of disks and os_variant (defined from distribution_id) should be used and passed to the domain.py so that one calls the least amount of hardware as possible. You already pass default flags of --noreboot and this would just be extending upon it. Otherwise there is little difference from calling the raw shell script with the appropriate variables.

from ansible-collection-jm1-libvirt.

LecrisUT avatar LecrisUT commented on July 19, 2024

That's partially it. You should consider this more for a bit. I am saying that the design of this role is incomplete and that ideally, the role should work with an empty hardware, because as you've mentioned in the other closed issue, the defaults in the role will be overridden and not merged.

from ansible-collection-jm1-libvirt.

JM1 avatar JM1 commented on July 19, 2024

Ansible lazy-evaluates variables. Why not define a default value for hardware which allows to use the role as shown in its README.md? If you want to change it, copy the value of hardware to a place you control and has higher precedence than role defaults and change it to your needs. Maybe you can give an example where this approach is not feasible?

from ansible-collection-jm1-libvirt.

LecrisUT avatar LecrisUT commented on July 19, 2024

The problem with that is that it is not user friendly when one wants to do only minimal changes, e.g. just changing the flavor with 4vcpus/8GB ram, then we still need to include boiler-plate variables of:

- disk: "vol='{{ libvirt_pool }}/{{ libvirt_volumes[0]['name'] }}',device=disk,bus=virtio,serial='root'"
- disk: "vol='{{ libvirt_pool }}/{{ libvirt_configdrive }}',device=disk,bus=virtio,serial='cidata'"

If instead we use a combine, then the user can simply make the minimal changes needed. It's mostly that it will make things more organized and easier to read, e.g. the hosts file can change from:

libvirt_domain: domain
libvirt_hardware:
- cpu: 'host'
- vcpus: '4'
- memory: '4096'
- virt_type: 'kvm'
- graphics: 'spice,listen=none'
- network: 'network=lan-bridge,model=virtio'
- disk: "vol='{{ libvirt_pool }}/{{ libvirt_volumes[0]['name'] }}',device=disk,bus=virtio,serial='root'"
- disk: "vol='{{ libvirt_pool }}/{{ libvirt_configdrive }}',device=disk,bus=virtio,serial='cidata'"
- os_variant: 'debian10'
other_role:
  var1: something
another_role:
  var1: something_else

to:

libvirt:
  domain: domain
  hardware:
    vcpus: '4'
    memory: '4096'
other_role:
  var1: something
another_role:
  var1: something_else

I realize that the hardware section should be changed to a flat dictionary and disk and network should be separate array variables, but that would still simplify thing.


As for things breaking and becoming not feasible, I have actually found a usecase in trying to make your role work on remote virtual hosts, specifically we cannot do such a playbook:

- name: Install virtual machines
  hosts: virtual_host
  gather_facts: no
  tasks:
  - name: Setup virtual machine 1
    ansible.builtin.import_role:
      name: jm1.libvirt.server
    vars: {{ vm1.virt_vars }}

vars being what breaks. This would be a prerequisite to make Setup virtual machine 1 on a loop. The solution of putting all of the libvirt vars inside the libvirt object is according to this workaround. To properly override variables from the defaults, we simply use default_virt_vars | combine(libvirt.virt_vars) in a set_fact. Creating a separate role that simply passes the variables down and sets its own defaults seems wasteful and a lot of black-box text for a new user reading the derived role.

An alternative I have considered is delegates_to, but I am still working on getting it to work. I suspect that that is a simpler solution to this and I will update here with the results, but if you have more insight on this, it would be useful.

PS: disclaimer I have only been doing ansible for 2 days, and these are some of the limitations of the language that I found. I do not know if there is an consensus on how to design this better

from ansible-collection-jm1-libvirt.

JM1 avatar JM1 commented on July 19, 2024

The problem with that is that it is not user friendly when one wants to do only minimal changes, e.g. just changing the flavor with 4vcpus/8GB ram, then we still need to include boiler-plate variables of:

- disk: "vol='{{ libvirt_pool }}/{{ libvirt_volumes[0]['name'] }}',device=disk,bus=virtio,serial='root'"
- disk: "vol='{{ libvirt_pool }}/{{ libvirt_configdrive }}',device=disk,bus=virtio,serial='cidata'"

If instead we use a combine, then the user can simply make the minimal changes needed. It's mostly that it will make things more organized and easier to read, e.g. the hosts file can change from:

#4 explains why combine or other postprocessing steps to hardware cannot be implemented in role jm1.libvirt.server.

libvirt:
  domain: domain
  hardware:
    vcpus: '4'
    memory: '4096'
other_role:
  var1: something
another_role:
  var1: something_else

I realize that the hardware section should be changed to a flat dictionary and disk and network should be separate array variables, but that would still simplify thing.

Dictionaries do not allow redundant keys. hardware must be a list, else passing two network parameters would not be possible.

As for things breaking and becoming not feasible, I have actually found a usecase in trying to make your role work on remote virtual hosts, specifically we cannot do such a playbook:

- name: Install virtual machines
  hosts: virtual_host
  gather_facts: no
  tasks:
  - name: Setup virtual machine 1
    ansible.builtin.import_role:
      name: jm1.libvirt.server
    vars: {{ vm1.virt_vars }}

vars being what breaks. This would be a prerequisite to make Setup virtual machine 1 on a loop. The solution of putting all of the libvirt vars inside the libvirt object is according to this workaround. To properly override variables from the defaults, we simply use default_virt_vars | combine(libvirt.virt_vars) in a set_fact. Creating a separate role that simply passes the variables down and sets its own defaults seems wasteful and a lot of black-box text for a new user reading the derived role.

import_role cannot be used in loops.

PS: disclaimer I have only been doing ansible for 2 days, and these are some of the limitations of the language that I found. I do not know if there is an consensus on how to design this better

Focus less on Ansible roles:

Customizations like virtual-machines-share-most-hardware-specs-but-differ-in-minor-aspects are better handled through inventory group_vars and host_vars instead of trying to add more and more variables to a role such as jm1.libvirt.server.

hardware in jm1.libvirt.server already allows maximum flexibility and thus does not have to be extended. Adding more variables to that role to tweak hardware a bit for your use case will not add any flexibility for others, hence you are looking at the wrong place. Try to grasp the power of Ansible's variable system, variable precedence rules and inventories!

from ansible-collection-jm1-libvirt.

Related Issues (3)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.