elastic / logstash

Logstash - transport and process your logs, events, or other data
https://www.elastic.co/products/logstash
Other
61 stars 3.5k forks source link

bin/plugin list show non installed plugins #4290

Open purbon opened 8 years ago

purbon commented 8 years ago

The bin/plugin list command is broken as it shows not installed plugins by default, this makes the user interaction confusing, see the example:

skywalker% bin/plugin list
logstash-codec-json
logstash-codec-json_lines
logstash-codec-line
logstash-codec-plain
logstash-filter-clone
logstash-filter-grok
logstash-filter-multiline
logstash-filter-mutate
logstash-input-generator
logstash-input-stdin
logstash-input-tcp
logstash-output-stdout
logstash-patterns-core
skywalker% mvim .
skywalker% bin/plugin uninstall logstash-codec-json
ERROR: Uninstall Aborted, message: This plugin has not been previously installed, aborting

I can see the json codec listed, but as is not installed I can not remove it.

purbon commented 8 years ago

@ph @colinsurprenant what do you think about this issue? for me is a bug.

colinsurprenant commented 8 years ago

@purbon I think its a regression, can you confirm? If it is we can probably easily find the cause.

purbon commented 8 years ago

@colinsurprenant I'm not sure is a regression per se, will investigate and report here.

ph commented 8 years ago

For me its a regression, probably the bundler env is not correctly setup in that method? (its usually the problem)

colinsurprenant commented 8 years ago

let's put it as a blocker of 2.1.1

purbon commented 8 years ago

@ph @colinsurprenant will investigate and report here and/or a solution

purbon commented 8 years ago

@ph @colinsurprenant this is not a regression, been there like the beginning of the plugin manager, but we never noticed. Code in https://github.com/elastic/logstash/blob/master/lib/pluginmanager/list.rb#L28-L40, see

 def filtered_specs
    @filtered_specs ||= begin
                          # start with all locally installed plugin gems regardless of the Gemfile content
                          specs = LogStash::PluginManager.find_plugins_gem_specs

                          # apply filters
                          specs = specs.select{|spec| gemfile.find(spec.name)} if installed?
                          specs = specs.select{|spec| spec.name =~ /#{plugin}/i} if plugin
                          specs = specs.select{|spec| spec.metadata['logstash_group'] == group} if group

                          specs
                        end
  end

the only difference in the last version is that in the uninstall we check for "installed gems", not as before. I do think we should change this interface to list only installed plugins.

ph commented 8 years ago

@purbon You are right the list command should only list "installed" plugins, also I think the definition we have of installed plugin is the following: they are listed in the lockfile and installed on the system.

In some cases we can have plugins that are present in the bundle directory but not listed in the lockfile or the gemfile, which is the case for logstash-codec-json in the issue description.

purbon commented 8 years ago

@ph then the definition of installed plugin is different in the list command than the one in the uninstall command (see https://github.com/elastic/logstash/blob/master/lib/pluginmanager/uninstall.rb#L18-L20 ). This is confusing, we should fix one way or another.

BTW, for me installed means in the Gemfile, others are dependencies of installed plugins, there only because there are plugins that depends on them.

ph commented 8 years ago

@purbon I agree this is confusing and we should clarify it.

Lets start with a clean install of logstash without any plugins.

If logstash-input-beats depends on logstash-codec-multiline and I install the beats input, both plugins will be installed in logstash but only one will be explicitly defined in the Gemfile, not the other since its a dependency. The two plugins are available when creating configuration.

What would be the result of bin/plugin list in that specific case?

purbon commented 8 years ago

@ph like I showed before as is today will show both. I think as booths are going to be used from configurations we should make them booths available in the Gemfile and then issue fix.

ph commented 8 years ago

You proposed solution is to make sure all logstash plugins are explicitly defined in the Gemfile. So following my previous case https://github.com/elastic/logstash/issues/4290#issuecomment-160978668

If my gemfile is clean of any plugins and I install logstash-input-beats which depends on the codec multiline I will get this content in the gemfile:

gem "logstash-input-beats"
gem "logstash-codec-multiline"

Correct? I certainly agree this would be more explicit than filtering the plugins using the metadata. How are you planning to do it?

purbon commented 8 years ago

@colinsurprenant your thoughts on this?

ph commented 8 years ago

just a note, this might be a bit complicated to update the gemfile with dependency :(

colinsurprenant commented 8 years ago

exactly, what would be the purpose and benefit of adding the dependencies in the Gemfile?

purbon commented 8 years ago

@colinsurprenant issue is different expectation to what is an installed plugin in the list command and in the uninstall command, we should unify them.

colinsurprenant commented 8 years ago

trying to manually fill the Gemfile with the dependencies that Bundler manages is absolutely not the right way to fix this problem. Also note that users are not exposed to the Gemfile.

Having an installed dependency listed in bin/plugin list is OK IMO since that dependency IS installed. So maybe the right approach here is to simply let users uninstall a dependency BUT it will most probably result in an error since IT IS still a dependency so it cannot be removed.

Or maybe we should just change the error message, instead of

ERROR: Uninstall Aborted, message: This plugin has not been previously installed, aborting

we could say something like

ERROR: Uninstall Aborted, message: This plugin cannot be uninstalled because it was not installed using bin/plugins install and is a dependecy to another plugin

Also, have we have reports for this problem? is that really confusing users? are users really uninstalling plugins?!

jakelandis commented 6 years ago

I just tested this against 5.6.3 and it is still an issue!

C:\Users\jake\Downloads\logstash-5.6.3\logstash-5.6.3>bin\logstash-plugin list
logstash-codec-cef
logstash-codec-collectd
logstash-codec-dots
logstash-codec-edn
logstash-codec-edn_lines
logstash-codec-es_bulk
logstash-codec-fluent
logstash-codec-graphite
logstash-codec-json
logstash-codec-json_lines
logstash-codec-line
logstash-codec-msgpack
logstash-codec-multiline
logstash-codec-netflow
logstash-codec-plain
logstash-codec-rubydebug
logstash-filter-cidr
logstash-filter-clone
logstash-filter-csv
logstash-filter-date
logstash-filter-dissect
logstash-filter-dns
logstash-filter-drop
logstash-filter-fingerprint
logstash-filter-geoip
logstash-filter-grok
logstash-filter-json
logstash-filter-kv
logstash-filter-metrics
logstash-filter-mutate
logstash-filter-ruby
logstash-filter-sleep
logstash-filter-split
logstash-filter-syslog_pri
logstash-filter-throttle
logstash-filter-translate
logstash-filter-urldecode
logstash-filter-useragent
logstash-filter-uuid
logstash-filter-xml
logstash-input-beats
logstash-input-couchdb_changes
logstash-input-dead_letter_queue
logstash-input-elasticsearch
logstash-input-exec
logstash-input-file
logstash-input-ganglia
logstash-input-gelf
logstash-input-generator
logstash-input-graphite
logstash-input-heartbeat
logstash-input-http
logstash-input-http_poller
logstash-input-imap
logstash-input-irc
logstash-input-jdbc
logstash-input-kafka
logstash-input-log4j
logstash-input-lumberjack
logstash-input-pipe
logstash-input-rabbitmq
logstash-input-redis
logstash-input-s3
logstash-input-snmptrap
logstash-input-sqs
logstash-input-stdin
logstash-input-syslog
logstash-input-tcp
logstash-input-twitter
logstash-input-udp
logstash-input-unix
logstash-input-xmpp
logstash-output-cloudwatch
logstash-output-csv
logstash-output-elasticsearch
logstash-output-file
logstash-output-graphite
logstash-output-http
logstash-output-irc
logstash-output-kafka
logstash-output-nagios
logstash-output-null
logstash-output-pagerduty
logstash-output-pipe
logstash-output-rabbitmq
logstash-output-redis
logstash-output-s3
logstash-output-sns
logstash-output-sqs
logstash-output-statsd
logstash-output-stdout
logstash-output-tcp
logstash-output-udp
logstash-output-webhdfs
logstash-output-xmpp
logstash-patterns-core
C:\Users\jake\Downloads\logstash-5.6.3\logstash-5.6.3>bin\logstash-plugin uninstall logstash-codec-xmpp
ERROR: Operation aborted, cannot remove plugin., message: This plugin has not been previously installed