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.

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

TELCODOCS-328 - Adding new VIP install params for IPv4 and IPv6 #51135

Merged
merged 1 commit into from
Nov 28, 2022

Conversation

rohennes
Copy link
Contributor

@rohennes rohennes commented Oct 3, 2022

TELCODOCS-328: Ingress VIP and API VIP currently support IPv4 address only. With this update, IPv6 addresses are also supported.

Version(s):
4.12+

Issue:
https://issues.redhat.com/browse/TELCODOCS-328

Link to docs preview:
IPI updates

  1. Updated install config code and added note:
    http://file.emea.redhat.com/rohennes/TELCODOCS-328-dual-vips/installing/installing_bare_metal_ipi/ipi-install-installation-workflow.html#configuring-the-install-config-file_ipi-install-installation-workflow
  2. Updated apiVIPs/IngressVIPs parameter definition and added a note in each definition:
    http://file.emea.redhat.com/rohennes/TELCODOCS-328-dual-vips/installing/installing_bare_metal_ipi/ipi-install-installation-workflow.html#additional-install-config-parameters_ipi-install-installation-workflow
  3. Updated dual-stack networking section:
    http://file.emea.redhat.com/rohennes/TELCODOCS-328-dual-vips/installing/installing_bare_metal_ipi/ipi-install-installation-workflow.html#modifying-install-config-for-dual-stack-network_ipi-install-installation-workflow
  4. Updated yaml code to match new params in this section:
    http://file.emea.redhat.com/rohennes/TELCODOCS-328-dual-vips/installing/installing_bare_metal_ipi/ipi-install-installation-workflow.html#modifying-install-config-for-no-provisioning-network_ipi-install-installation-workflow

OpenStack updates

  1. Updated note to change apiVIP to apiVIPs
    http://file.emea.redhat.com/rohennes/TELCODOCS-328-dual-vips/installing/installing_openstack/installing-openstack-installer-custom.html#installation-osp-custom-subnet_installing-openstack-installer-custom
  2. Update code snippet and added note about deprecation
    http://file.emea.redhat.com/rohennes/TELCODOCS-328-dual-vips/installing/installing_openstack/installing-openstack-installer-custom.html#installation-osp-custom-subnet_installing-openstack-installer-custom

vSphere updates

  1. http://file.emea.redhat.com/rohennes/TELCODOCS-328-dual-vips/installing/installing_vsphere/installing-vsphere-installer-provisioned-network-customizations.html#installation-configuration-parameters-additional-vsphere_installing-vsphere-installer-provisioned-network-customizations
  2. Edited code snippet and added note.
    http://file.emea.redhat.com/rohennes/TELCODOCS-328-dual-vips/installing/installing_vmc/installing-vmc-customizations.html#installation-installer-provisioned-vsphere-config-yaml_installing-vmc-customizations

RHV updates:

  1. http://file.emea.redhat.com/rohennes/TELCODOCS-328-dual-vips/installing/installing_rhv/installing-rhv-customizations.html#installation-configuration-parameters-additional-rhv_installing-rhv-customizations
  2. Updated code snippet and added note after each code snippet
    http://file.emea.redhat.com/rohennes/TELCODOCS-328-dual-vips/installing/installing_rhv/installing-rhv-customizations.html#installing-rhv-example-install-config-yaml_installing-rhv-customizations

QE review:

  • QE has approved this change.

@openshift-ci openshift-ci bot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Oct 3, 2022
@ocpdocs-previewbot
Copy link

ocpdocs-previewbot commented Oct 3, 2022

🤖 Updated build preview is available at:
https://51135--docspreview.netlify.app

Build log: https://circleci.com/gh/ocpdocs-previewbot/openshift-docs/4140

@rohennes rohennes force-pushed the TELCODOCS-328-dual-vips branch 3 times, most recently from 11456c1 to 1a4e04a Compare October 4, 2022 13:27
@openshift-ci 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 Oct 4, 2022
@rohennes rohennes force-pushed the TELCODOCS-328-dual-vips branch 5 times, most recently from 7a62177 to 9dd093d Compare October 4, 2022 14:15
Copy link
Member

@cybertron cybertron left a comment

Choose a reason for hiding this comment

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

Mostly looks good. A few comments on the wording of the IP version requirements so we aren't implying anything that isn't supported.


[NOTE]
====
Before {product-title} 4.12, the cluster installation program accepted only an IPv4 address for the `apiVIP` configuration setting. From {product-title} 4.12 or later, the `apiVIP` configuration setting is deprecated. Instead, use a list format for the `apiVIPs` configuration setting to specify an IPv4 addresses, an IPv6 addresses or both IP address formats.
Copy link
Member

Choose a reason for hiding this comment

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

apiVIP accepted both ipv4 and ipv6 addresses. ipv6 was only for single-stack ipv6 deployments though.

| `apiVIP` | | (Optional) The virtual IP address for Kubernetes API communication.
| `apiVIPs` | a| (Optional) The virtual IP address for Kubernetes API communication.

This setting must either be provided in the `install-config.yaml` file as a reserved IP from the MachineNetwork or pre-configured in the DNS so that the default name resolves correctly. Use the virtual IP address and not the FQDN when adding a value to the `apiVIPs` configuration setting in the `install-config.yaml` file. The IP address must be from the primary IPv4 or IPv6 network when using dual-stack networking. If not set, the installation program uses `api.<cluster_name>.<base_domain>` to derive the IP address from the DNS.
Copy link
Member

Choose a reason for hiding this comment

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

"The IP address must be from the primary IPv4 or IPv6 network when using dual-stack networking."

In dual stack, the primary network must be ipv4. The requirements for ip version in the new config are:

ipv4 single stack: one ipv4 address
ipv6 single stack: one ipv6 address
dual stack: one ipv4 and optionally one ipv6 address, in that order


This setting must either be provided in the `install-config.yaml` file as a reserved IP from the MachineNetwork or pre-configured in the DNS so that the default name resolves correctly. Use the virtual IP address and not the FQDN when adding a value to the `ingressVIP` configuration setting in the `install-config.yaml` file. The IP address must be from the primary IPv4 network when using dual stack networking. If not set, the installer uses `test.apps.<cluster_name>.<base_domain>` to derive the IP address from the DNS.
This setting must either be provided in the `install-config.yaml` file as a reserved IP from the MachineNetwork or pre-configured in the DNS so that the default name resolves correctly. Use the virtual IP address and not the FQDN when adding a value to the `ingressVIP` configuration setting in the `install-config.yaml` file. The IP address must be from the primary IPv4 or IPv6 network when using dual-stack networking. If not set, the installation program uses `test.apps.<cluster_name>.<base_domain>` to derive the IP address from the DNS.
Copy link
Member

Choose a reason for hiding this comment

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

The same IP requirements apply here too.


[NOTE]
====
Before {product-title} 4.12, the cluster installation program accepted only an IPv4 address for the `ingressVIP` configuration setting. From {product-title} 4.12 or later, the `ingressVIP` configuration setting is deprecated. Instead, use a list format for the `ingressVIPs` configuration setting to specify an IPv4 addresses, an IPv6 addresses or both IP address formats.
Copy link
Member

Choose a reason for hiding this comment

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

Likewise here.


You can also configure IPv4 or IPv6 virtual IP (VIP) address endpoints for the Ingress VIP and API VIP. This provides an interface to the cluster for applications that use IPv4 or IPv6 addresses.
Copy link
Member

Choose a reason for hiding this comment

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

This should probably be "IPv4 and IPv6". In dual stack you can't have just an IPv6 VIP.


.Procedure

. Edit the `machineNetwork`, `clusterNetwork`, and `serviceNetwork` configuration settings in the `install-config.yaml` file to configure IPv4 or IPv6 address endpoints for nodes in the cluster. Each setting must have two CIDR entries each. Ensure the first CIDR entry is the IPv4 setting and the second CIDR entry is the IPv6 setting.
Copy link
Member

Choose a reason for hiding this comment

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

This should also be an "and".

@rohennes rohennes force-pushed the TELCODOCS-328-dual-vips branch 2 times, most recently from 25fbba2 to f3d3af4 Compare October 17, 2022 11:31
ingressVIPs:
- <wildcard_ipv4>
- <wildcard_ipv6>
----

This comment was marked as resolved.

@cybertron
Copy link
Member

/lgtm

All of the ip version requirements look good now, thanks!

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Oct 20, 2022
@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Oct 21, 2022
@openshift-ci
Copy link

openshift-ci bot commented Oct 21, 2022

New changes are detected. LGTM label has been removed.

@rohennes rohennes force-pushed the TELCODOCS-328-dual-vips branch 5 times, most recently from 54d0ba6 to ef018a9 Compare October 28, 2022 15:19
@openshift-ci openshift-ci bot removed the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Oct 28, 2022
@@ -1233,12 +1233,24 @@ Additional {rh-virtualization} configuration parameters are described in the fol
|Required. The vNIC profile ID of the VM network interfaces. This can be inferred if the cluster network has a single profile.
|String. For example: `3fa86930-0be5-4052-b667-b79f0a729692`

|`platform.ovirt.api_vip`
|`platform.ovirt.api_vips`
|Required. An IP address on the machine network that will be assigned to the API virtual IP (VIP). You can access the OpenShift API at this endpoint.
Copy link
Member

Choose a reason for hiding this comment

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

Actually it is possible to specify up to two IP addresses (in dual-stack case). Same applies for the other apiVips and ingressVips fields.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks @creydr - I have implemented your feedback. Please let me know if this is accurate or if any other issues. Thanks!

api_vips:
- 10.46.8.230
ingress_vips:
- 192.168.1.5
Copy link
Member

Choose a reason for hiding this comment

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

IIRC the ovirt people wanted to check their config first and remove the validation, if the validation is wrong: https://coreos.slack.com/archives/C68TNFWA2/p1663242301646269?thread_ts=1662377646.622669&cid=C68TNFWA2 - anyhow I haven't seen an exception for ovirt in the installer for the validation.
@mburman5 how did you proceed on the IP expected to be in one of the machine networks issue on ovirt?

@rohennes rohennes force-pushed the TELCODOCS-328-dual-vips branch 2 times, most recently from afc1c72 to 5815095 Compare November 2, 2022 12:15
@eldar101
Copy link

eldar101 commented Nov 2, 2022

Other than a small comment above,
/lgtm.

@rohennes
Copy link
Contributor Author

rohennes commented Nov 9, 2022

/label telco
/label peer-review-needed

@openshift-ci openshift-ci bot added telco Label for all Telco PRs peer-review-needed Signifies that the peer review team needs to review this PR labels Nov 9, 2022
@abhatt-rh
Copy link
Contributor

/label peer-review-in-progress

@openshift-ci openshift-ci bot added the peer-review-in-progress Signifies that the peer review team is reviewing this PR label Nov 9, 2022
Copy link
Contributor

@abhatt-rh abhatt-rh left a comment

Choose a reason for hiding this comment

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

Hi @rohennes,
Nice work!
I added a few comments for your consideration.

/remove-label peer-review-in-progress
/remove-label peer-review-needed
/label peer-review-done


[NOTE]
====
From {product-title} 4.12 or later, the `api_vip` configuration setting is deprecated. Instead, use a list format to enter a value in the `api_vips` configuration setting. The order of the list indicates the primary and secondary VIP address for each service.
Copy link
Contributor

Choose a reason for hiding this comment

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

Global comment based on the general usage across OpenShift docs:

Suggested change
From {product-title} 4.12 or later, the `api_vip` configuration setting is deprecated. Instead, use a list format to enter a value in the `api_vips` configuration setting. The order of the list indicates the primary and secondary VIP address for each service.
In {product-title} 4.12 and later, the `api_vip` configuration setting is deprecated. Instead, use a list format to enter a value in the `api_vips` configuration setting. The order of the list indicates the primary and secondary VIP address for each service.

@@ -148,6 +150,11 @@ specify the base64-encoded user name and password for your mirror registry.
<12> Provide the `imageContentSources` section from the output of the command to mirror the repository.
endif::restricted[]

[NOTE]
====
From {product-title} 4.12 or later, the `apiVIP` and `IngressVIP` configuration settings are deprecated. Instead, use a list format to enter values in the `apiVIPs` and `IngressVIPs` configuration settings.
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
From {product-title} 4.12 or later, the `apiVIP` and `IngressVIP` configuration settings are deprecated. Instead, use a list format to enter values in the `apiVIPs` and `IngressVIPs` configuration settings.
From {product-title} 4.12 or later, the `apiVIP` and `ingressVIP` configuration settings are deprecated. Instead, use a list format to enter values in the `apiVIPs` and `ingressVIPs` configuration settings.

+
[NOTE]
====
From {product-title} 4.12 or later, the `apiVIP` and `IngressVIP` configuration settings are deprecated. Instead, use a list format to enter values in the `apiVIPs` and `IngressVIPs` configuration settings.
Copy link
Contributor

Choose a reason for hiding this comment

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

For RHV, should these be as suggested?

Suggested change
From {product-title} 4.12 or later, the `apiVIP` and `IngressVIP` configuration settings are deprecated. Instead, use a list format to enter values in the `apiVIPs` and `IngressVIPs` configuration settings.
From {product-title} 4.12 or later, the `api_vip` and `ingress_vip` configuration settings are deprecated. Instead, use a list format to enter values in the `api_vips` and `ingress_vips` configuration settings.

@@ -111,6 +120,10 @@ pullSecret: '{"auths": ...}'
sshKey: ssh-ed12345 AAAA...
----

[NOTE]
====
From {product-title} 4.12 or later, the `apiVIP` and `IngressVIP` configuration settings are deprecated. Instead, use a list format to enter values in the `apiVIPs` and `IngressVIPs` configuration settings.
Copy link
Contributor

Choose a reason for hiding this comment

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

For RHV, should these be as suggested?

Suggested change
From {product-title} 4.12 or later, the `apiVIP` and `IngressVIP` configuration settings are deprecated. Instead, use a list format to enter values in the `apiVIPs` and `IngressVIPs` configuration settings.
From {product-title} 4.12 or later, the `api_vip` and `ingress_vip` configuration settings are deprecated. Instead, use a list format to enter values in the `api_vips` and `ingress_vips` configuration settings.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, good catch.


This setting must either be provided in the `install-config.yaml` file as a reserved IP from the MachineNetwork or pre-configured in the DNS so that the default name resolves correctly. Use the virtual IP address and not the FQDN when adding a value to the `ingressVIP` configuration setting in the `install-config.yaml` file. The IP address must be from the primary IPv4 network when using dual stack networking. If not set, the installer uses `test.apps.<cluster_name>.<base_domain>` to derive the IP address from the DNS.
This setting must either be provided in the `install-config.yaml` file as a reserved IP from the MachineNetwork or pre-configured in the DNS so that the default name resolves correctly. Use the virtual IP address and not the FQDN when adding a value to the `ingressVIP` configuration setting in the `install-config.yaml` file. The primary IP address must be from the IPv4 network when using dual stack networking. If not set, the installation program uses `test.apps.<cluster_name>.<base_domain>` to derive the IP address from the DNS.
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
This setting must either be provided in the `install-config.yaml` file as a reserved IP from the MachineNetwork or pre-configured in the DNS so that the default name resolves correctly. Use the virtual IP address and not the FQDN when adding a value to the `ingressVIP` configuration setting in the `install-config.yaml` file. The primary IP address must be from the IPv4 network when using dual stack networking. If not set, the installation program uses `test.apps.<cluster_name>.<base_domain>` to derive the IP address from the DNS.
This setting must either be provided in the `install-config.yaml` file as a reserved IP from the MachineNetwork or pre-configured in the DNS so that the default name resolves correctly. Use the virtual IP address and not the FQDN when adding a value to the `ingressVIPs` configuration setting in the `install-config.yaml` file. The primary IP address must be from the IPv4 network when using dual stack networking. If not set, the installation program uses `test.apps.<cluster_name>.<base_domain>` to derive the IP address from the DNS.

@@ -157,6 +172,11 @@ pullSecret: '{"auths": ...}'
sshKey: ssh-ed25519 AAAA...
----

[NOTE]
====
From {product-title} 4.12 or later, the `apiVIP` and `IngressVIP` configuration settings are deprecated. Instead, use a list format to enter values in the `apiVIPs` and `IngressVIPs` configuration settings.
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as above

Suggested change
From {product-title} 4.12 or later, the `apiVIP` and `IngressVIP` configuration settings are deprecated. Instead, use a list format to enter values in the `apiVIPs` and `IngressVIPs` configuration settings.
From {product-title} 4.12 or later, the `api_vip` and `ingress_vip` configuration settings are deprecated. Instead, use a list format to enter values in the `api_vips` and `ingress_vips` configuration settings.

@@ -6,7 +6,7 @@
[id='modifying-install-config-for-dual-stack-network_{context}']
= Optional: Deploying with dual-stack networking

To deploy an {product-title} cluster with dual-stack networking, edit the `machineNetwork`, `clusterNetwork`, and `serviceNetwork` configuration settings in the `install-config.yaml` file. Each setting must have two CIDR entries each. Ensure the first CIDR entry is the IPv4 setting and the second CIDR entry is the IPv6 setting.
For dual-stack networking in {product-title} clusters, you can configure IPv4 and IPv6 address endpoints for nodes in the cluster. Edit the `machineNetwork`, `clusterNetwork`, and `serviceNetwork` configuration settings in the `install-config.yaml` file to configure IPv4 and IPv6 address endpoints for nodes in the cluster. Each setting must have two CIDR entries each. Ensure the first CIDR entry is the IPv4 setting and the second CIDR entry is the IPv6 setting.
Copy link
Contributor

Choose a reason for hiding this comment

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

It’s usually best to put the action first. Consider the suggested change:

Suggested change
For dual-stack networking in {product-title} clusters, you can configure IPv4 and IPv6 address endpoints for nodes in the cluster. Edit the `machineNetwork`, `clusterNetwork`, and `serviceNetwork` configuration settings in the `install-config.yaml` file to configure IPv4 and IPv6 address endpoints for nodes in the cluster. Each setting must have two CIDR entries each. Ensure the first CIDR entry is the IPv4 setting and the second CIDR entry is the IPv6 setting.
For dual-stack networking in {product-title} clusters, you can configure IPv4 and IPv6 address endpoints for nodes in the cluster. To configure IPv4 and IPv6 address endpoints for nodes in the cluster, edit the `machineNetwork`, `clusterNetwork`, and `serviceNetwork` configuration settings in the `install-config.yaml` file. Each setting must have two CIDR entries each. Ensure the first CIDR entry is the IPv4 setting and the second CIDR entry is the IPv6 setting.

@openshift-ci openshift-ci bot 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 Nov 11, 2022
Copy link
Member

@cybertron cybertron left a comment

Choose a reason for hiding this comment

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

LGTM modulo the comments from @abhatt-rh

@@ -111,6 +120,10 @@ pullSecret: '{"auths": ...}'
sshKey: ssh-ed12345 AAAA...
----

[NOTE]
====
From {product-title} 4.12 or later, the `apiVIP` and `IngressVIP` configuration settings are deprecated. Instead, use a list format to enter values in the `apiVIPs` and `IngressVIPs` configuration settings.
Copy link
Member

Choose a reason for hiding this comment

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

Yes, good catch.

@rohennes
Copy link
Contributor Author

/label merge-review-needed

@openshift-ci openshift-ci bot added the merge-review-needed Signifies that the merge review team needs to review this PR label Nov 28, 2022
@EricPonvelle EricPonvelle added merge-review-in-progress Signifies that the merge review team is reviewing this PR and removed merge-review-needed Signifies that the merge review team needs to review this PR labels Nov 28, 2022
@EricPonvelle EricPonvelle merged commit 9a89e6b into openshift:main Nov 28, 2022
@EricPonvelle
Copy link
Contributor

/cherrypick enterprise-4.12

@openshift-cherrypick-robot

@EricPonvelle: new pull request created: #53215

In response to this:

/cherrypick enterprise-4.12

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/test-infra repository.

@EricPonvelle EricPonvelle removed the merge-review-in-progress Signifies that the merge review team is reviewing this PR label Nov 28, 2022
@bergerhoffer bergerhoffer added this to the Planned for 4.12 GA milestone Jan 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
branch/enterprise-4.12 peer-review-done Signifies that the peer review team has reviewed this PR size/L Denotes a PR that changes 100-499 lines, ignoring generated files. telco Label for all Telco PRs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants