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

Collection: Add playbook for direct execution #842

Merged
merged 24 commits into from
Sep 11, 2024

Conversation

rhmk
Copy link
Member

@rhmk rhmk commented Aug 28, 2024

These are 2 playbooks which are added to the playbooks directory.

  • sap_hana_preconfigure.yml can be called directly from the CLI and interactively asked for a minimum viable parameter set for preparing or checking a system for HANA installation. After the variable collection, it imports the next playbook with the collected parameters.
  • sap_hana_preconfigure_exec.yml. This playbook can be called directly or imported into other playbooks with a prepared variable list. The host list is configurable so that the playbook can be used in generic workflows.
    Check the new README file in the playbooks directory for usage details of these roles.

@berndfinger berndfinger self-requested a review August 28, 2024 09:20
@sean-freeman
Copy link
Member

sean-freeman commented Aug 28, 2024

I'm inclined to reject the principle behind this PR:

  • there is already /playbooks/sample-sap-hana-preconfigure.yml that covers this use case
  • this appears to be (primarily focused on) using localhost as the end-users laptop and populate targets from comma-separated list of IP Addresses (into var _sap_systems)
  • this has a two-tier Playbook structure, Playbook calling Playbook, and I feel this will confuse end-users
  • this suggests to an end-user that the Playbook is "best practice" of some kind, but the underlying Ansible Roles have much more functionality - hence the reason why everything under /playbooks is prefixed as sample- because it is just the beginning of what can be achieved

If this is needed for some downstream purpose to provide Support or cleaner end-user "Just Run This" documentation/functionality, then I would suggest this code is instead added into the vendor-specific fork (aka. midstream).

@berndfinger
Copy link
Member

If this is needed for some downstream purpose to provide Support or cleaner end-user "Just Run This" documentation/functionality, then I would suggest this code is instead added into the vendor-specific fork (aka. midstream).

Having playbooks which can be called directly after installing the collection (including upstream collections) has the following advantages:

  • We can remove most of (if not all) examples from the role's README.md files, reducing their size. These sample playbooks in the README.md files can be replaced by an ansible-playbook command and the proper option for calling the interactive playbook.
  • First time users will get a much easier and also faster quick start.
  • It has the potential of increasing the adoption rate because there are fewer hurdles before getting a success when using the affected roles.
  • By offering these playbook in the upstream version, we could also increase the collaboration by the community.

Because we will be leaving the existing sample playbooks with the dashes in their names untouched, existing functionality and scope will remain in place. See also #726 .

@rhmk
Copy link
Member Author

rhmk commented Aug 28, 2024

@sean-freeman

  1. As Bernd said - these playbooks cannot be called directly after installing the collection due to the dash in the name
  2. the playbook adapts dynamically to localhost, all or any possible group defined inventory, so that the targets are not hardcoded as in sample- playbooks. This is a key reason to have them implemented from point of view.
  3. I agree, that the two-tier playbook structure may look confusing, but it is an interactive "pre-playbook" that collects variables for easy entry, the second one is so generic that it can be used almost anywhere (e.g by the ansible playbooks), it just calls two roles and it works.

My intervention is indeed to move away from sample- mid-term and have callable playbooks that can be imported with a few parameters, to do what is needed.
I think we can discuss about the "parameter asking playbook" to have this in downstream only, but the _exec playbook is worth putting here.

@rhmk
Copy link
Member Author

rhmk commented Aug 28, 2024

The PR is BTW a reaction to users who asked for a "directly usable" playbook

Copy link
Member

@berndfinger berndfinger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@rhmk
Copy link
Member Author

rhmk commented Sep 11, 2024

Removed the interactive part to provide a vendor-specific version of it and suggest to go forward with this PR. It makes the roles much easier consumable and reusable, e.g. in ansible-playbooks4sap

@berndfinger berndfinger changed the title Default playbooks for direct execution added to collection Collection: Add playbook for direct execution Sep 11, 2024
Copy link
Member

@berndfinger berndfinger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@berndfinger berndfinger merged commit 58c092a into sap-linuxlab:dev Sep 11, 2024
2 of 3 checks passed
@rhmk rhmk deleted the default-playbooks branch November 28, 2024 07:37
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.

3 participants