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

Make watcher settings reloadable #31746

Merged
merged 17 commits into from
Jul 13, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
import org.elasticsearch.node.Node;
import org.elasticsearch.plugins.ActionPlugin;
import org.elasticsearch.plugins.Plugin;
import org.elasticsearch.plugins.ReloadablePlugin;
import org.elasticsearch.plugins.ScriptPlugin;
import org.elasticsearch.rest.RestController;
import org.elasticsearch.rest.RestHandler;
Expand Down Expand Up @@ -123,6 +124,7 @@
import org.elasticsearch.xpack.watcher.input.simple.SimpleInputFactory;
import org.elasticsearch.xpack.watcher.input.transform.TransformInput;
import org.elasticsearch.xpack.watcher.input.transform.TransformInputFactory;
import org.elasticsearch.xpack.watcher.notification.NotificationService;
import org.elasticsearch.xpack.watcher.notification.email.Account;
import org.elasticsearch.xpack.watcher.notification.email.EmailService;
import org.elasticsearch.xpack.watcher.notification.email.HtmlSanitizer;
Expand Down Expand Up @@ -194,7 +196,7 @@

import static java.util.Collections.emptyList;

public class Watcher extends Plugin implements ActionPlugin, ScriptPlugin {
public class Watcher extends Plugin implements ActionPlugin, ScriptPlugin, ReloadablePlugin {

// This setting is only here for backward compatibility reasons as 6.x indices made use of it. It can be removed in 8.x.
@Deprecated
Expand All @@ -221,6 +223,7 @@ public class Watcher extends Plugin implements ActionPlugin, ScriptPlugin {
protected final boolean transportClient;
protected final boolean enabled;
protected final Environment env;
protected List<NotificationService> reloadableServices = new ArrayList<>();

public Watcher(final Settings settings) {
this.settings = settings;
Expand Down Expand Up @@ -275,6 +278,12 @@ public Collection<Object> createComponents(Client client, ClusterService cluster
SlackService slackService = new SlackService(settings, httpClient, clusterService.getClusterSettings());
PagerDutyService pagerDutyService = new PagerDutyService(settings, httpClient, clusterService.getClusterSettings());

reloadableServices.add(emailService);
reloadableServices.add(hipChatService);
reloadableServices.add(jiraService);
reloadableServices.add(slackService);
reloadableServices.add(pagerDutyService);

TextTemplateEngine templateEngine = new TextTemplateEngine(settings, scriptService);
Map<String, EmailAttachmentParser> emailAttachmentParsers = new HashMap<>();
emailAttachmentParsers.put(HttpEmailAttachementParser.TYPE, new HttpEmailAttachementParser(httpClient, httpTemplateParser,
Expand Down Expand Up @@ -613,4 +622,15 @@ public List<ScriptContext> getContexts() {
public void close() throws IOException {
IOUtils.closeWhileHandlingException(httpClient);
}

/**
* Reloads all the reloadable services in watcher.
*/
@Override
public void reload(Settings settings) {
if (enabled == false || transportClient) {
return;
}
reloadableServices.forEach(s -> s.reload(settings));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ public abstract class NotificationService<Account> extends AbstractComponent {
public NotificationService(Settings settings, String type,
ClusterSettings clusterSettings, List<Setting<?>> pluginSettings) {
this(settings, type);
clusterSettings.addSettingsUpdateConsumer(this::setAccountSetting, pluginSettings);
clusterSettings.addSettingsUpdateConsumer(this::reload, pluginSettings);
}

// Used for testing only
Expand All @@ -40,7 +40,7 @@ public NotificationService(Settings settings, String type,
this.type = type;
}

protected synchronized void setAccountSetting(Settings settings) {
public synchronized void reload(Settings settings) {
Tuple<Map<String, Account>, Account> accounts = buildAccounts(settings, this::createAccount);
this.accounts = Collections.unmodifiableMap(accounts.v1());
this.defaultAccount = accounts.v2();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ public EmailService(Settings settings, @Nullable CryptoService cryptoService, Cl
clusterSettings.addAffixUpdateConsumer(SETTING_SMTP_SEND_PARTIAL, (s, o) -> {}, (s, o) -> {});
clusterSettings.addAffixUpdateConsumer(SETTING_SMTP_WAIT_ON_QUIT, (s, o) -> {}, (s, o) -> {});
// do an initial load
setAccountSetting(settings);
reload(settings);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,13 +79,13 @@ public HipChatService(Settings settings, HttpClient httpClient, ClusterSettings
clusterSettings.addAffixUpdateConsumer(SETTING_PORT, (s, o) -> {}, (s, o) -> {});
clusterSettings.addAffixUpdateConsumer(SETTING_MESSAGE_DEFAULTS, (s, o) -> {}, (s, o) -> {});

setAccountSetting(settings);
reload(settings);
}

@Override
protected synchronized void setAccountSetting(Settings settings) {
public synchronized void reload(Settings settings) {
defaultServer = new HipChatServer(settings.getByPrefix("xpack.notification.hipchat."));
super.setAccountSetting(settings);
super.reload(settings);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ public JiraService(Settings settings, HttpClient httpClient, ClusterSettings clu
clusterSettings.addAffixUpdateConsumer(SETTING_SECURE_PASSWORD, (s, o) -> {}, (s, o) -> {});
clusterSettings.addAffixUpdateConsumer(SETTING_DEFAULTS, (s, o) -> {}, (s, o) -> {});
// do an initial load
setAccountSetting(settings);
reload(settings);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ public PagerDutyService(Settings settings, HttpClient httpClient, ClusterSetting
clusterSettings.addAffixUpdateConsumer(SETTING_SERVICE_API_KEY, (s, o) -> {}, (s, o) -> {});
clusterSettings.addAffixUpdateConsumer(SETTING_SECURE_SERVICE_API_KEY, (s, o) -> {}, (s, o) -> {});
clusterSettings.addAffixUpdateConsumer(SETTING_DEFAULTS, (s, o) -> {}, (s, o) -> {});
setAccountSetting(settings);
reload(settings);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ public SlackService(Settings settings, HttpClient httpClient, ClusterSettings cl
clusterSettings.addAffixUpdateConsumer(SETTING_URL, (s, o) -> {}, (s, o) -> {});
clusterSettings.addAffixUpdateConsumer(SETTING_URL_SECURE, (s, o) -> {}, (s, o) -> {});
clusterSettings.addAffixUpdateConsumer(SETTING_DEFAULTS, (s, o) -> {}, (s, o) -> {});
setAccountSetting(settings);
reload(settings);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,18 @@
import org.elasticsearch.test.IndexSettingsModule;
import org.elasticsearch.threadpool.ExecutorBuilder;
import org.elasticsearch.xpack.core.watcher.watch.Watch;
import org.elasticsearch.xpack.watcher.notification.NotificationService;

import java.util.List;

import static java.util.Collections.emptyMap;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.hasSize;
import static org.hamcrest.Matchers.is;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.verifyNoMoreInteractions;

public class WatcherPluginTests extends ESTestCase {

Expand Down Expand Up @@ -97,4 +102,36 @@ public void testThreadPoolSize() {
.build();
assertThat(Watcher.getWatcherThreadPoolSize(noDataNodeSettings), is(1));
}

public void testReload() {
Settings settings = Settings.builder()
.put("xpack.watcher.enabled", true)
.put("path.home", createTempDir())
.build();
NotificationService mockService = mock(NotificationService.class);
Watcher watcher = new TestWatcher(settings, mockService);

watcher.reload(settings);
verify(mockService, times(1)).reload(settings);
}

public void testReloadDisabled() {
Settings settings = Settings.builder()
.put("xpack.watcher.enabled", false)
.put("path.home", createTempDir())
.build();
NotificationService mockService = mock(NotificationService.class);
Watcher watcher = new TestWatcher(settings, mockService);

watcher.reload(settings);
verifyNoMoreInteractions(mockService);
}

private class TestWatcher extends Watcher {

TestWatcher(Settings settings, NotificationService service) {
super(settings);
reloadableServices.add(service);
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ private static class TestNotificationService extends NotificationService<String>

TestNotificationService(Settings settings) {
super(settings, "test");
setAccountSetting(settings);
reload(settings);
}

@Override
Expand Down