Simplify for loop/regex in Linux_installmq.yml

[CharlieParker]

Discussed early on with @scottcurtis2605, the following Find required package files task is rather confusing:

mq-ansible/ansible_collections/ibm/ibmmq/roles/installmq/tasks/Linux_installmq.yml

Lines 18 to 60 in 7243a4a

	- name: Find required package files
	find:
	paths: /var/MQServer
	use_regex: true
	patterns: "{{ item }}"
	register: package_files
	with_items:
	- (?i).*runtime.*
	- (?i).*gskit.*
	- (?i).*server.*
	- (?i).*java.*
	- (?i).*jre.*
	- (?i).*sdk.*
	- (?i).*samples.*
	- (?i).*man.*
	- (?i).*client.*
	- (?i).*amqp.*
	- (?i).*ams.*
	- (?i).*web.*
	- (?i).*(-|_)es.*
	- (?i).*(-|_)cn.*
	# - '(?i).*ftbase.*'
	# - '(?i).*ftlogger.*'
	# - '(?i).*fttools.*'
	# - '(?i).*ftagent.*'
	# - '(?i).*ftservice.*'
	# - '(?i).*xrservice.*'
	# - '(?i).*sfbridge.*'
	# - '(?i).*bcbridge.*'
	# - '(?i).*(-|_)de.*'
	# - '(?i).*(-|_)fr.*'
	# - '(?i).*(-|_)ja.*'
	# - '(?i).*(-|_)it.*'
	# - '(?i).*(-|_)ko.*'
	# - '(?i).*(-|_)ru.*'
	# - '(?i).*(-|_)pt.*'
	# - '(?i).*(-|_)hu.*'
	# - '(?i).*(-|_)pl.*'
	# - '(?i).*(-|_)cs.*'
	# - '(?i).*(-|_)tw.*'
	when:
	- '"MQSeriesRuntime" not in ansible_facts.packages'
	- '"ibmmq-runtime" not in ansible_facts.packages'

I'd like to discuss this more, we're fairly sure it can be made cleaner but there might be something we're missing.

Initial Thoughts

• find files in a certain directory or path /var/MQServer
• On each iteration the patterns key will have a single value "{{ item }}"
• What does package_files look like throughout this process? My understanding is it will be rewritten based on the register in every iteration

I think if the intention is to select only files that match a certain regex pattern, then this needs to be rethought.

Luckily it would be simplifying the code/process, I believe find and the following regex should be sufficient:

(?i)(?:.*runtime|gskit|server|java|jre|sdk|samples|man|client|amqp|ams|web|.*(-|_)es|.*(-|_)cn).*

[J-295]

The .* at the beginning of the non-capturing group would only work for "runtime" and not the other | separated parts. Placing the .* before the group instead will fix this:
(?i).*(?:runtime|gskit|server|java|jre|sdk|samples|man|client|amqp|ams|web|.*(-|_)es|.*(-|_)cn).*
This can be optimised to:
(?i)^.*?(?:runtime|gskit|server|java|jre|sdk|samples|man|client|amqp|ams|web|[-_](?:es|cn)).*$

[bimsara-yasitha01]

Hi @CharlieParker @James-Bennett-295, this is great stuff!

The initial design for this list was to give customers the freedom to select/deselect which components they'd like to install (across both debian and rpm package file patterns) - but you both raise a good point in that a lot of the default components in the list are part of the core MQ installation, where customisation doesn't make sense.

So I'll pass this back to you, do you guys think it would be valuable to have the core installation components as part of one regex pattern as you have proposed, PLUS the additional components (like samples, ams etc.) as single regex patterns for customers to select?

Either way, encapsulating the core mq installation components into one regex pattern would be a welcome optimisation to the codebase!

[CharlieParker]

If the intention is to have things like samples and ams (that's this right? 😅) as optional or configurable to the user who simply wants to automate the deployment of mq then I think it shouldn't be done by commenting out regex in an ansible playbook.

I think the options should be exposed perhaps on the cli or even be set in a config file if they're not trivial.
I'm imagining the commented out lines are options we don't typically need in a default deployment then?

I think it's perhaps worth us discussing this list and grouping options to make customer selection as intuitive as possible.

[martinevansibm]

I would recommend keeping the regex as simple as possible, if that means a couple of extra entries I would prefer that over a complex regex. Customers should be aware of and in control of the packages they are installing, the list makes a suggestion but it is easy to modify in its current form.

[CharlieParker]

Potentially useful for further conversation is this snippet that I used to check the logic here:

---
- name: Local test
  hosts: localhost
  # hosts: [amd64]
  tasks:
    - name: What dir are we in?
      ansible.builtin.debug:
        msg: The DIR we're in is {{ playbook_dir }}

    - name: Find required package files
      ansible.builtin.find:
        paths: "{{ playbook_dir }}"
        use_regex: true
        patterns: "{{ item }}"
      register: package_files
      with_items:
        - "*.yml"
        - .*mq.*
        - "*.py"

    - name: Debug package_files from above regex loop
      ansible.builtin.debug:
        var: package_files

I would say, even if you want to stick with the regex implementation for this, I'd break up the logic in the single task Find required package files as there's a few things going on there and it would be more readable broken up a little.

I'd also say we should perhaps define the lists of patterns elsewhere such that we only need to specify an abstracted variable or two (rather than look at a long list of regex patterns)