Skip to content

Update general inventory file vars appendix #3310

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 4 commits into from
Apr 29, 2025
Merged

Update general inventory file vars appendix #3310

merged 4 commits into from
Apr 29, 2025

Conversation

michellemacrh
Copy link
Contributor

Updates the general inventory file vars appendix and adds a redis vars file for redis specific vars.

General variables - Appendix Inventory file variables improvements

https://issues.redhat.com/browse/AAP-40600

@michellemacrh michellemacrh added Ready for technical review Content is ready for technical reviews 2.5 Content applies to AAP 2.5 Needs backport to 2.5 Changes still need to be applied to the AAP 2.5 branch labels Apr 22, 2025
dsavineau
dsavineau reviewed Apr 24, 2025
View reviewed changes
downstream/modules/platform/ref-redis-inventory-variables.adoc Outdated
Comment on lines 52 to 53
| `redis_tls_cert`
|
Copy link

Choose a reason for hiding this comment

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

Suggested change
| `redis_tls_cert`
|
| `redis_tls_cert`
| `redis_tls_cert`

Looks like we're missing redis_tls_{cert,key} variables from the containerized README but those exist in the code

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated thanks for catching that!
Will I create a Jira to update the README or is it OK?

downstream/modules/platform/ref-general-inventory-variables.adoc
| Optional
| `rhel8`

| `use_archive_compression`
Copy link

Choose a reason for hiding this comment

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

use_archive_compression and use_db_compression (as well as their component prefixed variables) also exist in containerized

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm hoping to add those as part of AAP-39129

simaishi
simaishi reviewed Apr 24, 2025
View reviewed changes
downstream/modules/platform/ref-general-inventory-variables.adoc Outdated
| `bundle_install`
| Controls whether or not to perform an offline or bundled installation. Set this variable to `true` to enable an offline or bundled installation.
| Optional
| `false`
Copy link

Choose a reason for hiding this comment

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

This value is set to true automatically if running bundle installer, for both RPM and containerized.

downstream/modules/platform/ref-general-inventory-variables.adoc Outdated
| `bundle_dir`
| Path to the bundle directory used when performing a bundle install.
| Required if `bundle_install=true`
| `false`
Copy link

Choose a reason for hiding this comment

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

The default is /var/lib/ansible-automation-platform-bundle for RPM installer, <current dir>/bundle for containerized.

downstream/modules/platform/ref-general-inventory-variables.adoc Outdated
| Optional
| `true`

| `precreate_partition_hours`
Copy link

Choose a reason for hiding this comment

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

This is a variable used only for controller.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

will move this var to the controller vars file

downstream/modules/platform/ref-general-inventory-variables.adoc Outdated

This variable is used as a host variable for particular hosts and not under the `[all:vars]` section.

| Required if `registry_auth=true`
Copy link

@simaishi simaishi Apr 24, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

same as password, need to be set if cred is required in RPM installer.

downstream/modules/platform/ref-general-inventory-variables.adoc Outdated

The username credential for access to the registry source defined in `registry_url`.

| Required if `registry_auth=true`
Copy link

Choose a reason for hiding this comment

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

registry_auth is a variable used in containerized installer only. For RPM installer, the var needs to be set if password is required to access registry_url.

downstream/modules/platform/ref-general-inventory-variables.adoc Outdated
| `routable_hostname`
| `routable_hostname`
| Used if the machine running the installation program can only route to the target host through a specific URL.
For example, if you use short names in your inventory, but the node running the installation program can only resolve that host by using a FQDN. If `routable_hostname` is not set, it should default to `ansible_host`.
Copy link

Choose a reason for hiding this comment

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

it should default to ansible_host -> It defaults to

@michellemacrh
Copy link
Contributor Author

Thank you for the feedback Satoe :)

@michellemacrh michellemacrh added Technical review complete Content has been reviewed for technical accuracy and completeness and removed Ready for technical review Content is ready for technical reviews labels Apr 28, 2025
g-murray
g-murray approved these changes Apr 29, 2025
View reviewed changes
Copy link
Contributor

@g-murray g-murray left a comment

Choose a reason for hiding this comment

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

Just the one comment, just a suggestion feel free to keep what you have. LGTM

downstream/modules/platform/ref-general-inventory-variables.adoc Outdated

| `aap_service_san_records`
|
| A list of additional SAN records for signing a service. These should be assigned to components in the inventory file as host variables rather than group or all variables. All strings should also contain their corresponding SAN option prefix such as `DNS:` or `IP:`.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested rewrite for active voice: "Assign these to components in the inventory file as host variables, not group or all variables."

Also for the last sentence, could you sub out "Should" for "must"? Should gives the user an option, but if they need these option prefixes lets just tell em.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Super feedback TY! Will implement

@michellemacrh
Update general inventory file vars appendix
d605531 
Updates the general inventory file vars appendix and adds a redis vars file for redis specific vars.

General variables - Appendix Inventory file variables improvements

https://issues.redhat.com/browse/AAP-40600
@michellemacrh
Updates based on SME feedback
21e0a5d
@michellemacrh
updates based on SME feedback
4d60bd7
@michellemacrh
Updates based on peer review feedback
26a6d2f
@g-murray g-murray merged commit 5617e9f into ansible:main Apr 29, 2025
michellemacrh added a commit to michellemacrh/aap-docs that referenced this pull request Apr 29, 2025
@michellemacrh
Update general inventory file vars appendix (ansible#3310)
fd3a1e8 
* Update general inventory file vars appendix

Updates the general inventory file vars appendix and adds a redis vars file for redis specific vars.

General variables - Appendix Inventory file variables improvements

https://issues.redhat.com/browse/AAP-40600

* Updates based on SME feedback

* updates based on SME feedback

* Updates based on peer review feedback
@michellemacrh michellemacrh mentioned this pull request Apr 29, 2025
Merged
ariordan-redhat pushed a commit that referenced this pull request Apr 29, 2025
@michellemacrh
2.5 Update general inventory file vars appendix (#3310) (#3357)
8c74dfe 
* Update general inventory file vars appendix

Updates the general inventory file vars appendix and adds a redis vars file for redis specific vars.

General variables - Appendix Inventory file variables improvements

https://issues.redhat.com/browse/AAP-40600

* Updates based on SME feedback

* updates based on SME feedback

* Updates based on peer review feedback
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dsavineau dsavineau dsavineau left review comments

@simaishi simaishi simaishi left review comments

@g-murray g-murray g-murray approved these changes

Assignees
No one assigned
Labels
2.5 Content applies to AAP 2.5 Needs backport to 2.5 Changes still need to be applied to the AAP 2.5 branch Technical review complete Content has been reviewed for technical accuracy and completeness
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@michellemacrh @dsavineau @simaishi @g-murray