ansible-role-for-splunk icon indicating copy to clipboard operation
ansible-role-for-splunk copied to clipboard

Published 20 hours ago • verified publisher icon splunk

feat(install_utilities): send list to package module

Open zyphermonkey opened this issue 3 years ago • 1 comments

Instead of looping through the list it is recommended to send a list to apt/yum when install multiple packages.

When used with a loop: each package will be processed individually, it is much more efficient to pass the list directly to the name option. https://docs.ansible.com/ansible/latest/collections/ansible/builtin/yum_module.html

zyphermonkey avatar Oct 12 '22 17:10 zyphermonkey

Thanks for the PR. Sorry I was out for a bit, so I wasn't able to have a look at it.

This is a goo suggestion, but the reason it loops it because if a certain package does not exist, the whole operation will fail, and it will not install any packages at all.

This has been suggested before, but the packages should be split out by platform and version. For example, policycoreutils-python is RHEL only.

Any suggestions?

dtwersky avatar Oct 24 '22 12:10 dtwersky

Okay to that explains the ignore_errors: true which I'm not a fan of and thought shouldn't be there.

I can come up with 2 idea.

  1. remove the packages from the list that don't have common names between the various repositories.
    This isn't ideal, but a quick fix

  2. Move lists to vars I like this idea the best, but will require me to move

    - name: Include OS family-specific variables
   include_vars: "{{ ansible_os_family }}.yml"

    from configure_os.yml to main.yml so the vars are available for all tasks.

    I'm willing to do the work, but want to make sure you're okay with the change first. If so I'll need a list of package names for Debian as I only have access to RHEL currently.

zyphermonkey avatar Oct 26 '22 12:10 zyphermonkey

Thoughts?

zyphermonkey avatar Nov 03 '22 10:11 zyphermonkey

@zyphermonkey I think that's the way to do it. The only problem is that some OS versions differ as well, and then you'll need to have separate vars for ansible_distribution_version as well. But I think this is a good start, and we can tackle the others as they come along.

dtwersky avatar Nov 08 '22 16:11 dtwersky

tested on ubuntu2204 and centos7

zyphermonkey avatar Nov 12 '22 12:11 zyphermonkey

@zyphermonkey Thanks for the PR. At first glance, it looks good, and the includes being in main.yml makes a lot more sense.

I will try to test it on some more platforms and scenarios sometime this week.

dtwersky avatar Nov 14 '22 16:11 dtwersky

RedHat8.yml var file was added for RHEL8 specific packages. Tested on Ubuntu 16/18/20/22.04 CentOS 7/8/8 Stream.

dtwersky avatar Nov 16 '22 02:11 dtwersky