Add option to configure worker type on AWS IPI step #13712
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: dsimansk The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
I guess I've placed the env var too low in the hierarchy. |
|
@dsimansk: The following test failed, say
Full PR test history. Your PR dashboard. DetailsInstructions 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. |
petr-muller
left a comment
petr-muller
left a comment
There was a problem hiding this comment.
This is currently discussed in https://issues.redhat.com/browse/DPTP-1740. This PR would do the job, but it would make jobs that would use this platform-dependent. We're discussing the option of providing a platform-agnostic set of parameters that would be respected by all workflows in that JIRA, so I'd like to wait for that discussion before merging this.
/hold
| default: 'null' | ||
| documentation: |- | ||
| The env var COMPUTE_WORKER defines type of flavour used for worker nodes, e.g. on AWS some tests may require larger m4.xlarge machines. | ||
| The default value "null" honours openshift-installer preset. |
There was a problem hiding this comment.
This should go directly to the step (the -ref.yaml file), not to the chain. The step needs to be usable on it's own.
There was a problem hiding this comment.
I tried that in my initial commit 5b4f423, but it failed other tests. Anyway let's wait for the outcome of DPTP-1740.
openshift-ci-robot
added
the
do-not-merge/hold
| - chain: ipi-conf-aws | ||
| - chain: ipi-install | ||
| env: | ||
| - name: COMPUTE_WORKER |
There was a problem hiding this comment.
nit: I think this should be COMPUTE_NODE_TYPE to distinguish from other compute properties (e.g. zones).
| - name: COMPUTE_WORKER | ||
| default: 'null' | ||
| documentation: |- | ||
| The env var COMPUTE_WORKER defines type of flavour used for worker nodes, e.g. on AWS some tests may require larger m4.xlarge machines. |
There was a problem hiding this comment.
nit: "The env var COMPUTE_WORKER" is already clear from context. Maybe just open with "The instance type to use for compute nodes, ..." or some such? Maybe link https://aws.amazon.com/ec2/instance-types/ ?
Personally, I'd prefer platform-agnostic values like memory_24G_cpu_10 that got translated in the step to a suitable instance type. That would allow jobs that needed larger compute to be platform-agnostic without needing folks to guess "hmm, I bet they don't need all of the m4.xlarge properties, and I can use $WHATEVER when I pivot the job to GCP", etc. But I'm fine with platform-specific values as a first pass.
|
Replaced by #14350, which just landed. /close |
|
@wking: Closed this PR. DetailsIn 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. |
5 participants
This PR adds ability to customize worker type of
ipi-awsworkflow.I'd appreciate feedback if that's a good direction how to add such customization.
Follow-up to https://issues.redhat.com/browse/DPTP-1740.