CORS-1770: Support pd-balanced disk types for GCP deployments #7337
Conversation
openshift-ci-robot
added
the
jira/valid-reference
|
@marcbrown2024: This pull request references CORS-1770 which is a valid jira issue. In response to this:
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. |
|
You also need to change this to this Let's keep the disk checking uniform by creating the supported list. |
marcbrown2024
force-pushed
the
pd-balanced
branch
2 times, most recently
from
7f40d4a
to
45ed748
Compare
| @@ -82,7 +83,7 @@ func ValidateDefaultDiskType(p *gcp.MachinePool, fldPath *field.Path) field.Erro | |||
| allErrs := field.ErrorList{} | |||
|
|
|||
| if p != nil && p.OSDisk.DiskType != "" { | |||
| diskTypes := sets.NewString("pd-ssd") | |||
| diskTypes := sets.NewString("pd-ssd", "pd-balanced") | |||
There was a problem hiding this comment.
The DefaultMachinePlatform provides a way to set values for both the compute and control plane machinepools. In order to prevent pd-balanced from being used with control-plane machines, you should probably not include this change.
marcbrown2024
force-pushed
the
pd-balanced
branch
from
45ed748
to
690be69
Compare
|
/retest |
| @@ -48,7 +48,7 @@ func TestValidateMachinePool(t *testing.T) { | |||
| DiskType: "pd-", | |||
| }, | |||
| }, | |||
| expected: `^test-path\.diskType: Unsupported value: "pd-": supported values: "pd-ssd", "pd-standard"$`, | |||
| expected: `^test-path\.diskType: Unsupported value: "pd-": supported values: "pd-ssd", "pd-balanced", "pd-standard"$`, | |||
There was a problem hiding this comment.
Ordering is alphabetical so this needs to be.
| expected: `^test-path\.diskType: Unsupported value: "pd-": supported values: "pd-ssd", "pd-balanced", "pd-standard"$`, | |
| expected: `^test-path\.diskType: Unsupported value: "pd-": supported values: "pd-balanced", "pd-ssd", "pd-standard"$`, |
marcbrown2024
force-pushed
the
pd-balanced
branch
from
f4cf70f
to
4d29613
Compare
|
/retest |
2 similar comments
|
/retest |
|
/retest |
|
/approve |
|
@marcbrown2024: This pull request references CORS-1770 which is a valid jira issue. In response to this:
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. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: patrickdillon The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
bot
added
the
approved
|
/lgtm |
|
/retest |
|
We shouldn't have merge commits in a PR. You need to remove |
marcbrown2024
force-pushed
the
pd-balanced
branch
from
c6718fb
to
c9966f3
Compare
|
/retest |
|
|
||
| if p.Name == "master" && p.Platform.GCP.OSDisk.DiskType == "pd-standard" { | ||
| allErrs = append(allErrs, field.Invalid(fldPath.Child("diskType"), p.Platform.GCP.OSDisk.DiskType, fmt.Sprintf("%s not compatible with control planes.", p.Platform.GCP.OSDisk.DiskType))) | ||
| if p.Name == "master" { |
There was a problem hiding this comment.
| if p.Name == "master" { | |
| if p.Name == "master" && p.Platform.GCP.OSDisk.DiskType != "" { |
marcbrown2024
force-pushed
the
pd-balanced
branch
from
c9966f3
to
d737d49
Compare
There was a problem hiding this comment.
@marcbrown2024 I am not exactly sure how it happened but code-gen thinks you added pd-ssd rather than pd-balanced and it seems to be complaining about it
| @@ -379,6 +379,7 @@ spec: | |||
| control plane nodes, the valid value is pd-ssd. | |||
| enum: | |||
| - pd-ssd | |||
| - pd-balanced | |||
There was a problem hiding this comment.
Put this above pd-ssd. I think it's the ordering.
| @@ -1142,6 +1143,7 @@ spec: | |||
| plane nodes, the valid value is pd-ssd. | |||
| enum: | |||
| - pd-ssd | |||
| - pd-balanced | |||
| @@ -2570,6 +2572,7 @@ spec: | |||
| plane nodes, the valid value is pd-ssd. | |||
| enum: | |||
| - pd-ssd | |||
| - pd-balanced | |||
OCPBUGS-16245: Make nmstateconfig.yaml optional in config-drive
marcbrown2024
force-pushed
the
pd-balanced
branch
from
d737d49
to
badf67c
Compare
|
/retest |
4 similar comments
|
/retest |
|
/retest |
|
/retest |
|
/retest |
|
/lgtm |
|
/retest |
|
/retest |
|
/retest |
|
/hold Revision badf67c was retested 3 times: holding |
openshift-ci
bot
added
the
do-not-merge/hold
|
/hold cancel |
openshift-ci
bot
removed
the
do-not-merge/hold
|
@rna-afk: /override requires failed status contexts, check run or a prowjob name to operate on.
Only the following failed contexts/checkruns were expected:
If you are trying to override a checkrun that has a space in it, you must put a double quote on the context. In response to this:
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. |
|
/override ci/prow/okd-scos-images |
|
@rna-afk: Overrode contexts on behalf of rna-afk: ci/prow/okd-scos-images In response to this:
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. |
|
@marcbrown2024: The following tests failed, say
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. |
Adding support for
pd-balanceddisk types for the compute nodes by allowing users to provide it in the disk type field under compute in the install config.