-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Added: support for multiple network interfaces in openstack datasource #6877
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -7,7 +7,7 @@ | |||||
| import logging | ||||||
| import time | ||||||
|
|
||||||
| from cloudinit import dmi, sources, url_helper, util | ||||||
| from cloudinit import dmi, net, sources, url_helper, util | ||||||
| from cloudinit.event import EventScope, EventType | ||||||
| from cloudinit.net.dhcp import NoDHCPLeaseError | ||||||
| from cloudinit.net.ephemeral import EphemeralDHCPv4 | ||||||
|
|
@@ -73,11 +73,9 @@ def __str__(self): | |||||
| mstr = "%s [%s,ver=%s]" % (root, self.dsmode, self.version) | ||||||
| return mstr | ||||||
|
|
||||||
| def wait_for_metadata_service(self): | ||||||
| def wait_for_metadata_service(self, iface): | ||||||
| DEF_MD_URLS = [ | ||||||
| "http://[fe80::a9fe:a9fe%25{iface}]".format( | ||||||
| iface=self.distro.fallback_interface | ||||||
| ), | ||||||
| "http://[fe80::a9fe:a9fe%25{iface}]".format(iface=iface), | ||||||
| "http://169.254.169.254", | ||||||
| ] | ||||||
| urls = self.ds_cfg.get("metadata_urls", DEF_MD_URLS) | ||||||
|
|
@@ -158,14 +156,22 @@ def _get_data(self): | |||||
| """ | ||||||
|
|
||||||
| if self.perform_dhcp_setup: # Setup networking in init-local stage. | ||||||
| try: | ||||||
|
|
||||||
| with EphemeralDHCPv4( | ||||||
| self.distro, self.distro.fallback_interface | ||||||
| ): | ||||||
| results = self._crawl_metadata() | ||||||
| except (NoDHCPLeaseError, sources.InvalidMetaDataException) as e: | ||||||
| util.logexc(LOG, str(e)) | ||||||
| results = None | ||||||
| for iface in self.get_interface_list(): | ||||||
| try: | ||||||
| with EphemeralDHCPv4(self.distro, iface): | ||||||
| results = self._crawl_metadata(iface) | ||||||
| break | ||||||
| except ( | ||||||
| NoDHCPLeaseError, | ||||||
| sources.InvalidMetaDataException, | ||||||
|
Comment on lines
+166
to
+167
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This approach aligns with what we have currently in GCE datasource with two things missing: I think we want to distinguish NoDHCPLeaseError condition by logging "Unable to obtain a DHCP lease for " |
||||||
| ) as e: | ||||||
| util.logexc(LOG, str(e)) | ||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Generally there is not a lot of value in logging the related traceback via util.logexc if we can valid it. Our common logging pattern gives is the module cloud-init is logging from, so the additional traceback info is nothing more than a lot of noise in our already busy logs. Since you are touching this area. Maybe we make an effort to LOG.debug("OpenStack metadata error: %s") vs "Unable to obtain DHCP lease on %s" etc. |
||||||
| if not results: | ||||||
| LOG.warning( | ||||||
| "All interfaces failed to get dhcp lease" | ||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
| "Metadata service will NOT be called" | ||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
| ) | ||||||
| return False | ||||||
| else: | ||||||
| try: | ||||||
|
|
@@ -202,15 +208,17 @@ def _get_data(self): | |||||
|
|
||||||
| return True | ||||||
|
|
||||||
| def _crawl_metadata(self): | ||||||
| def _crawl_metadata(self, iface=None): | ||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since you are touching functions, please add type hints to function params and expected return values to aid in mypy verification. |
||||||
| """Crawl metadata service when available. | ||||||
|
|
||||||
| @returns: Dictionary with all metadata discovered for this datasource. | ||||||
| @raise: InvalidMetaDataException on unreadable or broken | ||||||
| metadata. | ||||||
| """ | ||||||
| if not iface: | ||||||
| iface = self.distro.fallback_interface | ||||||
| try: | ||||||
| if not self.wait_for_metadata_service(): | ||||||
| if not self.wait_for_metadata_service(iface): | ||||||
| raise sources.InvalidMetaDataException( | ||||||
| "No active metadata service found" | ||||||
| ) | ||||||
|
|
@@ -239,6 +247,15 @@ def _crawl_metadata(self): | |||||
| raise sources.InvalidMetaDataException(msg) from e | ||||||
| return result | ||||||
|
|
||||||
| def get_interface_list(self): | ||||||
| ifaces = [] | ||||||
| for iface in net.find_candidate_nics(): | ||||||
| if "dummy" in iface: | ||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wonder if we want to promote this logic into net.find_candidate_nics directly. I see Vultr also attempts to ignore this iface name and I think it's generally irrelevant as a candidate. |
||||||
| continue | ||||||
| ifaces.append(iface) | ||||||
|
|
||||||
| return ifaces | ||||||
|
|
||||||
| def ds_detect(self): | ||||||
| """Return True when a potential OpenStack platform is detected.""" | ||||||
| accept_oracle = "Oracle" in self.sys_cfg.get("datasource_list") | ||||||
|
|
||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add typing hints for function returns and params to aid our mypy CI validation.