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

Deprecation check for File Discovery plugin #36190

Merged
merged 1 commit into from Dec 7, 2018

Conversation

gwbrown
Copy link
Contributor

@gwbrown gwbrown commented Dec 4, 2018

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
Collaborator

Pinging @elastic/es-core-features

Copy link
Contributor

@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.

@gwbrown
Copy link
Contributor Author

gwbrown commented Dec 4, 2018

@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
Collaborator

Pinging @elastic/es-distributed

@DaveCTurner
Copy link
Contributor

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.

@gwbrown
Copy link
Contributor Author

gwbrown commented Dec 6, 2018

@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
Contributor

@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.

None yet

3 participants