openstack-k8s-operators / edpm-ansible

External Dataplane Management Ansible Playbooks
https://openstack-k8s-operators.github.io/edpm-ansible/
Apache License 2.0
9 stars 65 forks source link

Replacing shell wrapped python script with a module #667

Closed jpodivin closed 1 month ago

jpodivin commented 3 months ago

Existing implementation of the role is working fine. But it doesn't align with our current best practice. This PR replace script with proper module, including error handling, parameter sanity check, documentation etc. This module will also provide generic way to get contents of any ini file on target machine.

Approach utilizing ansible.builitin.ini lookup proved workable. However, since the look up requires file to be present in one of pre-set search paths, its use would necessitate further changes to the role.Possibly including provision for more arguments.

When compared with that, a simple custom module seemed preferable.

openshift-ci[bot] commented 3 months ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jpodivin

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/openstack-k8s-operators/edpm-ansible/blob/main/OWNERS)~~ [jpodivin] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment
softwarefactory-project-zuul[bot] commented 3 months ago

Build failed (check pipeline). Post recheck (without leading slash) to rerun all jobs. Make sure the failure cause has been resolved before you rerun jobs.

https://review.rdoproject.org/zuul/buildset/ef510c55ec9e430ebd9f20e7a225c99a

:heavy_check_mark: openstack-k8s-operators-content-provider SUCCESS in 3h 11m 55s :x: podified-multinode-edpm-deployment-crc FAILURE in 2h 15m 11s :heavy_check_mark: cifmw-crc-podified-edpm-baremetal SUCCESS in 1h 49m 59s :heavy_check_mark: edpm-ansible-molecule-edpm_bootstrap SUCCESS in 7m 24s :heavy_check_mark: edpm-ansible-molecule-edpm_podman SUCCESS in 5m 18s :heavy_check_mark: edpm-ansible-molecule-edpm_module_load SUCCESS in 4m 39s :heavy_check_mark: edpm-ansible-molecule-edpm_kernel SUCCESS in 9m 39s :heavy_check_mark: edpm-ansible-molecule-edpm_libvirt SUCCESS in 9m 15s :heavy_check_mark: edpm-ansible-molecule-edpm_nova SUCCESS in 9m 33s :heavy_check_mark: edpm-ansible-molecule-edpm_frr SUCCESS in 6m 59s :heavy_check_mark: edpm-ansible-molecule-edpm_iscsid SUCCESS in 4m 35s :heavy_check_mark: edpm-ansible-molecule-edpm_ovn_bgp_agent SUCCESS in 8m 08s :heavy_check_mark: edpm-ansible-molecule-edpm_ovs SUCCESS in 5m 00s :heavy_check_mark: edpm-ansible-molecule-edpm_tripleo_cleanup SUCCESS in 3m 59s

softwarefactory-project-zuul[bot] commented 3 months ago

Build failed (check pipeline). Post recheck (without leading slash) to rerun all jobs. Make sure the failure cause has been resolved before you rerun jobs.

https://review.rdoproject.org/zuul/buildset/b0c7afb66ce84f9f97ae857e984f36c5

:heavy_check_mark: openstack-k8s-operators-content-provider SUCCESS in 2h 35m 38s :x: podified-multinode-edpm-deployment-crc FAILURE in 2h 18m 36s :heavy_check_mark: cifmw-crc-podified-edpm-baremetal SUCCESS in 1h 57m 25s :heavy_check_mark: edpm-ansible-molecule-edpm_bootstrap SUCCESS in 6m 11s :heavy_check_mark: edpm-ansible-molecule-edpm_podman SUCCESS in 5m 11s :heavy_check_mark: edpm-ansible-molecule-edpm_module_load SUCCESS in 4m 41s :heavy_check_mark: edpm-ansible-molecule-edpm_kernel SUCCESS in 9m 25s :heavy_check_mark: edpm-ansible-molecule-edpm_libvirt SUCCESS in 8m 10s :heavy_check_mark: edpm-ansible-molecule-edpm_nova SUCCESS in 8m 25s :heavy_check_mark: edpm-ansible-molecule-edpm_frr SUCCESS in 6m 21s :heavy_check_mark: edpm-ansible-molecule-edpm_iscsid SUCCESS in 4m 31s :heavy_check_mark: edpm-ansible-molecule-edpm_ovn_bgp_agent SUCCESS in 6m 45s :heavy_check_mark: edpm-ansible-molecule-edpm_ovs SUCCESS in 4m 55s :heavy_check_mark: edpm-ansible-molecule-edpm_tripleo_cleanup SUCCESS in 4m 04s

softwarefactory-project-zuul[bot] commented 3 months ago

Build failed (check pipeline). Post recheck (without leading slash) to rerun all jobs. Make sure the failure cause has been resolved before you rerun jobs.

https://review.rdoproject.org/zuul/buildset/d8a93171abb048af8d6ec08fe8e50cfe

:heavy_check_mark: openstack-k8s-operators-content-provider SUCCESS in 2h 29m 07s :x: podified-multinode-edpm-deployment-crc FAILURE in 2h 08m 12s :heavy_check_mark: cifmw-crc-podified-edpm-baremetal SUCCESS in 1h 51m 45s :heavy_check_mark: edpm-ansible-molecule-edpm_bootstrap SUCCESS in 6m 02s :heavy_check_mark: edpm-ansible-molecule-edpm_podman SUCCESS in 5m 11s :heavy_check_mark: edpm-ansible-molecule-edpm_module_load SUCCESS in 4m 38s :heavy_check_mark: edpm-ansible-molecule-edpm_kernel SUCCESS in 9m 22s :heavy_check_mark: edpm-ansible-molecule-edpm_libvirt SUCCESS in 8m 01s :heavy_check_mark: edpm-ansible-molecule-edpm_nova SUCCESS in 8m 42s :heavy_check_mark: edpm-ansible-molecule-edpm_frr SUCCESS in 6m 22s :heavy_check_mark: edpm-ansible-molecule-edpm_iscsid SUCCESS in 4m 33s :heavy_check_mark: edpm-ansible-molecule-edpm_ovn_bgp_agent SUCCESS in 6m 53s :heavy_check_mark: edpm-ansible-molecule-edpm_ovs SUCCESS in 5m 07s :heavy_check_mark: edpm-ansible-molecule-edpm_tripleo_cleanup SUCCESS in 4m 03s

softwarefactory-project-zuul[bot] commented 3 months ago

Build failed (check pipeline). Post recheck (without leading slash) to rerun all jobs. Make sure the failure cause has been resolved before you rerun jobs.

https://review.rdoproject.org/zuul/buildset/a0f897dc344a4bbc9ca3e7a78be14be5

:heavy_check_mark: openstack-k8s-operators-content-provider SUCCESS in 2h 45m 09s :x: podified-multinode-edpm-deployment-crc FAILURE in 2h 08m 40s :heavy_check_mark: cifmw-crc-podified-edpm-baremetal SUCCESS in 1h 54m 02s :heavy_check_mark: edpm-ansible-molecule-edpm_bootstrap SUCCESS in 6m 13s :heavy_check_mark: edpm-ansible-molecule-edpm_podman SUCCESS in 5m 10s :heavy_check_mark: edpm-ansible-molecule-edpm_module_load SUCCESS in 4m 40s :heavy_check_mark: edpm-ansible-molecule-edpm_kernel SUCCESS in 9m 18s :heavy_check_mark: edpm-ansible-molecule-edpm_libvirt SUCCESS in 8m 09s :heavy_check_mark: edpm-ansible-molecule-edpm_nova SUCCESS in 9m 42s :heavy_check_mark: edpm-ansible-molecule-edpm_frr SUCCESS in 6m 15s :heavy_check_mark: edpm-ansible-molecule-edpm_iscsid SUCCESS in 4m 32s :heavy_check_mark: edpm-ansible-molecule-edpm_ovn_bgp_agent SUCCESS in 6m 55s :heavy_check_mark: edpm-ansible-molecule-edpm_ovs SUCCESS in 5m 03s :heavy_check_mark: edpm-ansible-molecule-edpm_tripleo_cleanup SUCCESS in 4m 01s

jpodivin commented 3 months ago

recheck

softwarefactory-project-zuul[bot] commented 3 months ago

Build failed (check pipeline). Post recheck (without leading slash) to rerun all jobs. Make sure the failure cause has been resolved before you rerun jobs.

https://review.rdoproject.org/zuul/buildset/df541c71ac5d46c8ab83031f115a0104

:heavy_check_mark: openstack-k8s-operators-content-provider SUCCESS in 1h 32m 58s :x: podified-multinode-edpm-deployment-crc FAILURE in 1h 01m 06s :x: cifmw-crc-podified-edpm-baremetal FAILURE in 1h 09m 13s :heavy_check_mark: edpm-ansible-molecule-edpm_bootstrap SUCCESS in 6m 12s :heavy_check_mark: edpm-ansible-molecule-edpm_podman SUCCESS in 5m 11s :heavy_check_mark: edpm-ansible-molecule-edpm_module_load SUCCESS in 4m 37s :heavy_check_mark: edpm-ansible-molecule-edpm_kernel SUCCESS in 9m 23s :heavy_check_mark: edpm-ansible-molecule-edpm_libvirt SUCCESS in 8m 03s :heavy_check_mark: edpm-ansible-molecule-edpm_nova SUCCESS in 8m 39s :heavy_check_mark: edpm-ansible-molecule-edpm_frr SUCCESS in 6m 18s :heavy_check_mark: edpm-ansible-molecule-edpm_iscsid SUCCESS in 4m 34s :heavy_check_mark: edpm-ansible-molecule-edpm_ovn_bgp_agent SUCCESS in 6m 47s :heavy_check_mark: edpm-ansible-molecule-edpm_ovs SUCCESS in 5m 00s :heavy_check_mark: edpm-ansible-molecule-edpm_tripleo_cleanup SUCCESS in 3m 57s

rebtoor commented 3 months ago

recheck

SeanMooney commented 2 months ago

im not sure which best pratics this is meant to be conforming too.

i know we have stated that we should prefer using existing pyton modules instead of shell commands but i don't generally agree we should write and maintain our own modules to replace all usage of command or shell.

SeanMooney commented 2 months ago

im kind of neutral on this. without proper unit testing of the module i don't think this adds much