[Bug 1818670] Review Request: sensu-go - Sensu Go Open Source (Monitoring Program)
https://bugzilla.redhat.com/show_bug.cgi?id=1818670 Robert-André Mauchin changed: What|Removed |Added Flags|needinfo?(zebo...@gmail.com |needinfo?(devin@linuxstack. |) |cloud) -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component ___ package-review mailing list -- package-review@lists.fedoraproject.org To unsubscribe send an email to package-review-le...@lists.fedoraproject.org Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/package-review@lists.fedoraproject.org Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure
needinfo canceled: [Bug 1818670] Review Request: sensu-go - Sensu Go Open Source (Monitoring Program)
Product: Fedora Version: rawhide Component: Package Review Robert-André Mauchin has canceled Package Review 's request for Robert-André Mauchin 's needinfo: Bug 1818670: Review Request: sensu-go - Sensu Go Open Source (Monitoring Program) https://bugzilla.redhat.com/show_bug.cgi?id=1818670 ___ package-review mailing list -- package-review@lists.fedoraproject.org To unsubscribe send an email to package-review-le...@lists.fedoraproject.org Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/package-review@lists.fedoraproject.org Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure
[Bug 1818670] Review Request: sensu-go - Sensu Go Open Source (Monitoring Program)
https://bugzilla.redhat.com/show_bug.cgi?id=1818670
Robert-André Mauchin changed:
What|Removed |Added
Status|NEW |ASSIGNED
CC||zebo...@gmail.com
Assignee|nob...@fedoraproject.org|zebo...@gmail.com
Flags||fedora-review?
--- Comment #7 from Robert-André Mauchin ---
Most of these are already packaged, the work to be done would be low. Anyhow
EPEL would need to be vendored as we don't have macros available there.
A few remarks:
- You need a logrotate file for your log
- Don't use /opt for the home of the Sensu user, but %{_sharedstatedir}/sensu:
%pre backend
getent group sensu >/dev/null || groupadd -r sensu
getent passwd sensu >/dev/null || \
useradd -r -g sensu -d %{_sharedstatedir}/sensu -s /sbin/nologin \
-c "Sensu User" sensu
exit 0
%pre agent
getent group sensu >/dev/null || groupadd -r sensu
getent passwd sensu >/dev/null || \
useradd -r -g sensu -d %{_sharedstatedir}/sensu -s /sbin/nologin \
-c "Sensu User" sensu
exit 0
- You should use macros for the various directories used in the SPEC:
%install
install -m 0755 -vd %{buildroot}%{_sbindir}
install -m 0755 -vp %{gobuilddir}/bin/* %{buildroot}%{_sbindir}/
install -m 0755 -vd %{buildroot}%{_tmpfilesdir}
install -m 0755 -vd %{buildroot}%{_unitdir}
install -m 0755 -vd %{buildroot}%{_sysconfdir}/sensu
install -m 0755 -vd %{buildroot}%{_sharedstatedir}/sensu
install -m 0755 -vd
%{buildroot}%{_localstatedir}/cache/sensu
install -m 0755 -vd %{buildroot}%{_localstatedir}/log/sensu
install -m 0755 -vd %{buildroot}%{_rundir}/sensu
install -m 0755 -vd %{buildroot}%{_sysconfdir}/logrotate.d
install -pm 0644 %{SOURCE2}
%{buildroot}%{_unitdir}/sensu-backend.service
install -pm 0640 %{SOURCE4}
%{buildroot}%{_sysconfdir}/sensu/backend.yml
install -pm 0644 %{SOURCE3}
%{buildroot}%{_unitdir}/sensu-agent.service
install -pm 0640 %{SOURCE5}
%{buildroot}%{_sysconfdir}/sensu/agent.yml
install -pm 0644 %{SOURCE6}
%{buildroot}%{_tmpfilesdir}/%{name}.conf
install -pm 0644 %{SOURCE7}
%{buildroot}%{_sysconfdir}/logrotate.d/%{name}.conf
Here are SPEC to use bundled dependencies:
First for Fedora: https://koji.fedoraproject.org/koji/taskinfo?taskID=46060718
=
# Generated by go2rpm 1
%bcond_without check
%bcond_without vendor
# https://github.com/sensu/sensu-go
%global goipath github.com/sensu/sensu-go
Version:5.21.0
%gometa
%global common_description %{expand:
Sensu is an open source monitoring tool for ephemeral infrastructure and
distributed applications. It is an agent based monitoring system with built-in
auto-discovery, making it very well-suited for cloud environments. Sensu uses
service checks to monitor service health and collect telemetry data. It also
has
a number of well defined APIs for configuration, external data input, and to
provide access to Sensu's data. Sensu is extremely extensible and is commonly
referred to as "the monitoring router".}
%global golicenses LICENSE
%global godocs CHANGELOG.md CODE_OF_CONDUCT.md CONTRIBUTING.md\\\
FAQ.md README.md
Name: sensu-go
Release:3%{?dist}
Summary:Simple, Scalable, Multi-cloud monitoring
License:MIT
URL:%{gourl}
Source0:%{gosource}
# git clone https://github.com/sensu/sensu-go
# cd sensu-go
# git checkout v%%{version}
# go mod vendor
# tar czvf vendor-%%{version}.tar.gz vendor/
Source1:vendor-%{version}.tar.gz
Source2:sensu-backend.service
Source3:sensu-agent.service
Source4:backend.yml
Source5:agent.yml
Source6:%{name}-tmpfiles.conf
Source7:%{name}-logrotate.conf
BuildRequires: systemd-rpm-macros
Requires: logrotate
Requires(pre): shadow-utils
%if %{without vendor}
BuildRequires: golang(github.com/AlecAivazis/survey)
BuildRequires: golang(github.com/atlassian/gostatsd)
BuildRequires: golang(github.com/atlassian/gostatsd/pkg/statsd)
BuildRequires: golang(github.com/coreos/etcd/clientv3)
BuildRequires: golang(github.com/coreos/etcd/clientv3/concurrency)
BuildRequires: golang(github.com/coreos/etcd/embed)
BuildRequires: golang(github.com/coreos/etcd/etcdserver/api/v3client)
BuildRequires: golang(github.com/coreos/etcd/etcdserver/api/v3rpc/rpctypes)
BuildRequires: golang(github.com/coreos/etcd/etcdserver/etcdserverpb)
BuildRequires: golang(github.com/coreos/etcd/mvcc/mvccpb)
BuildRequires:
[Bug 1818670] Review Request: sensu-go - Sensu Go Open Source (Monitoring Program)
https://bugzilla.redhat.com/show_bug.cgi?id=1818670 --- Comment #6 from Devin Acosta --- Dominik, I will see what I can do to try to adhere to the Go way of doing things, however I am not a GoLang developer by any means. The Go Binary that I am building requires 79 additional Go Language dependencies, so it seems like a huge task for me to have to maintain 79 additional packages in addition to the one that I am trying to get added to EPEL 7/EPEL 8. For Example this is all the required GoLang dependencies as of right now. So seems like the Fedora way would say that I have to create 79 packages for each one of these requirements? I understand why bundling everything with the package can be not optimal but seems like a really steep hill to try to have to maintain 79 other packages in addition. Maybe why no one YET has tried to get Sensu Go into EPEL because of the requirements. Thoughts? github.com/AlecAivazis/survey v1.4.1 github.com/Azure/go-ansiterm v0.0.0-20170929234023-d6e3b3328b78 // indirect github.com/NYTimes/gziphandler v0.0.0-20180227021810-5032c8878b9d github.com/StackExchange/wmi v0.0.0-20180725035823-b12b22c5341f // indirect github.com/ash2k/stager v0.0.0-20170622123058-6e9c7b0eacd4 // indirect github.com/atlassian/gostatsd v0.0.0-20180514010436-af796620006e github.com/coreos/bbolt v1.3.3 // indirect github.com/coreos/etcd v3.3.17+incompatible github.com/coreos/go-semver v0.3.0 // indirect github.com/coreos/go-systemd v0.0.0-20190719114852-fd7a80b32e1f // indirect github.com/coreos/pkg v0.0.0-20180928190104-399ea9e2e55f github.com/dave/jennifer v0.0.0-20171207062344-d8bdbdbee4e1 github.com/dgrijalva/jwt-go v3.2.0+incompatible github.com/docker/docker v0.0.0-20180409082103-cbde00b44273 github.com/echlebek/crock v1.0.1 github.com/echlebek/timeproxy v1.0.0 github.com/emicklei/proto v1.1.0 github.com/frankban/quicktest v1.7.2 // indirect github.com/ghodss/yaml v1.0.0 github.com/go-ole/go-ole v0.0.0-20170209151332-de8695c8edbf // indirect github.com/go-resty/resty/v2 v2.1.0 github.com/gogo/protobuf v1.3.1 github.com/golang/groupcache v0.0.0-20191002201903-404acd9df4cc // indirect github.com/golang/protobuf v1.3.2 github.com/google/uuid v1.1.1 github.com/gorilla/context v0.0.0-20160226214623-1ea25387ff6f // indirect github.com/gorilla/mux v1.6.2 github.com/gorilla/websocket v1.4.1 github.com/gotestyourself/gotestyourself v2.2.0+incompatible // indirect github.com/graph-gophers/dataloader v0.0.0-20180104184831-78139374585c github.com/graphql-go/graphql v0.7.9-0.20191125031726-2e2b648ecbe4 github.com/grpc-ecosystem/go-grpc-middleware v1.1.0 // indirect github.com/grpc-ecosystem/grpc-gateway v1.11.3 // indirect github.com/gxed/GoEndian v0.0.0-20160916112711-0f5c6873267e // indirect github.com/gxed/eventfd v0.0.0-20160916113412-80a92cca79a8 // indirect github.com/hashicorp/go-version v1.2.0 github.com/ipfs/go-log v0.0.0-2018041604-7ecd3df29a4a // indirect github.com/jbenet/go-reuseport v0.0.0-20180416043609-15a1cd37f050 // indirect github.com/json-iterator/go v1.1.7 github.com/konsorten/go-windows-terminal-sequences v1.0.2 // indirect github.com/libp2p/go-reuseport v0.0.0-20180416043609-15a1cd37f050 // indirect github.com/libp2p/go-sockaddr v0.0.0-20180329070516-f3e9f73a53d1 // indirect github.com/mattn/go-colorable v0.0.9 // indirect github.com/mattn/go-isatty v0.0.2 // indirect github.com/mattn/go-runewidth v0.0.2 // indirect github.com/mgutz/ansi v0.0.0-20170206155736-9520e82c474b github.com/mholt/archiver/v3 v3.3.1-0.20191129193105-44285f7ed244 github.com/mitchellh/go-homedir v1.1.0 github.com/mitchellh/mapstructure v1.1.2 github.com/olekukonko/tablewriter v0.0.0-20180506121414-d4647c9c7a84 github.com/prometheus/client_golang v1.2.0 github.com/prometheus/client_model v0.0.0-20190812154241-14fe0d1b01d4 github.com/robertkrimen/otto v0.0.0-20180617131154-15f95af6e78d github.com/robfig/cron/v3 v3.0.0 github.com/sensu/lasr v1.2.1 github.com/shirou/gopsutil v0.0.0-20180801053943-8048a2e9c577 github.com/shirou/w32 v0.0.0-20160930032740-bb4de0191aa4 // indirect github.com/sirupsen/logrus v1.4.2 github.com/spf13/cobra v0.0.5 github.com/spf13/pflag v1.0.5 github.com/spf13/viper v1.4.0 github.com/stretchr/testify v1.4.0 github.com/whyrusleeping/go-logging v0.0.0-20170515211332-0457bb6b88fc // indirect github.com/willf/pad v0.0.0-20160331131008-b3d780601022 go.etcd.io/bbolt v1.3.2 go.uber.org/multierr v1.2.0 // indirect golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550 golang.org/x/net v0.0.0-20191014212845-da9a3fd4c582 golang.org/x/sys v0.0.0-20191113165036-4c7a9d0fe056 golang.org/x/text v0.3.2 // indirect golang.org/x/time v0.0.0-20190921001708-c4c64cad1fd0 google.golang.org/genproto v0.0.0-20191009194640-548a555dbc03 // indirect google.golang.org/grpc v1.24.0 gopkg.in/AlecAivazis/survey.v1 v1.4.0 // indirect gopkg.in/h2non/filetype.v1 v1.0.3 gopkg.in/sourcemap.v1 v1.0.5 // indirect gopkg.in/yaml.v2 v2.2.4 gotest.tools v2.2.0+incompatible // indirect sigs.k8s.io/yaml v1.1.0 -- You are
[Bug 1818670] Review Request: sensu-go - Sensu Go Open Source (Monitoring Program)
https://bugzilla.redhat.com/show_bug.cgi?id=1818670
Dominik 'Rathann' Mierzejewski changed:
What|Removed |Added
CC||domi...@greysector.net
--- Comment #5 from Dominik 'Rathann' Mierzejewski ---
%define debug_package %{nil} is no-go, a proper debug{info,source} packages
must be generated. See
https://docs.fedoraproject.org/en-US/packaging-guidelines/Debuginfo/ .
Fedora has Golang specific guidelines at
https://docs.fedoraproject.org/en-US/packaging-guidelines/Golang/ .
Is there any reason why you're not following them?
The empty %check section is useless. Please either run some after-build tests
there or drop it. Golang guidelines mention a %gochecks macro which runs
built-in tests in a standard way.
ExclusiveArch: x86_64 is not justified, please see:
https://docs.fedoraproject.org/en-US/packaging-guidelines/#_architecture_support
As for bundling so many build dependencies, this is something that should be
avoided in general:
https://docs.fedoraproject.org/en-US/packaging-guidelines/#bundling .
Please make an effort to unbundle them first. This may require submitting
additional packages for review.
I can sponsor you, but this package requires a bit more work to pass review. It
would also help if you could do a couple of informal reviews of other pending
packages.
--
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org
Fedora Code of Conduct:
https://docs.fedoraproject.org/en-US/project/code-of-conduct/
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives:
https://lists.fedoraproject.org/archives/list/package-review@lists.fedoraproject.org
[Bug 1818670] Review Request: sensu-go - Sensu Go Open Source (Monitoring Program)
https://bugzilla.redhat.com/show_bug.cgi?id=1818670 --- Comment #4 from Devin Acosta --- I have spent a little over an entire week making lots of modifications per your request. I changed the following: - Removed my cleanup jobs so they don't delete users, or cleanup any directories - I fixed my installation section to all use common (install -m xxx) method. - I cleaned up a lot of the %files, %attr() section, also fixed the /var/run issue I ran into, which i talked in IRC channel about. - I had it end up creating 3 RPMS like the Sensu Enterprise edition does to keep in-line with what they are doing. - I fixed the dynamic user creation as requested. The only thing that I have yet to really tackle is the way that I am TARing up the Go Dependencies and just extracting them (since the Koji) doesn't allow for external access to download the modules it needs to compile. I am still working on seeing what options i have there to clean up the code a little bit more. please check out my latest SPEC file located at: https://raw.githubusercontent.com/devinacosta/sensugo-koji/master/rpmbuild/SPECS/sensu-go.spec Also You can see my latest build output here: https://koji.fedoraproject.org/koji/taskinfo?taskID=43104293 Let me know if you have more suggestions or if i missed anything? -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component ___ package-review mailing list -- package-review@lists.fedoraproject.org To unsubscribe send an email to package-review-le...@lists.fedoraproject.org Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/package-review@lists.fedoraproject.org
[Bug 1818670] Review Request: sensu-go - Sensu Go Open Source (Monitoring Program)
https://bugzilla.redhat.com/show_bug.cgi?id=1818670 --- Comment #3 from Zbigniew Jędrzejewski-Szmek --- (In reply to Zbigniew Jędrzejewski-Szmek from comment #2) > /var/run/sensu/ → /run/sensu. It also needs %ghost. > https://docs.fedoraproject.org/en-US/packaging-guidelines/#_run I saw that you asked about this on IRC. /var/run is a symlink to /run, so using the /var/run path anywhere doesn't make much sense. Usually, it's just a longer and uglier legacy name. It also matters in one particular case: when /var is separately mounted. In that case, /var/run cannot be resolved before /var is mounted, so using /var/run creates an ordering dependency. Both in case of /var/run/foo and /run/foo, the directory is stored on a tmpfs, so it doesn't survive reboots. Rpm needs to be told that this is the case, by adding %ghost in the %file list, so that e.g. 'rpm -V' doesn't complain about a missing directory. -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component ___ package-review mailing list -- package-review@lists.fedoraproject.org To unsubscribe send an email to package-review-le...@lists.fedoraproject.org Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/package-review@lists.fedoraproject.org
[Bug 1818670] Review Request: sensu-go - Sensu Go Open Source (Monitoring Program)
https://bugzilla.redhat.com/show_bug.cgi?id=1818670
Zbigniew Jędrzejewski-Szmek changed:
What|Removed |Added
CC||zbys...@in.waw.pl
Doc Type|--- |If docs needed, set a value
--- Comment #2 from Zbigniew Jędrzejewski-Szmek ---
The description is supposed to go in the spec file primarily. (Usually the one
in the
review request is just a copy. What counts is the one in the file.)
Please wrap it to 80 columns when copying into the file.
Sources should be extracted in %prep, not in %build.
The scripts to add 'sensu' user and group need to be a bit different:
please use the scriptlet from
https://docs.fedoraproject.org/en-US/packaging-guidelines/UsersAndGroups/#_dynamic_allocation
Users cannot be removed from the system, because this generally cannot be done
safely.
Also, do not remove non-cache data on package removal.
The scriptlets to define the user should be present in just one subpackage,
and the other subpackage should Require it. If both packages are fully
independent
and both need the user, add a comment to the spec file about that.
The %postun systemd scriptlets are missing, see
https://docs.fedoraproject.org/en-US/packaging-guidelines/Scriptlets/#_scriptlets
You can drop full paths like in /usr/sbin/userdel. Both /usr/bin and /usr/sbin
are guaranteed to be in the path.
There is also no gain in using macros like %{__install}. They serve no purpose
whatsoever
and make the spec file much harder to read.
(There is a guidelines to use macros for some _directories_, because they
change occasionally,
but binary names don't change.)
Systemd unit files go to /usr/lib/systemd/system, not in etc, see
https://www.freedesktop.org/software/systemd/man/systemd.unit.html#Description.
/var/run/sensu/ → /run/sensu. It also needs %ghost.
https://docs.fedoraproject.org/en-US/packaging-guidelines/#_run
BuildRequires: systemd
→ systemd-rpm-macros should be enough, it is much more lightweight.
> Group: Development/Languages
Not necessary, please remove.
Requires(post): systemd
Requires(preun):systemd
Requires(postun): systemd
→ not necessary, should be removed.
Following
https://docs.fedoraproject.org/en-US/packaging-guidelines/Golang/#_dependencies,
please add:
BuildRequires: go-rpm-macros.
The big question at the end: would it be possible to unbundle any of the
go dependencies, i.e. to use some packages from Fedora. It seems that doing
that
for all 70+ modules is infeasible, but maybe a partial unbundling would work. I
don't know about go to have an informed opinion. If everything is bundled, this
needs a comment in the spec file and justification.
--
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list -- package-review@lists.fedoraproject.org
To unsubscribe send an email to package-review-le...@lists.fedoraproject.org
Fedora Code of Conduct:
https://docs.fedoraproject.org/en-US/project/code-of-conduct/
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives:
https://lists.fedoraproject.org/archives/list/package-review@lists.fedoraproject.org
[Bug 1818670] Review Request: sensu-go - Sensu Go Open Source (Monitoring Program)
https://bugzilla.redhat.com/show_bug.cgi?id=1818670 Weiping changed: What|Removed |Added CC||zwp10...@gmail.com --- Comment #1 from Weiping --- (In reply to Devin Acosta from comment #0) > Spec URL: > https://github.com/devinacosta/sensugo-koji/blob/master/rpmbuild/SPECS/sensu- > go.spec This is a informal review: Please use raw format for github, there is a raw button, like https://raw.githubusercontent.com/devinacosta/sensugo-koji/master/rpmbuild/SPECS/sensu-go.spec -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component ___ package-review mailing list -- package-review@lists.fedoraproject.org To unsubscribe send an email to package-review-le...@lists.fedoraproject.org Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/package-review@lists.fedoraproject.org
[Bug 1818670] Review Request: sensu-go - Sensu Go Open Source (Monitoring Program)
https://bugzilla.redhat.com/show_bug.cgi?id=1818670 Devin Acosta changed: What|Removed |Added Blocks||177841 (FE-NEEDSPONSOR) Referenced Bugs: https://bugzilla.redhat.com/show_bug.cgi?id=177841 [Bug 177841] Tracker: Review requests from new Fedora packagers who need a sponsor -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component ___ package-review mailing list -- package-review@lists.fedoraproject.org To unsubscribe send an email to package-review-le...@lists.fedoraproject.org Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/package-review@lists.fedoraproject.org
