[Bug 1826621] Review Request: RMD - Resource Manager Deamon

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

Neil Horman <nhorman@xxxxxxxxxx> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
              Flags|                            |needinfo?(arunprabhu.vijaya
                   |                            |n@xxxxxxxxx)



--- Comment #12 from Neil Horman <nhorman@xxxxxxxxxx> ---
Thank you for your quick reply. In response:

>[Arun] 
>1. Sure. We will add all packages present in fedora as BuildRequires and drop from go.mod file & other packages as bundled exception in go.mod file.
Perfect, thank you!
>2. We will also add BuildRequires : git-core & BuildRequires : go-rpm-macros. 
Excellent

>Note: In my fedora machine,am getting this error if I add go-rpm-macros. Can you please let me why I'm getting this error?
>
>[root]# dnf install go-rpm-macros
>Last metadata expiration check: 3:07:12 ago on Wed 13 May 2020 00:52:11 IST.
>No match for argument: go-rpm-macros
>Error: Unable to find a match: go-rpm-macros
Hmm...odd, what Fedora release are you running on?  I managed to install it on
my F32 system just fine.  I wonder if you're using an older fedora release that
doesn't have those macros packaged yet?

>> >1.rmd.x86_64: W: pem-certificate /usr/local/etc/rmd/acl/roles/admin/cert.pem
>> >This is not fixed as these files are only used for testing.
>> I'm not sure I follow here.  What testing are you referring to?  I don't see
>> any %check script in your spec file, so I'm not sure what testing you are
>> doing.  As such, why not just not package the pem certificates at all?

>[Arun] PEM certificates can be used as reference by System Admin to configure. So, it is more like reference files or fast and easy check of functionality.. Sorry for mentioning it as used for testing >purposes.
>https://github.com/intel/rmd/blob/master/etc/rmd/rmd.toml  has reference usage in [default] section. Please refer to it.
Hmm, ok, I see the need, but I'll need to check a little bit on what the policy
is here.  I would hate to have people use this with a default certificate in
your name.  I expect we can mark those in the file manafest with a %conf, and
make a note in the documentation and the config file that production systems
should generate their own cert, but I'll check and confirm.

>> >2. Below error is not fixed as our SW needs to copy config files to /usr/local/etc/
>> > rmd.x86_64: E: dir-or-file-in-usr-local /usr/local/etc/rmd/policy.toml
>> >A file in the package is located in /usr/local. It's not permitted for
>> >packages to install files in this directory.
>> >Added exception in rpmlint --> addFilter("E: dir-or-file-in-usr-local") in /etc/rpmlint/config
>> Why?  It looks to me like the rmd utility allows you to specify the location
>> of a config file on the command line, so I don't understand why the config
>> files can't be placed in the appropriate directory (%_sysconfdir}/etc) here.
>> Note also that, if you add a systemd unit file as noted in comment 5, you
>> can set the config directory there without having to modify the source
>> defaults.

>[Arun] If all config files need to be moved to /etc/, we will move them from /usr/local/etc/ folder. We also have some install related scripts which needs to be moved from /usr/local/etc/rmd to /etc/rmd. >Please let us know if it is OK?
Yes, that is exactly correct.  You are completely permitted to create
subdirectories under /etc to store your config files in, it just all needs to
be rooted in /etc (or more specifically %{_sysconfdir}, which currently expands
to /etc)

>> The review of the package itself looks ok, though you didn't address the
>> following items from comment 5:
>> [!]: %config files are marked noreplace or the reason is justified.
>>      Note: No (noreplace) in %config(missingok) /usr/bin/scripts
>>      %config(missingok) /usr/bin/etc>
>
>[Arun] This was removed from the latest spec file as we corrected in the spec file by installing directly from the SPEC file in the correct path instead of running script from /usr/bin directory which will >copy these files to /usr/local/etc/. 
Understood, thank you!

>> [!]: Package contains systemd file(s) if in need.  <= you probably want to
>> add a systemd unit to start rmd
>
>[Arun] We will discuss internally and get back if its needed to keep the systemd file for this release.
I'm sorry, I'm not sure I understand.  I didn't see a systemd unit file
previously (hence the error).  I presume you want to run rmd as a daemon,
correct?  If so, I would strongly recommend that you add one, so you have a
good user experience when the package is installed.


>> [!]: Package is not known to require an ExcludeArch tag. <= It should be
>> buildable on non-x86 platforms, you either need to allow that, or document
>> the fact that rmd only operates on data found in x86 systems
>> 
>[Arun] RMD addresses Resource Management for Intel X86 architectures. HW support is documented in https://github.com/intel/rmd/blob/master/docs/Prerequisite.md file. Please let us know if this Ok or needs >further documentation.
Yes, the documentation is fine, if you could just add a comment in the spec
file above the ExclusiveArch tag pointing to that URL, that would be great

>> [!]: Package functions as described. <= description in spec file needs to be
>> more verbose

>[Arun] We added description in the SPEC file in the last version. It has the details, kindly let us know if something more needs to be added. I saw the Lint warning disappear after addition in the >description.
This is the description that was in the latest spec file that you submitted:
%description
Resource Management Daemon (RMD) is a system daemon running on Linux platforms

I'm asking that you provide a little more description around what RMD actually
does (i.e. why people might want to run it).


-- 
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@xxxxxxxxxxxxxxxxxxxxxxx
To unsubscribe send an email to package-review-leave@xxxxxxxxxxxxxxxxxxxxxxx
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@xxxxxxxxxxxxxxxxxxxxxxx




[Index of Archives]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite Conditions]     [KDE Users]

  Powered by Linux