OCPBUGS-45800#updating resources info #89851

brendan-daly-red-hat · 2025-03-07T13:12:55Z

Versions:
4.17+

Issue:
https://issues.redhat.com/browse/OCPBUGS-45800

Link to docs preview:
The updated module is reflected in these pages:

Restricting the API server to private (Control plane configuration options for Amazon Web Services)
Restricting the API server to private (Control plane configuration options for Microsoft Azure)
Restricting the API server to private (Configuring a private cluster)

QE review:

QE has approved this change.

Additional information:
From what I understand, the Vale messages below aren't valid. It is suggesting the ifdef statement requires an endif, but that's not required here. See #86647.

ocpdocs-previewbot · 2025-03-07T13:35:39Z

🤖 Mon Mar 24 09:19:46 - Prow CI generated the docs preview:

https://89851--ocpdocs-pr.netlify.app/openshift-enterprise/latest/machine_management/control_plane_machine_management/cpmso_provider_configurations/cpmso-config-options-aws.html
https://89851--ocpdocs-pr.netlify.app/openshift-enterprise/latest/machine_management/control_plane_machine_management/cpmso_provider_configurations/cpmso-config-options-azure.html
https://89851--ocpdocs-pr.netlify.app/openshift-enterprise/latest/post_installation_configuration/configuring-private-cluster.html

brendan-daly-red-hat · 2025-03-07T13:48:57Z

@jinyunma, PTAL at the draft update.

jinyunma · 2025-03-14T08:46:50Z

modules/private-clusters-setting-api-private.adoc

-*** For Azure, delete the `api-internal-v4` rule for the public load balancer.
+*** For {azure-short}, you must delete the following resources:
+ ** The `api-v4` rule for the public load balancer.
+ ** The `frontenIPConfiguration` that is associated with the public load balancer.


Here frontenIPConfiguration is associated with api-v4 rule in public load balancer

jinyunma · 2025-03-14T08:52:33Z

modules/private-clusters-setting-api-private.adoc

+*** For {azure-short}, you must delete the following resources:
+ ** The `api-v4` rule for the public load balancer.
+ ** The `frontenIPConfiguration` that is associated with the public load balancer.
+ ** The public IP that is associated with the public load balancer.


public IP refers to the ip specified in frontenIPConfiguration

jinyunma · 2025-03-14T08:54:14Z

modules/private-clusters-setting-api-private.adoc

+ ** The `api-v4` rule for the public load balancer.
+ ** The `frontenIPConfiguration` that is associated with the public load balancer.
+ ** The public IP that is associated with the public load balancer.
+ ** The Network Security Group (NSG) rule for port 22.


sorry, this should be incorrect, no nsg rule to be removed in this case. I updated description in the bug as well.

jinyunma · 2025-03-14T10:02:52Z

/lgtm

openshift-ci · 2025-03-21T13:32:13Z

New changes are detected. LGTM label has been removed.

ocpdocs-vale-bot · 2025-03-21T13:45:44Z

modules/private-clusters-setting-api-private.adoc

+. In the web portal or console for your cloud provider, take the following actions:
+
+.. Locate and delete the appropriate load balancer component:
+ifndef::cpmso-using-azure[]


🤖 [error] AsciiDoc.ValidConditions: File contains unbalanced if statements. Review the file to ensure it contains matching opening and closing if statements.

ocpdocs-vale-bot · 2025-03-21T13:45:45Z

modules/private-clusters-setting-api-private.adoc

+ifndef::cpmso-using-azure[]
+*** For {aws-short}, delete the external load balancer. The API DNS entry in the private zone already points to the internal load balancer, which uses an identical configuration, so you do not need to modify the internal load balancer.
+endif::cpmso-using-azure[]
+ifndef::cpmso-using-aws[]


🤖 [error] AsciiDoc.ValidConditions: File contains unbalanced if statements. Review the file to ensure it contains matching opening and closing if statements.

ocpdocs-vale-bot · 2025-03-21T13:45:48Z

modules/private-clusters-setting-api-private.adoc

+endif::cpmso-using-aws[]
+
+.. Delete the
+ifdef::cpmso-using-aws[`api.$clustername.$yourdomain`]


🤖 [error] AsciiDoc.ValidConditions: File contains unbalanced if statements. Review the file to ensure it contains matching opening and closing if statements.

ocpdocs-vale-bot · 2025-03-21T13:45:49Z

modules/private-clusters-setting-api-private.adoc

+
+.. Delete the
+ifdef::cpmso-using-aws[`api.$clustername.$yourdomain`]
+ifdef::post-install[`api.$clustername.$yourdomain` or]


🤖 [error] AsciiDoc.ValidConditions: File contains unbalanced if statements. Review the file to ensure it contains matching opening and closing if statements.

ocpdocs-vale-bot · 2025-03-21T13:45:50Z

modules/private-clusters-setting-api-private.adoc

+ifndef::cpmso-using-aws[`api.$clustername`]
+DNS entry in the public zone.
+
+ifdef::cpmso-using-aws[]


🤖 [error] AsciiDoc.ValidConditions: File contains unbalanced if statements. Review the file to ensure it contains matching opening and closing if statements.

ocpdocs-vale-bot · 2025-03-21T13:45:51Z

modules/private-clusters-setting-api-private.adoc

+<2> Delete the `type` value for the external load balancer.
+endif::cpmso-using-aws[]
+
 ifdef::post-install[]


🤖 [error] AsciiDoc.ValidConditions: File contains unbalanced if statements. Review the file to ensure it contains matching opening and closing if statements.

brendan-daly-red-hat · 2025-03-21T14:06:43Z

/label peer-review-needed

skopacz1

Left a few comments, but nice job navigating the complex conditional stuff. It can be like a logic puzzle sometimes. Some of my comments are a little long and complicated, so let me know if you have questions about any of them!

skopacz1 · 2025-03-21T15:20:56Z

modules/private-clusters-setting-api-private.adoc

+
+.. Locate and delete the appropriate load balancer component:
+ifndef::cpmso-using-azure[]
+*** For {aws-short}, delete the external load balancer. The API DNS entry in the private zone already points to the internal load balancer, which uses an identical configuration, so you do not need to modify the internal load balancer.


I understand that existing docs may have this "For [platform name], do this step" format, but I'd suggest checking out the ISG section on indicating conditional steps. In short, you're supposed to preface the step with a condition or use an "if" statement before writing your step as normal.

I do think that your current structure is probably close enough to follow the spirit of that rule, so I'll suggest rewrites of this step based on the rule, but it's up to you if you think the feedback should be applied:

Suggested change

*** For {aws-short}, delete the external load balancer. The API DNS entry in the private zone already points to the internal load balancer, which uses an identical configuration, so you do not need to modify the internal load balancer.

*** {aws-short} clusters: Delete the external load balancer. The API DNS entry in the private zone already points to the internal load balancer, which uses an identical configuration, so you do not need to modify the internal load balancer.

skopacz1 · 2025-03-21T15:27:34Z

modules/private-clusters-setting-api-private.adoc

+*** For {aws-short}, delete the external load balancer. The API DNS entry in the private zone already points to the internal load balancer, which uses an identical configuration, so you do not need to modify the internal load balancer.
+endif::cpmso-using-azure[]
+ifndef::cpmso-using-aws[]
+*** For {azure-short}, you must delete the following resources:


Same suggestion as before:

Suggested change

*** For {azure-short}, you must delete the following resources:

*** {azure-short} clusters: Delete the following resources:

However, even if you don't use that suggestion, you must rewrite the first sentence to be in the imperative mood either way. That rule is also stated on the same ISG page:

Suggested change

*** For {azure-short}, you must delete the following resources:

*** For {azure-short}, delete the following resources:

skopacz1 · 2025-03-21T15:29:34Z

modules/private-clusters-setting-api-private.adoc

+*** For {azure-short}, you must delete the following resources:
+ ** The `api-v4` rule for the public load balancer.
+ ** The `frontenIPConfiguration` that is associated with the `api-v4` rule for the public load balancer.
+ ** The public IP that is specified in the `frontenIPConfiguration`.


Could you add some sort of noun so that the backticks phrase/term is used as an adjective and not a noun? In general, doing so for all terms in backticks helps increase clarity:

Suggested change

** The public IP that is specified in the `frontenIPConfiguration`.

** The public IP that is specified in the `frontenIPConfiguration` parameter.

I'm guessing the noun would be parameter but feel free to swap with anything more appropriate.

skopacz1 · 2025-03-21T15:31:16Z

modules/private-clusters-setting-api-private.adoc

+ ** The `frontenIPConfiguration` that is associated with the `api-v4` rule for the public load balancer.
+ ** The public IP that is specified in the `frontenIPConfiguration`.


I looked around the docs and I think this is a typo here, please confirm with SME/QE:

Suggested change

** The `frontenIPConfiguration` that is associated with the `api-v4` rule for the public load balancer.

** The public IP that is specified in the `frontenIPConfiguration`.

** The `frontendIPConfiguration` that is associated with the `api-v4` rule for the public load balancer.

** The public IP that is specified in the `frontendIPConfiguration`.

skopacz1 · 2025-03-21T15:49:32Z

modules/private-clusters-setting-api-private.adoc

+ ** The `frontenIPConfiguration` that is associated with the `api-v4` rule for the public load balancer.
+ ** The public IP that is specified in the `frontenIPConfiguration`.
+
+.. For {azure-short}, configure the Ingress Controller endpoint publishing scope to `Internal`.


Same suggestion as before, which I'm now noticing would align with how this line is written anyways:

Suggested change

.. For {azure-short}, configure the Ingress Controller endpoint publishing scope to `Internal`.

.. {azure-short} clusters: Configure the Ingress Controller endpoint publishing scope to `Internal`.

skopacz1 · 2025-03-21T16:16:01Z

modules/private-clusters-setting-api-private.adoc

+
+.. For the {azure-short} public load balancer, if you configure the Ingress Controller endpoint publishing scope to `Internal` and there are no existing inbound rules in the public load balancer, you must create an outbound rule explicitly to provide outbound traffic for the backend address pool.
+For more information, see the Microsoft Azure documentation about adding outbound rules.


This step is a tough one but I feel like it needs to be rewritten somehow. Here are the issues I see with it:

The beginning condition is so long to describe, and seems more like a substep of the previous step (i.e. it's worded like "if you did the last step and there are no existing inbound rules on the load balancer, you need to do this").

It seems redundant (to me) to start with "for the load balancer" when the condition is really for Azure clusters and the load balancer context is mentioned later in the sentence

As mentioned in a previous comment, this instruction is written as "you must do this" rather than "do this".

Here are my suggestions for you to consider.

Keep this as a step but clean it up to make it more like an instruction, maybe something like this:

Suggested change

.. For the {azure-short} public load balancer, if you configure the Ingress Controller endpoint publishing scope to `Internal` and there are no existing inbound rules in the public load balancer, you must create an outbound rule explicitly to provide outbound traffic for the backend address pool.

For more information, see the Microsoft Azure documentation about adding outbound rules.

.. {azure-short} clusters: If you configure the Ingress Controller endpoint publishing scope to `Internal` and there are no existing inbound rules in the public load balancer, create an outbound rule explicitly to provide outbound traffic for the backend address pool.

For more information, see the Microsoft Azure documentation about adding outbound rules.

I don't like that option as much since it has two conditional statements, one of which seems to be based off the previous step. You can also make this a part of the previous step, either as another line or as a NOTE/IMPORTANT admonition:

Suggested change

.. For the {azure-short} public load balancer, if you configure the Ingress Controller endpoint publishing scope to `Internal` and there are no existing inbound rules in the public load balancer, you must create an outbound rule explicitly to provide outbound traffic for the backend address pool.

For more information, see the Microsoft Azure documentation about adding outbound rules.

+

[IMPORTANT]

====

If you configure the Ingress Controller endpoint publishing scope to `Internal` and there are no existing inbound rules in the public load balancer, you must create an outbound rule explicitly to provide outbound traffic for the backend address pool.

For more information, see the Microsoft Azure documentation about adding outbound rules.

====

more

openshift-ci · 2025-03-24T09:20:59Z

@brendan-daly-red-hat: all tests passed!

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

bergerhoffer · 2025-06-16T18:48:23Z

The branch/enterprise-4.20 label has been added to this PR.

This is because your PR targets the main branch and is labeled for enterprise-4.19. And any PR going into main must also target the latest version branch (enterprise-4.20).

If the update in your PR does NOT apply to version 4.20 onward, please re-target this PR to go directly into the appropriate version branch or branches (enterprise-4.x) instead of main.

jinyunma · 2025-06-23T02:27:12Z

In section Restricting the API server to private of previewed doc, procedure 1 & 2 are similar, procedure 2 should be removed, right?

same issues in AWS doc, procedure 1&2 are duplicated with procedure 3&4.

jinyunma · 2025-06-23T02:30:13Z

modules/private-clusters-setting-api-private.adoc

+
+[IMPORTANT]
+====
+If you configure the Ingress Controller endpoint publishing scope to `Internal` and there are no existing inbound rules in the public load balancer, you must create an outbound rule explicitly to provide outbound traffic for the backend address pool.


Starting from 4.17+, installer create the outbound rule explicitly in public load balancer to provide outbound traffic, so I think there is no need to add this IMPORTANT admonition.

openshift-bot · 2025-09-21T09:00:55Z

Issues go stale after 90d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.
Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle stale

bergerhoffer · 2025-10-20T15:28:34Z

The branch/enterprise-4.21 label has been added to this PR.

This is because your PR targets the main branch and is labeled for enterprise-4.20. And any PR going into main must also target the latest version branch (enterprise-4.21).

If the update in your PR does NOT apply to version 4.21 onward, please re-target this PR to go directly into the appropriate version branch or branches (enterprise-4.x) instead of main.

openshift-ci bot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Mar 7, 2025

brendan-daly-red-hat force-pushed the OCPBUGS-45800 branch from 7a07f2f to 24f4a79 Compare March 7, 2025 13:28

openshift-ci bot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Mar 7, 2025

brendan-daly-red-hat force-pushed the OCPBUGS-45800 branch from 24f4a79 to 7414928 Compare March 14, 2025 08:42

jinyunma reviewed Mar 14, 2025

View reviewed changes

brendan-daly-red-hat force-pushed the OCPBUGS-45800 branch from 7414928 to 69ad32e Compare March 14, 2025 09:36

openshift-ci bot assigned jinyunma Mar 14, 2025

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Mar 14, 2025

brendan-daly-red-hat force-pushed the OCPBUGS-45800 branch from 69ad32e to e4c0a0b Compare March 21, 2025 13:32

openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Mar 21, 2025

openshift-ci bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Mar 21, 2025

ocpdocs-vale-bot reviewed Mar 21, 2025

View reviewed changes

openshift-ci bot added the peer-review-needed Signifies that the peer review team needs to review this PR label Mar 21, 2025

skopacz1 added peer-review-in-progress Signifies that the peer review team is reviewing this PR branch/enterprise-4.17 branch/enterprise-4.18 branch/enterprise-4.19 labels Mar 21, 2025

skopacz1 added this to the Continuous Release milestone Mar 21, 2025

skopacz1 reviewed Mar 21, 2025

View reviewed changes

skopacz1 added peer-review-done Signifies that the peer review team has reviewed this PR and removed peer-review-in-progress Signifies that the peer review team is reviewing this PR peer-review-needed Signifies that the peer review team needs to review this PR labels Mar 21, 2025

More

046ab01

more

brendan-daly-red-hat force-pushed the OCPBUGS-45800 branch from e4c0a0b to 046ab01 Compare March 24, 2025 09:11

bergerhoffer added the branch/enterprise-4.20 label Jun 16, 2025

jinyunma reviewed Jun 23, 2025

View reviewed changes

openshift-ci bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Sep 21, 2025

bergerhoffer added the branch/enterprise-4.21 label Oct 20, 2025

	*** For {aws-short}, delete the external load balancer. The API DNS entry in the private zone already points to the internal load balancer, which uses an identical configuration, so you do not need to modify the internal load balancer.
	*** {aws-short} clusters: Delete the external load balancer. The API DNS entry in the private zone already points to the internal load balancer, which uses an identical configuration, so you do not need to modify the internal load balancer.

	*** For {azure-short}, you must delete the following resources:
	*** {azure-short} clusters: Delete the following resources:

	*** For {azure-short}, you must delete the following resources:
	*** For {azure-short}, delete the following resources:

	** The public IP that is specified in the `frontenIPConfiguration`.
	** The public IP that is specified in the `frontenIPConfiguration` parameter.

		** The `frontenIPConfiguration` that is associated with the `api-v4` rule for the public load balancer.
		** The public IP that is specified in the `frontenIPConfiguration`.

	.. For {azure-short}, configure the Ingress Controller endpoint publishing scope to `Internal`.
	.. {azure-short} clusters: Configure the Ingress Controller endpoint publishing scope to `Internal`.


		.. For the {azure-short} public load balancer, if you configure the Ingress Controller endpoint publishing scope to `Internal` and there are no existing inbound rules in the public load balancer, you must create an outbound rule explicitly to provide outbound traffic for the backend address pool.
		For more information, see the Microsoft Azure documentation about adding outbound rules.

Uh oh!

OCPBUGS-45800#updating resources info #89851

Are you sure you want to change the base?

OCPBUGS-45800#updating resources info #89851

Uh oh!

Conversation

brendan-daly-red-hat commented Mar 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ocpdocs-previewbot commented Mar 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

brendan-daly-red-hat commented Mar 7, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jinyunma Mar 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jinyunma commented Mar 14, 2025

Uh oh!

openshift-ci bot commented Mar 21, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

brendan-daly-red-hat commented Mar 21, 2025

Uh oh!

skopacz1 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

openshift-ci bot commented Mar 24, 2025

Uh oh!

bergerhoffer commented Jun 16, 2025

Uh oh!

jinyunma commented Jun 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

openshift-bot commented Sep 21, 2025

Uh oh!

bergerhoffer commented Oct 20, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

brendan-daly-red-hat commented Mar 7, 2025 •

edited

Loading

ocpdocs-previewbot commented Mar 7, 2025 •

edited

Loading

jinyunma Mar 14, 2025 •

edited

Loading

jinyunma commented Jun 23, 2025 •

edited

Loading