SPLAT-657: ipi/aws-local-zones: increase zone coverage in opted-out zones #40606
Conversation
openshift-ci-robot
added
the
jira/valid-reference
openshift-ci
bot
added
the
do-not-merge/work-in-progress
|
@mtulio: This pull request references SPLAT-657 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. |
|
Skipping CI for Draft Pull Request. |
mtulio
force-pushed
the
splat-657-presubmits-local-zones-coverage
branch
from
f46c5b8
to
c5a9a76
Compare
openshift-ci
bot
removed
the
do-not-merge/work-in-progress
|
[REHEARSALNOTIFIER]
A total of 7099 jobs have been affected by this change. The above listing is non-exhaustive and limited to 35 jobs. A full list of affected jobs can be found here Interacting with pj-rehearseComment: Once you are satisfied with the results of the rehearsals, comment: |
|
[REHEARSALNOTIFIER]
A total of 7099 jobs have been affected by this change. The above listing is non-exhaustive and limited to 35 jobs. A full list of affected jobs can be found here Interacting with pj-rehearseComment: Once you are satisfied with the results of the rehearsals, comment: |
|
More improvements to increase the coverage in installer picking random zones even if it's not yet opted-in. Ref PR openshift/installer#7137 For IPI owners, ptal? |
|
/assign @dgoodwin |
|
/pj-rehearse skip |
openshift-ci-robot
added
the
rehearsals-ack
|
@vrutkovs would you mind taking a look? it's an small improvement in the zone query |
|
Why do we want to increase the coverage? This should be in commit message / PR description |
|
@mtulio: This pull request references SPLAT-657 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. |
|
@mtulio: This pull request references SPLAT-657 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. |
Updated. That update is also also covered in the RFE for Local Zones. |
|
Oh, I see. I don't know the implications of using not-yet-supported zones, I'll leave it for Devan to approve. I don't think picking random zone is a good idea cost-wise tbh |
|
@vrutkovs - Thanks for sharing your thoughts.
Which zone is not supported? We are declaring support for Local Zones in 4.12. If we'll not test in CI, we are not testing what we are declaring as supported to our customers.
What is the cost increase of getting static zone and random zone?[1][2] This suggestion was to prevent high costs from getting all zones in all tests (alongside increasing the total execution time, and risk for failures). The cost will increase for compute almost 14x (in us-east-1, N*zones) for each execution if we'll not pick one zone for each execution. Another cons of getting one static zone is we'll also not cover the customer scenario. [1] https://github.com/openshift/enhancements/blob/master/enhancements/installer/aws-custom-edge-machineset-local-zones.md#infrastructure-costs |
|
Hi Yunfei, as you are familiar with Local Zones, could you please take a look too? |
|
This LGTM but why skip the rehearsals? I would feel more confident with rehearsals. I think marco is out for the moment, so I'm going to go ahead and run them /pj-rehearse |
Thanks. Reason for skip: rehearsals history exercised when introducing this job:
Here you can see the merged version of this change (job |
|
/pj-rehearse pull-ci-openshift-installer-master-e2e-aws-ovn-localzones |
|
@patrickdillon, If the problem persists, please contact Test Platform. |
failed as expected (due the open PR): https://prow.ci.openshift.org/view/gs/origin-ci-test/pr-logs/pull/openshift_release/40606/rehearse-40606-pull-ci-openshift-installer-master-e2e-aws-ovn-localzones/1674173677668667392#1:build-log.txt%3A129 zone selected: Triggering other tests which are more related to this change: |
|
/pj-rehearse pull-ci-openshift-installer-master-e2e-aws-ovn-imdsv2 |
|
Unrelated failure in the test /pj-rehearse pull-ci-openshift-installer-master-e2e-aws-ovn |
@patrickdillon what is the pass ratio of those jobs to get confidence in your PoV? |
Just a sanity check to make sure nothing is obviously broken. Is this an expected failure https://prow.ci.openshift.org/view/gs/origin-ci-test/pr-logs/pull/openshift_release/40606/rehearse-40606-pull-ci-openshift-installer-master-e2e-aws-ovn-localzones/1674173677668667392 |
@patrickdillon - Yes, expected fail preventing to install in non-existing VPC: |
|
/lgtm |
|
/pj-rehearse pull-ci-openshift-installer-master-e2e-aws-ovn-fips |
|
/pj-rehearse pull-ci-openshift-installer-master-e2e-aws-ovn |
@patrickdillon can you see any blocker in this PR? |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: mtulio, patrickdillon, rvanderp3 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
|
@mtulio: 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. |
openshift-merge-robot
merged commit a23dafa
into
openshift:master
AWS is constantly adding new locations in Local Zones, we need to test it to prevent unwanted scenarios in features we are declaring supported. It will also not increase the cost of the test as it is selecting randomly one single zone.
openshift/enhancements#1232
Increasing the zone coverage by setting the flag
--all-availability-zonesto show zones which are not yet opted-in - the installer must opt-in when creating the cluster and that zone name is set in install-config.yaml:us-east-1in CI account without the flag--all-availability-zonesus-east-1in CI account with the flag--all-availability-zonesus-west-2in CI account without the flag--all-availability-zonesus-west-2in CI account with the flag--all-availability-zones