Skip to content

net/dhcp: surface type of DHCP lease failure to caller#1276

Merged
TheRealFalcon merged 1 commit into
canonical:mainfrom
cjp256:net-dhcp-errors
Mar 1, 2022
Merged

net/dhcp: surface type of DHCP lease failure to caller#1276
TheRealFalcon merged 1 commit into
canonical:mainfrom
cjp256:net-dhcp-errors

Conversation

@cjp256
Copy link
Copy Markdown
Contributor

@cjp256 cjp256 commented Feb 17, 2022

When performing DHCP, it is useful for the caller to have context
on the type of failure. This can be done with some new exceptions
types, subclassing NoDHCPLeaseError so the caller's current
contract remains.

  • Add the following errors:

    • NoDHCPLeaseInterfaceError if there are problems finding
      the (possibly specified) interface.

    • NoDHCPLeaseMissingDhclientError for missing dhclient.

  • Update InvalidDHCPLeaseFileError to subclass NoDHCPLeaseError.

  • Pass through these errors rather than catching it in obtain_lease().

Tests:

  • Add missing mock for test_provided_nic_does_not_exist().

  • Add new test coverage for EphemeralDHCPv4 errors.

  • Update existing tests for maybe_perform_dhcp_discovery() to match
    new behavior.

Signed-off-by: Chris Patterson cpatterson@microsoft.com

@cjp256 cjp256 changed the title net/dhcp: surface type of DHCP lease failures to caller net/dhcp: surface type of DHCP lease failure to caller Feb 17, 2022
Copy link
Copy Markdown
Contributor

@anhvoms anhvoms left a comment

Choose a reason for hiding this comment

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

LGTM!

When performing DHCP, it is useful for the caller to have context
on the type of failure.  This can be done with some new exceptions
types, subclassing NoDHCPLeaseError so the caller's current
contract remains.

- Add the following errors:

  - NoDHCPLeaseInterfaceError if there are problems finding
  the (possibly specified) interface.

  - NoDHCPLeaseMissingDhclientError for missing dhclient.

- Update InvalidDHCPLeaseFileError to subclass NoDHCPLeaseError.

- Pass through these errors rather than catching it in obtain_lease().

Tests:

- Add missing mock for test_provided_nic_does_not_exist().

- Add new test coverage for EphemeralDHCPv4 errors.

- Update existing tests for maybe_perform_dhcp_discovery() to match
  new behavior.

Signed-off-by: Chris Patterson <cpatterson@microsoft.com>
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.

Thanks!

@TheRealFalcon TheRealFalcon merged commit f99bb8c into canonical:main Mar 1, 2022
@cjp256 cjp256 deleted the net-dhcp-errors branch April 25, 2022 16:32
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