ansible-middleware / amq

A collection to manage AMQ brokers
Apache License 2.0
17 stars 11 forks source link

Tasks configure.yml and upgrade.yml are part of the systemd.yml? #10

Closed RobertFloor closed 1 year ago

RobertFloor commented 2 years ago
SUMMARY

Hi, thank you so far for the nice project. I was reviewing the code and was wondering why the tasks configure.yml and upgrade.yml are part of the systemd.yml task. I think it would be more logical to include them as separate tasks in the main.yml. Would it be possible to change this or are you open to a pull request which would include them as separate tasks in the main.yml?

Thank you in advance,

Also the current main.yaml is a combination of include_tasks and specific log tasks which is not the most logical combination

- name: Check prerequisites
  ansible.builtin.include_tasks: prereqs.yml
  tags:
    - prereqs

- name: Include firewall config tasks
  ansible.builtin.include_tasks: firewalld.yml
  when: amq_broker_configure_firewalld
  tags:
    - firewall

- name: Include install tasks
  ansible.builtin.include_tasks: install.yml
  tags:
    - install

- name: Include systemd tasks
  ansible.builtin.include_tasks: systemd.yml
  tags:
    - systemd

- name: Create default logs directory
  become: yes
  ansible.builtin.file:
    state: directory
    dest: "/var/log/{{ amq_broker.service_name }}"
    owner: "{{ amq_broker_service_user }}"
    group: "{{ amq_broker_service_group }}"
    mode: 0750

- name: Link default logs directory
  become: yes
  ansible.builtin.file:
    state: link
    src: "{{ amq_broker.instance_home }}/log"
    dest: "/var/log/{{ amq_broker.service_name }}/{{ amq_broker.instance_name }}"
guidograzioli commented 2 years ago

Hello @RobertFloor , PRs are always welcomed. If you wish to separate the setup tasks from the systemd tasks I have nothing in contrary. At that point we could even provide a flag to indicate whether to execute the systemd steps or not. Mind that we are about to rename all variables shortly... you might prefer to base your PR on top the renaming

RobertFloor commented 2 years ago

Hi, Thanks for your response. Previously we developed AMQ playbooks based on this code (https://github.com/redhat-cop/jboss_amq) with several modifications. We are investigating if we can switch over to this playbook. Thank you for the help so far

guidograzioli commented 1 year ago

Closed with #43