Replace installer-scenario attribute with foreman-installer when possible #2943

ekohl · 2024-04-05T16:26:11Z

On existing installations it's preferable to simply use foreman-installer with the options you wish to change. This replaces it where possible.

Includes #2942.

I am familiar with the contributing guidelines.

Please cherry-pick my commits into:

Foreman 3.10/Katello 4.12
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/6.7)
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)
We do not accept PRs for Foreman older than 3.3.

ekohl · 2024-04-05T16:27:15Z

guides/common/modules/proc_configuring-dynamic-dns-update-with-gss-tsig-authentication.adoc

 . Use the `{foreman-installer}` command to configure the {Project} or {SmartProxy} that manages the DNS Service for the domain:
-* On {Project}, enter the following command:
+* On {Project} or {SmartProxy}, enter the following command:


I wasn't sure how to write these instructions, but they feel too complex with the simplification.

github-actions · 2024-04-05T16:28:25Z

The PR preview for 4502f8c is available at theforeman-foreman-documentation-preview-pr-2943.surge.sh

The following output files are affected by this PR:

show diff

show diff as HTML

maximiliankolb

One comment, rest LGTM. Overall change makes sense to me: After you have installed Foreman Server/Smart Proxy Server, you don't need to specify the scenario anymore.

I am not even sure if changing the scenario is supported.

maximiliankolb · 2024-04-08T06:36:55Z

guides/common/modules/proc_configuring-dynamic-dns-update-with-gss-tsig-authentication.adoc

@@ -138,24 +138,11 @@ grant {smart-proxy-principal}\047__{foreman-example-com}@EXAMPLE.COM__ wildcard
 .Configuring the {Project} or {SmartProxyServer} that manages the DNS service for the domain

 . Use the `{foreman-installer}` command to configure the {Project} or {SmartProxy} that manages the DNS Service for the domain:
-* On {Project}, enter the following command:
+* On {Project} or {SmartProxy}, enter the following command:


Suggested change

* On {Project} or {SmartProxy}, enter the following command:

* On your {SmartProxy}, enter the following command:

Alternative: Adjust your suggestion to "On your {ProjectServer} or {SmartProxyServer}"

ekohl · 2024-04-08T10:00:16Z

I am not even sure if changing the scenario is supported.

No. There is code to do so, but it really isn't tested and pretty much guaranteed to be broken in my experience. Just too many small details you need to think about. The code warns you about it as well:

https://github.com/theforeman/kafo/blob/200815048a1c37f94e76682ad1e82a2a47d7807e/lib/kafo/scenario_manager.rb#L160C4-L160C9

asteflova · 2024-04-08T10:53:25Z

guides/common/modules/proc_configuring-dynamic-dns-update-with-gss-tsig-authentication.adoc

@@ -138,24 +138,11 @@ grant {smart-proxy-principal}\047__{foreman-example-com}@EXAMPLE.COM__ wildcard
 .Configuring the {Project} or {SmartProxyServer} that manages the DNS service for the domain

 . Use the `{foreman-installer}` command to configure the {Project} or {SmartProxy} that manages the DNS Service for the domain:


You can delete this line. It just repeats the heading on line 138 and the command name from line 145; neither is necessary.

asteflova · 2024-04-08T10:57:38Z

guides/common/modules/proc_configuring-dynamic-dns-update-with-gss-tsig-authentication.adoc

@@ -138,24 +138,11 @@ grant {smart-proxy-principal}\047__{foreman-example-com}@EXAMPLE.COM__ wildcard
 .Configuring the {Project} or {SmartProxyServer} that manages the DNS service for the domain

 . Use the `{foreman-installer}` command to configure the {Project} or {SmartProxy} that manages the DNS Service for the domain:
-* On {Project}, enter the following command:
+* On {Project} or {SmartProxy}, enter the following command:


Can you add some wording that would briefly describe what the command actually does?

Suggested change

* On {Project} or {SmartProxy}, enter the following command:

* Configure your {ProjectServer} or {SmartProxyServer} to manage the DNS service for the domain:

or

Suggested change

* On {Project} or {SmartProxy}, enter the following command:

* On the {ProjectServer} or {SmartProxyServer} you want to use to manage the DNS service for the domain, configure <description of what the command does>:

or perhaps you'll be able to think of something even better.

Both my suggestions are significantly longer than the original, but my reasoning is that rather than writing "enter this command" (users can see that you're asking them to run a command), it's much more useful to describe what the command does.

I thought this would cover it:

Suggested change

* On {Project} or {SmartProxy}, enter the following command:

* Configure your {ProjectServer} or {SmartProxyServer} to connect to your DNS service:

But then the header above it is:

.Configuring the {Project} or {SmartProxyServer} that manages the DNS service for the domain

IMHO that's too similar.

Taking a step back, we have this situation:

Foreman knows about a Foreman Proxy. In this step we configure the Foreman Proxy DNS module. It does a few things:

--foreman-proxy-dns enables the DNS module

--foreman-proxy-dns-provider configures the DNS module to use the nsupdate_gss provider

--foreman-proxy-dns-managed tells the installer not to install a DNS service

--foreman-proxy-dns-server tells the DNS module where it can find the DNS server

--foreman-proxy-dns-tsig-keytab & --foreman-proxy-dns-tsig-principal tells the DNS provider which credentials to use when connecting

I know that's too many details to cover, but I'm hoping it helps you to come up with a good suggestion.

I think your suggestion would work just fine... if you merge .Configuring the {Project} or {SmartProxyServer} [...] with .Updating the configuration in the {ProjectWebUI}. Which is something I wanted to suggest before, but it seemed out of scope of this PR.

I couldn't find words to explain it, so I'm sending a patch (and I can only hope I'm doing this patch thing correctly 🤞).

diff --git a/guides/common/modules/proc_configuring-dynamic-dns-update-with-gss-tsig-authentication.adoc b/guides/common/modules/proc_configuring-dynamic-dns-update-with-gss-tsig-authentication.adoc index 94fdd8cbd9..3248c40880 100644 --- a/guides/common/modules/proc_configuring-dynamic-dns-update-with-gss-tsig-authentication.adoc +++ b/guides/common/modules/proc_configuring-dynamic-dns-update-with-gss-tsig-authentication.adoc @@ -137,8 +137,7 @@ grant {smart-proxy-principal}\047__{foreman-example-com}@EXAMPLE.COM__ wildcard .Configuring the {Project} or {SmartProxyServer} that manages the DNS service for the domain -. Use the `{foreman-installer}` command to configure the {Project} or {SmartProxy} that manages the DNS Service for the domain: -* On {Project} or {SmartProxy}, enter the following command: +. Configure your {ProjectServer} or {SmartProxyServer} to connect to your DNS service: + [options="nowrap" subs="+quotes,attributes"] ---- @@ -150,17 +149,14 @@ grant {smart-proxy-principal}\047__{foreman-example-com}@EXAMPLE.COM__ wildcard --foreman-proxy-dns-tsig-principal="{smart-proxy-principal}/_{foreman-example-com}@EXAMPLE.COM_" \ --foreman-proxy-dns=true ---- - -After you run the `{foreman-installer}` command to make any changes to your {SmartProxy} configuration, you must update the configuration of each affected {SmartProxy} in the {ProjectWebUI}. - -.Updating the configuration in the {ProjectWebUI} -. In the {ProjectWebUI}, navigate to *Infrastructure* > *{SmartProxies}*, locate the {ProductName}, and from the list in the *Actions* column, select *Refresh*. -. Configure the domain: -.. In the {ProjectWebUI}, navigate to *Infrastructure* > *Domains* and select the domain name. -.. In the *Domain* tab, ensure *DNS {SmartProxy}* is set to the {SmartProxy} where the subnet is connected. -. Configure the subnet: -.. In the {ProjectWebUI}, navigate to *Infrastructure* > *Subnets* and select the subnet name. -.. In the *Subnet* tab, set *IPAM* to *None*. -.. In the *Domains* tab, select the domain that you want to manage using the IdM server. -.. In the *{SmartProxies}* tab, ensure *Reverse DNS {SmartProxy}* is set to the {SmartProxy} where the subnet is connected. -.. Click *Submit* to save the changes. +. For each affected {SmartProxy}, update the configuration of that {SmartProxy} in the {ProjectWebUI}: +.. In the {ProjectWebUI}, navigate to *Infrastructure* > *{SmartProxies}*, locate the {ProductName}, and from the list in the *Actions* column, select *Refresh*. +.. Configure the domain: +... In the {ProjectWebUI}, navigate to *Infrastructure* > *Domains* and select the domain name. +... In the *Domain* tab, ensure *DNS {SmartProxy}* is set to the {SmartProxy} where the subnet is connected. +.. Configure the subnet: +... In the {ProjectWebUI}, navigate to *Infrastructure* > *Subnets* and select the subnet name. +... In the *Subnet* tab, set *IPAM* to *None*. +... In the *Domains* tab, select the domain that you want to manage using the IdM server. +... In the *{SmartProxies}* tab, ensure *Reverse DNS {SmartProxy}* is set to the {SmartProxy} where the subnet is connected. +... Click *Submit* to save the changes.

Now, as is usually the case, this creates a whole new problem, namely procedure steps nested in 3 levels. But I still think this is a cleaner solution, especially since I assume you never want users to just connect to the DNS service, but not update the config in the web UI right after.

WDYT?

Yes, that's exactly the issue I had and I think your suggestion looks better.

ekohl · 2024-04-10T08:44:30Z

guides/common/modules/proc_configuring-dynamic-dns-update-with-gss-tsig-authentication.adoc

+. For each affected {SmartProxy}, update the configuration of that {SmartProxy} in the {ProjectWebUI}:
+.. In the {ProjectWebUI}, navigate to *Infrastructure* > *{SmartProxies}*, locate the {ProductName}, and from the list in the *Actions* column, select *Refresh*.


This is redundant, but I chose to keep it in to keep to keep the PR focused

maximiliankolb

Thanks Ewoud, diff LGTM; handing over to @asteflova for additional/final ACK.

asteflova

There is just one bullet point (*) that should in fact be a procedure step (.). Other than that, all looks good.

guides/common/modules/proc_configuring-dynamic-dns-update-with-gss-tsig-authentication.adoc

maximiliankolb · 2024-05-02T12:24:15Z

From triage: @ekohl Please rebase to "master"; then we'll re-review and merge.

…ible On existing installations it's preferable to simply use foreman-installer with the options you wish to change. This replaces it where possible.

ekohl · 2024-05-16T17:33:15Z

Rebased

maximiliankolb

Thanks Ewoud, LGTM.

maximiliankolb · 2024-06-13T12:11:14Z

TODO: cherry-pick to 3.11 too

pr-processor bot added the Not yet reviewed label Apr 5, 2024

ekohl mentioned this pull request Apr 5, 2024

Merge the DNS/DHCP/TFTP sections between server/proxy #2932

Merged

9 tasks

ekohl commented Apr 5, 2024

View reviewed changes

maximiliankolb reviewed Apr 8, 2024

View reviewed changes

asteflova reviewed Apr 8, 2024

View reviewed changes

ekohl force-pushed the remove-installer-scenario-where-possible branch 2 times, most recently from 2bcecf9 to d1590f2 Compare April 10, 2024 08:31

ekohl marked this pull request as ready for review April 10, 2024 08:43

ekohl commented Apr 10, 2024

View reviewed changes

maximiliankolb approved these changes Apr 10, 2024

View reviewed changes

asteflova approved these changes Apr 10, 2024

View reviewed changes

guides/common/modules/proc_configuring-dynamic-dns-update-with-gss-tsig-authentication.adoc Outdated Show resolved Hide resolved

ekohl force-pushed the remove-installer-scenario-where-possible branch from d1590f2 to 4e9336f Compare April 10, 2024 14:01

Replace installer-scenario attribute with foreman-installer when poss…

4502f8c

…ible On existing installations it's preferable to simply use foreman-installer with the options you wish to change. This replaces it where possible.

ekohl force-pushed the remove-installer-scenario-where-possible branch from 4e9336f to 4502f8c Compare May 16, 2024 12:17

maximiliankolb approved these changes May 17, 2024

View reviewed changes

pr-processor bot removed the Not yet reviewed label May 17, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Replace installer-scenario attribute with foreman-installer when possible #2943

Replace installer-scenario attribute with foreman-installer when possible #2943

ekohl commented Apr 5, 2024

ekohl Apr 5, 2024

github-actions bot commented Apr 5, 2024 •

edited

maximiliankolb left a comment

maximiliankolb Apr 8, 2024

ekohl commented Apr 8, 2024

asteflova Apr 8, 2024

asteflova Apr 8, 2024

ekohl Apr 8, 2024

asteflova Apr 9, 2024

ekohl Apr 9, 2024

ekohl Apr 10, 2024

maximiliankolb left a comment

asteflova left a comment

maximiliankolb commented May 2, 2024

ekohl commented May 16, 2024

maximiliankolb left a comment

maximiliankolb commented Jun 13, 2024

	* On {Project} or {SmartProxy}, enter the following command:
	* On your {SmartProxy}, enter the following command:

		@@ -138,24 +138,11 @@ grant {smart-proxy-principal}\047__{foreman-example-com}@EXAMPLE.COM__ wildcard
		.Configuring the {Project} or {SmartProxyServer} that manages the DNS service for the domain

		. Use the `{foreman-installer}` command to configure the {Project} or {SmartProxy} that manages the DNS Service for the domain:

	* On {Project} or {SmartProxy}, enter the following command:
	* Configure your {ProjectServer} or {SmartProxyServer} to manage the DNS service for the domain:

	* On {Project} or {SmartProxy}, enter the following command:
	* On the {ProjectServer} or {SmartProxyServer} you want to use to manage the DNS service for the domain, configure <description of what the command does>:

		. For each affected {SmartProxy}, update the configuration of that {SmartProxy} in the {ProjectWebUI}:
		.. In the {ProjectWebUI}, navigate to Infrastructure > {SmartProxies}, locate the {ProductName}, and from the list in the Actions column, select Refresh.

Replace installer-scenario attribute with foreman-installer when possible #2943

Are you sure you want to change the base?

Replace installer-scenario attribute with foreman-installer when possible #2943

Conversation

ekohl commented Apr 5, 2024

Choose a reason for hiding this comment

github-actions bot commented Apr 5, 2024 • edited

maximiliankolb left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ekohl commented Apr 8, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

maximiliankolb left a comment

Choose a reason for hiding this comment

asteflova left a comment

Choose a reason for hiding this comment

maximiliankolb commented May 2, 2024

ekohl commented May 16, 2024

maximiliankolb left a comment

Choose a reason for hiding this comment

maximiliankolb commented Jun 13, 2024

github-actions bot commented Apr 5, 2024 •

edited