meffie / molecule-proxmox

Molecule driver for Proxmox VE
MIT License
36 stars 8 forks source link

Add requests and add ssh_port to options #19

Closed akrami closed 2 months ago

akrami commented 2 months ago

In our infra, we are doing the whole templates with custom ssh port, which was not supported by your module. in this change I'm reading the ssh_port from the options but also with a fallback to 22. The other change is dependency to requests module.

meffie commented 2 months ago

Thank you. Do you know why we need to add the requests module dependency?

akrami commented 2 months ago

in the create step on the TASK [Create molecule instance(s).] I get this error:

failed: [localhost] (item=test-srv) => {"ansible_loop_var": "p", "changed": false, "module_stderr": "Chosen backend requires 'requests' module\n\n", "module_stdout": "", "msg": "MODULE FAILURE\nSee stdout/stderr for the exact error", "p": {"name": "test-srv", "newid": 1200}, "rc": 1}

it seems requests is a dependency for proxmox_kvm: https://docs.ansible.com/ansible/latest/collections/community/general/proxmox_kvm_module.html#requirements

meffie commented 2 months ago

Thank you. Yes, I see I missed that dependency. Thank you for the fix! Let me get this one merged.

IamLunchbox commented 2 months ago

Wouldn't it be helpful to enable alternative ports not only at the options level, but as well on the platform level?

For example like that:

user: "{{ options.ssh_user | d(ra.rc.p.ssh_user, true) | d('molecule') }}" # while we are at it
port: "{{ options.ssh_port  | d(ra.rc.p.ssh_port, true) | d('22') }}"
meffie commented 2 months ago

Wouldn't it be helpful to enable alternative ports not only at the options level, but as well on the platform level?

For example like that:

user: "{{ options.ssh_user | d(ra.rc.p.ssh_user, true) | d('molecule') }}" # while we are at it
port: "{{ options.ssh_port  | d(ra.rc.p.ssh_port, true) | d('22') }}"

Yes, I think that could be added later. I think it would first check the platform level, and if not found, check the drivers options, and finally fall back to default value, something like (untested)

port: "{{ ra.rc.p.ssh_port | d(options.ssh_port, true) | d(22, true) }}" (and similar for the user)