Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Display plugins versions #18683

Merged
merged 1 commit into from
Sep 12, 2016
Merged

Conversation

ggrossetie
Copy link
Contributor

This is useful to determine if a plugin needs to be updated when using deployment automation solution (like Ansible).

A similar feature is now available in Kibana: elastic/kibana#7221

This is useful to determine if a plugin needs to be updated when using deployment automation solution (like Ansible).
@rjernst
Copy link
Member

rjernst commented Jun 1, 2016

The verbose option was extended to display all plugin properties in #18051.

@ggrossetie
Copy link
Contributor Author

ggrossetie commented Jun 1, 2016

Yes I saw but the output is on multiples lines (ie. harder to parse with an automation tool).
The format name@version is quite common and the behavior will be the same across Kibana and Elasticsearch (and soon Logstash ?)

The verbose output is still useful for human.

@rjernst rjernst added discuss :Core/Infra/Plugins Plugin API and infrastructure labels Jun 1, 2016
@rjernst
Copy link
Member

rjernst commented Jun 1, 2016

Human consumption is all you can really do. Knowing the plugin version doesn't tell you if it needs updating. Any time you update elasticsearch, plugins need to be updated, because they currently must be built against each specific version of elasticsearch.

I think the non verbose output should stay as simply listing the plugin names installed, eg like ls just lists file names. But I have marked this for discussion.

@jasontedor
Copy link
Member

I think the non verbose output should stay as simply listing the plugin names installed, eg like ls just lists file names.

+1

@ggrossetie
Copy link
Contributor Author

Human consumption is all you can really do. Knowing the plugin version doesn't tell you if it needs updating. Any time you update elasticsearch, plugins need to be updated, because they currently must be built against each specific version of elasticsearch.

But you can still have a new version of a plugin without a new version of Elasticsearch ?

I think the non verbose output should stay as simply listing the plugin names installed, eg like ls just lists file names. But I have marked this for discussion.

"Make everything as simple as possible, but not simpler".
If I want to know the content of a directory, I will use ls not a Java program 😉

I think the plugin version is an important information and ListPluginsCommand already knows this information, why not make it available and easy to read by an automation tool ? What are the drawbacks of adding this piece of information ?

Here's some benefits:

  • Check that a plugin need to be updated
  • Check that the plugin version is compatible with Elasticsearch version
  • Do not remove/install a plugin if the version did not change

Right now, in Ansible the basic idea is remove everything, install everything: https://github.com/elastic/ansible-elasticsearch/blob/master/tasks/elasticsearch-plugins.yml#L20 😒

@rjernst
Copy link
Member

rjernst commented Jun 1, 2016

Right now, in Ansible the basic idea is remove everything, install everything

That is what must happen for all official elasticsearch plugins. You are correct that community plugins may release multiple versions per elasticsearch version. But knowing what version is currently installed of a community plugin does not tell you anything about whether there is another version, and upgrading elasticsearch means another version of that community plugin must be installed.

@ggrossetie
Copy link
Contributor Author

But knowing what version is currently installed of a community plugin does not tell you anything about whether there is another version

Ansible will know because I will tell him to install a (newer) version for this plugin. Being able to determine if I need to remove a community plugin in order to install a newer version will be really useful.
IMHO this is just tiny addition that can be useful to users and will certainly make my life easier 😄

@dadoonet
Copy link
Member

We discussed it on fix it friday and found that it could be a nice feature to have. We don't see any drawback by adding this.
It's also consistent with the rest of the stack (kibana as explained in the PR) and if it makes our users life easier, let's do it.

I'm changing the label to "review".

@rjernst As you were not available for this Fix It friday session, feel free to comment and raise any objection if any and change back the label to "discuss".

@dadoonet dadoonet added review and removed :Core/Infra/Plugins Plugin API and infrastructure discuss labels Jun 24, 2016
@ggrossetie
Copy link
Contributor Author

Thanks @dadoonet !

@clintongormley clintongormley added >enhancement :Core/Infra/Plugins Plugin API and infrastructure labels Jun 27, 2016
@ggrossetie
Copy link
Contributor Author

@dadoonet Any news ?

@dakrone
Copy link
Member

dakrone commented Sep 12, 2016

This LGTM, I will merge this

@dakrone
Copy link
Member

dakrone commented Sep 12, 2016

@Mogztter merged this, thanks!

@dakrone
Copy link
Member

dakrone commented Sep 12, 2016

Backported to 5.x and 5.0

@dadoonet
Copy link
Member

Thanks @dakrone! I was planning to merge it today! \o/

@ggrossetie
Copy link
Contributor Author

Thanks \o/
Looking forward to the next 5.0 release ;)

@jasontedor
Copy link
Member

This has been reverted in #20807.

@ggrossetie
Copy link
Contributor Author

Wait what... but why ? 😭

@jasontedor
Copy link
Member

For the initial reasons given against this PR and additionally as elaborated in #20668.

@ggrossetie
Copy link
Contributor Author

Just read #20668

I was against that change because it's conflating a human-readable API with a machine-readable API.

Fair enough but where can I find a machine-readable API ?
As a workaround, you can use localhost:9200/_cat/plugins API but the server must be running.
I think I will just create a tiny script to read the properties file.

@jasontedor
Copy link
Member

Fair enough but where can I find a machine-readable API ?

There isn't one, but that does not mean we should mix the two.

I think I will just create a tiny script to read the properties file.

I'm wondering why you don't unconditionally uninstall and then install the right version?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants