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

OSDOCS#8773: Agent configuration reference assembly #69292

Merged
merged 1 commit into from Jan 18, 2024

Conversation

skopacz1
Copy link
Contributor

@skopacz1 skopacz1 commented Dec 12, 2023

OSDOCS-8773

Version(s):
4.14+

(content should be backported in later doc effort, I believe the main difference would be which platforms are supported)

This PR adds a new assembly containing all the available configuration parameters for the install-config.yaml and agent-config.yaml files used during an Agent-based installation.

QE review:

  • QE has approved this change.

Preview: Installation configuration parameters for the Agent-based Installer

@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Dec 12, 2023
@openshift-ci-robot
Copy link

openshift-ci-robot commented Dec 12, 2023

@skopacz1: This pull request references OSDOCS-8773 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.16.0" version, but no target version was set.

In response to this:

OSDOCS-8773

Version(s):
4.12+

This PR adds a new assembly containing all the available configuration parameters for the install-config.yaml and agent-config.yaml files used during an Agent-based installation.

QE review:

  • QE has approved this change.

Preview:

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.

@openshift-ci openshift-ci bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Dec 12, 2023
@ocpdocs-previewbot
Copy link

ocpdocs-previewbot commented Dec 12, 2023

🤖 Thu Jan 18 16:44:11 - Prow CI generated the docs preview: https://69292--ocpdocs-pr.netlify.app

@skopacz1 skopacz1 changed the title OSDOCS-8773: Agent configuration reference assembly OSDOCS#8773: Agent configuration reference assembly Dec 12, 2023
@openshift-ci-robot openshift-ci-robot removed the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Dec 12, 2023
@openshift-ci-robot
Copy link

@skopacz1: No Jira issue is referenced in the title of this pull request.
To reference a jira issue, add 'XYZ-NNN:' to the title of this pull request and request another refresh with /jira refresh.

In response to this:

OSDOCS-8773

Version(s):
4.12+

This PR adds a new assembly containing all the available configuration parameters for the install-config.yaml and agent-config.yaml files used during an Agent-based installation.

QE review:

  • QE has approved this change.

Preview:

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.

toc::[]

Before you deploy an {product-title} cluster using the Agent-based Installer, you provide parameters to customize your cluster and the platform that hosts it.
When you create the `install-config.yaml` and `agent-config.yaml` files, you provide values for the required parameters through the command line. You can then modify the `install-config.yaml` and `agent-config.yaml` files to customize your cluster further.

Choose a reason for hiding this comment

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

Note sure what is meant by "through the command line" here as this is a referring to the yaml file only. There isn't a command to create an agent-config file

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure either, I took this wording from similar reference pages in other installer sections and modified it to include agent-config. Would this phrasing seem better to you?

Suggested change
When you create the `install-config.yaml` and `agent-config.yaml` files, you provide values for the required parameters through the command line. You can then modify the `install-config.yaml` and `agent-config.yaml` files to customize your cluster further.
When you create the `install-config.yaml` and `agent-config.yaml` files, you must provide values for the required parameters, and you can use the optional parameters to customize your cluster further.

Choose a reason for hiding this comment

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

Yes that looks good, thanks.


[NOTE]
====
After installation, you cannot modify these parameters in the `agent-config.yaml` file.

Choose a reason for hiding this comment

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

Maybe say that these settings are used for installation only and therefore can't be modified after install.


|kind:
|(needs info)
|(needs info)

Choose a reason for hiding this comment

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

As this is optional and not needed for the agent-config.yaml it can be removed.
Note that this is the definition of Kind, this resource isn't retrieved in an API
// Kind is a string value representing the REST resource this object represents.
// Servers may infer this from the endpoint the client submits requests to.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good, I can delete this parameter

|rendezvousIP:
|The IP address of the node that performs the bootstrapping process as well as running the `assisted-service` component.
You must provide the rendezvous IP address when you do not specify at least one host's IP address in the `networkConfig` parameter.
If this address is not provided, one IP address is selected from the provided hosts' `networkConfig`.

Choose a reason for hiding this comment

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

We probably should define here in more details how the the RendezvousIP is selected from the hosts if not provided. TBD.


|additionalNTPSources:
|(needs info)
|(needs info)

Choose a reason for hiding this comment

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

AdditionalNTPSources is a list of NTP sources (hostname or IP) to be added
to all cluster hosts. They are added to any NTP sources that were configured through other means. This is an optional setting.

|Host name.
Overrides the hostname obtained from either the Dynamic Host Configuration Protocol (DHCP) or a reverse DNS lookup.
Each host must have a unique hostname supplied by one of these methods.
|(needs info)

Choose a reason for hiding this comment

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

This seems reasonable, could also note that the hostname supplied here is optional.

|hosts:
interfaces:
|(needs info)
|(needs info)

Choose a reason for hiding this comment

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

The interface table provides a name and mac address mapping for the interfaces on the host. The settings must match the configuration in the NetworkConfig section. This table is optional, but if the NetworkConfig section is included it must also e provided.

|Enables provisioning of the {op-system-first} image to a particular device.
It examines the devices in the order it discovers them, and compares the discovered values with the hint values.
It uses the first discovered device that matches the hint value.
|(needs info)

Choose a reason for hiding this comment

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

Maybe in addition state that this is the device that the OS will be written during installation.

|(needs info)

|hosts:
networkConfig:

Choose a reason for hiding this comment

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

The networkConfig field is the host network definition. It must match the Network Management API defined in https://nmstate.io/.

networkConfig:
|(needs info)
|(needs info)

Choose a reason for hiding this comment

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

I think all of the fields below can be removed. The NetworkConfig must match the fields in https://nmstate.io/, its not necessary to redefine them here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good, I'll delete these remaining NetworkConfig parameters

It examines the devices in the order it discovers them, and compares the discovered values with the hint values.
It uses the first discovered device that matches the hint value.
This is the device that the operating system will be written on during installation.
|(needs accepted/valid values)
Copy link

Choose a reason for hiding this comment

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

I don't think need to list accepted/valid values here, it should just refer to other section in the docs that describes rootDeviceHints e.g. https://docs.openshift.com/container-platform/4.13/installing/installing_bare_metal_ipi/ipi-install-installation-workflow.html#root-device-hints_ipi-install-installation-workflow

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I still think there might be a need to define the type of data that should be used with this parameter (and all others, even if we don't want to specify which specific values are accepted). Would this be an accurate entry to include here?

A dictionary of key-value pairs. For more information, see Root device hints.

With a link to the section you provide in your comment

Copy link

Choose a reason for hiding this comment

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

Sure

interfaces:
name:
|The name of an interface on the host.
|(needs accepted/valid values)
Copy link

Choose a reason for hiding this comment

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

Interface names aren't really checked for validity so don't need to list valid values here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If a string of characters is accepted in general, I can just put "String" here. Let me know if that's not accurate.

Copy link

Choose a reason for hiding this comment

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

Yes that's fine

@skopacz1
Copy link
Contributor Author

skopacz1 commented Jan 5, 2024

@bfournie and other Agent devs, I think I have enough information to move this PR on to QE review, thanks for all the help! If you'd like to make more comments/feedback while I'm waiting on or implementing QE review, feel free to keep on doing so.

@skopacz1
Copy link
Contributor Author

skopacz1 commented Jan 5, 2024

@mhanss could you PTAL when you have a chance! Thank you!

Comment on lines +33 to +40
|metadata:
|Kubernetes resource `ObjectMeta`, from which only the `name` parameter is consumed.
|Object

|metadata:
name:
|The name of the cluster.
Copy link

Choose a reason for hiding this comment

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

The parameter metadata.name is optional; however, when included in the agent-config file, it takes precedence over the value specified in the install-config file.

Copy link

Choose a reason for hiding this comment

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

If included in the agent-config file, the metadata.name parameter neither takes precedence nor serves as a required or optional setting; rather, it is simply ignored, and the value specified in the install-config file is used.
@bfournie please confirm.

Copy link

Choose a reason for hiding this comment

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

The metadata name and namespace are required. From the agent-config.yaml defintion: https://github.com/openshift/installer/blob/master/pkg/types/agent/agent_config_type.go#L19-L20

However they are not validated or used. As Manoj indicated, the fields are ignored

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll add a sentence to this entry stating "The value entered in the agent-config.yaml file is ignored, and the instead the value specified in the install-config.yaml file is used."

Let me know if that sounds right, or if I need to add to/change that sentence.

Copy link

Choose a reason for hiding this comment

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

That sounds right, although I did discover that if the install-config.yaml is ALSO not supplied that a default cluster name of agent-cluster will be used, see https://github.com/openshift/installer/blob/master/pkg/asset/agent/installconfig.go#L253-L258

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@bfournie just out of curiosity, how is this value a required parameter in both the agent-config and install-config files if the installation proceeds without them specified? I'm not familiar with OCP configuration, does "required" not mean "necessary for the installation and thus the installer will terminate with an error without this value"?

Copy link

Choose a reason for hiding this comment

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

@bfournie If the value is not specified in the install-config, the following error message will be generated: "FATAL failed to fetch Agent Manifests: failed to load asset "Install Config": invalid install-config configuration: metadata.name: Invalid value: "": a lowercase RFC 1123 subdomain must consist of lower case alphanumeric characters, '-' or '.', and must start and end with an alphanumeric character"

Choose a reason for hiding this comment

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

For the install-config.yaml, if it's provided the metadata.name must be included. However install-config.yaml is optional for the agent installer (when using just the ZTP manifests). In that case the cluster name will be set to agent-cluster

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Proposed sentence to add to metadata.name descriptions for both agent-config and install-config:

"When you do not provide metadata.name through either the install-config.yaml or agent-config.yaml files, for example when you use only ZTP manifests, the cluster name will be set to agent-cluster."

@mhanss
Copy link

mhanss commented Jan 9, 2024

@skopacz1 I would suggest omitting unsupported platforms from the install-config in case of agent-based installation.

@skopacz1
Copy link
Contributor Author

skopacz1 commented Jan 9, 2024

@mhanss thanks for the feedback, let me know if I'm missing anything or need to further refine this PR.

|`alibabacloud`, `aws`, `azure`, `gcp`, `ibmcloud`, `nutanix`, `openstack`, `powervs`, `vsphere`, or `{}`
endif::agent[]
ifdef::agent[]
|`vsphere`
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mhanss do I need to add {} and/or external to this parameter?

Copy link

Choose a reason for hiding this comment

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

I suggest adding the {} as it involves user-provided infrastructure that doesn't require machine configuration through install-config.yaml.

platform: {} or

platform:
  baremetal: {}
platform:
  vsphere: {} 
platform:
  external: {}

@bfournie @pawanpinjarkar What are your thoughts on this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Until/unless anyone says otherwise, I'll add {} as a valid value for this parameter and for controlPlane.platform, and I can modify as needed.

Choose a reason for hiding this comment

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

@skopacz1 You can set controlPlane.Platform and compute.Platform to {} for agent installer like below:

            "controlPlane": {
                "name": "master",
                "replicas": 1,
                "platform": {},
                "hyperthreading": "Enabled",
                "architecture": "amd64"
            },
            "compute": [
                {
                    "name": "worker",
                    "replicas": 0,
                    "platform": {},
                    "hyperthreading": "Enabled",
                    "architecture": "amd64"
                }
            ],

I don't think you need to specify other possible platform values in there. The possible values could be ( no external here),

"controlPlane": {
                "name": "master",
                "replicas": 1,
                "platform": {
                    "aws": {
                        "zones": ["us-east", "us-west"],
                        "type": "test"
                    }
                },
                "hyperthreading": "Enabled",
                "architecture": "amd64"
            },

Here is the schema definition for platform section for controlPlane and compute https://github.com/openshift/installer/blob/master/pkg/types/machinepools.go#L64

Each of the underlying platform schema definition is here https://github.com/openshift/installer/blob/master/pkg/types/machinepools.go#L84 and any specific ones are here https://github.com/openshift/installer/blob/master/pkg/types/aws/machinepool.go#L5

Long story short, for agent installer, you can just set controlPlane.Platform and compute.Platform to {}

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 for the detailed response, Pawan!

|`alibabacloud`, `aws`, `azure`, `gcp`, `ibmcloud`, `nutanix`, `openstack`, `powervs`, `vsphere`, or `{}`
endif::agent[]
ifdef::agent[]
|`vsphere`
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mhanss do I need to add {} and/or external to this parameter?

Copy link

Choose a reason for hiding this comment

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

see this #69292 (comment)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Until/unless anyone says otherwise, I'll add {} as a valid value for this parameter and for compute.platform, and I can modify as needed.

You must provide the rendezvous IP address when you do not specify at least one host's IP address in the `networkConfig` parameter.
If this address is not provided, one IP address is selected from the provided hosts' `networkConfig`.
|IPv4 or IPv6 address.
IPv6 is supported only on bare metal platforms.
Copy link

Choose a reason for hiding this comment

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

It is also supported on vSphere platforms.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Based on our previous conversation I'll remove this line entirely.

Copy link

Choose a reason for hiding this comment

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

We need to update the network configuration and some other values in the install-config. You can refer to this documentation for guidance on network configuration. https://docs.openshift.com/container-platform/4.14/installing/installing_bare_metal/installation-config-parameters-bare-metal.html#installation-configuration-parameters-network_installation-config-parameters-bare-metal.

  • The architecture values for Compute and controlPlane should be either amd64 or arm64.
  • In the case of SNO, the controlPlane replicas value can be set to 1.
  • IPv6 is supported for both bare-metal and vSphere platforms. Therefore, the statement indicating that IPv6 is only supported for bare-metal can be removed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've taken the following actions in response to this comment:

  1. Made the network config section in Agent look the same as the network config in the bare metal page you linked to, which includes the content that mentions both IPv4 and IPv6.
  2. Modified the compute.architecture and controlPlane.architecture parameters to include both of those architectures in the Agent doc.
  3. Modified controlPlane.replicas to say that 1 is a valid value for SNO. This change will appear in all the install sections that have this reference info, not just Agent.
  4. Removed content in the Agent page that states either "only IPv4 is supported" or "IPv6 is supported only for bare metal", these changes are applied to both the install-config and agent-config sections.

|`alibabacloud`, `aws`, `azure`, `gcp`, `ibmcloud`, `nutanix`, `openstack`, `powervs`, `vsphere`, or `{}`
endif::agent[]
ifdef::agent[]
|`vsphere` or `{}`
Copy link

Choose a reason for hiding this comment

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

Include 'baremetal', 'vsphere', as well as {} as supported values, or simply use {} alone.

|`alibabacloud`, `aws`, `azure`, `gcp`, `ibmcloud`, `nutanix`, `openstack`, `powervs`, `vsphere`, or `{}`
endif::agent[]
ifdef::agent[]
|`vsphere` or `{}`
Copy link

Choose a reason for hiding this comment

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

Include 'baremetal', 'vsphere', as well as {} as supported values, or simply use {} alone.

@mhanss
Copy link

mhanss commented Jan 17, 2024

thanks @skopacz1, LGTM.

@skopacz1
Copy link
Contributor Author

/label peer-review-needed

@openshift-ci openshift-ci bot added the peer-review-needed Signifies that the peer review team needs to review this PR label Jan 17, 2024
@kcarmichael08 kcarmichael08 added peer-review-in-progress Signifies that the peer review team is reviewing this PR and removed peer-review-needed Signifies that the peer review team needs to review this PR labels Jan 18, 2024
Copy link
Contributor

@kcarmichael08 kcarmichael08 left a comment

Choose a reason for hiding this comment

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

Great job with all the new content! I marked a few minor style things and some questions I had. Hope this helps!

|apiVersion:
|The API version for the `agent-config.yaml` content.
The current version is `v1beta1`.
The installation program may also support older API versions.
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
The installation program may also support older API versions.
The installation program might also support older API versions.

See IBM Style

[role="_additional-resources"]
.Additional resources
* xref:../../installing/installing_with_agent_based_installer/prepare-pxe-assets-agent.adoc#prepare-pxe-assets-agent[Preparing PXE assets for {product-title}].
* xref:../../installing/installing_bare_metal_ipi/ipi-install-installation-workflow.adoc#root-device-hints_ipi-install-installation-workflow[Root device hints].
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
* xref:../../installing/installing_bare_metal_ipi/ipi-install-installation-workflow.adoc#root-device-hints_ipi-install-installation-workflow[Root device hints].
* xref:../../installing/installing_bare_metal_ipi/ipi-install-installation-workflow.adoc#root-device-hints_ipi-install-installation-workflow[Root device hints]

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a duplicate of the comment below, so I'm assuming this is meant for the first list item in this Additional Resources list. I'll remove periods from both items.

Copy link
Contributor

Choose a reason for hiding this comment

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

ooops. yes :)

[role="_additional-resources"]
.Additional resources
* xref:../../installing/installing_with_agent_based_installer/prepare-pxe-assets-agent.adoc#prepare-pxe-assets-agent[Preparing PXE assets for {product-title}].
* xref:../../installing/installing_bare_metal_ipi/ipi-install-installation-workflow.adoc#root-device-hints_ipi-install-installation-workflow[Root device hints].
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
* xref:../../installing/installing_bare_metal_ipi/ipi-install-installation-workflow.adoc#root-device-hints_ipi-install-installation-workflow[Root device hints].
* xref:../../installing/installing_bare_metal_ipi/ipi-install-installation-workflow.adoc#root-device-hints_ipi-install-installation-workflow[Root device hints]


:_mod-docs-content-type: CONCEPT
[id="agent-configuration-parameters_{context}"]
= Available Agent configuration parameters
Copy link
Contributor

Choose a reason for hiding this comment

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

missing blank line after header?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch, thank you

name:
|The name of the cluster.
DNS records for the cluster are all subdomains of `{{.metadata.name}}.{{.baseDomain}}`.
The value entered in the `agent-config.yaml` file is ignored, and the instead the value specified in the `install-config.yaml` file is used.
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
The value entered in the `agent-config.yaml` file is ignored, and the instead the value specified in the `install-config.yaml` file is used.
The value entered in the `agent-config.yaml` file is ignored, and instead the value specified in the `install-config.yaml` file is used.

|hosts:
interfaces:
|Provides a table of the name and MAC address mappings for the interfaces on the host.
If a `NetworkConfig` section is provided in the `agent-config.yaml`, this table must be included and the values must match the mappings provided in the `NetworkConfig` section.
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
If a `NetworkConfig` section is provided in the `agent-config.yaml`, this table must be included and the values must match the mappings provided in the `NetworkConfig` section.
If a `NetworkConfig` section is provided in the `agent-config.yaml` file, this table must be included and the values must match the mappings provided in the `NetworkConfig` section.

|hosts:
rootDeviceHints:
|Enables provisioning of the {op-system-first} image to a particular device.
It examines the devices in the order it discovers them, and compares the discovered values with the hint values.
Copy link
Contributor

Choose a reason for hiding this comment

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

It = The installation program ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I believe so, good catch! Thanks

ifdef::agent[]
|compute:
architecture:
|Determines the instruction set architecture of the machines in the pool. Currently, clusters with varied architectures are not supported. All pools must specify the same architecture. Valid values are `amd64` and `arm64`.
Copy link
Contributor

Choose a reason for hiding this comment

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

Would the "valid values" part go in the values column?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Based on the way this attribute is written for other install pages (see lines 458-487) I guess not. I'd prefer to follow the existing format unless you'd prefer me to put these values in the other column.

ifdef::agent[]
|controlPlane:
architecture:
|Determines the instruction set architecture of the machines in the pool. Currently, clusters with varied architectures are not supported. All pools must specify the same architecture. Valid values are `amd64` and `arm64`.
Copy link
Contributor

Choose a reason for hiding this comment

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

see previous "values" comment

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Based on the way this attribute is written for other install pages (see lines 551-580) I guess not. I'd prefer to follow the existing format unless you'd prefer me to put these values in the other column.

@@ -173,7 +173,7 @@ EOF
You must provide the rendezvous IP address when you do not specify at least one host's IP address in the `networkConfig` parameter. If this address is not provided, one IP address is selected from the provided hosts' `networkConfig`.
<2> Optional: Host configuration. The number of hosts defined must not exceed the total number of hosts defined in the `install-config.yaml` file, which is the sum of the values of the `compute.replicas` and `controlPlane.replicas` parameters.
<3> Optional: Overrides the hostname obtained from either the Dynamic Host Configuration Protocol (DHCP) or a reverse DNS lookup. Each host must have a unique hostname supplied by one of these methods.
<4> Enables provisioning of the Red Hat Enterprise Linux CoreOS (RHCOS) image to a particular device. It examines the devices in the order it discovers them, and compares the discovered values with the hint values. It uses the first discovered device that matches the hint value.
<4> Enables provisioning of the {op-system-first} image to a particular device. It examines the devices in the order it discovers them, and compares the discovered values with the hint values. It uses the first discovered device that matches the hint value.
Copy link
Contributor

Choose a reason for hiding this comment

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

It = installation program?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think so, yeah.

@kcarmichael08 kcarmichael08 added peer-review-done Signifies that the peer review team has reviewed this PR branch/enterprise-4.14 branch/enterprise-4.15 and removed peer-review-in-progress Signifies that the peer review team is reviewing this PR labels Jan 18, 2024
@kcarmichael08 kcarmichael08 added this to the Continuous Release milestone Jan 18, 2024
@bscott-rh
Copy link
Contributor

Reviewed the conditionals in the install config parameters module and they LGTM.

Copy link

openshift-ci bot commented Jan 18, 2024

@skopacz1: 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/test-infra repository. I understand the commands that are listed here.

@skopacz1
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 Jan 18, 2024
@ShaunaDiaz ShaunaDiaz merged commit 12afe2f into openshift:main Jan 18, 2024
2 checks passed
@ShaunaDiaz
Copy link
Contributor

/remove-label merge-review-needed

@openshift-ci openshift-ci bot removed the merge-review-needed Signifies that the merge review team needs to review this PR label Jan 18, 2024
@ShaunaDiaz
Copy link
Contributor

/cherrypick enterprise-4.14

@ShaunaDiaz
Copy link
Contributor

/cherrypick enterprise-4.15

@openshift-cherrypick-robot

@ShaunaDiaz: new pull request created: #70531

In response to this:

/cherrypick enterprise-4.14

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.

@openshift-cherrypick-robot

@ShaunaDiaz: new pull request created: #70532

In response to this:

/cherrypick enterprise-4.15

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
branch/enterprise-4.14 branch/enterprise-4.15 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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

10 participants