Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DataSourceEc2: use metadata's NIC ordering to determine route-metrics #342

Merged
merged 4 commits into from
May 1, 2020

Conversation

OddBloke
Copy link
Collaborator

@OddBloke OddBloke commented May 1, 2020

We want to set route-metrics such that NICs are configured with the priority that they are given in the network metadata that we receive from the IMDS. (This switches away from using MAC ordering.)

This also required the following test changes:

  • reverse the sort order of the MACs in test data (so that they would trigger the bug being fixed)
  • fix up the key names in NIC2_MD (which were under_scored instead of dash-separated)
  • use a full interface dict (rather than a minimal one) for TestConvertEc2MetadataNetworkConfig

LP: #1876312

OddBloke added 4 commits May 1, 2020 10:31
This exposes a bug in our code.
EC2 uses dash-separated-names, not underscore_separated_names.  (It's
not clear to me how this data ended up with underscores at all.)
We want to set route-metrics such that NICs are configured with the
priority that they are given in the network metadata that we receive
from the IMDS.  (This switches away from using MAC ordering.)
Copy link
Collaborator

@raharper raharper left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, this looks good. I'd like another pass to look at some possible issues:

  • get_interfaces_by_mac may vary it's output depending on platform, do we need to consistently apply a .lower() or .upper() here?
  • in DataSourceEc2 (and likely others) are we dealing with mixed case mac address values?

tests/unittests/test_datasource/test_ec2.py Show resolved Hide resolved
tests/unittests/test_datasource/test_ec2.py Show resolved Hide resolved
@OddBloke
Copy link
Collaborator Author

OddBloke commented May 1, 2020 via email

@OddBloke OddBloke merged commit 70dbccb into canonical:master May 1, 2020
@OddBloke OddBloke deleted the ec2 branch May 1, 2020 20:57
TheRealFalcon added a commit to TheRealFalcon/cloud-init that referenced this pull request Mar 8, 2021
canonical#342 (70dbccb) introduced the ability to determine route-metrics based on
the `device-number` provided by the EC2 IMDS. Not all datasources that
subclass EC2 will have this attribute, so allow the old behavior if
`device-number` is not present.

LP: #1917875
TheRealFalcon added a commit that referenced this pull request Mar 8, 2021
#342 (70dbccb) introduced the ability to determine route-metrics based on
the `device-number` provided by the EC2 IMDS. Not all datasources that
subclass EC2 will have this attribute, so allow the old behavior if
`device-number` is not present.

LP: #1917875
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants