Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(install_utilities): send list to package module #138

Merged
merged 4 commits into from
Nov 16, 2022

Conversation

zyphermonkey
Copy link
Contributor

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

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.*
@dtwersky
Copy link
Collaborator

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?

@zyphermonkey
Copy link
Contributor Author

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
Copy link
Contributor Author

Thoughts?

@dtwersky
Copy link
Collaborator

dtwersky commented Nov 8, 2022

@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.

Merrel Raney III added 2 commits November 12, 2022 11:48
vars are needed in various task files
best to include them early to ensure they're always available
@zyphermonkey
Copy link
Contributor Author

tested on ubuntu2204 and centos7

@dtwersky
Copy link
Collaborator

@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
Copy link
Collaborator

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

@dtwersky dtwersky merged commit 8050090 into splunk:master Nov 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants