Option 1: Simplify how we find extras package #33
Conversation
|
Short of testing every kernel for versions the cookbook already supports, personally I'd much rather not break compatibility for the edge cases I've heard uname not working. Soon enough I'm sure we'll not even require AUFS (as shown by devmapper support on CentOS/Fedora/RHEL family machines), which will remove this need. In the meantime, could we just limit the platform_version to just 12.04, 12.10, and 13.04? Or maybe just make the regex support two digit numbers via something like [0-9]{1,2}? Curious what you think. Thanks! |
|
I honestly can't think of a single situation where the longer grep would find something and not just return the entire output of uname -r. I can understand the paranoia, and will resubmit with the version gate per your request because I don't use anything older than Precise, but I honestly don't think there are any cases that |
|
Actually, given in the docs you show only support for 12.04, 12.10, and 13.04, I'm not sure keeping the old way makes sense at all. A cursory apt-cache search on the three supported distros don't show me any kernels that this would break for, I'd suggest leaving it alone. I'll leave this one (with fixes for the style failures in the build) and submit an alternate PR where we can hide the old kernel-finding logic behind an attribute, so anyone who might be broken could re-enable the old way of finding it, but honestly I think that's overkill. |
|
PR #35 makes using the old logic an option. FWIW; I prefer completely removing it instead, but either solution should be amenable for Saucy. |
- Current regexp breaks on kernels with double-digit versions (i.e. 3.11.0-13-generic) - Can't find a sensible reason why we can't plug in uname -r directly
|
aha; I have found a way this will fail. This no longer checks to see if the linux-image-extra package exists before trying to install it. I'll add this check |
- Only attempt to install package if it exists -- use apt-cache search to check for it - Use ohai's kernel['release'] in lieu of shelling out to uname
|
Tested this PR on saucy (13.10) and found that it fixes the problem I had with the current stable version of this cookbook ( The old and clunky way of searching |
|
Looks like the naming scheme is quite ugly for the linux kernel image packages across Ubuntu releases. I just tested on 12.04 precise and found that Turns out that the only similar kernel images in precise (12.04) are In the Docker Installation Documentation, for Ubuntu So this code probably has the best way to decide which one to use in general, but for 12.04 use those packages above. Haven't tested on any other VMs yet, but if this still doesn't do the trick: maybe try to first split the output of either |
|
Something to note: Docker's Kernel Requirements. This pretty much limits us to installing a 3.8 kernel anyway. Since the For Ubuntu For Ubuntu |
|
It's really crummy that uname -r returns -generic for some of those :-X I kinda agree that the packages they suggest are better, however, the earlier version of the kernel for Precise does work, and I don't think it's valuable to force users of the cookbook to upgrade kernel if they don't want to separately (not to mention no way to enforce reboot in chef, so it gives kinda inconsitent results if the user isn't expecting this). I'll rework this to work with your failing case. |
|
@trinitronx please see PR #35 for a version of this which shouldn't break precise. |
This completely removes the old logic. PR #35 keeps the older logic, hidden behind an attribute.
3.11.0-13-generic)
This is tested working on Ubuntu Precise 12.04 and tested in theory on Ubuntu Saucy 13.10 (although other problems prevent this cookbook from working there yet; see my recently filed issue).