-
Notifications
You must be signed in to change notification settings - Fork 25k
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
Add Azure ARM discovery plugin #22679
Conversation
* Azure ARM secret | ||
*/ | ||
public static final Setting<String> SECRET_SETTING = | ||
Setting.simpleString("cloud.azure-arm.secret", Property.NodeScope, Property.Filtered); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we should be adding new secret settings that are plain strings.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed.
You can filter virtual machines you would like to connect to by entering a name here. It can be a wildcard | ||
like `azure-esnode-*`. | ||
|
||
`discovery.azure-arm.host.group_name`:: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we rename this to discovery.azure-arm.resource_group
, group
by itself is a tad ambiguous and resource group
is a well understood azure concept
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure! Will do!
@Mpdreamz I updated the repo with new changes:
|
Now getting security manager exceptions @dadoonet
|
We need the entire stack trace here. |
@Mpdreamz Interesting. Can you start elasticsearch with this option ES_JAVA_OPTS='-Djava.security.debug="access,failure"' ./bin/elasticsearch |
My bad @jasontedor: click to expand
|
I opened Azure/autorest-clientruntime-for-java#136 on azure side. Let see what Azure team can do to help us in that context. If they can't do anything, I believe that we would have to shade somehow a version of Azure SDK and Jackson within a single flat jar where we can relocate then Jackson classes... I'm not a fan of this TBH. |
That looks like an issue in jackson itself. The |
Thanks @rjernst. I opened FasterXML/jackson-core#347 |
Will this support scalesets? |
@dadoonet, just wondering if this is still getting merged? |
Supported settings so far: ```yml cloud: azure-arm: client_id: FILL_WITH_YOUR_CLIENT_ID secret: FILL_WITH_YOUR_SECRET tenant_id: FILL_WITH_YOUR_TENANT subscription_id: FILL_WITH_YOUR_SUBSCRIPTION_ID discovery: zen.hosts_provider: azure-arm azure-arm: host: type: private_ip name: azure-esnode-master-* group_name: azure-preprod region: westeurope refresh_interval: 10s ``` Closes elastic#19146
And cleanup a bit gradle build
Also adds more Javadoc on settings. Also fixed a bug when using wildcards for group names. Azure API does not support wildcards so we need to get all the VMs in that case and filter on our side.
Applying here advices I got at Azure/azure-sdk-for-java#1387 (comment)
@elasticmachine retest this please |
I don't know why Anyway, someone from the @elastic/es-distributed team would like to review? |
Merging in latest master should fix this (see #32430) |
@dadoonet I'd like to test this with the Azure ARM template; is it possible to build a version for 6.3.1? |
@ywelsch I was sure I did. And actually I did not. 😄 I think I have a trickier thing to solve now.
But with JDK8, this is failing with:
(Source: https://elasticsearch-ci.elastic.co/job/elastic+elasticsearch+pull-request/14717/console) Do you know how I can make this check optional depending on the JVM version? Unless that's a bad idea and that should be solved in another way? |
yes, it should be made optional depending on JVM version, see the build file for |
Thanks @ywelsch that worked! Should I rebase and squash before anyone review it? |
wondering when discovery-azure plugin should be available? |
# Conflicts: # docs/reference/cat/plugins.asciidoc # docs/reference/modules/discovery/azure.asciidoc
# Conflicts: # docs/reference/cat/plugins.asciidoc
We discussed this PR during the distributed sync and, while generally useful, think that we currently cannot take this one on, in particular give it the proper testing, support and maintenance that we provide for the other plugins in the ES repo. This might be a better fit as a community plugin for now. |
Can we then remove this part of the documentation? https://www.elastic.co/guide/en/elasticsearch/plugins/current/discovery-azure-classic.html Or fix its content saying that this plugin will be removed in the future? |
sure, thank you for pointing this out. |
Supported settings so far:
Closes #19146