Return only supported instances-types with describe-instance-type-offing #22399
Conversation
ci-operator/step-registry/ipi/conf/aws/ipi-conf-aws-commands.sh
Outdated
Show resolved
Hide resolved
austincunningham
changed the title
| @@ -42,7 +42,7 @@ fi | |||
| # Generate working availability zones from the region | |||
| mapfile -t AVAILABILITY_ZONES < <(aws --region "${REGION}" ec2 describe-availability-zones | jq -r '.AvailabilityZones[] | select(.State == "available") | .ZoneName' | sort -u) | |||
| # Generate availability zones with OpenShift Installer required instance types | |||
| mapfile -t INSTANCE_ZONES < <(aws --region "${REGION}" ec2 describe-instance-type-offerings --location-type availability-zone --filters Name=instance-type,Values="${BOOTSTRAP_NODE_TYPE}","${master_type}","${COMPUTE_NODE_TYPE}" | jq -r '.InstanceTypeOfferings[].Location' | sort -u) | |||
| mapfile -t INSTANCE_ZONES < <(aws --region "${REGION}" ec2 describe-instance-type-offerings --location-type availability-zone --filters Name=instance-type,Values="${BOOTSTRAP_NODE_TYPE}","${master_type}","${COMPUTE_NODE_TYPE}" | jq -r '.InstanceTypeOfferings[].Location' | sort | uniq -d) | |||
There was a problem hiding this comment.
uniq -d will fail in two cases:
- Only a single available instance type is requested, e.g.
m5.large,m5.large,nullorm5.large,null,null. - Three separate instance types are requested, and there are regions where only two are available.
I think what we want is some pre-processing to build a Values argument without dups or null, and then jq post-processing to only accept zones which have the expected count of entries.
There was a problem hiding this comment.
As ${master_type} is always set to null in this script
${BOOTSTRAP_NODE_TYPE} is defaulted to m5.large #14350
The only var we need to check is ${COMPUTE_NODE_TYPE}
so either
| ${BOOTSTRAP_NODE_TYPE} | ${master_type} | ${COMPUTE_NODE_TYPE} |
|---|---|---|
| m5.large | null | m5.large |
| m5.large | null | null |
| m5.large | null | m4.xlarge (or some other type) |
if ${COMPUTE_NODE_TYPE} is a duplicate of BOOTSTRAP_NODE_TYPE or a null then the original sort works
if not then the uniq -d sort works
There was a problem hiding this comment.
actually I'm wrong master_type and BOOTSTRAP_NODE_TYPE are subject to change.
…rings addressing pr comments addressing pr comments
austincunningham
force-pushed
the
bug-exclude-region
branch
from
b6a613a
to
5f8d172
Compare
|
/cc e-tienne |
|
/retest-required |
|
@austincunningham: 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. |
Let's just keep an eye on the CI results as soon as it merges to make sure it's working as expected. |
|
@e-tienne looks like tide needs an |
|
/test build-farm |
|
@austincunningham: The specified target(s) for
The following commands are available to trigger optional jobs:
Use
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. |
|
/test build03-dry |
|
@e-tienne do you have some time to take a look at this one? I see that you've lgtm'd it but it needs an approve to merge. Until this merges our e2e tests are non functional on all of our prs so it is a high priority for us atm. If you had some time that would be great. 🙏 |
|
ping @csrwng do you have some time to take a look at this pr? |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: austincunningham, dgoodwin, e-tienne 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
|
@austincunningham: Updated the
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. |
WHAT
Issue provision prow clusters on us-east-1e m5.large not supported
HOW
Setting the filter to use
sortanduniq -callows ec2 describe-instance-type-offerings to return the correct valuesbelow script can be used to test the conditionals