kubevirt / kubevirt.core

Lean Ansible bindings for KubeVirt
Apache License 2.0
13 stars 11 forks source link

Inventory: supporting `annotation_variable` #141

Open VannTen opened 2 months ago

VannTen commented 2 months ago
SUMMARY

Hi.

The no-longer maintained community.kubevirt.kubevirt supported an annotation_variable which contained json to be put in the host_vars of the particular hosts. Is that something you would support ?

I've seen in the code that annotations are translated to the {prefix}_annotations host var, but that require post processing to set the actual vars for the host.

ISSUE TYPE
0xFelix commented 2 months ago

Hi!

I think you should be able to do this with a composed host variable using a filter like from_json that takes the {prefix}_annotation var value as input.

See https://kubevirt.io/kubevirt.core/main/plugins/kubevirt.html#parameter-compose https://docs.ansible.com/ansible/latest/collections/ansible/builtin/constructed_inventory.html

VannTen commented 2 months ago

How would you do that ? compose needs static keys, doesn't it ? (I mean:

compose:
  my_var: var1 + var2 
=> valid
---
compose: some_var | from_json
=> not valid

Or am I mistaken ? (I'll try to test that asap)

0xFelix commented 2 months ago

See this example:

server_type: "ansible_hostname | regex_replace ('(.{6})(.{2}).*', '\\2')"

compose takes jinja2 expressions and should be able to access variables discovered earlier (e.g. like ansible_hostname or also annotations).

VannTen commented 2 months ago

Yeah but my point is that server_type can not be dynamic here, i-e, you need to define the composed variables directly in your inventory file.

0xFelix commented 2 months ago

Playbook play-create-annotation-host-vars.yml:

---
- name: Playbook creating a virtual machine with annotations
  hosts: localhost
  tasks:
    - name: Create VM
      kubevirt.core.kubevirt_vm:
        state: present
        name: testvm-with-annotations
        namespace: default
        running: false
        annotations:
          ansible: '{"ann_var_1": "ann_val_1", "ann_var_2": "ann_val_2"}'

Inventory configuration annotation.kubevirt.yml:

---
plugin: kubevirt.core.kubevirt
compose:
  ansible_annotation: vm_annotations["ansible"] | from_json

gives me following output

{
    "_27_0_0_1_42645": {
        "children": [
            "namespace_default"
        ]
    },
    "_meta": {
        "hostvars": {
            "default-testvm-with-annotations": {
                "ansible_annotation": {
                    "ann_var_1": "ann_val_1",
                    "ann_var_2": "ann_val_2"
                },
                "vm_annotations": {
                    "ansible": "{\"ann_var_1\": \"ann_val_1\", \"ann_var_2\": \"ann_val_2\"}",
                    "kubevirt.io/latest-observed-api-version": "v1",
                    "kubevirt.io/storage-observed-api-version": "v1"
                },
                "vm_conditions": [
                    {
                        "lastProbeTime": "2024-09-16T14:38:48Z",
                        "lastTransitionTime": "2024-09-16T14:38:48Z",
                        "message": "VMI does not exist",
                        "reason": "VMINotExists",
                        "status": "False",
                        "type": "Ready"
                    }
                ],
                "vm_printable_status": "Stopped",
                "vm_resource_version": "4297",
                "vm_uid": "abf369db-7718-447b-81d1-abfefcd7f3aa"
            }
        }
    },
    "all": {
        "children": [
            "ungrouped",
            "_27_0_0_1_42645"
        ]
    },
    "namespace_default": {
        "hosts": [
            "default-testvm-with-annotations"
        ]
    }
}

With this I am able to access the parsed vars with hostvars["default-testvm-with-annotations"]["ansible_annotation"]["ann_var_1"].

Does that work for you?

VannTen commented 2 months ago

Yes, this works, but ansible_annotation is static here, there is no way to set variables in the "hostvars root" i.e if you want hostvars["default-testvm-with-annotations"]["ann_var_1"] you need to modify annotation.kubevirt.yml.

I could go around that with set_fact (I think) but it would not exactly be elegant.

0xFelix commented 2 months ago

I see, but does it really hurt to have this level grouping? Instead of maintaining special code for this feature, we use the constructed inventory already provided by Ansible. With regards to that I think it is an acceptable "downside".

VannTen commented 2 months ago

It does not hurt that much but it would certainly be convenient. Looking at the code, I think I can see a way to add it without much more code. Would you consider a PR implementing this ?

0xFelix commented 2 months ago

Do other inventories apart from the old community.kubevirt inventory have such a feature? I'd like to stick to Ansible conventions if possible.

VannTen commented 2 months ago

Checking for gcp or aws, they don't.

What they do, however, is controlling the prefix of variables:

Maybe an alternative to achieve the same purpose would be to have the option to flatten the vm_annotations host_var and control the prefix ?

0xFelix commented 2 months ago

Annotations can be added from VMs and VMIs, that's a distinction in KubeVirt. I'm all for customizations and such but I don't think this adds a lot of value. In the end some name has to be chosen anyway. Why make it more complicated than it needs to be?