Initial ABRT proxy support #133

mmilata · 2014-02-12T18:12:33Z

This change makes it possible for the proxy to pretend it is ABRT
(Automatic Bug Reporting Tool[1]) server. When it receives a problem
report, it forwards notification of the error to Foreman, and optionally
also forwards the original report to an actual ABRT server.

Non strictly ABRT-related changes:

Move module Proxy::ChefProxy to Proxy::Request
Add :foreman_ssl_ options to settings example

[1] https://github.com/abrt/abrt/wiki/ABRT-Project

For some notes regarding usage, see https://groups.google.com/forum/#!topic/foreman-dev/Yk2GrW9ne20

I am quite new to Ruby so please let me know if anything should be done in a different way:)

lzap · 2014-02-12T18:31:10Z

Thanks for your contribution, the formal review will follow shortly. [test]

lzap · 2014-02-12T18:41:16Z

lib/proxy.rb

@@ -18,7 +18,8 @@ module Proxy
  require "proxy/dns"        if SETTINGS.dns
  require "proxy/dhcp"       if SETTINGS.dhcp
  require "proxy/bmc"        if SETTINGS.bmc
-  require "proxy/chefproxy"  if SETTINGS.chefproxy
+  #require "proxy/chefproxy"  if SETTINGS.chefproxy


Oops. Should I reopen the PR or wait after you finish the review?

Let us finish the review, then fix it, amend the changes and force-push into the same branch. Github will notice the changes, no need to close/open/reopen the PR.

When amending, please reword your commit message to:

fixes #4332 - initial abrt proxy support

I created feature for this: http://projects.theforeman.org/issues/4332

lzap · 2014-02-12T18:48:31Z

Initial code reading looks fine, can you paste some info how to configure the ABRT client. I will be happy to test this tomorrow.

mmilata · 2014-02-12T21:12:19Z

The client configuration is described on https://groups.google.com/forum/#!topic/foreman-dev/Yk2GrW9ne20 (please note that # abrt-auto-reporting enabled and other lines below starting with # are commands to be run as root; lines starting with # above are config file comments).

If anything is unclear, you can also contact me via IRC (either internal or as b42 on #abrt@freenode).

lzap · 2014-02-13T10:01:12Z

config/settings.yml.example

 #:foreman_url: http://127.0.0.1:3000
+#:foreman_ssl_ca: ssl/certs/ca.pem
+#:foreman_ssl_cert: ssl/certs/fqdn.pem


Hmmm I am trying to find use of foreman_ssl_* variables in this patch and I am not succesful. I only see validations for the abrt_server_ssl* variables bellow. Am I searching correctly, or is this expected. I wonder what is unused settings for then.

The variables are used in lib/proxy/request.rb (which was renamed from lib/proxy/chefproxy.rb in the commit). They should probably have been added in the patch that introduced the chef proxy.

I see, thanks.

lzap · 2014-02-13T10:18:49Z

Sidenote: In the instructions for this feature, we must make sure that abrt-addon-ccpp package is installed.

lzap · 2014-02-13T10:42:52Z

Ok this is working fine. The only issue I had was that abrt autoupload feature was not working properly on Fedora 20 minimal install. Workaround:

yum -y install abrt abrt-addon-ccpp abrt-python

I will look on the ruby code now.

lzap · 2014-02-13T10:54:36Z

lib/abrtproxy_api.rb

+    ureport_json = request['file'][:tempfile].read
+    ureport = JSON.parse(ureport_json)
+
+    #forward to FAF


Can you also enclose the whole FAF sending to a begin/rescue block catching all the Exceptions with a message "Unable to send to FAF"? Use log_halt to generate the error and return 503.

Thanks. [test]

Ok this needs some additional opinions from the team.

This change makes it possible for the proxy to pretend it is ABRT (Automatic Bug Reporting Tool[1]) server. When it receives a problem report, it forwards notification of the error to Foreman, and optionally also forwards the original report to an actual ABRT server. Non strictly ABRT-related changes: - Move module Proxy::ChefProxy to Proxy::Request - Add :foreman_ssl_ options to settings example [1] https://github.com/abrt/abrt/wiki/ABRT-Project

domcleal · 2014-02-13T16:46:59Z

I don't think this should be in the proxy core, especially as it masquerades via configuration management reports. This should be written as a plugin once the proxy has plugin support of its own.

lzap · 2014-02-14T09:16:59Z

Well my assumption was to generalize reports to some degree. I am fine
with the plugin.

Later,

Lukas "lzap" Zapletal
irc: lzap #theforeman

mmilata · 2014-02-17T14:29:00Z

@domcleal Afaik it is planned for foreman to support multiple report types: ~~http://projects.theforeman.org/issues/4332~~ http://projects.theforeman.org/issues/4151. Then the ABRT proxy can be rewritten to use its own report format. Would that be acceptable? Do you have any estimate for the smart-proxy plugin support?

domcleal · 2014-02-17T14:35:20Z

Yes, Foreman should support different report types (http://projects.theforeman.org/issues/4151), but I don't think that would be appropriate still. Reports in Foreman's context are configuration management reports, so it would additionally require different treatment - e.g. we show a host as out of sync when it doesn't send in reports. If an ABRT "report" arrived, that would incorrectly imply that configuration management is working. I think it would require even more changes to how "reports" are handled, or simply to define a new object type.. which can already be done today in a plugin.

I don't know when proxy plugin support (http://projects.theforeman.org/issues/3351) will be done, as it's not on our current sprint and not prioritised in the backlog.

mmilata · 2014-02-17T14:50:30Z

Oops, #4151 was what I meant to link.

I'll take a closer look at foreman plugins. From what you say it certainly makes sense to treat them separately from the configuration management reports.

iNecas · 2014-02-18T16:48:45Z

@domcleal what about having reports more generic, and a subtype of them would be ConfigManagement related: this subtypes would update the sync status, while ABRT type would not cause it. Since the reports are all that similar, I don't see why we would need something completely different for the ABRT type of reports?

domcleal · 2014-02-18T16:53:44Z

@iNecas indeed. Tthough I don't know what ABRT sends, that'd be sensible.

mmilata · 2014-02-18T16:56:37Z

The ABRT reports look like this: https://github.com/mmilata/smart-proxy/blob/abrtproxy/test/fixtures/abrt/ureport1.json

ares · 2014-02-19T09:43:16Z

I think report is still a report. We may set host status based on config mgmt reports only but I think we could reuse a lot of existing report functionality.

if ABRT report was imported, it would be displayed on dashboard
ABRT report will later aggregate crashes, so counters (e.g. for failures) seems appropriate and each failure should be separate log in report

I think it would be better to make reports more general. If we make STI which separates "config reports" and "crash reports" it will be easy to keep hosts statuses based only on config reports.

Also maybe later we could have more host statuses based on reports type. E.g. crash report could make host in some failing state. I can imagine other reports from let's say monitoring tools.

EDIT: I should have reloaded the page before submitting, seems like @iNecas already said it.

lzap · 2014-02-20T09:14:37Z

I agree with Marek.

Having that said, I think this deserves some investigation of the field.
We should look on technologies we would like to see in Foreman and then
refactor this properly for once.

I suppose we aim for:

puppet reports
chef reports
ansible reports
abrt reports

In addition to that, it would be good to consider support for syslog to
some degree (syslog server integration). But this should be perhaps
better candidate for a plugin as this is something else than reporting.
It's a constant stream of logs.

Later,

Lukas "lzap" Zapletal
irc: lzap #theforeman

iNecas · 2014-02-20T09:23:30Z

Maybe SCAP reports as well?

mmilata · 2014-03-26T16:12:51Z

Thanks for the feedback! I've opened new pull request for this, since the code changed significantly: #144

lzap reviewed Feb 12, 2014
View reviewed changes

lzap reviewed Feb 13, 2014
View reviewed changes

mmilata mentioned this pull request Mar 26, 2014

Initial abrt proxy support (2nd version) #144

Closed

mmilata closed this Mar 26, 2014

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Initial ABRT proxy support #133

Initial ABRT proxy support #133

mmilata commented Feb 12, 2014

lzap commented Feb 12, 2014

lzap Feb 12, 2014

mmilata Feb 12, 2014

lzap Feb 13, 2014

lzap commented Feb 12, 2014

mmilata commented Feb 12, 2014

lzap Feb 13, 2014

mmilata Feb 13, 2014

lzap Feb 13, 2014

lzap commented Feb 13, 2014

lzap commented Feb 13, 2014

lzap Feb 13, 2014

lzap Feb 13, 2014

domcleal commented Feb 13, 2014

lzap commented Feb 14, 2014

mmilata commented Feb 17, 2014

domcleal commented Feb 17, 2014

mmilata commented Feb 17, 2014

iNecas commented Feb 18, 2014

domcleal commented Feb 18, 2014

mmilata commented Feb 18, 2014

ares commented Feb 19, 2014

lzap commented Feb 20, 2014

iNecas commented Feb 20, 2014

mmilata commented Mar 26, 2014

Initial ABRT proxy support #133

Initial ABRT proxy support #133

Conversation

mmilata commented Feb 12, 2014

lzap commented Feb 12, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lzap commented Feb 12, 2014

mmilata commented Feb 12, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lzap commented Feb 13, 2014

lzap commented Feb 13, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

domcleal commented Feb 13, 2014

lzap commented Feb 14, 2014

mmilata commented Feb 17, 2014

domcleal commented Feb 17, 2014

mmilata commented Feb 17, 2014

iNecas commented Feb 18, 2014

domcleal commented Feb 18, 2014

mmilata commented Feb 18, 2014

ares commented Feb 19, 2014

lzap commented Feb 20, 2014

iNecas commented Feb 20, 2014

mmilata commented Mar 26, 2014