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

[7.x] Add action to decommission legacy monitoring cluster alerts (#64373) #66309

Merged
merged 2 commits into from
Dec 15, 2020

Conversation

jbaiera
Copy link
Member

@jbaiera jbaiera commented Dec 14, 2020

Backports the following commits to 7.x:

…64373)

Adds an action that will proactively remove any watches that monitoring has configured. The action
toggles on a new setting that informs the cluster to tear down any previously created cluster alerts,
and after that is accepted, the action immediately attempts a best-effort refresh of cluster alert
resources in order to force their removal in case collection is disabled or delayed.

Since resources are controlled lazily by the existing monitoring exporters, extra care was taken to
ensure that any in-flight resource management operations do not race against any resource actions
taken by the migration action. Resource installation code was updated with callbacks to report any
errors instead of just logging them.
# Conflicts:
#	x-pack/plugin/monitoring/src/internalClusterTest/java/org/elasticsearch/xpack/monitoring/exporter/http/HttpExporterIT.java
#	x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/Monitoring.java
#	x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/exporter/local/LocalExporter.java
Comment on lines 68 to 80
@Override
protected Collection<Class<? extends Plugin>> transportClientPlugins() {
return Arrays.asList(LocalStateMonitoring.class, MockClusterAlertScriptEngine.TestPlugin.class,
MockIngestPlugin.class, CommonAnalysisPlugin.class);
}

@Override
protected Settings transportClientSettings() {
return Settings.builder().put(super.transportClientSettings())
.put(XPackSettings.SECURITY_ENABLED.getKey(), false)
.put(XPackSettings.WATCHER_ENABLED.getKey(), false)
.build();
}
Copy link
Member Author

Choose a reason for hiding this comment

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

These were added:

  1. To disable security in the tests. The tests were failing because security expects the Transport client type to be a secure one, but the tests install the mock-nio client.
  2. To mirror the test plugins here in transportClientPlugins so that the client in the test has the watcher and monitoring actions available on it.

Copy link
Contributor

Choose a reason for hiding this comment

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

are MockIngestPlugin.class, CommonAnalysisPlugin.class needed ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Nope! I took them off, along with MockClusterAlertScriptEngine.TestPlugin.class

Comment on lines +57 to +63

@Override
public Collection<Module> createGuiceModules() {
return XPackPlugin.transportClientMode(settings) ?
Collections.emptyList() :
super.createGuiceModules();
}
Copy link
Member Author

Choose a reason for hiding this comment

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

Added in order to ignore watcher's feature set object which expects a client to be injected. When creating a transport client, these feature set objects are created at some point and there are no clients available to the injector to give their constructors. This results in a Guice exception.

@jbaiera jbaiera requested a review from jakelandis December 15, 2020 00:06
Copy link
Contributor

@jakelandis jakelandis left a comment

Choose a reason for hiding this comment

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

LGTM (minor question if a test plugins are needed)

@jbaiera jbaiera merged commit e42d267 into elastic:7.x Dec 15, 2020
@jbaiera jbaiera deleted the backport/7.x/pr-64373 branch December 15, 2020 18:45
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.

2 participants