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

Issue with EC2 Discovery Plugin - colon (:) doesn't work in discovery.ec2.tag setting #38406

Closed
DenisMartyanov opened this issue Feb 5, 2019 · 5 comments · Fixed by #41630
Closed
Assignees
Labels
:Core/Infra/Settings Settings infrastructure and APIs

Comments

@DenisMartyanov
Copy link

DenisMartyanov commented Feb 5, 2019

Elasticsearch version (bin/elasticsearch --version): 6.5.4

Plugins installed: [discovery-ec2]

JVM version (java -version): 1.8.0_191

OS version (uname -a if on a Unix-like system): Linux

Description of the problem including expected versus actual behavior:
Based on documentation, I see that we can make use of discovery.ec2.tag. in the EC2 discovery plugin as a Prefix to filter down the EC2 instances to be included in the discovery process. I use OpsWorks and filter by opsworks:stack tag:

discovery.ec2.tag.opsworks:stack: StackName

But while starting up Elastic Search instance after including the above tag in elasticsearch.yml file, it complains

StartupException: java.lang.IllegalArgumentException: unknown setting [discovery.ec2.tag.opsworks:stack] please check that any required plugins are installed, or check the breaking changes documentation for removed settings. 

I tried to enclose in quotes (both single ' and double "), but nothing changes. If I try to use any tag without colon (:), everything works correctly.
It worked without issues in 5.6.4, but doesn't work in both 6.6.0 and 6.5.4

Steps to reproduce:

  1. Install ec2-discovery plugin
  2. Add discovery.ec2.tag.opsworks:stack: StackName line in elasticsearch.yml file
  3. Restart the Elastic Search instance. As soon as you start, you will see the following error pasted in logs section

Provide logs (if relevant):

[o.e.b.ElasticsearchUncaughtExceptionHandler] [IntappOpenNode1] uncaught exception in thread [main]
org.elasticsearch.bootstrap.StartupException: java.lang.IllegalArgumentException: unknown setting [discovery.ec2.tag.opsworks:stack] please check that any required plugins are installed, or check the breaking changes documentation for removed settings
        at org.elasticsearch.bootstrap.Elasticsearch.init(Elasticsearch.java:140) ~[elasticsearch-6.5.4.jar:6.5.4]
        at org.elasticsearch.bootstrap.Elasticsearch.execute(Elasticsearch.java:127) ~[elasticsearch-6.5.4.jar:6.5.4]
        at org.elasticsearch.cli.EnvironmentAwareCommand.execute(EnvironmentAwareCommand.java:86) ~[elasticsearch-6.5.4.jar:6.5.4]
        at org.elasticsearch.cli.Command.mainWithoutErrorHandling(Command.java:124) ~[elasticsearch-cli-6.5.4.jar:6.5.4]
        at org.elasticsearch.cli.Command.main(Command.java:90) ~[elasticsearch-cli-6.5.4.jar:6.5.4]
        at org.elasticsearch.bootstrap.Elasticsearch.main(Elasticsearch.java:93) ~[elasticsearch-6.5.4.jar:6.5.4]
        at org.elasticsearch.bootstrap.Elasticsearch.main(Elasticsearch.java:86) ~[elasticsearch-6.5.4.jar:6.5.4]
Caused by: java.lang.IllegalArgumentException: unknown setting [discovery.ec2.tag.opsworks:stack] please check that any required plugins are installed, or check the breaking changes documentation for removed settings
        at org.elasticsearch.common.settings.AbstractScopedSettings.validate(AbstractScopedSettings.java:476) ~[elasticsearch-6.5.4.jar:6.5.4]
        at org.elasticsearch.common.settings.AbstractScopedSettings.validate(AbstractScopedSettings.java:421) ~[elasticsearch-6.5.4.jar:6.5.4]
        at org.elasticsearch.common.settings.AbstractScopedSettings.validate(AbstractScopedSettings.java:392) ~[elasticsearch-6.5.4.jar:6.5.4]
        at org.elasticsearch.common.settings.AbstractScopedSettings.validate(AbstractScopedSettings.java:363) ~[elasticsearch-6.5.4.jar:6.5.4]
        at org.elasticsearch.common.settings.SettingsModule.<init>(SettingsModule.java:148) ~[elasticsearch-6.5.4.jar:6.5.4]
        at org.elasticsearch.node.Node.<init>(Node.java:373) ~[elasticsearch-6.5.4.jar:6.5.4]
        at org.elasticsearch.node.Node.<init>(Node.java:265) ~[elasticsearch-6.5.4.jar:6.5.4]
        at org.elasticsearch.bootstrap.Bootstrap$5.<init>(Bootstrap.java:212) ~[elasticsearch-6.5.4.jar:6.5.4]
        at org.elasticsearch.bootstrap.Bootstrap.setup(Bootstrap.java:212) ~[elasticsearch-6.5.4.jar:6.5.4]
        at org.elasticsearch.bootstrap.Bootstrap.init(Bootstrap.java:333) ~[elasticsearch-6.5.4.jar:6.5.4]
        at org.elasticsearch.bootstrap.Elasticsearch.init(Elasticsearch.java:136) ~[elasticsearch-6.5.4.jar:6.5.4]
        ... 6 more
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-distributed

@colings86 colings86 added the :Distributed Coordination/Discovery-Plugins Anything related to our integration plugins with EC2, GCP and Azure label Feb 5, 2019
@DaveCTurner
Copy link
Contributor

DaveCTurner commented Feb 5, 2019

This is a general problem with this kind of AffixSetting which only allows characters matching [-\w] in the key, and that doesn't include colons.

pattern = Pattern.compile("(" + Pattern.quote(prefix) + "((?:[-\\w]+[.])*[-\\w]+$))");

For instance, this setting also doesn't work:

node.attr.has:colon: foo

Fixing this doesn't look too hard, but given that colons are meaningful in many places perhaps it would be better to keep this as-is. Marking this for further discussion.

Edit: if we do decide to keep it as-is then we should document this restriction.

@DaveCTurner DaveCTurner added the :Core/Infra/Settings Settings infrastructure and APIs label Feb 5, 2019
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra

@DaveCTurner DaveCTurner added team-discuss and removed :Distributed Coordination/Discovery-Plugins Anything related to our integration plugins with EC2, GCP and Azure labels Feb 5, 2019
@DaveCTurner
Copy link
Contributor

We discussed this as a team and decided that supporting colons in setting names would be trappy so we'd rather not make this change. We'll document this restriction.

We discussed ideas for supporting colons in the tags used in EC2 discovery without adding too much extra overhead for today's colon-free tags. One possibility might be to tie the tag name and value together with an "internal identifier" like this:

discovery.ec2.tag.foobar: baz 
discovery.ec2.tag_name.foobar: quux:gruly

Here we are matching a tag quux:gruly with value baz using the "internal identifier" foobar. The discovery.ec2.tag_name.* setting would be optional and we would fall back to today's behaviour if it were missing. The consensus was that this would add too much administrative overhead, and we think it's simpler to define a custom tag that is compatible with Elasticsearch's naming rules. We additionally noted that you should only tag the master-eligible nodes in the cluster for discovery purposes, whereas a tag like opsworks:stack looks like the sort of thing that would be applied to every node in the cluster.

@Backgammon-
Copy link

Backgammon- commented Apr 27, 2019

While I understand your reasoning regarding the unwieldiness of the possible solutions, I hope you'll also consider that many useful AWS-generated tags (e.g. https://docs.aws.amazon.com/autoscaling/ec2/userguide/autoscaling-tagging.html#tag-lifecycle) use colons as separators, and that it is also common practice to use a similar format for user-defined tags. In my case, it is not a big deal to add a user-defined tag outside the usual format, but I was hoping to be able to build the configuration using AWS-generated ASG group tags in the template to identify the cluster.

DaveCTurner added a commit to DaveCTurner/elasticsearch that referenced this issue Apr 29, 2019
This commit reworks and clarifies the docs for the `discovery-ec2` plugin:

- folds the tiny "Getting started with AWS" into the page on configuration
- spells out the name of each setting in full instead of noting the
  `discovery.ec2` prefix at the top of the page.
- replaces each `(Secure)` marker with a sentence describing what that means in
  situ
- notes some missing defaults
- clarifies the behaviour of `discovery.ec2.groups` (dependent on `.any_group`)
- clarifies what `discovery.ec2.host_type` is for
- adds `discovery.ec2.tag.TAGNAME` as a (meta-)setting rather than describing
  it in a separate section
- notes that the tags mentioned in `discovery.ec2.tag.TAGNAME` cannot contain
  colons (see elastic#38406)
- clarifies the EC2-specific interface names and what they're for
- reorders and rewords the recommendations for storage
- expands on why you should not span a cluster across regions
- adds a suggestion on protecting instances against termination during scale-in
- reformat to 80 columns where possible

Fixes elastic#38406
DaveCTurner added a commit that referenced this issue May 22, 2019
This commit reworks and clarifies the docs for the `discovery-ec2` plugin:

- folds the tiny "Getting started with AWS" into the page on configuration
- spells out the name of each setting in full instead of noting the
  `discovery.ec2` prefix at the top of the page.
- replaces each `(Secure)` marker with a sentence describing what that means in
  situ
- notes some missing defaults
- clarifies the behaviour of `discovery.ec2.groups` (dependent on `.any_group`)
- clarifies what `discovery.ec2.host_type` is for
- adds `discovery.ec2.tag.TAGNAME` as a (meta-)setting rather than describing
  it in a separate section
- notes that the tags mentioned in `discovery.ec2.tag.TAGNAME` cannot contain
  colons (see #38406)
- clarifies the EC2-specific interface names and what they're for
- reorders and rewords the recommendations for storage
- expands on why you should not span a cluster across regions
- adds a suggestion on protecting instances against termination during scale-in
- reformat to 80 columns where possible

Fixes #38406
DaveCTurner added a commit that referenced this issue May 22, 2019
This commit reworks and clarifies the docs for the `discovery-ec2` plugin:

- folds the tiny "Getting started with AWS" into the page on configuration
- spells out the name of each setting in full instead of noting the
  `discovery.ec2` prefix at the top of the page.
- replaces each `(Secure)` marker with a sentence describing what that means in
  situ
- notes some missing defaults
- clarifies the behaviour of `discovery.ec2.groups` (dependent on `.any_group`)
- clarifies what `discovery.ec2.host_type` is for
- adds `discovery.ec2.tag.TAGNAME` as a (meta-)setting rather than describing
  it in a separate section
- notes that the tags mentioned in `discovery.ec2.tag.TAGNAME` cannot contain
  colons (see #38406)
- clarifies the EC2-specific interface names and what they're for
- reorders and rewords the recommendations for storage
- expands on why you should not span a cluster across regions
- adds a suggestion on protecting instances against termination during scale-in
- reformat to 80 columns where possible

Fixes #38406
DaveCTurner added a commit that referenced this issue May 22, 2019
This commit reworks and clarifies the docs for the `discovery-ec2` plugin:

- folds the tiny "Getting started with AWS" into the page on configuration
- spells out the name of each setting in full instead of noting the
  `discovery.ec2` prefix at the top of the page.
- replaces each `(Secure)` marker with a sentence describing what that means in
  situ
- notes some missing defaults
- clarifies the behaviour of `discovery.ec2.groups` (dependent on `.any_group`)
- clarifies what `discovery.ec2.host_type` is for
- adds `discovery.ec2.tag.TAGNAME` as a (meta-)setting rather than describing
  it in a separate section
- notes that the tags mentioned in `discovery.ec2.tag.TAGNAME` cannot contain
  colons (see #38406)
- clarifies the EC2-specific interface names and what they're for
- reorders and rewords the recommendations for storage
- expands on why you should not span a cluster across regions
- adds a suggestion on protecting instances against termination during scale-in
- reformat to 80 columns where possible

Fixes #38406
DaveCTurner added a commit that referenced this issue May 22, 2019
This commit reworks and clarifies the docs for the `discovery-ec2` plugin:

- folds the tiny "Getting started with AWS" into the page on configuration
- spells out the name of each setting in full instead of noting the
  `discovery.ec2` prefix at the top of the page.
- replaces each `(Secure)` marker with a sentence describing what that means in
  situ
- notes some missing defaults
- clarifies the behaviour of `discovery.ec2.groups` (dependent on `.any_group`)
- clarifies what `discovery.ec2.host_type` is for
- adds `discovery.ec2.tag.TAGNAME` as a (meta-)setting rather than describing
  it in a separate section
- notes that the tags mentioned in `discovery.ec2.tag.TAGNAME` cannot contain
  colons (see #38406)
- clarifies the EC2-specific interface names and what they're for
- reorders and rewords the recommendations for storage
- expands on why you should not span a cluster across regions
- adds a suggestion on protecting instances against termination during scale-in
- reformat to 80 columns where possible

Fixes #38406
gurkankaymak pushed a commit to gurkankaymak/elasticsearch that referenced this issue May 27, 2019
This commit reworks and clarifies the docs for the `discovery-ec2` plugin:

- folds the tiny "Getting started with AWS" into the page on configuration
- spells out the name of each setting in full instead of noting the
  `discovery.ec2` prefix at the top of the page.
- replaces each `(Secure)` marker with a sentence describing what that means in
  situ
- notes some missing defaults
- clarifies the behaviour of `discovery.ec2.groups` (dependent on `.any_group`)
- clarifies what `discovery.ec2.host_type` is for
- adds `discovery.ec2.tag.TAGNAME` as a (meta-)setting rather than describing
  it in a separate section
- notes that the tags mentioned in `discovery.ec2.tag.TAGNAME` cannot contain
  colons (see elastic#38406)
- clarifies the EC2-specific interface names and what they're for
- reorders and rewords the recommendations for storage
- expands on why you should not span a cluster across regions
- adds a suggestion on protecting instances against termination during scale-in
- reformat to 80 columns where possible

Fixes elastic#38406
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Core/Infra/Settings Settings infrastructure and APIs
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants