Skip to content

Conversation

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

🤖 Fri Sep 26 20:43:16 - Prow CI generated the docs preview:
https://99801--ocpdocs-pr.netlify.app
Complete list of updated preview URLs: artifacts/updated_preview_urls.txt

Copy link

openshift-ci bot commented Sep 26, 2025

@sslocket: 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.

@sslocket
Copy link
Contributor Author

sslocket commented Oct 8, 2025

@jcpowermac PTAL

@jcpowermac
Copy link
Contributor

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Oct 8, 2025
@sslocket
Copy link
Contributor Author

@jinyunma PTAL

platform:
azure:
type: Standard_D4s_v5
dataDisks:
Copy link

@jinyunma jinyunma Oct 10, 2025

Choose a reason for hiding this comment

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

In install-config.yaml parameters, dataDisks objects are described under controlPlane machinepool.
Suggest to keep consistent here, something as below:

controlPlane:
  architecture: amd64
  hyperthreading: Enabled
  name: master
  platform:
    azure:
      dataDisks:
      - cachingType: ReadWrite
         diskSizeGB: 256
         lun: 1
         nameSuffix: etcddisk
  diskSetup:
  - type: etcd
    etcd:
      platformDiskID: etcddisk

And parameter cachingType is also missed under dataDisks.

$ ./openshift-install explain installconfig.controlPlane.platform.azure.dataDisks
KIND:     InstallConfig
VERSION:  v1

RESOURCE: <[]object>
  DataDisk specifies the parameters that are used to add one or more data disks to the machine.

FIELDS:
    cachingType <string>
      Valid Values: "None","ReadOnly","ReadWrite"
      CachingType specifies the caching requirements.

    diskSizeGB <integer> -required-
      Format: int32
      DiskSizeGB is the size in GB to assign to the data disk.

    lun <integer>
      Format: int32
      Lun Specifies the logical unit number of the data disk. This value is used to identify data disks within the VM and therefore must be unique for each data disk attached to a VM.
The value must be between 0 and 63.

    managedDisk <object>
      ManagedDisk specifies the Managed Disk parameters for the data disk.

    nameSuffix <string> -required-
      NameSuffix is the suffix to be appended to the machine name to generate the disk name.
Each disk name will be in format <machineName>_<nameSuffix>.

Copy link
Contributor

Choose a reason for hiding this comment

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

my concern with indicating readwrite for cachetype is data loss, maybe a link to azure documentation explaining the risks? for etcd though I would think None is more appropriate

also in researching this I see we have an issue that should be resolved:
https://learn.microsoft.com/en-us/azure/virtual-machines/premium-storage-performance#optimize-performance-on-linux-vms
I will put in a bug to fix this.

Copy link
Contributor Author

@sslocket sslocket Oct 10, 2025

Choose a reason for hiding this comment

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

@jinyunma @jcpowermac If cachingType: None is the safe option and there isn't a specific use case that necessitates the other two options (which also entail significant risks/unclear benefits), I would prefer to make cachingType: None the requirement for this 4.20 version of the documentation due to time constraints, and revisit later.

----

<1> Specify `etcd`.
<2> Specify a name to identify the disk.

Choose a reason for hiding this comment

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

The platformDiskID has limitation, it could not be longer than 12 characters.

<1> Specify `etcd`.
<2> Specify a name to identify the disk.
<3> Specify the same value given for `platformDiskID`.
<4> Specify a disk size in GB. This can be any integer from `8` through `32`.

Choose a reason for hiding this comment

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

hmm, why does disk size have limitation? It works when I set to more than 32. Or anything that I missed? cc @jcpowermac

Copy link
Contributor

Choose a reason for hiding this comment

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

I thought it was anything greater than 0, let me double check

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

lgtm Indicates that a PR is ready to be merged. 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.

4 participants