Skip to content

Fix stacktrace in DataSourceRbxCloud if no metadata disk is found.#632

Merged
OddBloke merged 2 commits into
canonical:masterfrom
smoser:fix/rbx-stacktrace-on-notfound
Nov 10, 2020
Merged

Fix stacktrace in DataSourceRbxCloud if no metadata disk is found.#632
OddBloke merged 2 commits into
canonical:masterfrom
smoser:fix/rbx-stacktrace-on-notfound

Conversation

@smoser
Copy link
Copy Markdown
Collaborator

@smoser smoser commented Oct 28, 2020

Proposed Commit Message

Fix stacktrace in DataSourceRbxCloud if no metadata disk is found.

Largely speaking, ds-identify protects from this scenario being
hit, but if DataSourceRbxCloud ran and there was no metadata
disks found (LABEL=CLOUDMD), then it would stacktrace.

The fix is just to clean up the get_md function a little
bit, and the explicitly check for False as a return value in _get_data.

Additional Context

This was seen in a log attached to LP: #1901831.

Checklist:

  • My code follows the process laid out in the documentation
  • I have updated or added any unit tests accordingly
  • I have updated or added any documentation accordingly

Largely speaking, ds-identify protects from this scenario being
hit, but if DataSourceRbxCloud ran and there was no metadata
disks found (LABEL=CLOUDMD), then it would stacktrace.

The fix is just to clean up the get_md function a little
bit, and the explicitly check for False as a return value in _get_data.
@smoser smoser force-pushed the fix/rbx-stacktrace-on-notfound branch from 767b912 to 28376d1 Compare October 28, 2020 16:27
Copy link
Copy Markdown
Collaborator

@igalic igalic left a comment

Choose a reason for hiding this comment

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

🙋🏻‍♀️


def get_md():
rbx_data = None
"""Returns False (not found or error) or a dictionary with metadata."""
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

instead of (or in addition to) this long-winded doc-string, we could add a type annotation

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

instead of (or in addition to) this long-winded doc-string, we could add a type annotation

The return type is Union[dict, bool] which we can't express in our still-supporting-3.4 codebase.

@TheRealFalcon TheRealFalcon self-assigned this Nov 4, 2020
Copy link
Copy Markdown
Contributor

@TheRealFalcon TheRealFalcon left a comment

Choose a reason for hiding this comment

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

Do you think it makes sense to have get_md return an empty dictionary rather than false if nothing is found and then in generate_network_config check if rbx_data is truthy? Either way, I think this looks good.

Copy link
Copy Markdown
Collaborator

@OddBloke OddBloke left a comment

Choose a reason for hiding this comment

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

Thanks, Scott!


def get_md():
rbx_data = None
"""Returns False (not found or error) or a dictionary with metadata."""
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

instead of (or in addition to) this long-winded doc-string, we could add a type annotation

The return type is Union[dict, bool] which we can't express in our still-supporting-3.4 codebase.

@OddBloke OddBloke merged commit 2730521 into canonical:master Nov 10, 2020
@mitechie
Copy link
Copy Markdown
Contributor

mitechie commented Dec 9, 2020

@smoser we're looking at SRU verification for this, do you have access to test this or was this a side fix that is not easily verifiable?

Thanks

@smoser
Copy link
Copy Markdown
Collaborator Author

smoser commented Dec 10, 2020

Recreating the failure can be done like:

# force-enable cloud-init with ds-identify.cfg and put RbxCloud in the search list before NoCloud
$ lxc init ubuntu-daily:bionic b1
$ echo "datasource_list: [ RbxCloud, NoCloud ]" | lxc file push - b1/etc/cloud/cloud.cfg.d/90_dpkg.cfg
$ echo policy: enabled | lxc file push - b1/etc/cloud/ds-identify.cfg

# start the system and wait till cloud-init is done.  cloud-init shoudl run to completion and find the
# lxc provided NoCloud
$ lxc start b1
$ lxc exec b1 -- cloud-init status --wait

# but, there will be a stack trace and warnings in the log from when RbxCloud did not find anything.
$ lxc exec b1 -- grep WARN /var/log/cloud-init.log
2020-12-10 00:31:50,348 - util.py[WARNING]: Failed to load metadata and userdata
2020-12-10 00:31:50,348 - util.py[WARNING]: Getting data from <class 'cloudinit.sources.DataSourceRbxCloud.DataSourceRbxCloud'> failed

TheRealFalcon added a commit to TheRealFalcon/cloud-init that referenced this pull request Dec 11, 2020
Verify that if cloud-init is using DataSourceRbxCloud, there is
no traceback if the metadata disk cannot be found.
@TheRealFalcon TheRealFalcon mentioned this pull request Dec 11, 2020
3 tasks
TheRealFalcon added a commit to TheRealFalcon/cloud-init that referenced this pull request Dec 11, 2020
TheRealFalcon added a commit that referenced this pull request Dec 11, 2020
Verify that if cloud-init is using DataSourceRbxCloud, there is
no traceback if the metadata disk cannot be found.
TheRealFalcon added a commit to TheRealFalcon/cloud-init that referenced this pull request Jan 12, 2021
TheRealFalcon added a commit to TheRealFalcon/cloud-init that referenced this pull request Jan 12, 2021
OddBloke pushed a commit that referenced this pull request Jan 12, 2021
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.

5 participants