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.

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

Add more details on host group settings and nested host groups #2685

Merged
merged 17 commits into from
Jan 26, 2024
Merged

Add more details on host group settings and nested host groups #2685

merged 17 commits into from
Jan 26, 2024

Conversation

aneta-petrova
Copy link
Member

BZ#2255404 requests expanding the existing description of host group settings, nested host groups, and how the settings are inherited through the hierarchy of nested host groups. Because the conceptual information is now more extensive, I split it into a separate module, which then required moving all host group-related modules into a separate assembly.

Please cherry-pick my commits into:

  • Foreman 3.9/Katello 4.11 (planned Satellite 6.15)
  • Foreman 3.8/Katello 4.10
  • Foreman 3.7/Katello 4.9 (Satellite 6.14)
  • Foreman 3.6/Katello 4.8
  • Foreman 3.5/Katello 4.7 (Satellite 6.13; orcharhino 6.6)
  • Foreman 3.4/Katello 4.6 (EL8 only)
  • Foreman 3.3/Katello 4.5 on EL7 & EL8 (Satellite 6.12 on EL8 only; orcharhino 6.4/6.5 on EL8 only)
  • Foreman 3.2/Katello 4.4 on EL7 & EL8
  • Foreman 3.1/Katello 4.3 on EL7 & EL8 (Satellite 6.11 EL7/8; orcharhino 6.3 on EL7/8)
  • We do not accept PRs for Foreman older than 3.1.

@github-actions GitHub Actions
Copy link

github-actions bot commented Jan 19, 2024 •
edited
Loading

The PR preview for 906b853 is available at theforeman-foreman-documentation-preview-pr-2685.surge.sh

The following output files are affected by this PR:

show diff

show diff as HTML

@aneta-petrova aneta-petrova force-pushed the 2255404_hgroup_field_inherit branch from 049fad1 to a410693 Compare January 19, 2024 17:43
@aneta-petrova aneta-petrova added Needs tech review Requires a review from the technical perspective and removed Not yet reviewed labels Jan 19, 2024
@aneta-petrova
Copy link
Member Author

Hi @jeremylenz This is my attempt to resolve BZ#2255404, can you please review? It turned out to be pretty tricky, so I'd appreciate it if you could read it carefully. For you tech review, reading only con_host-group-settings-and-nested-host-groups.adoc (2.8.1. Host Group Settings and Nested Host Groups) should be enough.

jeremylenz
jeremylenz reviewed Jan 19, 2024
View reviewed changes
Copy link
Contributor

@jeremylenz jeremylenz left a comment

Choose a reason for hiding this comment

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

This is looking great! Everything seems accurate so far, besides the few comments below.

Also would be good to get input from @ShimShtein and/or @ares

guides/common/modules/con_host-group-settings-and-nested-host-groups.adoc Outdated Show resolved Hide resolved
guides/common/modules/con_host-group-settings-and-nested-host-groups.adoc Outdated Show resolved Hide resolved
guides/common/modules/con_host-group-settings-and-nested-host-groups.adoc Outdated Show resolved Hide resolved
aneta-petrova and others added 2 commits January 19, 2024 19:08
@aneta-petrova @jeremylenz
Fixes from tech review
169c7bd 
Co-authored-by: Jeremy Lenz <jlenz@redhat.com>
@aneta-petrova
s/NOTE:/IMPORTANT:/
f1c81d4
@aneta-petrova
Copy link
Member Author

This is looking great! Everything seems accurate so far, besides the few comments below.

Also would be good to get input from @ShimShtein and/or @ares

Thanks!

Also, @ares, just in case -- I'm aware of BZ#2253496 which seems related, but I want to handle it separately.

@jeremylenz
Copy link
Contributor

And see also :) Katello/katello#10841

ShimShtein
ShimShtein reviewed Jan 21, 2024
View reviewed changes
Copy link
Member

@ShimShtein ShimShtein left a comment

Choose a reason for hiding this comment

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

Worth also mentioning that the values are copied into the host object. This means that changes made later to the hostgroup are not propagated to hosts that are already provisioned.
In the UI there is an option to edit the host instance and force re-inheritance of the properties for the current state.

guides/common/modules/con_host-group-settings-and-nested-host-groups.adoc Outdated Show resolved Hide resolved
guides/common/modules/con_host-group-settings-and-nested-host-groups.adoc Outdated Show resolved Hide resolved
@aneta-petrova @ShimShtein
Fixes from SME review
4d850ef 
Co-authored-by: Shimon Shtein <sshtein@redhat.com>
@aneta-petrova aneta-petrova mentioned this pull request Jan 23, 2024
Closed
10 tasks
@aneta-petrova
Copy link
Member Author

Thanks for the review @ShimShtein! I applied the changes you suggested, except for this comment:

Worth also mentioning that the values are copied into the host object. This means that changes made later to the hostgroup are not propagated to hosts that are already provisioned. In the UI there is an option to edit the host instance and force re-inheritance of the properties for the current state.

Because this information is needed when users actually change host groups, it will be best to include it in a follow-up PR I'm preparing that targets procedure modules: #2689

@aneta-petrova aneta-petrova added tech review done No issues from the technical perspective Needs style review Requires a review from docs style/grammar perspective and removed Needs tech review Requires a review from the technical perspective labels Jan 23, 2024
@aneta-petrova aneta-petrova marked this pull request as ready for review January 23, 2024 06:10
Lennonka
Lennonka requested changes Jan 23, 2024
View reviewed changes
Copy link
Contributor

@Lennonka Lennonka left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Great job!
I'll review it more thoroughly once the new assembly is available in the preview.

guides/common/assembly_working-with-host-groups.adoc Outdated Show resolved Hide resolved
@pr-processor pr-processor bot added the Waiting on contributor Requires an action from the author label Jan 23, 2024
@pr-processor pr-processor bot added Needs re-review and removed Waiting on contributor Requires an action from the author labels Jan 23, 2024
@pr-processor pr-processor bot added Needs re-review and removed Waiting on contributor Requires an action from the author labels Jan 24, 2024
@Lennonka Lennonka added Waiting on contributor Requires an action from the author and removed Needs re-review labels Jan 24, 2024
@pr-processor pr-processor bot added Needs re-review and removed Waiting on contributor Requires an action from the author labels Jan 25, 2024
@aneta-petrova aneta-petrova marked this pull request as draft January 25, 2024 10:54
@aneta-petrova
Copy link
Member Author

Switching back to draft because I think I should rearrange a few bits around the example blocks. But feel free to keep reviewing in the meantime.

@aneta-petrova aneta-petrova marked this pull request as ready for review January 25, 2024 11:43
@aneta-petrova
Copy link
Member Author

I moved a piece of general information about nested host groups out of the example boxes, and slightly tweaked the example boxes to match each other.

Lennonka
Lennonka approved these changes Jan 25, 2024
View reviewed changes
Copy link
Contributor

@Lennonka Lennonka left a comment

Choose a reason for hiding this comment

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

Supercool.

jeremylenz
jeremylenz approved these changes Jan 25, 2024
View reviewed changes
Copy link
Contributor

@jeremylenz jeremylenz left a comment

Choose a reason for hiding this comment

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

Nice work! LGTM 👍

@aneta-petrova aneta-petrova added style review done No issues from docs style/grammar perspective and removed Needs style review Requires a review from docs style/grammar perspective labels Jan 25, 2024
@Lennonka Lennonka merged commit a986bbe into theforeman:master Jan 26, 2024
8 checks passed
Lennonka pushed a commit that referenced this pull request Jan 26, 2024
@aneta-petrova @jeremylenz
@ShimShtein @Lennonka
Add details on host groups (#2685)
f97ea09 
* Define host group settings persistance
* Edit host group settings hierarchy and priorities
* Split host group settings into a separate module
* Split host group-related modules into a separate assembly

---------

Co-authored-by: Jeremy Lenz <jlenz@redhat.com>
Co-authored-by: Shimon Shtein <sshtein@redhat.com>
@Lennonka
Copy link
Contributor

Cherry-picked:

@aneta-petrova aneta-petrova mentioned this pull request Apr 22, 2024
Merged
3 tasks
@aneta-petrova aneta-petrova deleted the 2255404_hgroup_field_inherit branch September 27, 2024 19:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jeremylenz jeremylenz jeremylenz approved these changes

@Lennonka Lennonka Lennonka approved these changes

@ShimShtein ShimShtein ShimShtein left review comments

Assignees
No one assigned
Labels
style review done No issues from docs style/grammar perspective tech review done No issues from the technical perspective
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@aneta-petrova @jeremylenz @Lennonka @ShimShtein