Skip to content

Deprecation check for File Discovery plugin#36190

Merged
AthenaEryma merged 1 commit intoelastic:6.xfrom
AthenaEryma:depcheck/file-discovery-plugin
Dec 7, 2018
Merged

Deprecation check for File Discovery plugin#36190
AthenaEryma merged 1 commit intoelastic:6.xfrom
AthenaEryma:depcheck/file-discovery-plugin

Conversation

@AthenaEryma
Copy link
Copy Markdown
Contributor

Adds a deprecation warning that the file discovery plugin is no longer
needed and that the path it uses has changed.

Relates to #36024 and #33257

Adds a deprecation warning that the file discovery plugin is no longer
needed and that the path it uses has changed.
@elasticmachine
Copy link
Copy Markdown
Collaborator

Pinging @elastic/es-core-features

Copy link
Copy Markdown
Member

@DaveCTurner DaveCTurner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this needs to be two independent checks:

  1. the discovery-file plugin is not in use, and
  2. the path to the unicast hosts file is correct

This is because in versions ≥ 6.5.0 you can be using the old path without using the plugin, and in that situation AIUI this warning won't fire.

@AthenaEryma
Copy link
Copy Markdown
Contributor Author

@DaveCTurner I agree that would be better, however it looks like a fair bit of plumbing work to get that information through the NodeInfo API (or modify the deprecation check framework to support getting information another way). Am I wrong there?

This is an interesting one, as there doesn't look like a clean way to expose this easily.

@DaveCTurner DaveCTurner added the :Distributed/Discovery-Plugins Anything related to our integration plugins with EC2, GCP and Azure label Dec 5, 2018
@elasticmachine
Copy link
Copy Markdown
Collaborator

Pinging @elastic/es-distributed

@DaveCTurner
Copy link
Copy Markdown
Member

I see. Yes, you're right, apart from logging a deprecation warning we don't record that the discovery file isn't in the new place.

I think if the file is not in the right place on 7.0+ nodes during an upgrade then the upgraded nodes will struggle to join the cluster, which seems ok. It depends a bit on whether they're using file-based discovery exclusively or some combination of other discovery mechanisms too, but if it's exclusively file-based discovery then no file means no discovery. Am I right that the migration assistant is intended to be a best-effort thing? If so then this is going to be ok.

@AthenaEryma
Copy link
Copy Markdown
Contributor Author

@DaveCTurner I believe that yes, the migration assistant/deprecation info API is intended to be best-effort. Users will still need to check the deprecation logs for any request/response format changes they'll need to make anyway.

I've opened #36343 to track the larger issue, but it's unlikely to be addressed before the release of 7.0, so I think it might be best to move forward with this as-is.

Copy link
Copy Markdown
Member

@DaveCTurner DaveCTurner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, this LGTM then, thanks for the explanations @gwbrown.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Distributed/Discovery-Plugins Anything related to our integration plugins with EC2, GCP and Azure >enhancement v6.6.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants