Spec URL: http://juergh.fedorapeople.org/review/cloud-initramfs-0.20-0.1.bzr85/cloud-initramfs.spec SRPM URL: http://juergh.fedorapeople.org/review/cloud-initramfs-0.20-0.1.bzr85/cloud-initramfs-0.20-0.1.bzr85.fc18.src.rpm Description: cloud image initramfs management utilities Fedora Account System Username: juergh Scratch build: http://koji.fedoraproject.org/koji/taskinfo?taskID=5060243
Since the subpackage is only just installing a dracut module, I wonder whether the subpackage shouldn't be named dracut-growroot instead of cloud-initramfs-growroot. It's true that the SRPM (and spec file) have to be named like upstream but plugins and modules are usually named after the tool they are used for. Also, I think an additional /etc/dracut.conf.d/xx-growroot.conf to automatically enable inclusion of the module in new initramfs builds would make sense. IMHO that's what someone would expect to happen after one installed the package. That's only some food for thought, not a formal review.
(In reply to comment #1) > Since the subpackage is only just installing a dracut module, I wonder > whether the subpackage shouldn't be named dracut-growroot instead of > cloud-initramfs-growroot. It's true that the SRPM (and spec file) have to be > named like upstream but plugins and modules are usually named after the tool > they are used for. Is that submodule useful outside of cloud-init on its own, or does it need other stuff? If the former, yeah, that makes sense.
Followup question: do we want to include some of the other modules here while we're at it? Maybe rescuevol? Not all of them make sense, and don't want to block this on that. :)
I notice that this puts everything under /usr/share/dracut. Shouldn't that be /usr/lib/dracut? (Also, it shouldn't own /usr/lib/dracut and /usr/lib/dracut/modules.d, because dracut already does.)
(In reply to comment #4) > I notice that this puts everything under /usr/share/dracut. Shouldn't that > be /usr/lib/dracut? Hmm... it's /usr/lib for Fedora 18 and /usr/share for RHEL6. When I built it locally under Fedora 18 it was correctly using /usr/lib but the koji built is using /usr/share. What gives? Am I using the wrong macro (%{_datadir})? > (Also, it shouldn't own /usr/lib/dracut and > /usr/lib/dracut/modules.d, because dracut already does.) So I need to use something like the following in the %files section? %{_datadir}/dracut/modules.d/*
> Is that submodule useful outside of cloud-init on its own, or does it need other stuff? If the former, yeah, that makes sense. It can be used outside of cloud-init but it requires cloud-utils which provides the growpart script. Not sure how much sense this makes in a non-cloud image though. Wouldn't a name like dracut-growroot suggest that this is a dracut subpackage? Note that I'm not strictly following the convention that the package name should match upstream since upstream is named cloud-initramfs-tools.
(In reply to comment #1) > Also, I think an additional /etc/dracut.conf.d/xx-growroot.conf to > automatically enable inclusion of the module in new initramfs builds would > make sense. IMHO that's what someone would expect to happen after one > installed the package. I'm not following you. The module does get included on the next initramfs build since the check() function from module-setup.sh returns 0.
(In reply to comment #5) > Hmm... it's /usr/lib for Fedora 18 and /usr/share for RHEL6. When I built it > locally under Fedora 18 it was correctly using /usr/lib but the koji built > is using /usr/share. What gives? Am I using the wrong macro (%{_datadir})? Hmmm I'm getting /usr/share both in mock under F18 and built from my home dir. To my knowledge, %{_datadir} is /usr/share in all versions of Fedora. To get arch-independent lib, I don't think there's anything better than "%{_prefix}/lib/". > > (Also, it shouldn't own /usr/lib/dracut and > > /usr/lib/dracut/modules.d, because dracut already does.) > So I need to use something like the following in the %files section? > %{_datadir}/dracut/modules.d/* Well, my preference is to not use wildcards at all. That can cause a little bit more work if the upstream changes, but helps keep accidental junk from creeping in -- and lets you easily tell if something changed and you didn't expect it to.
(In reply to comment #6) > It can be used outside of cloud-init but it requires cloud-utils which > provides the growpart script. Not sure how much sense this makes in a > non-cloud image though. Could be used for something where a prebuilt image is copied to physical hardware, right? > Wouldn't a name like dracut-growroot suggest that this is a dracut > subpackage? Note that I'm not strictly following the convention that the > package name should match upstream since upstream is named > cloud-initramfs-tools. Well, there's a "dracut-modules-olpc". Maybe one of the dracut maintainers has an opinion here?
New feedback to the spec file: The Requires _need_ to be *under* the %package line. Right now the Requires count towards the *main* package (which is never built) and the subpackage doesn't have any Requires. (In reply to comment #5) > (In reply to comment #4) > > I notice that this puts everything under /usr/share/dracut. Shouldn't that > > be /usr/lib/dracut? > > Hmm... it's /usr/lib for Fedora 18 and /usr/share for RHEL6. When I built it > locally under Fedora 18 it was correctly using /usr/lib but the koji built > is using /usr/share. What gives? Am I using the wrong macro (%{_datadir})? Something's wrong with your F18 then ;) %_datadir should always point to /usr/share. Go with %{_prefix}/lib for Fedora. > > (Also, it shouldn't own /usr/lib/dracut and > > /usr/lib/dracut/modules.d, because dracut already does.) > > So I need to use something like the following in the %files section? > %{_datadir}/dracut/modules.d/* Yes. Or, since it's only two files, specify both with the complete path (on two lines). (In reply to comment #6) > > Is that submodule useful outside of cloud-init on its own, or does it need other stuff? If the former, yeah, that makes sense. > > It can be used outside of cloud-init but it requires cloud-utils which > provides the growpart script. Not sure how much sense this makes in a > non-cloud image though. It can technically, that's what counts. IMHO :) > Wouldn't a name like dracut-growroot suggest that this is a dracut > subpackage? Note that I'm not strictly following the convention that the > package name should match upstream since upstream is named > cloud-initramfs-tools. I don't think it does suggest that. For example, most perl-* or R-* or <any language>-* are not subpackages but separate SRPMS. Matt is right though, it should be dracut-modules-growroot. Why "-growroot" and not "-growpart" (like the tool that is used), though? If that's an upstream decision, okay. Is there any reason it's not called cloud-initramfs-tools, then? If not, that needs to be fixed. The guideline is rather strict. (In reply to comment #7) > (In reply to comment #1) > > Also, I think an additional /etc/dracut.conf.d/xx-growroot.conf to > > automatically enable inclusion of the module in new initramfs builds would > > make sense. IMHO that's what someone would expect to happen after one > > installed the package. > > I'm not following you. The module does get included on the next initramfs > build since the check() function from module-setup.sh returns 0. Seems to work, so you probably know better how that works than me. Taking my comment back. :)
(In reply to comment #10) > Something's wrong with your F18 then ;) Well that would be Matt's cloud image then since that's what I'm running :-) > %_datadir should always point to > /usr/share. Go with %{_prefix}/lib for Fedora. Got it. > > > (Also, it shouldn't own /usr/lib/dracut and > > > /usr/lib/dracut/modules.d, because dracut already does.) > > > > So I need to use something like the following in the %files section? > > %{_datadir}/dracut/modules.d/* > > Yes. Or, since it's only two files, specify both with the complete path (on > two lines). Yep, makes sense. > I don't think it does suggest that. For example, most perl-* or R-* or <any > language>-* are not subpackages but separate SRPMS. Matt is right though, it > should be dracut-modules-growroot. Why "-growroot" and not "-growpart" (like > the tool that is used), though? If that's an upstream decision, okay. Because the module specifically grows (only) the root partition (using the growpart script from cloud-utils). > Is there any reason it's not called cloud-initramfs-tools, then? If not, > that needs to be fixed. The guideline is rather strict. Ok. I was under the impression that a subpackage always needs to include the name of the parent package which would have resulted in cloud-initramfs-tools-growpart which I though was a rather awkward name :-) Will fix. So do I need to open a new review request since the package names changes?
(In reply to comment #11) > > Something's wrong with your F18 then ;) > Well that would be Matt's cloud image then since that's what I'm running :-) Weird. Cannot reproduce. > Ok. I was under the impression that a subpackage always needs to include the > name of the parent package which would have resulted in > cloud-initramfs-tools-growpart which I though was a rather awkward name :-) > Will fix. Yeah, you can use "%package -n" to change the base name. > So do I need to open a new review request since the package names changes? Nope. Just edit this one.
Also, please make the description a little more descriptive than just copying the summary. I'd suggest something along: "This dracut module will re-write the partition table of a disk so that the root partition has as much space as possible, bumping it up to the edge of the disk, or the edge of the next partition." Text taken from the README file, s/initramfs/dracut/.
(In reply to comment #12) > Yeah, you can use "%package -n" to change the base name. Just to make things clear: you'll need to "-n dracut-modules-growroot" every macro that refers directly to the subpackage (%description, %files, ...) not only %package :)
Renamed the parent and sub-packages. Fixed the specfile per reviewers comments. Spec URL: http://juergh.fedorapeople.org/review/cloud-initramfs-tools-0.20-0.2.bzr85/cloud-initramfs-tools.spec SRPM URL: http://juergh.fedorapeople.org/review/cloud-initramfs-tools-0.20-0.2.bzr85/cloud-initramfs-tools-0.20-0.2.bzr85.fc18.src.rpm Scratch build: http://koji.fedoraproject.org/koji/taskinfo?taskID=5093835
Personally, I would use /usr/lib and %{_datadir} instead of %{_prefix}/lib and %{_prefix}/share, but I don't think there's hard rules for either which makes it simply a matter of taste. Also, you're writing "bzr85" a lot, I'd recommend to define something like "%global rev 85" at the very top and use "bzr%{rev}" everywhere else instead. Will save you some editing when updating the package in the future. Anyway, looks good to me from a strictly technical point of view but I haven't done a formal review as I understand Matthew will do that.
What does it take to get this reviewed and approved?
Gah. Sorry. Thanks for the ping. I'll look at it today.
Review passed. Sorry for the delay! Package Review ============== Key: [x] = Pass [!] = Fail [-] = Not applicable [?] = Not evaluated ===== MUST items ===== Generic: [x]: Package is licensed with an open-source compatible license and meets other legal requirements as defined in the legal section of Packaging Guidelines. [x]: Package contains no bundled libraries without FPC exception. [x]: Changelog in prescribed format. [x]: Sources contain only permissible code or content. [-]: Package contains desktop file if it is a GUI application. [-]: Development files must be in a -devel package [x]: Package requires other packages for directories it uses. [x]: Package uses nothing in %doc for runtime. [-]: Package is not known to require ExcludeArch. [-]: Fully versioned dependency in subpackages, if present. [x]: Package complies to the Packaging Guidelines [x]: License field in the package spec file matches the actual license. [x]: Package consistently uses macro is (instead of hard-coded directory names). [x]: Package is named according to the Package Naming Guidelines. [x]: Package does not generate any conflict. [x]: Package obeys FHS, except libexecdir and /usr/target. [-]: If the package is a rename of another package, proper Obsoletes and Provides are present. [x]: Package must own all directories that it creates. [x]: Package does not own files or directories owned by other packages. [x]: Requires correct, justified where necessary. [x]: Spec file is legible and written in American English. [-]: Package contains systemd file(s) if in need. [-]: Large documentation must go in a -doc subpackage. [x]: All build dependencies are listed in BuildRequires, except for any that are listed in the exceptions section of Packaging Guidelines. [x]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the beginning of %install. [x]: Each %files section contains %defattr if rpm < 4.4 [x]: Macros in Summary, %description expandable at SRPM build time. [x]: Package does not contain duplicates in %files. [x]: Permissions on files are set properly. [x]: Spec file lacks Packager, Vendor, PreReq tags. [x]: If (and only if) the source package includes the text of the license(s) in its own file, then that file, containing the text of the license(s) for the package is included in %doc. [x]: Package use %makeinstall only when make install' ' DESTDIR=... doesn't work. [x]: Package is named using only allowed ASCII characters. [x]: Package do not use a name that already exist [x]: Package is not relocatable. [x]: Sources used to build the package match the upstream source, as provided in the spec URL. [x]: Spec file name must match the spec package %{name}, in the format %{name}.spec. [x]: File names are valid UTF-8. [x]: Packages must not store files under /srv, /opt or /usr/local [x]: Package successfully compiles and builds into binary rpms on at least one supported primary architecture. [x]: Package installs properly. [x]: Rpmlint is run on all rpms the build produces. Note: There are rpmlint messages (see attachment). ===== SHOULD items ===== Generic: [-]: If the source package does not include license text(s) as a separate file from upstream, the packager SHOULD query upstream to include it. [x]: Final provides and requires are sane (see attachments). [x]: Package functions as described. [x]: Latest version is packaged. [x]: Package does not include license text files separate from upstream. [x]: SourceX tarball generation or download is documented. Note: Package contains tarball without URL, check comments [?]: Description and summary sections in the package spec file contains translations for supported Non-English languages, if available. [-]: Package should compile and build into binary rpms on all supported architectures. [?]: %check is present and all tests pass. [x]: Packages should try to preserve timestamps of original installed files. [x]: Reviewer should test that the package builds in mock. [x]: Buildroot is not present [x]: Package has no %clean section with rm -rf %{buildroot} (or $RPM_BUILD_ROOT) [x]: Dist tag is present. [x]: No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin. [x]: SourceX is a working URL. [x]: Spec use %global instead of %define. ===== EXTRA items ===== Generic: [x]: Large data in /usr/share should live in a noarch subpackage if package is arched. [x]: Rpmlint is run on all installed packages. Note: There are rpmlint messages (see attachment). [x]: Spec file according to URL is the same as in SRPM. Rpmlint ------- Checking: dracut-modules-growroot-0.20-0.2.bzr85.fc19.noarch.rpm dracut-modules-growroot.noarch: W: spelling-error Summary(en_US) resize -> reside, re size, re-size dracut-modules-growroot.noarch: W: only-non-binary-in-usr-lib 1 packages and 0 specfiles checked; 0 errors, 2 warnings. Rpmlint (installed packages) ---------------------------- # rpmlint dracut-modules-growroot dracut-modules-growroot.noarch: W: spelling-error Summary(en_US) resize -> reside, re size, re-size dracut-modules-growroot.noarch: W: only-non-binary-in-usr-lib 1 packages and 0 specfiles checked; 0 errors, 2 warnings. # echo 'rpmlint-done:' Requires -------- dracut-modules-growroot (rpmlib, GLIBC filtered): /bin/bash /bin/sh cloud-utils dracut util-linux Provides -------- dracut-modules-growroot: dracut-modules-growroot Generated by fedora-review 0.4.0 (660ce56) last change: 2013-01-29 Buildroot used: fedora-rawhide-x86_64 Command line :/usr/bin/fedora-review -b 916087
New Package SCM Request ======================= Package Name: cloud-initramfs-tools Short Description: Cloud image initramfs management utilities Owners: juergh Branches: f18 f19 el6 InitialCC:
Git done (by process-git-requests).
Hi there, I try to build cloud-initramfs-tools-0.20-0.2.bzr85.fc18.src.rpm on Centos 6.4 to provide resize on root partition. I built .rpm form src but when I was install it nothing is happening. I have all dependecy installed from el6 (cloud-init, cloud-utils, dracut, util-linux). Is there somwhere builded .rpm package or the main question is when cloud-initramfs-tools will be in el6 so simply do "yum install cloud-initramfs-tools"
(In reply to comment #22) > Hi there, > > I try to build cloud-initramfs-tools-0.20-0.2.bzr85.fc18.src.rpm on Centos > 6.4 to provide resize on root partition. I built .rpm form src but when I > was install it nothing is happening. I have all dependecy installed from el6 > (cloud-init, cloud-utils, dracut, util-linux). Is there somwhere builded > .rpm package or the main question is when cloud-initramfs-tools will be in > el6 so simply do "yum install cloud-initramfs-tools" 0.20-0.2.bzr85 had a bug which prevented it from building on EPEL. Here's the link to the latest (fixed) build: http://kojipkgs.fedoraproject.org//packages/cloud-initramfs-tools/0.20/0.3.bzr85.el6/noarch/dracut-modules-growroot-0.20-0.3.bzr85.el6.noarch.rpm It'll slowly trickle through the system and should hit testing soon. You need to rebuild the initrd after installing the package so that the tools get pulled in: $ mkinitrd /boot/initramfs-$(uname -r).img $(uname -r) To check: $ lsinitrd /boot/initramfs-$(uname -r).img | grep growroot
Also, this does not work if you have any LVM crazyness.
First of all thanks for your replay! I installed module like this: rpm -Uvh dracut-modules-growroot-0.20-0.3.bzr85.el6.noarch.rpm Then i rebuild initrd with: mkinitrd --force /boot/initramfs-$(uname -r).img $(uname -r) Chek that module is existing: lsinitrd /boot/initramfs-$(uname -r).img | grep growroot And the output is: -rwxr-xr-x 1 root root 2828 May 7 10:27 pre-pivot/00growroot.sh I reboot instance and again check for module and it is existing but the root disk is still same. I dont use LVM in the image. It is ext4 /dev/vda1 and swap /dev/vda2. Here is a output of df -h: Filesystem Size Used Avail Use% Mounted on /dev/vda1 245G 6.6G 226G 3% / tmpfs 3.9G 0 3.9G 0% /dev/shm /usr/tmpDSK 2.3G 68M 2.1G 4% /tmp And output from fdisk -l /dev/vda Disk /dev/vda: 644.2 GB, 644245094400 bytes 255 heads, 63 sectors/track, 78325 cylinders Units = cylinders of 16065 * 512 = 8225280 bytes Sector size (logical/physical): 512 bytes / 512 bytes I/O size (minimum/optimal): 512 bytes / 512 bytes Disk identifier: 0x00043f73 Device Boot Start End Blocks Id System /dev/vda1 * 1 32375 260045824 83 Linux /dev/vda2 32375 32636 2097152 82 Linux swap / Solaris Actually I use custom flavor with 600GB Disk. With fdisk I see that vda is shrink to 600GB but vda1 (root partition) is still 250GB.
The problem is your swap partition. vda1 can not be grown because of vda2 (the two partitions are back to back). You need to either: - remove the swap partition OR - make vda1 the swap partition and vda2 the root partition To check that everything is working otherwise, you can delete vda2 and reboot and then vda1 should consume the whole disk.
You are right! With image without swap partition everything is working well. Thanks!
cloud-initramfs-tools-0.20-0.3.bzr85.fc18 has been submitted as an update for Fedora 18. https://admin.fedoraproject.org/updates/cloud-initramfs-tools-0.20-0.3.bzr85.fc18
cloud-initramfs-tools-0.20-0.3.bzr85.fc18 has been pushed to the Fedora 18 testing repository.
cloud-initramfs-tools-0.20-0.3.bzr85.fc18 has been pushed to the Fedora 18 stable repository.
Any interest in maintaining this in EPEL7?
This package is not really required anymore for EPEL7 or F21 for that matter. The RHEL7 and F21 kernels can do online partition resizing so it's not necessary anymore to grow the root partition in the initramfs. It can be done later on by cloud-init.