Bug 1822656 - Container Review Request: pcp - Performance Co-Pilot
Summary: Container Review Request: pcp - Performance Co-Pilot
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora Container Images
Classification: Fedora
Component: Container Review
Version: rawhide
Hardware: Unspecified
OS: Unspecified
unspecified
unspecified
Target Milestone: ---
Assignee: Nathan Scott
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2020-04-09 14:27 UTC by Andreas Gerstmayr
Modified: 2020-04-15 18:48 UTC (History)
3 users (show)

Fixed In Version:
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed: 2020-04-15 18:48:19 UTC
Type: ---
Embargoed:
nathans: fedora-review+


Attachments (Terms of Use)

Description Andreas Gerstmayr 2020-04-09 14:27:49 UTC
Container Build Info URL: https://github.com/andreasgerstmayr/specs/tree/master/reviews/containers/pcp
Description: Performance Co-Pilot is a system performance analysis toolkit.
Fedora Account System Username: agerstmayr

Comment 1 Nathan Scott 2020-04-15 07:07:40 UTC
Checklist is based on https://docs.fedoraproject.org/en-US/containers/guidelines/guidelines/

Looks good, a couple of very minor questions below; if they're all OK this is ready to go - Ack'd!


[x] Naming
    [x] matches main service it delivers
    [x] follows general fedora naming guidelines

[x] Dockerfile Creation
    [x] FROM instruction is first line
    [?] FROM instruction is a fully-qualifies fedora registry/image:tag name
        eg FROM registry.fedoraproject.org/fedora:latest

	(pcp container using rawhide - should it use latest?)

    [x] required labels present:
	[x] com.redhat.component
	[x] name
	[x] version (usually 0)
	[?] architecture (usually x86_64)

	(double check - missing?  needed?  may be fine without)

	[x] maintainer

	(should be an external list I guess - pcp?  pcp-masters?)

	[x] run or usage
	[x] summary
    [x] optional labels present:
        [-] url
        [-] help

	(double check - missing?  wanted?  fine without)

    [x] labels defined in a single line
    [x] CMD or ENTRYPOINT present
    [x] Volumes used for persistent data
    [x] Volumes used for application data
    [x] Volumes listed in the help file
    [x] example run command contains volume with a persistent name
    [x] Volumes defined as narrowly as possible
    [x] Volumes in help file have full path of the volume
    [x] Volumes in help file show why its marked as a volume
    [x] Volumes in help file should describe space, permissions and performance needs
    [x] readme may contain suggested additional volumes that aren’t made mandatory

[x] Image Contents
    [x] contains no net new code (ie must use rpms)
    [x] each container image should provide only one "service"

	(exposing just pmproxy, which is good - someone will ask for pmcd later :)
	(consider naming 'PMPROXY_PMSERIES_SERVERS' as 'REDIS_SERVERS' - in time,
	 they may be used for more than pmseries, e.g. full text search service :)

[x] Help File Creation
    [x] must have a help.1 or README.md
    [?] must also be COPYed into the container, to live in the base directory

	(check - is this missing?  or handled via the 'COPY root /' line?)

    [x] help file should contain
        [x] brief description of what service/software the image contains
        [x] the purpose it fulfills in a larger infrastructure
        [x] contain directions on how to configure the contained service
        [x] detail dependencies on any dependent services
        [x] detail what each of the VOLUMES is for, if any
        [x] explanation of each PORT the image listens on (protocol, purpose)
        [x] links to any external documentation if any exist
        [x] any special requirements of the container must be listed
        [x] information about major variants on how it can be built

Comment 2 Andreas Gerstmayr 2020-04-15 17:22:05 UTC
(In reply to Nathan Scott from comment #1)
>     [x] FROM instruction is first line
>     [?] FROM instruction is a fully-qualifies fedora registry/image:tag name
>         eg FROM registry.fedoraproject.org/fedora:latest
> 
> 	(pcp container using rawhide - should it use latest?)

Done

> 
>     [x] required labels present:
> 	[?] architecture (usually x86_64)
> 
> 	(double check - missing?  needed?  may be fine without)

Added.

> 
> 	[x] maintainer
> 
> 	(should be an external list I guess - pcp?  pcp-masters?)

Updated

> 
>     [x] optional labels present:
>         [-] url
>         [-] help
> 
> 	(double check - missing?  wanted?  fine without)

Added

>     [x] each container image should provide only one "service"
> 
> 	(exposing just pmproxy, which is good - someone will ask for pmcd later :)
> 	(consider naming 'PMPROXY_PMSERIES_SERVERS' as 'REDIS_SERVERS' - in time,
> 	 they may be used for more than pmseries, e.g. full text search service :)

Heh, yeah ;)
Updated.

> 
> [x] Help File Creation
>     [x] must have a help.1 or README.md
>     [?] must also be COPYed into the container, to live in the base directory
> 
> 	(check - is this missing?  or handled via the 'COPY root /' line?)

Yep, this is handled by the COPY line.
There will be /README.md file inside the container.


Thanks a lot for the review!

Comment 3 Gwyn Ciesla 2020-04-15 18:15:40 UTC
(fedscm-admin):  The Pagure repository was created at https://src.fedoraproject.org/container/pcp


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