[Bug 1290523] New: Review Request: oci-systemd-hooks - OCI systemd hook for Docker

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1290523

Bug ID: 1290523
           Summary: Review Request: oci-systemd-hooks - OCI systemd hook
                    for Docker
           Product: Fedora
           Version: rawhide
         Component: Package Review
          Severity: medium
          Priority: medium
          Assignee: [e-mail]
          Reporter: [e-mail]
        QA Contact: [e-mail]
                CC: [e-mail], [e-mail],
                    [e-mail],
                    [e-mail]

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-hoo...

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.

[Bug 1290523] Review Request: oci-systemd-hooks - OCI systemd hook for Docker

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1290523

--- Comment #1 from Lokesh Mandvekar  ---
- 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....

[Bug 1290523] Review Request: oci-systemd-hooks - OCI systemd hook for Docker

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1290523

Lokesh Mandvekar  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
           Assignee|[e-mail]    |[e-mail]

--- Comment #2 from Lokesh Mandvekar  ---
Jan, you're my default victim for rpm reviews. Sucks to be you :P

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

[Bug 1290523] Review Request: oci-systemd-hooks - OCI systemd hook for Docker

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1290523

--- Comment #3 from Jan Chaloupka  ---
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

[Bug 1290523] Review Request: oci-systemd-hooks - OCI systemd hook for Docker

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1290523

Daniel Walsh  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]

--- Comment #4 from Daniel Walsh  ---
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.

[Bug 1290523] Review Request: oci-systemd-hooks - OCI systemd hook for Docker

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1290523

Jan Chaloupka  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
              Flags|                            |needinfo?([e-mail]
                   |                            |)

--- Comment #5 from Jan Chaloupka  ---
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.

[Bug 1290523] Review Request: oci-systemd-hooks - OCI systemd hook for Docker

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1290523

Daniel Walsh  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
              Flags|needinfo?([e-mail] |
                   |)                           |

--- Comment #6 from Daniel Walsh  ---
Rewrote the README.md and create a 0.1.4 release.

[Bug 1290523] Review Request: oci-systemd-hooks - OCI systemd hook for Docker

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1290523

--- Comment #7 from Jan Chaloupka  ---
Change commit in the spec to b1cd5c2ca3f3aebe6848f798bcbfaad8b5682752 so it
picks 1.4.0.

[Bug 1290523] Review Request: oci-systemd-hooks - OCI systemd hook for Docker

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1290523

Nalin Dahyabhai  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]

--- Comment #8 from Nalin Dahyabhai  ---
* 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".)

[Bug 1290523] Review Request: oci-systemd-hooks - OCI systemd hook for Docker

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1290523

--- Comment #9 from Daniel Walsh  ---
Fixed.

[Bug 1290523] Review Request: oci-systemd-hooks - OCI systemd hook for Docker

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1290523

--- Comment #10 from Daniel Walsh  ---
Also updated commit message.

[Bug 1290523] Review Request: oci-systemd-hooks - OCI systemd hook for Docker

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1290523

--- Comment #11 from Jan Chaloupka  ---
$ 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  - 0.1.4

should be

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

[Bug 1290523] Review Request: oci-systemd-hooks - OCI systemd hook for Docker

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1290523

--- Comment #12 from Daniel Walsh  ---
Fixed.

[Bug 1290523] Review Request: oci-systemd-hooks - OCI systemd hook for Docker

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1290523

Jan Chaloupka  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
              Flags|fedora-review?              |fedora-review+

--- Comment #13 from Jan Chaloupka  ---
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

[Bug 1290523] Review Request: oci-systemd-hooks - OCI systemd hook for Docker

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1290523

--- Comment #14 from Jon Ciesla  ---
Package request has been approved:
https://admin.fedoraproject.org/pkgdb/package/oci-systemd-hook

[Bug 1290523] Review Request: oci-systemd-hooks - OCI systemd hook for Docker

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1290523

--- Comment #15 from Jan Chaloupka  ---
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.

[Bug 1290523] Review Request: oci-systemd-hooks - OCI systemd hook for Docker

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1290523

--- Comment #16 from Daniel Walsh  ---
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.

[Bug 1290523] Review Request: oci-systemd-hooks - OCI systemd hook for Docker

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1290523

--- Comment #17 from Jan Chaloupka  ---
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}

[Bug 1290523] Review Request: oci-systemd-hooks - OCI systemd hook for Docker

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1290523

--- Comment #18 from Daniel Walsh  ---
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']

[Bug 1290523] Review Request: oci-systemd-hooks - OCI systemd hook for Docker

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1290523

--- Comment #19 from Lokesh Mandvekar  ---
(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.

[Bug 1290523] Review Request: oci-systemd-hooks - OCI systemd hook for Docker

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1290523

--- Comment #20 from Fedora Update System  ---
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

[Bug 1290523] Review Request: oci-systemd-hooks - OCI systemd hook for Docker

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1290523

Fedora Update System  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |MODIFIED

[Bug 1290523] Review Request: oci-systemd-hooks - OCI systemd hook for Docker

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1290523

--- Comment #21 from Jan Chaloupka  ---
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?

[Bug 1290523] Review Request: oci-systemd-hooks - OCI systemd hook for Docker

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1290523

Fedora Update System  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|MODIFIED                    |ON_QA

--- Comment #22 from Fedora Update System  ---
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

[Bug 1290523] Review Request: oci-systemd-hooks - OCI systemd hook for Docker

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1290523

--- Comment #23 from Fedora Update System  ---
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.

[Bug 1290523] Review Request: oci-systemd-hooks - OCI systemd hook for Docker

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=1290523

Fedora Update System  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ON_QA                       |CLOSED
         Resolution|---                         |ERRATA
        Last Closed|                            |2016-04-02 11:55:12