Bug 1290523 - Review Request: oci-systemd-hooks - OCI systemd hook for Docker
Summary: Review Request: oci-systemd-hooks - OCI systemd hook for Docker
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Jan Chaloupka
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2015-12-10 17:35 UTC by Lokesh Mandvekar
Modified: 2016-04-02 15:55 UTC (History)
7 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2016-04-02 15:55:12 UTC
Type: ---
Embargoed:
jchaloup: fedora-review+


Attachments (Terms of Use)

Description Lokesh Mandvekar 2015-12-10 17:35:59 UTC
Spec URL: https://github.com/lsm5/hooks/blob/hooks-rpm/oci-systemd-hook.spec

SRPM URL: https://kojipkgs.fedoraproject.org//work/tasks/9789/12139789/oci-systemd-hook-0.1.3-2.fc24.src.rpm

Upstream: https://github.com/mrunalp/hooks

Description:
OCI systemd hook enables running systemd in docker and OCI compatible runtimes such as runc.

It reads state over stdin and mounts a tmpfs at /run, /tmp, links in a journal directory from the host and creates /etc/machine-id file for a container.


Fedora Account System Username: lsm5

Koji: https://koji.fedoraproject.org/koji/taskinfo?taskID=12139788

$ rpmlint -i SRPMS/* RPMS/* oci-systemd-hook.spec 
3 packages and 1 specfiles checked; 0 errors, 0 warnings.

Comment 1 Lokesh Mandvekar 2015-12-10 17:50:37 UTC
- use separate macro for license when fedora 23 or higher
- update release tag when using git commits

Spec URL: https://github.com/lsm5/hooks/blob/hooks-rpm/oci-systemd-hook.spec
SRPM URL: https://github.com/lsm5/hooks/blob/hooks-rpm/SRPMS/oci-systemd-hook-0.1.3-3.gitebdb622.fc24.src.rpm

Comment 2 Lokesh Mandvekar 2015-12-10 20:12:34 UTC
Jan, you're my default victim for rpm reviews. Sucks to be you :P

(..just kidding, feel free to re-assign or whatever)

Comment 3 Jan Chaloupka 2015-12-11 10:44:12 UTC
Heh :D

- remove BuildRequires: go-srpm-macros as the macro is in the minimal buildroot
- ExclusiveArch: %{go_arches} is ok as the package is supposed to be built only on architectures where docker is

This can be shortened

%files
%if 0%{?fedora} >= 23
%license LICENSE
%else
%doc LICENSE
%doc README.md
%endif

to

#define license tag if not already defined
%{!?_licensedir:%global license %doc}

%files
%license LICENSE
%doc README.md

Comment 4 Daniel Walsh 2016-02-18 21:13:28 UTC
I have done a lot of cleanups on this and moved it to projectatomic.

We need to move forward on this review.

https://github.com/projectatomic/oci-systemd-hook

Has the latest code.

Comment 5 Jan Chaloupka 2016-03-02 13:33:36 UTC
The spec file states:

Version: 0.1.4

However, I don't see any release under https://github.com/projectatomic/oci-systemd-hook/releases

At the same time comment on the line 5:
# https://github.com/projectatomic/oci-register-machine

It should be
# https://github.com/projectatomic/oci-oci-systemd-hook

Those are the two last nits before I can approve the spec file.

Comment 6 Daniel Walsh 2016-03-02 14:35:33 UTC
Rewrote the README.md and create a 0.1.4 release.

Comment 7 Jan Chaloupka 2016-03-02 14:53:37 UTC
Change commit in the spec to b1cd5c2ca3f3aebe6848f798bcbfaad8b5682752 so it picks 1.4.0.

Comment 8 Nalin Dahyabhai 2016-03-02 20:50:36 UTC
* Package should probably own /usr/libexec/oci and /usr/libexec/oci/hooks.d, because nothing else does if other hooks are not installed.
* Versions given in .spec file and configure.in don't agree.
* Guidelines recommend replacing BuildRequires: on yajl-devel, libselinux-devel, and libmount-devel with BuildRequires: on pkgconfig(yajl), pkgconfig(libselinux), and pkgconfig(mount), respectively.  (Not a "must", only a "should".)

Comment 9 Daniel Walsh 2016-03-02 21:15:00 UTC
Fixed.

Comment 10 Daniel Walsh 2016-03-02 21:23:56 UTC
Also updated commit message.

Comment 11 Jan Chaloupka 2016-03-03 08:38:08 UTC
$ rpmlint oci-systemd-hook-0.1.4-1.gitde345df.fc20.src.rpm oci-systemd-hook-0.1.4-1.gitde345df.fc25.x86_64.rpm
oci-systemd-hook.src: W: spelling-error %description -l en_US runc -> run, runic, rune
oci-systemd-hook.x86_64: W: spelling-error %description -l en_US runc -> run, runic, rune
oci-systemd-hook.x86_64: W: incoherent-version-in-changelog 0.1.4 ['0.1.4-1.gitde345df.fc25', '0.1.4-1.gitde345df']
2 packages and 0 specfiles checked; 0 errors, 3 warnings.

The version-release in the changelog is incorrect. 

* Thu Feb 18 2016 Dan Walsh <dwalsh> - 0.1.4

should be

* Thu Feb 18 2016 Dan Walsh <dwalsh> - 0.1.4-1.gitde345df

Comment 12 Daniel Walsh 2016-03-03 13:22:24 UTC
Fixed.

Comment 13 Jan Chaloupka 2016-03-03 13:32:34 UTC
Thanks.

Summary:
- license is correct
- Source tag is correct
- rpmlint output is fine
- version correct in spec and configure.in
- /usr/libexec/oci and usr/libexec/oci/hooks.d owned by this package
- buildable in Koji

Comment 14 Gwyn Ciesla 2016-03-07 19:51:39 UTC
Package request has been approved: https://admin.fedoraproject.org/pkgdb/package/oci-systemd-hook

Comment 15 Jan Chaloupka 2016-03-29 11:30:20 UTC
Daniel, if you want f24 build to get into stable, update needs to be created. At the same time this bug will get closed once the f24 built gets into stable.

Just asking so this bug get closed.

Comment 16 Daniel Walsh 2016-03-29 12:46:44 UTC
What exactly do I need to do.

oci-systemd-hook-0.1.4-1.gitde345df.fc24 is in f24-updates-candidate

koji latest-pkg f24-updates-candidate oci-systemd-hookWarning: the pkgurl option is obsolete
Build                                     Tag                   Built by
----------------------------------------  --------------------  ----------------
oci-systemd-hook-0.1.4-1.gitde345df.fc24  f24                   dwalsh


I tried to add this to https://bodhi.fedoraproject.org/updates/new

But it is being rejected.

Sorry it has been a while since I have done this.

Comment 17 Jan Chaloupka 2016-03-29 13:04:02 UTC
Getting the following errors when trying to create new update:

> Builds : Unable to determine release from build:
> oci-systemd-hook-0.1.4-1.gitde345df.fc24

> Builds : Invalid tag: oci-systemd-hook-0.1.4-1.gitde345df.fc24 tagged with
> [u'f24-updates-candidate', u'epel7-testing-candidate',
> u'dist-6E-epel-testing-candidate', u'dist-5E-epel-testing-candidate',
> u'f22-updates-candidate', u'f23-updates-candidate', u'f21-updates-candidate']

Looking into the nvr:

oci-systemd-hook-0.1.4-1.gitde345df.fc24

it should be:

oci-systemd-hook-0.1.4-0.1.gitde345df.fc24

Release is incorrect, rpm does not like the integer before git. It can be fixed by updating release from:

Release:        1.git%{shortcommit}%{?dist}

to

Release:        1%{?dist}

Comment 18 Daniel Walsh 2016-03-29 13:21:24 UTC
Made that change but updates still breaks with

Builds : Cannot find release associated with build: oci-systemd-hook-0.1.4-1.fc24, tags: 

Builds : Invalid tag: oci-systemd-hook-0.1.4-1.fc24 tagged with [u'f24-updates-candidate', u'epel7-testing-candidate', u'dist-6E-epel-testing-candidate', u'dist-5E-epel-testing-candidate', u'f22-updates-candidate', u'f23-updates-candidate', u'f21-updates-candidate']

Comment 19 Lokesh Mandvekar 2016-03-29 13:29:11 UTC
(In reply to Jan Chaloupka from comment #17)
 
> Looking into the nvr:
> 
> oci-systemd-hook-0.1.4-1.gitde345df.fc24
> 
> it should be:
> 
> oci-systemd-hook-0.1.4-0.1.gitde345df.fc24
> 
> Release is incorrect, rpm does not like the integer before git. It can be
> fixed by updating release from:
> 
> Release:        1.git%{shortcommit}%{?dist}
> 
> to
> 
> Release:        1%{?dist}


docker has had N.git for quite sometime. I think the issue is that bodhi updates haven't been turned on f24 yet, so it's in the same boat as rawhide as far as bodhi goes. Let me get back once I hear from #fedora-devel.

Comment 20 Fedora Update System 2016-03-29 14:49:45 UTC
oci-systemd-hook-0.1.4-1.fc24 has been submitted as an update to Fedora 24. https://bodhi.fedoraproject.org/updates/FEDORA-2016-d091f541f1

Comment 21 Jan Chaloupka 2016-03-29 17:51:28 UTC
I have been updating f24 builds for quite some time so it is already on (with 3 days to push update to stable manually).

Lokesh, what was the magic?

Comment 22 Fedora Update System 2016-03-30 22:27:00 UTC
oci-systemd-hook-0.1.4-1.fc24 has been pushed to the Fedora 24 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2016-d091f541f1

Comment 23 Fedora Update System 2016-04-02 15:55:08 UTC
oci-systemd-hook-0.1.4-1.fc24 has been pushed to the Fedora 24 stable repository. If problems still persist, please make note of it in this bug report.


Note You need to log in before you can comment on or make changes to this bug.