Skip to content

Conversation

@ekohl
Copy link
Member

@ekohl ekohl commented Apr 3, 2024

This takes the idea that there should be a central place with documentation on all DNS providers. Not spread over multiple guides.

It's not complete, but it moves forward with something I've raised multiple times. Most recently in #2781 (review). I'd like to see a discussion on what should be the goal.

One possible goal is to also include "Configuring Smart Proxy server with external DNS" and keep it all in the Installing a Smart Proxy Server guide. That means we also need to include the DNS assembly in the Installing Server guide, but that would be (correctly) documenting what's possible.

Another is to move it all to its own guide and make the Installing Server/Proxy/Provisioning Hosts guides link to it when needed. That can also be done in multiple smaller steps.

Al of this can also be done for DHCP and we should be consistent. We should also discuss the "Configuring DNS, DHCP, and TFTP" chapter and what we want with it.

It includes #2932 because I needed that for better readability.

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.

@github-actions
Copy link

github-actions bot commented Apr 3, 2024

The PR preview for 809e85d could not be generated

For more information, see xref:configuring_dns_libvirt_{context}[].
endif::[]
* `dns_nsupdate` {endash} Dynamic DNS update using nsupdate.
For more information, see {ProvisioningDocURL}Using_Infoblox_as_DHCP_and_DNS_Providers_provisioning[Using Infoblox as DHCP and DNS Providers] in _{ProvisioningDocTitle}_.
Copy link
Member Author

Choose a reason for hiding this comment

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

This part triggered me when looking at it with @apinnick, because it's wrong. It escalated into a larger PR.

@ekohl ekohl force-pushed the dns-integration branch from f495fb5 to 257b29a Compare April 10, 2024 14:03
@ekohl ekohl marked this pull request as ready for review April 10, 2024 14:03
Copy link
Contributor

@maximiliankolb maximiliankolb left a comment

Choose a reason for hiding this comment

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

I cannot comment much on the technical details but left some minor suggestions.


The _dns_nsupdate_ DNS provider manages DNS records using nsupdate.
It can be used with any https://www.rfc-editor.org/rfc/rfc2136[RFC2136] compatible nameserver, but by default ISC BIND is installed.
For installation without ISC BIND, see xref:configuring-external-dns_{context}[]
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
For installation without ISC BIND, see xref:configuring-external-dns_{context}[]
For installation without ISC BIND, see xref:configuring-external-dns_{context}[].

For installation without ISC BIND, see xref:configuring-external-dns_{context}[]

.Procedure
* You can use `{foreman-installer}` to configure `dns_nsupdate`:
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* You can use `{foreman-installer}` to configure `dns_nsupdate`:
* Configure `dns_nsupdate` on your {SmartProxy}:

Copy link
Member Author

Choose a reason for hiding this comment

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

I think eventually we want to include it for {ProjectServer} too so I'd be conservative in making it proxy only. Not sure if that should hold us back now, but something to keep in mind.

Copy link
Contributor

Choose a reason for hiding this comment

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

But it's "SmartProxy", not "SmartProxyServer"; so it'd be valid for both Foreman server and any Smart Proxy server.

Copy link
Member Author

Choose a reason for hiding this comment

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

That's fair.

Copy link
Member Author

Choose a reason for hiding this comment

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

For Satellite SmartProxy resolves to Capsule and I'm not sure that won't mislead readers.

= Installing the DNS Infoblox module

Use this procedure to install the DNS Infoblox module on {SmartProxy}.
ifeval::["{context}" != "{smart-proxy-context}"]
Copy link
Contributor

Choose a reason for hiding this comment

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

I am unsure about this change content-wise (why?) and style-wise (it says "also" in some cases but there's no first item)

Copy link
Member Author

Choose a reason for hiding this comment

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

It may not be the right way, but my reasoning is that the procedure is now included in both Provisioning Hosts (as it previously was) and in Installing Proxy, which is new. The xref only works in the Provisioning Hosts guide.

Perhaps the proper way is to create a separate file just for the procedure. That also allows using a consistent header as for the other DNS providers and a little introduction that's relevant.

Thoughts on how to move forward?

Copy link
Member

Choose a reason for hiding this comment

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

Users can follow the first procedure (Installing the DHCP Infoblox module) and then the other (Installing the DNS Infoblox module), right? It will give the same result as "combining" both, right? If that's the case, I'd just drop the hint about combining the procedures.

This would be in line with the recommended approach in RH d/s to avoid documenting multiple ways of accomplishing the same thing. In other words, we're encouraged to pick one approach (the easiest one, the least error-prone one, the most user-friendly one, the 'best practices' one, ...) and avoid mentioning other approaches. Also, instructions to "combine" procedures might cause unnecessary confusion on the users' side ("Does 'combine' mean following one procedure and then the other? Or running all the installer options with one installer command? And what about the rest of the procedure steps, in what order should I follow them?").

Copy link
Member Author

Choose a reason for hiding this comment

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

Users can follow the first procedure (Installing the DHCP Infoblox module) and then the other (Installing the DNS Infoblox module), right? It will give the same result as "combining" both, right? If that's the case, I'd just drop the hint about combining the procedures.

Correct, both are independent. Logically, you could say it's A + B == B + A == AB.

This would be in line with the recommended approach in RH d/s to avoid documenting multiple ways of accomplishing the same thing. In other words, we're encouraged to pick one approach (the easiest one, the least error-prone one, the most user-friendly one, the 'best practices' one, ...) and avoid mentioning other approaches. Also, instructions to "combine" procedures might cause unnecessary confusion on the users' side ("Does 'combine' mean following one procedure and then the other? Or running all the installer options with one installer command? And what about the rest of the procedure steps, in what order should I follow them?").

I agree, but there are many counter examples in our current documentation.

You could argue this PR is a step in untangling that mess. For example, documenting the nsupdate provider means we could pull the DNS part out of https://docs.theforeman.org/nightly/Installing_Server/index-katello.html#configuring-dns-dhcp-and-tftp_foreman. If we then do the same with DHCP then you remain with just TFTP installation. That's my ultimate goal with #2957.

Thinking more about this, perhaps we need a generic section that talks about combining installer arguments if we want to encourage users to do so. There are good reasons, most come down to speed: the installer has a certain overhead so running it just once with more arguments is faster. Plus, if you install a DNS module you will restart services, refresh features. Then if you install a DHCP module, you do the same. If it's in a single run, you only need to restart and refresh once.

Copy link
Member

@aneta-petrova aneta-petrova Apr 11, 2024

Choose a reason for hiding this comment

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

This would be in line with the recommended approach in RH d/s to avoid documenting multiple ways of accomplishing the same thing. In other words, we're encouraged to pick one approach (the easiest one, the least error-prone one, the most user-friendly one, the 'best practices' one, ...) and avoid mentioning other approaches. Also, instructions to "combine" procedures might cause unnecessary confusion on the users' side ("Does 'combine' mean following one procedure and then the other? Or running all the installer options with one installer command? And what about the rest of the procedure steps, in what order should I follow them?").

I agree, but there are many counter examples in our current documentation.

Very true :)

You could argue this PR is a step in untangling that mess. For example, documenting the nsupdate provider means we could pull the DNS part out of https://docs.theforeman.org/nightly/Installing_Server/index-katello.html#configuring-dns-dhcp-and-tftp_foreman. If we then do the same with DHCP then you remain with just TFTP installation. That's my ultimate goal with #2957.

I think it's the current messy-ish situation that prevents me from proposing anything useful when it comes to structuring this content. In #2957, I proposed starting by defining the assemblies first. That's my preferred approach because reasonably defined assemblies can then be moved around guides with relative ease.

Thinking more about this, perhaps we need a generic section that talks about combining installer arguments if we want to encourage users to do so. There are good reasons, most come down to speed: the installer has a certain overhead so running it just once with more arguments is faster. Plus, if you install a DNS module you will restart services, refresh features. Then if you install a DHCP module, you do the same. If it's in a single run, you only need to restart and refresh once.

That sounds like material for the installer man page rather than product guides (IMHO). I guess if this behavior was documented in the man page, it would be possible to reference it from the procedure modules we're discussing (from Additional resources). And then you could drop the hint about combining the procedures..? But it's hard for me to say at this point.

Copy link
Contributor

Choose a reason for hiding this comment

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

That sounds like material for the installer man page rather than product guides

That sounds reasonable. But we could also mention that combining the installer args is faster when we first mention the installer in the Installing docs.

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.

From my point of view, the goal should be to take the whole Infoblox chapter out of the Provisioning guide. It can be done in smaller steps.


include::modules/proc_configuring-dns-nsupdate.adoc[leveloffset=+1]

include::modules/proc_installing-the-dns-infoblox-module.adoc[leveloffset=+1]
Copy link
Contributor

Choose a reason for hiding this comment

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

If you include info only from this module, it's missing the Infoblox limitations and prerequisites info. Is it possible that some of that information only applies to combining DNS and DHCP Infoblox modules? Not sure how to handle this content-wise.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think you're right that some of them should be included. However, the chapter really is written to reflect on both DHCP and DNS. I wonder if it makes sense to move the prerequisites into the specific chapters. But I also see the CA certificate installation step is missing.

How about we exclude Infoblox for now and handle it as a separate follow up?

Copy link
Contributor

Choose a reason for hiding this comment

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

Sure, that's an option.

* `dns_dnscmd` {endash} Static DNS records in Microsoft Active Directory.
endif::[]
* `dns_infoblox` {endash} For more information, see {ProvisioningDocURL}Using_Infoblox_as_DHCP_and_DNS_Providers_provisioning[Using Infoblox as DHCP and DNS Providers] in _{ProvisioningDocTitle}_.
* `dns_infoblox` {endash} For more information, see xref:Installing_the_DHCP_Infoblox_Module_{context}.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* `dns_infoblox` {endash} For more information, see xref:Installing_the_DHCP_Infoblox_Module_{context}.
* `dns_infoblox` {endash} For more information, see xref:Installing_the_DHCP_Infoblox_Module_{context}[].

@maximiliankolb maximiliankolb added Waiting on contributor Requires an action from the author and removed Not yet reviewed labels May 2, 2024
@maximiliankolb
Copy link
Contributor

FYI: there's a general agreement that we want to move forward with this 👍

ekohl added 2 commits August 8, 2024 14:06
This takes the idea that there should be a central place with
documentation on all DNS providers. Not spread over multiple guides. It
starts with the dns_nsupdate provider, but this should be completed with
all of the providers.
@ekohl ekohl force-pushed the dns-integration branch from 257b29a to 809e85d Compare August 8, 2024 12:09
@pr-processor pr-processor bot added Needs re-review and removed Waiting on contributor Requires an action from the author labels Aug 8, 2024
@ekohl ekohl marked this pull request as draft August 8, 2024 12:10
@ekohl
Copy link
Member Author

ekohl commented Aug 8, 2024

I've opened #3200 to at least handle the dns_nsupdate provider. Until that's merged, this is a draft.

@Lennonka would you be willing to look at moving Infoblox out of the provisioning hosts guide?

@Lennonka
Copy link
Contributor

Lennonka commented Aug 8, 2024

would you be willing to look at moving Infoblox out of the provisioning hosts guide?

Yes, I don't mind doing that. I would prefer to have a whole guide about configuring the networking services, so I will be able to move Infoblox afterwards. Right now, there isn't a good place to move it to :)

@ekohl
Copy link
Member Author

ekohl commented Aug 8, 2024

I proposed a separate DNS guide in #2957, but depending on how we shape it then it could also be more of a networking guide.

IMHO the path is to take this assembly and move it to a better place. Also make it obvious that it can be used on all servers that have a Smart Proxy (so also point to it with Installing Server).

@Lennonka
Copy link
Contributor

Lennonka commented Aug 8, 2024

Okay #3204

@maximiliankolb
Copy link
Contributor

@ekohl Is this PR still relevant after #3530?

@ekohl
Copy link
Member Author

ekohl commented May 2, 2025

No, #3530 is the proper implementation of this.

@ekohl ekohl closed this May 2, 2025
@ekohl ekohl deleted the dns-integration branch May 2, 2025 14:07
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.

5 participants