Fixes #6396: an error removing a DHCP record (record doesn't exist) stop...

[pccowboy]

...s the delete process for a host

[domcleal]

[test]

There's a check above in dhcp_api.rb for records that don't exist, any idea why that isn't matching? The correct response for a record that doesn't exist should be a 404, which Foreman itself handles and ignores.

[pccowboy]

I believe this behavior is actually due to #175 (6412?) - when the host being checked is marked as 'deleted' in the dhcp data, but was originally from a different subnet than the subnet under evaluation, it is not found, and throws this error.

[domcleal]

I think this should log_halt 404.

[GregSutcliffe]

Hmm, would a 404 here not cause much the same issue? The foreman code parses proxy responses with this:

if response and response.code >= 200 and response.code < 300

So a 404 will still trigger the same result as the catchall 400 below.

[domcleal]

I'm expecting this to ignore it: https://github.com/theforeman/foreman/blob/develop/lib/proxy_api/dhcp.rb#L75-L77

[GregSutcliffe]

Ah true, in that case, +1 for a 404, for consistency

[GregSutcliffe]

@pccowboy i'm with @domcleal on the 404, and a test would be awesome, if you can.

[pccowboy]

@greg I'll try and think one up, I only thing that occurs to me is
checking the return type of the

find_record_by_hostname

method to make sire it is zero or one host object, and not an array.

Do you have any other suggestions?

Greg Sutcliffe mailto:notifications@github.com
Wednesday, July 02, 2014 9:12 AM

@pccowboy https://github.com/pccowboy i'm with @domcleal
https://github.com/domcleal on the 404, and a test would be awesome,
if you can.

---

Reply to this email directly or view it on GitHub
#174 (comment).

[GregSutcliffe]

@pccowboy i think we want to stub out some methods and then check that delete actually raises a Proxy::DHCP::InvalidRecord error. Something like https://github.com/theforeman/smart-proxy/blob/develop/test/tftp/tftp_test.rb#L35-L42 where we check the raise type of a piece of tftp code. Ping me on IRC if you need a hand, I'd like to get this merged :)

[domcleal]

[test]

[domcleal]

Merged as 67bfd9a for Foreman 1.5.2, thanks @pccowboy and @sodabrew!