DHCP Lease file

[pbertera]

Would be a good improvement to have the lease file saved on the file system. This will avoid IP address conflicts after service restarts. What do you think about ? If it's considered useful I'll start working on it.

[psychomario]

This would be useful, and a relatively simple file format.

You're welcome to start implementing it but it looks like we're going to have a feature hold on development until we can get it cleaned up and merged.

[mmattioli]

@pbertera @psychomario How does JSON sound?

[psychomario]

Sounds sensible, It's what we're using elsewhere. Also it means we could just dump self.leases, optionally pruning some fields (ipxe).

[psychomario]

@mmattioli Unless I missed something, I don't think this was. This is referencing saving the created leases, not static leasing.

[mmattioli]

@psychomario my bad, I was confused

[psychomario]

@pbertera If you want to start on this, development is now accepting new features. If you don't want to anymore I can do so.

[pbertera]

Hi, unfortunately at this moment I have no free time :( I see an occasion to start doing something maybe after mid June.. if you have a chance to do it more quickly you're welcome, I'm not jealous ;)

PS: compliments for the nice works on the release!

[psychomario]

In that case, I'll take this one off your hands, but we'd love more contributions if you have them when you've got spare time.

[psychomario]

@mmattioli When do you think is the right time to write out? If we do it on lease we might end up thrashing the disk. If we do it on SIGINT we won't dump it when we're killed some other way. The easiest way is probably at the end of the while loop in listen(), although again this might end up thrashing disk.

[mmattioli]

As far as when we should write out, why can't we write out on SIGINT, SIGTERM, SITQUIT, SIGKILL, and/or SIGSTOP (covers all the bases)?

[psychomario]

That should work, although doesn't account for a hard crash of us or OS or anything, but bad timing on that part would undo any carefully placed saving code.

I'll get on this this weekend.

[mmattioli]

If whatever happens is outside the scope of SIGINT, SIGTERM, SITQUIT, SIGKILL, and/or SIGSTOP then I think it's fair to say that there are bigger problems that are out of our hands :smile:

[psychomario]

I've just realised that the chroot is going to cause issues for this, unless we pass in the file handle we overwrite, but this seems a little messy.

Perhaps the changes fixing directory and chroot usage should come first.

[icb-]

From a quick read through the code, I think we're not doing any IPC between the modules. Is there any reason to stick with threading instead of moving to multiprocessing and doing the chroot only places that need it (tftp and http)?

[psychomario]

As far as I'm aware there's no reason for threads over processes, so as long as we can safely drop in the changes that sounds like a reasonable solution. Http might still want per-client threads for speed(?) but the module itself could be either.

We still need to clean up path parsing I believe, as partially addressed in the PR (apologies, on my mobile so I can't easily link the number)

[psychomario]

Something is wrong with my environment apparently, and I can't get multiprocessing working, but once I've got that fixed I will try to get something written for this.

[psychomario]

Handling SIGINT in DHCP causes the except KeyboardInterrupt in server.py to stop working. I'm not sure what to do about this. Any suggestions?

[psychomario]

I have found a solution to the SIGINT issue, however I've come across a new issue. The DHCPD.leases object contains quite a lot of binary data (keys are raw mac addresses, we have an options dict which contains raw packet contents), which the json encoder is not having fun with.

We could potentially use something like pickle to keep the data as is, or we could convert the mac addresses (using DHCPD.get_mac()) and remove the options dictionary before we export. This would mean we lose the options dict upon restart, but I don't think that this will be an issue as it's only used when we give out leases.

[psychomario]

@mmattioli I would value any input you could offer on the above.

I'd like to get all out open issues closed over the next few weeks and have another merge to master.

[mmattioli]

@psychomario using pickle seems like the cleanest way while preserving functionality. Thoughts?

[psychomario]

I've done this in #110 with json mostly because we already have the json module loaded. We don't actually need to keep the binary data from options so I've just moved it into a different object. The only slightly messy part of doing it this way is that we're converting to and from human readable (json safe) mac reprentations.