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

fix(dhcpcd): Make lease parsing more robust #5129

Merged
merged 3 commits into from Apr 2, 2024

Conversation

holmanb
Copy link
Member

@holmanb holmanb commented Apr 1, 2024

Proposed Commit Message

fix(dhcpcd): Make lease parsing more robust

Additional Context

Followup to #5128 (rebase before merge)

Merge type

  • Squash merge using "Proposed Commit Message"
  • Rebase and merge unique commits. Requires commit messages per-commit each referencing the pull request number (#<PR_NUM>)

@holmanb holmanb force-pushed the holmanb/dhcpcd-robust-parsing branch from 1ae918f to aec9035 Compare April 2, 2024 16:12
Copy link
Collaborator

@blackboxsw blackboxsw left a comment

Choose a reason for hiding this comment

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

Thanks for the improvement to avoid breakage. Can we also take this a step further to do the following:

  • log in the ValueError handling the source dump_lease content for reference in the error-level logs
  • raise an error when the processed dhcpcd.lease file has no viable parsed config options

cloudinit/net/dhcp.py Show resolved Hide resolved
tests/unittests/net/test_dhcp.py Outdated Show resolved Hide resolved
@holmanb holmanb requested a review from blackboxsw April 2, 2024 19:46
@holmanb
Copy link
Member Author

holmanb commented Apr 2, 2024

Thanks for the review @blackboxsw. I think that I've addressed all of your comments.

Requesting re-review.

@holmanb holmanb force-pushed the holmanb/dhcpcd-robust-parsing branch from 92d91ec to 0ef7ea8 Compare April 2, 2024 19:57
Copy link
Collaborator

@blackboxsw blackboxsw left a comment

Choose a reason for hiding this comment

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

LGTM!

@holmanb holmanb merged commit f6ac6ee into canonical:main Apr 2, 2024
28 checks passed
blackboxsw pushed a commit to blackboxsw/cloud-init that referenced this pull request Apr 2, 2024
blackboxsw pushed a commit to blackboxsw/cloud-init that referenced this pull request Apr 2, 2024
blackboxsw pushed a commit to blackboxsw/cloud-init that referenced this pull request Apr 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants