AWS: Arbitrarily choose first (lexicographically) subnet in AZ #50255
Conversation
k8s-ci-robot
added
cncf-cla: yes
|
Hi @mattlandis. Thanks for your PR. I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with I understand the commands that are listed here. 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. |
k8s-github-robot
added
size/M
|
/assign @justinsb |
| // If we have two subnets for the same AZ we arbitrarily choose the one that is first lexicographically. | ||
| // TODO: Should this be an error. | ||
| if strings.Compare(*existing.SubnetId, *subnet.SubnetId) > 0 { | ||
| glog.Warningf("Found multiple subnets in AZ %q; choosing %q between subnets %q and %q", az, *subnet.SubnetId, *existing.SubnetId, *subnet.SubnetId) |
There was a problem hiding this comment.
I think we'll end up glog.Warningf twice here
There was a problem hiding this comment.
Should have had a continue inside the if block. The log statements differ in which one it is choosing. Updated.
justinsb
changed the title
justinsb
added
release-note
|
I think we don't need both glog.Warningf statements, as otherwise we'll log twice per loop. Otherwise LGTM. /ok-to-test |
k8s-ci-robot
removed
the
needs-ok-to-test
When there is more than one subnet for an AZ on AWS choose arbitrarily chose the first one lexicographically for consistency.
mattlandis
force-pushed
the
aws-duplicate-subnets
branch
from
da88a57 to
10794e7
Compare
|
/unassign |
|
@justinsb I've updated to add a continue inside the block so only the correct log messages is output. |
|
Looks like this needs /lgtm |
|
/lgtm |
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: justinsb, mattlandis Associated issue: 45983 The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these OWNERS Files:
You can indicate your approval by writing |
k8s-github-robot
added
the
approved
|
/test pull-kubernetes-e2e-gce-etcd3 |
|
Automatic merge from submit-queue (batch tested with PRs 50255, 50885) |
When there is more than one subnet for an AZ on AWS choose arbitrarily
chose the first one lexicographically for consistency.
What this PR does / why we need it:
If two subnets were to be used appear in the same aws az which one is chosen is currently not consistent. This could lead to difficulty in diagnosing issues.
Which issue this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)format, will close that issue when PR gets merged): fixes #45983Special notes for your reviewer:
Release note: