ci/installer-5327 : aws-ec2 - change instance to m6i#23031
ci/installer-5327 : aws-ec2 - change instance to m6i#23031mtulio wants to merge 3 commits intoopenshift:masterfrom
Conversation
|
/hold |
openshift-ci
Bot
added
the
do-not-merge/hold
mtulio
force-pushed
the
fix-installer-5327
branch
from
e8b7e5c to
dcbbf02
Compare
|
/retest |
There was a problem hiding this comment.
In practice this probably ends up the same, but I would have expected that the preferred family be used as long as there was at least one zone (or perhaps 2 zones) offering the preferred family.
There was a problem hiding this comment.
iiuc, I am not sure if it will ends up the same, it will help to select the correct instance on the region which does not support the preferred one (see the original PR for the table of availability):
output of $ cat install-config.yaml |yq -r '. | (.platform.aws, .controlPlane.platform.aws)',
for LEASED_RESOURCE=sa-east-1:
{
"region": "sa-east-1",
"userTags": {
"expirationDate": "2021-12-01T22:34+00:00"
}
}
{
"zones": [],
"type": "m5.2xlarge"
}
for LEASED_RESOURCE=us-east-1:
{
"region": "us-east-1",
"userTags": {
"expirationDate": "2021-12-01T22:35+00:00"
}
}
{
"zones": [],
"type": "m6i.2xlarge"
}There was a problem hiding this comment.
but I would have expected that the preferred family be used as long as there was at least one zone (or perhaps 2 zones)
if it's not a problem, we could change to something like:
if [[ ${zones_cnt_pref} -ge 1 ]]; thenwdyt?
There was a problem hiding this comment.
updating the latest availability:
count(m5.xlarge) count(m6i.xlarge) diff(m5_m6i)
region
af-south-1 3 0 3
ap-east-1 3 0 3
ap-northeast-1 3 2 1
ap-northeast-2 4 3 1
ap-northeast-3 3 0 3
ap-south-1 3 3 0
ap-southeast-1 3 3 0
ap-southeast-2 3 2 1
ca-central-1 3 0 3
eu-central-1 3 3 0
eu-north-1 3 0 3
eu-south-1 3 0 3
eu-west-1 3 3 0
eu-west-2 3 0 3
eu-west-3 3 3 0
me-south-1 3 0 3
sa-east-1 3 2 1
us-east-1 5 5 0
us-east-2 3 3 0
us-west-1 2 2 0
us-west-2 6 4 2
There was a problem hiding this comment.
it's not. I just removed it, and replace the ${master_type} to ${MASTER_TYPE} used in this script. 👍🏼
There was a problem hiding this comment.
This does not take into account whether AZs are actually available or whether the AZs were specified already in the install-config.yaml.
There was a problem hiding this comment.
you're right. I just fixed it:
AWS just change the availability of
sa-east-1, now I am changing the region toaf-south-1will get the expected results
$ yq .controlPlane.platform.aws install-config.yaml
{
"zones": [
"af-south-1a",
"af-south-1b"
]
}
$ export LEASED_RESOURCE=af-south-1
$ bash ipi-conf-aws-commands.sh
zones already set in install-config.yaml, skipped
$ yq .controlPlane.platform.aws install-config.yaml
{
"zones": [
"af-south-1a",
"af-south-1b"
],
"type": "m5.xlarge"
}
$ export LEASED_RESOURCE=us-east-1
$ cp install-config.yaml-use1 install-config.yaml
$ yq .controlPlane.platform.aws install-config.yaml
{
"zones": [
"us-east-1a",
"us-east-1b"
]
}
$ bash ipi-conf-aws-commands.sh
zones already set in install-config.yaml, skipped
$ yq .controlPlane.platform.aws install-config.yaml
{
"zones": [
"us-east-1a",
"us-east-1b"
],
"type": "m6i.xlarge"
}
mtulio
force-pushed
the
fix-installer-5327
branch
from
dcbbf02 to
64c47e6
Compare
openshift-ci
Bot
added
the
needs-rebase
mtulio
force-pushed
the
fix-installer-5327
branch
from
64c47e6 to
6b2e3f3
Compare
openshift-ci
Bot
added
needs-rebase
mtulio
force-pushed
the
fix-installer-5327
branch
4 times, most recently
from
85e2ade to
44691fd
Compare
openshift-ci
Bot
added
the
do-not-merge/work-in-progress
mtulio
force-pushed
the
fix-installer-5327
branch
from
44691fd to
8bd275b
Compare
openshift-ci
Bot
removed
the
do-not-merge/work-in-progress
|
@staebler ptal? |
|
/retest-required |
|
/unhold |
openshift-ci
Bot
removed
the
do-not-merge/hold
| | wc -l) | ||
| fi | ||
|
|
||
| if [[ ${zones_cnt_pref} -ge ${zones_cnt_backup} ]]; then |
There was a problem hiding this comment.
When you switch this logic to check if there are enough zones for the preferred or backup type, then make sure that the zones with availability matches exactly when the zones are explicitly specified in the install config.
|
Issues in openshift/release go stale after 30d of inactivity. Mark the issue as fresh by commenting If this issue is safe to close now please do so with /lifecycle stale |
openshift-ci
Bot
added
the
lifecycle/stale
|
Stale issue in openshift/release rot after 15d of inactivity. Mark the issue as fresh by commenting If this issue is safe to close now please do so with /lifecycle rotten |
openshift-ci
Bot
added
lifecycle/rotten
|
Rotten issues in openshift/release close after 15d of inactivity. Reopen the issue by commenting /close |
|
@openshift-bot: 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. |
|
I need to raise the priority on this. Patric, I will work on this next week. Please let me know if you also can see something else to change or any other suggestions. |
|
@mtulio: Reopened 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. |
|
/remove-lifecycle rotten |
openshift-ci
Bot
added
do-not-merge/hold
|
@mtulio: The 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. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: mtulio 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 |
Sure Marco, but I think I need to get caught up. I don't understand the goal here. It looks like you are developing logic to choose the best instance type but what good does that do to put it in ci? Should this logic go in the installer? |
|
@mtulio: PR needs rebase. 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. |
openshift-ci
Bot
added
the
needs-rebase
|
@mtulio: The following tests 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. |
|
@mtulio What is the status of this PR? |
|
/close |
|
@droslean: 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. |
@patrickdillon : The goal in this PR is to simulate the installer behavior when the m6i is not available in all zones within the region - but tbh it seems to be very specific and, as you can see, is becoming very complex to be implemented here. |
6 participants
fixes for installer PR openshift/installer#5327
The idea is to choose the preferred instance type (in general newer gen) when it's available on the equal or higher number of zones on a given region, otherwise, the backup will be chosen.
/cc @staebler