From 21f882a96207f151645b271d86f3890e2ae06163 Mon Sep 17 00:00:00 2001 From: mgodwan Date: Fri, 5 Jul 2024 17:16:03 +0530 Subject: [PATCH 01/10] SPI for loading ABC templates Signed-off-by: mgodwan --- .../service/ClusterManagerService.java | 8 +- .../cluster/service/ClusterService.java | 9 +- .../cluster/service/MasterService.java | 10 +- .../ClusterStateComponentTemplateLoader.java | 65 +++++++++++ .../applicationtemplates/SystemTemplate.java | 41 +++++++ .../SystemTemplateInfo.java | 53 +++++++++ .../SystemTemplatesPlugin.java | 28 +++++ .../SystemTemplatesService.java | 104 ++++++++++++++++++ .../applicationtemplates/TemplateLoader.java | 24 ++++ .../TemplateRepository.java | 35 ++++++ .../TemplateRepositoryInfo.java | 31 ++++++ .../main/java/org/opensearch/node/Node.java | 4 +- .../TestSystemTemplatesRepositoryPlugin.java | 62 +++++++++++ .../test/OpenSearchIntegTestCase.java | 2 + 14 files changed, 468 insertions(+), 8 deletions(-) create mode 100644 server/src/main/java/org/opensearch/cluster/service/applicationtemplates/ClusterStateComponentTemplateLoader.java create mode 100644 server/src/main/java/org/opensearch/cluster/service/applicationtemplates/SystemTemplate.java create mode 100644 server/src/main/java/org/opensearch/cluster/service/applicationtemplates/SystemTemplateInfo.java create mode 100644 server/src/main/java/org/opensearch/cluster/service/applicationtemplates/SystemTemplatesPlugin.java create mode 100644 server/src/main/java/org/opensearch/cluster/service/applicationtemplates/SystemTemplatesService.java create mode 100644 server/src/main/java/org/opensearch/cluster/service/applicationtemplates/TemplateLoader.java create mode 100644 server/src/main/java/org/opensearch/cluster/service/applicationtemplates/TemplateRepository.java create mode 100644 server/src/main/java/org/opensearch/cluster/service/applicationtemplates/TemplateRepositoryInfo.java create mode 100644 test/framework/src/main/java/org/opensearch/cluster/service/applicationtemplates/TestSystemTemplatesRepositoryPlugin.java diff --git a/server/src/main/java/org/opensearch/cluster/service/ClusterManagerService.java b/server/src/main/java/org/opensearch/cluster/service/ClusterManagerService.java index fa8c965b4d538..4f3f5ecb86d1f 100644 --- a/server/src/main/java/org/opensearch/cluster/service/ClusterManagerService.java +++ b/server/src/main/java/org/opensearch/cluster/service/ClusterManagerService.java @@ -9,11 +9,14 @@ package org.opensearch.cluster.service; import org.opensearch.cluster.ClusterManagerMetrics; +import org.opensearch.cluster.service.applicationtemplates.SystemTemplatesPlugin; import org.opensearch.common.annotation.PublicApi; import org.opensearch.common.settings.ClusterSettings; import org.opensearch.common.settings.Settings; import org.opensearch.threadpool.ThreadPool; +import java.util.List; + /** * Main Cluster Manager Node Service * @@ -30,8 +33,9 @@ public ClusterManagerService( Settings settings, ClusterSettings clusterSettings, ThreadPool threadPool, - ClusterManagerMetrics clusterManagerMetrics + ClusterManagerMetrics clusterManagerMetrics, + List systemTemplatesPlugins ) { - super(settings, clusterSettings, threadPool, clusterManagerMetrics); + super(settings, clusterSettings, threadPool, clusterManagerMetrics, systemTemplatesPlugins); } } diff --git a/server/src/main/java/org/opensearch/cluster/service/ClusterService.java b/server/src/main/java/org/opensearch/cluster/service/ClusterService.java index c3c48dd8b87ef..d3282bb158774 100644 --- a/server/src/main/java/org/opensearch/cluster/service/ClusterService.java +++ b/server/src/main/java/org/opensearch/cluster/service/ClusterService.java @@ -46,6 +46,7 @@ import org.opensearch.cluster.node.DiscoveryNode; import org.opensearch.cluster.routing.OperationRouting; import org.opensearch.cluster.routing.RerouteService; +import org.opensearch.cluster.service.applicationtemplates.SystemTemplatesPlugin; import org.opensearch.common.annotation.PublicApi; import org.opensearch.common.lifecycle.AbstractLifecycleComponent; import org.opensearch.common.settings.ClusterSettings; @@ -58,6 +59,7 @@ import org.opensearch.threadpool.ThreadPool; import java.util.Collections; +import java.util.List; import java.util.Map; /** @@ -94,19 +96,20 @@ public class ClusterService extends AbstractLifecycleComponent { private IndexingPressureService indexingPressureService; public ClusterService(Settings settings, ClusterSettings clusterSettings, ThreadPool threadPool) { - this(settings, clusterSettings, threadPool, new ClusterManagerMetrics(NoopMetricsRegistry.INSTANCE)); + this(settings, clusterSettings, threadPool, new ClusterManagerMetrics(NoopMetricsRegistry.INSTANCE), null); } public ClusterService( Settings settings, ClusterSettings clusterSettings, ThreadPool threadPool, - ClusterManagerMetrics clusterManagerMetrics + ClusterManagerMetrics clusterManagerMetrics, + List systemTemplatesPlugins ) { this( settings, clusterSettings, - new ClusterManagerService(settings, clusterSettings, threadPool, clusterManagerMetrics), + new ClusterManagerService(settings, clusterSettings, threadPool, clusterManagerMetrics, systemTemplatesPlugins), new ClusterApplierService(Node.NODE_NAME_SETTING.get(settings), settings, clusterSettings, threadPool, clusterManagerMetrics) ); } diff --git a/server/src/main/java/org/opensearch/cluster/service/MasterService.java b/server/src/main/java/org/opensearch/cluster/service/MasterService.java index 686e9793a8fd3..de420b420dbad 100644 --- a/server/src/main/java/org/opensearch/cluster/service/MasterService.java +++ b/server/src/main/java/org/opensearch/cluster/service/MasterService.java @@ -53,6 +53,8 @@ import org.opensearch.cluster.node.DiscoveryNode; import org.opensearch.cluster.node.DiscoveryNodes; import org.opensearch.cluster.routing.RoutingTable; +import org.opensearch.cluster.service.applicationtemplates.SystemTemplatesPlugin; +import org.opensearch.cluster.service.applicationtemplates.SystemTemplatesService; import org.opensearch.common.Nullable; import org.opensearch.common.Priority; import org.opensearch.common.annotation.DeprecatedApi; @@ -140,16 +142,18 @@ public class MasterService extends AbstractLifecycleComponent { private final ClusterManagerThrottlingStats throttlingStats; private final ClusterStateStats stateStats; private final ClusterManagerMetrics clusterManagerMetrics; + private final SystemTemplatesService systemTemplatesService; public MasterService(Settings settings, ClusterSettings clusterSettings, ThreadPool threadPool) { - this(settings, clusterSettings, threadPool, new ClusterManagerMetrics(NoopMetricsRegistry.INSTANCE)); + this(settings, clusterSettings, threadPool, new ClusterManagerMetrics(NoopMetricsRegistry.INSTANCE), null); } public MasterService( Settings settings, ClusterSettings clusterSettings, ThreadPool threadPool, - ClusterManagerMetrics clusterManagerMetrics + ClusterManagerMetrics clusterManagerMetrics, + List systemTemplatesPlugins ) { this.nodeName = Objects.requireNonNull(Node.NODE_NAME_SETTING.get(settings)); @@ -169,6 +173,7 @@ public MasterService( this.stateStats = new ClusterStateStats(); this.threadPool = threadPool; this.clusterManagerMetrics = clusterManagerMetrics; + this.systemTemplatesService = new SystemTemplatesService(systemTemplatesPlugins, clusterSettings, settings); } private void setSlowTaskLoggingThreshold(TimeValue slowTaskLoggingThreshold) { @@ -395,6 +400,7 @@ void onPublicationSuccess(ClusterChangedEvent clusterChangedEvent, TaskOutputs t try { taskOutputs.clusterStatePublished(clusterChangedEvent); + threadPool.executor(ThreadPool.Names.GENERIC).submit(() -> systemTemplatesService.refreshTemplates()); } catch (Exception e) { logger.error( () -> new ParameterizedMessage( diff --git a/server/src/main/java/org/opensearch/cluster/service/applicationtemplates/ClusterStateComponentTemplateLoader.java b/server/src/main/java/org/opensearch/cluster/service/applicationtemplates/ClusterStateComponentTemplateLoader.java new file mode 100644 index 0000000000000..75b5ba8ee09f8 --- /dev/null +++ b/server/src/main/java/org/opensearch/cluster/service/applicationtemplates/ClusterStateComponentTemplateLoader.java @@ -0,0 +1,65 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.cluster.service.applicationtemplates; + +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; +import org.opensearch.action.admin.indices.template.put.PutComponentTemplateAction; +import org.opensearch.client.Client; +import org.opensearch.client.OriginSettingClient; +import org.opensearch.cluster.ClusterState; +import org.opensearch.cluster.metadata.ComponentTemplate; +import org.opensearch.common.unit.TimeValue; +import org.opensearch.common.xcontent.json.JsonXContent; +import org.opensearch.core.xcontent.DeprecationHandler; +import org.opensearch.core.xcontent.NamedXContentRegistry; +import org.opensearch.core.xcontent.XContentParser; +import org.opensearch.threadpool.ThreadPool; + +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.util.function.Supplier; + +public class ClusterStateComponentTemplateLoader implements TemplateLoader { + + private Client client; + + private Supplier clusterStateSupplier; + + private static final Logger logger = LogManager.getLogger(TemplateLoader.class); + + public static final String TEMPLATE_LOADER_IDENTIFIER = "system_template_loader"; + + public ClusterStateComponentTemplateLoader(Client client, + ThreadPool threadPool, + Supplier clusterStateSupplier) { + this.client = new OriginSettingClient(client, TEMPLATE_LOADER_IDENTIFIER); + this.clusterStateSupplier = clusterStateSupplier; + } + + @Override + public void loadTemplate(SystemTemplate template) throws IOException { + ComponentTemplate existingTemplate = clusterStateSupplier.get().metadata().componentTemplates().get(template.templateInfo().fullyQualifiedName()); + + XContentParser contentParser = JsonXContent.jsonXContent.createParser(NamedXContentRegistry.EMPTY, DeprecationHandler.IGNORE_DEPRECATIONS, + template.templateContent().utf8ToString()); + ComponentTemplate newTemplate = ComponentTemplate.parse(contentParser); + + if (existingTemplate != null && existingTemplate.version() >= newTemplate.version()) { + logger.debug("Skipping putting template {} as its existing version [{}] is >= fetched version [{}]", template.templateInfo().fullyQualifiedName(), + existingTemplate.version(), + newTemplate.version()); + } + + PutComponentTemplateAction.Request request = new PutComponentTemplateAction.Request(template.templateInfo().fullyQualifiedName()) + .componentTemplate(newTemplate); + + client.admin().indices().execute(PutComponentTemplateAction.INSTANCE, request).actionGet(TimeValue.timeValueMillis(30000)); + } +} diff --git a/server/src/main/java/org/opensearch/cluster/service/applicationtemplates/SystemTemplate.java b/server/src/main/java/org/opensearch/cluster/service/applicationtemplates/SystemTemplate.java new file mode 100644 index 0000000000000..1baf452768f69 --- /dev/null +++ b/server/src/main/java/org/opensearch/cluster/service/applicationtemplates/SystemTemplate.java @@ -0,0 +1,41 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.cluster.service.applicationtemplates; + +import org.opensearch.core.common.bytes.BytesReference; + +/** + * Encapsulates the information and content about a system template available within a repository. + */ +public class SystemTemplate { + + private final BytesReference templateContent; + + private final SystemTemplateInfo templateInfo; + + private final TemplateRepositoryInfo repositoryInfo; + + public SystemTemplate(BytesReference templateContent, SystemTemplateInfo templateInfo, TemplateRepositoryInfo repositoryInfo) { + this.templateContent = templateContent; + this.templateInfo = templateInfo; + this.repositoryInfo = repositoryInfo; + } + + public BytesReference templateContent() { + return templateContent; + } + + public SystemTemplateInfo templateInfo() { + return templateInfo; + } + + public TemplateRepositoryInfo repositoryInfo() { + return repositoryInfo; + } +} diff --git a/server/src/main/java/org/opensearch/cluster/service/applicationtemplates/SystemTemplateInfo.java b/server/src/main/java/org/opensearch/cluster/service/applicationtemplates/SystemTemplateInfo.java new file mode 100644 index 0000000000000..209fcc885d250 --- /dev/null +++ b/server/src/main/java/org/opensearch/cluster/service/applicationtemplates/SystemTemplateInfo.java @@ -0,0 +1,53 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.cluster.service.applicationtemplates; + +/** + * Metadata information about a template available in a template repository. + */ +public class SystemTemplateInfo { + + private final long version; + private final String type; + private final String name; + + private static final String DELIMITER = "@"; + + public static final String COMPONENT_TEMPLATE_TYPE = "@abc_template"; + + public SystemTemplateInfo(long version, String type, String name) { + this.version = version; + this.type = type; + this.name = name; + } + + public String type() { + return type; + } + + public String name() { + return name; + } + + public long version() { + return version; + } + + public static SystemTemplateInfo fromComponentTemplate(String fullyQualifiedName) { + return new SystemTemplateInfo(Long.parseLong(fullyQualifiedName.substring(fullyQualifiedName.lastIndexOf(DELIMITER))), COMPONENT_TEMPLATE_TYPE, fullyQualifiedName.substring(0, fullyQualifiedName.lastIndexOf(DELIMITER))); + } + + public static SystemTemplateInfo createComponentTemplateInfo(String name, long version) { + return new SystemTemplateInfo(version, COMPONENT_TEMPLATE_TYPE, name); + } + + public final String fullyQualifiedName() { + return name + DELIMITER + version; + } +} diff --git a/server/src/main/java/org/opensearch/cluster/service/applicationtemplates/SystemTemplatesPlugin.java b/server/src/main/java/org/opensearch/cluster/service/applicationtemplates/SystemTemplatesPlugin.java new file mode 100644 index 0000000000000..3d4d662beff18 --- /dev/null +++ b/server/src/main/java/org/opensearch/cluster/service/applicationtemplates/SystemTemplatesPlugin.java @@ -0,0 +1,28 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.cluster.service.applicationtemplates; + +import java.io.IOException; + +/** + * Plugin interface to expose the template maintaining logic. + */ +public interface SystemTemplatesPlugin { + + /** + * @return repository implementation from which templates are to be fetched. + */ + TemplateRepository loadRepository() throws IOException; + + /** + * @param templateInfo Metadata about the template to load + * @return Implementation of TemplateLoader which determines how to make the template available at runtime. + */ + TemplateLoader loaderFor(SystemTemplateInfo templateInfo); +} diff --git a/server/src/main/java/org/opensearch/cluster/service/applicationtemplates/SystemTemplatesService.java b/server/src/main/java/org/opensearch/cluster/service/applicationtemplates/SystemTemplatesService.java new file mode 100644 index 0000000000000..c9f1c4de477e0 --- /dev/null +++ b/server/src/main/java/org/opensearch/cluster/service/applicationtemplates/SystemTemplatesService.java @@ -0,0 +1,104 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.cluster.service.applicationtemplates; + +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; +import org.opensearch.common.settings.ClusterSettings; +import org.opensearch.common.settings.Setting; +import org.opensearch.common.settings.Settings; + +import java.io.IOException; +import java.util.List; +import java.util.concurrent.atomic.AtomicBoolean; + +/** + * Service class to orchestrate execution around available templates' management. + */ +public class SystemTemplatesService { + + public static final int APPLICATION_BASED_CONFIGURATION_TEMPLATES_LOAD_DEFAULT_COUNT = 50; + + public static final Setting SETTING_APPLICATION_BASED_CONFIGURATION_TEMPLATES_LOAD = Setting.intSetting( + "cluster.application_templates.load", + APPLICATION_BASED_CONFIGURATION_TEMPLATES_LOAD_DEFAULT_COUNT, + 0, + 1000, + Setting.Property.Dynamic, + Setting.Property.NodeScope + ); + + private final List systemTemplatesPluginList; + + private final Settings settings; + + private final AtomicBoolean loaded = new AtomicBoolean(false); + + private volatile int templatesToLoad = APPLICATION_BASED_CONFIGURATION_TEMPLATES_LOAD_DEFAULT_COUNT; + + private static final Logger logger = LogManager.getLogger(SystemTemplatesService.class); + + public SystemTemplatesService(List systemTemplatesPluginList, + ClusterSettings clusterSettings, + Settings settings) { + this.systemTemplatesPluginList = systemTemplatesPluginList; + clusterSettings.addSettingsUpdateConsumer(SETTING_APPLICATION_BASED_CONFIGURATION_TEMPLATES_LOAD, this::setTemplatesToLoad); + this.settings = settings; + } + + public void refreshTemplates() { + if (loaded.compareAndSet(false, true)) { + if (SETTING_APPLICATION_BASED_CONFIGURATION_TEMPLATES_LOAD.get(settings) > 0) { + int countOfTemplatesToLoad = SETTING_APPLICATION_BASED_CONFIGURATION_TEMPLATES_LOAD.get(settings); + int templatesLoaded = 0; + int failedLoadingTemplates = 0; + int failedLoadingRepositories = 0; + + for (SystemTemplatesPlugin plugin : systemTemplatesPluginList) { + try (TemplateRepository repository = plugin.loadRepository()) { + TemplateRepositoryInfo repositoryInfo = repository.info(); + logger.debug("Loading templates from repository: {} at version {}", repositoryInfo.id(), repositoryInfo.version()); + for (SystemTemplateInfo templateInfo : repository.listTemplates()) { + + if (templatesLoaded == countOfTemplatesToLoad) { + logger.debug("Not loading template: {} from repository: {} as we've breached the max count [{}] allowed", + templateInfo.fullyQualifiedName(), repositoryInfo.id(), countOfTemplatesToLoad); + break; + } + try { + SystemTemplate template = repository.fetchTemplate(templateInfo); + plugin.loaderFor(templateInfo).loadTemplate(template); + countOfTemplatesToLoad--; + } catch (Exception ex) { + logger.error("Failed loading template {} from repository: {}", + templateInfo.fullyQualifiedName(), repositoryInfo.id(), ex); + failedLoadingTemplates++; + } + } + + if (templatesLoaded == countOfTemplatesToLoad) { + logger.debug("Loaded maximum permitted templates"); + break; + } + } catch (Exception ex) { + failedLoadingRepositories++; + logger.error("Failed loading repository from plugin: {}", plugin.getClass().getName(), ex); + } + } + + logger.debug("Stats: Total Loaded Templates: [{}], Failed Loading Templates: [{}], Failed Loading Repositories: [{}]", + templatesLoaded, failedLoadingTemplates, failedLoadingRepositories); + } + } + } + + private void setTemplatesToLoad(int templatesToLoad) { + this.templatesToLoad = templatesToLoad; + } +} diff --git a/server/src/main/java/org/opensearch/cluster/service/applicationtemplates/TemplateLoader.java b/server/src/main/java/org/opensearch/cluster/service/applicationtemplates/TemplateLoader.java new file mode 100644 index 0000000000000..7c43d19ae2526 --- /dev/null +++ b/server/src/main/java/org/opensearch/cluster/service/applicationtemplates/TemplateLoader.java @@ -0,0 +1,24 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.cluster.service.applicationtemplates; + +import java.io.IOException; + + +/** + * Interface to load template into the OpenSearch runtime. + */ +public interface TemplateLoader { + + /** + * @param template Templated to be loaded + * @throws IOException If an exceptional situation is encountered while parsing/loading the template + */ + void loadTemplate(SystemTemplate template) throws IOException; +} diff --git a/server/src/main/java/org/opensearch/cluster/service/applicationtemplates/TemplateRepository.java b/server/src/main/java/org/opensearch/cluster/service/applicationtemplates/TemplateRepository.java new file mode 100644 index 0000000000000..6e968a29e0dfb --- /dev/null +++ b/server/src/main/java/org/opensearch/cluster/service/applicationtemplates/TemplateRepository.java @@ -0,0 +1,35 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.cluster.service.applicationtemplates; + +import java.io.IOException; +import java.util.List; + +/** + * Repository interface around the templates provided by a store (e.g. code repo, remote file store, etc) + */ +public interface TemplateRepository extends AutoCloseable { + + /** + * @return Metadata about the repository + */ + TemplateRepositoryInfo info(); + + /** + * @return Metadata for all available templates + */ + Iterable listTemplates() throws IOException; + + /** + * + * @param template + * @return + */ + SystemTemplate fetchTemplate(SystemTemplateInfo template) throws IOException; +} diff --git a/server/src/main/java/org/opensearch/cluster/service/applicationtemplates/TemplateRepositoryInfo.java b/server/src/main/java/org/opensearch/cluster/service/applicationtemplates/TemplateRepositoryInfo.java new file mode 100644 index 0000000000000..9328cd24c3981 --- /dev/null +++ b/server/src/main/java/org/opensearch/cluster/service/applicationtemplates/TemplateRepositoryInfo.java @@ -0,0 +1,31 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.cluster.service.applicationtemplates; + +/** + * The information to uniquely identify a template repository. + */ +public class TemplateRepositoryInfo { + + private final String id; + private final long version; + + public TemplateRepositoryInfo(String id, long version) { + this.id = id; + this.version = version; + } + + public String id() { + return id; + } + + public long version() { + return version; + } +} diff --git a/server/src/main/java/org/opensearch/node/Node.java b/server/src/main/java/org/opensearch/node/Node.java index 85ef547e27787..95dcebfc463f7 100644 --- a/server/src/main/java/org/opensearch/node/Node.java +++ b/server/src/main/java/org/opensearch/node/Node.java @@ -84,6 +84,7 @@ import org.opensearch.cluster.routing.allocation.AwarenessReplicaBalance; import org.opensearch.cluster.routing.allocation.DiskThresholdMonitor; import org.opensearch.cluster.service.ClusterService; +import org.opensearch.cluster.service.applicationtemplates.SystemTemplatesPlugin; import org.opensearch.common.SetOnce; import org.opensearch.common.StopWatch; import org.opensearch.common.cache.module.CacheModule; @@ -663,7 +664,8 @@ protected Node( settings, settingsModule.getClusterSettings(), threadPool, - clusterManagerMetrics + clusterManagerMetrics, + pluginsService.filterPlugins(SystemTemplatesPlugin.class) ); clusterService.addStateApplier(scriptService); resourcesToClose.add(clusterService); diff --git a/test/framework/src/main/java/org/opensearch/cluster/service/applicationtemplates/TestSystemTemplatesRepositoryPlugin.java b/test/framework/src/main/java/org/opensearch/cluster/service/applicationtemplates/TestSystemTemplatesRepositoryPlugin.java new file mode 100644 index 0000000000000..bb04136adb6da --- /dev/null +++ b/test/framework/src/main/java/org/opensearch/cluster/service/applicationtemplates/TestSystemTemplatesRepositoryPlugin.java @@ -0,0 +1,62 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.cluster.service.applicationtemplates; + +import org.opensearch.core.common.bytes.BytesReference; +import org.opensearch.plugins.Plugin; + +import java.io.IOException; +import java.nio.ByteBuffer; +import java.nio.charset.StandardCharsets; +import java.util.List; + +public class TestSystemTemplatesRepositoryPlugin extends Plugin implements SystemTemplatesPlugin { + + private SystemTemplateInfo info = SystemTemplateInfo.createComponentTemplateInfo("dummy", 1); + + private TemplateRepositoryInfo repoInfo = new TemplateRepositoryInfo("test", 1); + + private SystemTemplate systemTemplate = new SystemTemplate(BytesReference.fromByteBuffer(ByteBuffer.wrap("content".getBytes(StandardCharsets.UTF_8))), info, repoInfo); + + @Override + public TemplateRepository loadRepository() throws IOException { + return new TemplateRepository() { + @Override + public TemplateRepositoryInfo info() { + return repoInfo; + } + + @Override + public List listTemplates() throws IOException { + return List.of(info); + } + + @Override + public SystemTemplate fetchTemplate(SystemTemplateInfo template) throws IOException { + return systemTemplate; + } + + @Override + public void close() throws Exception { + } + }; + } + + @Override + public TemplateLoader loaderFor(SystemTemplateInfo templateInfo) { + return new TemplateLoader() { // Asserting Loader + @Override + public void loadTemplate(SystemTemplate template) throws IOException { + assert template.templateInfo() == info; + assert template.repositoryInfo() == repoInfo; + assert template.templateContent() == systemTemplate; + } + }; + } +} diff --git a/test/framework/src/main/java/org/opensearch/test/OpenSearchIntegTestCase.java b/test/framework/src/main/java/org/opensearch/test/OpenSearchIntegTestCase.java index ca5ddf21710af..2955c988a12c6 100644 --- a/test/framework/src/main/java/org/opensearch/test/OpenSearchIntegTestCase.java +++ b/test/framework/src/main/java/org/opensearch/test/OpenSearchIntegTestCase.java @@ -90,6 +90,7 @@ import org.opensearch.cluster.routing.allocation.decider.AwarenessAllocationDecider; import org.opensearch.cluster.routing.allocation.decider.EnableAllocationDecider; import org.opensearch.cluster.service.ClusterService; +import org.opensearch.cluster.service.applicationtemplates.TestSystemTemplatesRepositoryPlugin; import org.opensearch.common.Nullable; import org.opensearch.common.Priority; import org.opensearch.common.collect.Tuple; @@ -2168,6 +2169,7 @@ protected Collection> getMockPlugins() { if (addMockTelemetryPlugin()) { mocks.add(MockTelemetryPlugin.class); } + mocks.add(TestSystemTemplatesRepositoryPlugin.class); return Collections.unmodifiableList(mocks); } From d4c64721fe85ca53ef7a5044c5ffd1755f17cacc Mon Sep 17 00:00:00 2001 From: Mohit Godwani Date: Thu, 11 Jul 2024 14:07:14 +0530 Subject: [PATCH 02/10] Change template load trigger Signed-off-by: Mohit Godwani --- .../service/ClusterManagerService.java | 8 ++------ .../cluster/service/ClusterService.java | 9 +++------ .../cluster/service/MasterService.java | 10 ++-------- .../SystemTemplatesService.java | 19 ++++++++++++++++--- .../common/settings/ClusterSettings.java | 5 ++++- .../main/java/org/opensearch/node/Node.java | 9 ++++++--- 6 files changed, 33 insertions(+), 27 deletions(-) diff --git a/server/src/main/java/org/opensearch/cluster/service/ClusterManagerService.java b/server/src/main/java/org/opensearch/cluster/service/ClusterManagerService.java index 4f3f5ecb86d1f..fa8c965b4d538 100644 --- a/server/src/main/java/org/opensearch/cluster/service/ClusterManagerService.java +++ b/server/src/main/java/org/opensearch/cluster/service/ClusterManagerService.java @@ -9,14 +9,11 @@ package org.opensearch.cluster.service; import org.opensearch.cluster.ClusterManagerMetrics; -import org.opensearch.cluster.service.applicationtemplates.SystemTemplatesPlugin; import org.opensearch.common.annotation.PublicApi; import org.opensearch.common.settings.ClusterSettings; import org.opensearch.common.settings.Settings; import org.opensearch.threadpool.ThreadPool; -import java.util.List; - /** * Main Cluster Manager Node Service * @@ -33,9 +30,8 @@ public ClusterManagerService( Settings settings, ClusterSettings clusterSettings, ThreadPool threadPool, - ClusterManagerMetrics clusterManagerMetrics, - List systemTemplatesPlugins + ClusterManagerMetrics clusterManagerMetrics ) { - super(settings, clusterSettings, threadPool, clusterManagerMetrics, systemTemplatesPlugins); + super(settings, clusterSettings, threadPool, clusterManagerMetrics); } } diff --git a/server/src/main/java/org/opensearch/cluster/service/ClusterService.java b/server/src/main/java/org/opensearch/cluster/service/ClusterService.java index d3282bb158774..c3c48dd8b87ef 100644 --- a/server/src/main/java/org/opensearch/cluster/service/ClusterService.java +++ b/server/src/main/java/org/opensearch/cluster/service/ClusterService.java @@ -46,7 +46,6 @@ import org.opensearch.cluster.node.DiscoveryNode; import org.opensearch.cluster.routing.OperationRouting; import org.opensearch.cluster.routing.RerouteService; -import org.opensearch.cluster.service.applicationtemplates.SystemTemplatesPlugin; import org.opensearch.common.annotation.PublicApi; import org.opensearch.common.lifecycle.AbstractLifecycleComponent; import org.opensearch.common.settings.ClusterSettings; @@ -59,7 +58,6 @@ import org.opensearch.threadpool.ThreadPool; import java.util.Collections; -import java.util.List; import java.util.Map; /** @@ -96,20 +94,19 @@ public class ClusterService extends AbstractLifecycleComponent { private IndexingPressureService indexingPressureService; public ClusterService(Settings settings, ClusterSettings clusterSettings, ThreadPool threadPool) { - this(settings, clusterSettings, threadPool, new ClusterManagerMetrics(NoopMetricsRegistry.INSTANCE), null); + this(settings, clusterSettings, threadPool, new ClusterManagerMetrics(NoopMetricsRegistry.INSTANCE)); } public ClusterService( Settings settings, ClusterSettings clusterSettings, ThreadPool threadPool, - ClusterManagerMetrics clusterManagerMetrics, - List systemTemplatesPlugins + ClusterManagerMetrics clusterManagerMetrics ) { this( settings, clusterSettings, - new ClusterManagerService(settings, clusterSettings, threadPool, clusterManagerMetrics, systemTemplatesPlugins), + new ClusterManagerService(settings, clusterSettings, threadPool, clusterManagerMetrics), new ClusterApplierService(Node.NODE_NAME_SETTING.get(settings), settings, clusterSettings, threadPool, clusterManagerMetrics) ); } diff --git a/server/src/main/java/org/opensearch/cluster/service/MasterService.java b/server/src/main/java/org/opensearch/cluster/service/MasterService.java index de420b420dbad..686e9793a8fd3 100644 --- a/server/src/main/java/org/opensearch/cluster/service/MasterService.java +++ b/server/src/main/java/org/opensearch/cluster/service/MasterService.java @@ -53,8 +53,6 @@ import org.opensearch.cluster.node.DiscoveryNode; import org.opensearch.cluster.node.DiscoveryNodes; import org.opensearch.cluster.routing.RoutingTable; -import org.opensearch.cluster.service.applicationtemplates.SystemTemplatesPlugin; -import org.opensearch.cluster.service.applicationtemplates.SystemTemplatesService; import org.opensearch.common.Nullable; import org.opensearch.common.Priority; import org.opensearch.common.annotation.DeprecatedApi; @@ -142,18 +140,16 @@ public class MasterService extends AbstractLifecycleComponent { private final ClusterManagerThrottlingStats throttlingStats; private final ClusterStateStats stateStats; private final ClusterManagerMetrics clusterManagerMetrics; - private final SystemTemplatesService systemTemplatesService; public MasterService(Settings settings, ClusterSettings clusterSettings, ThreadPool threadPool) { - this(settings, clusterSettings, threadPool, new ClusterManagerMetrics(NoopMetricsRegistry.INSTANCE), null); + this(settings, clusterSettings, threadPool, new ClusterManagerMetrics(NoopMetricsRegistry.INSTANCE)); } public MasterService( Settings settings, ClusterSettings clusterSettings, ThreadPool threadPool, - ClusterManagerMetrics clusterManagerMetrics, - List systemTemplatesPlugins + ClusterManagerMetrics clusterManagerMetrics ) { this.nodeName = Objects.requireNonNull(Node.NODE_NAME_SETTING.get(settings)); @@ -173,7 +169,6 @@ public MasterService( this.stateStats = new ClusterStateStats(); this.threadPool = threadPool; this.clusterManagerMetrics = clusterManagerMetrics; - this.systemTemplatesService = new SystemTemplatesService(systemTemplatesPlugins, clusterSettings, settings); } private void setSlowTaskLoggingThreshold(TimeValue slowTaskLoggingThreshold) { @@ -400,7 +395,6 @@ void onPublicationSuccess(ClusterChangedEvent clusterChangedEvent, TaskOutputs t try { taskOutputs.clusterStatePublished(clusterChangedEvent); - threadPool.executor(ThreadPool.Names.GENERIC).submit(() -> systemTemplatesService.refreshTemplates()); } catch (Exception e) { logger.error( () -> new ParameterizedMessage( diff --git a/server/src/main/java/org/opensearch/cluster/service/applicationtemplates/SystemTemplatesService.java b/server/src/main/java/org/opensearch/cluster/service/applicationtemplates/SystemTemplatesService.java index c9f1c4de477e0..c665fe7421c6c 100644 --- a/server/src/main/java/org/opensearch/cluster/service/applicationtemplates/SystemTemplatesService.java +++ b/server/src/main/java/org/opensearch/cluster/service/applicationtemplates/SystemTemplatesService.java @@ -10,6 +10,9 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; +import org.opensearch.cluster.ClusterChangedEvent; +import org.opensearch.cluster.ClusterStateListener; +import org.opensearch.cluster.LocalNodeClusterManagerListener; import org.opensearch.common.settings.ClusterSettings; import org.opensearch.common.settings.Setting; import org.opensearch.common.settings.Settings; @@ -21,7 +24,7 @@ /** * Service class to orchestrate execution around available templates' management. */ -public class SystemTemplatesService { +public class SystemTemplatesService implements LocalNodeClusterManagerListener { public static final int APPLICATION_BASED_CONFIGURATION_TEMPLATES_LOAD_DEFAULT_COUNT = 50; @@ -52,10 +55,20 @@ public SystemTemplatesService(List systemTemplatesPluginL this.settings = settings; } - public void refreshTemplates() { + @Override + public void onClusterManager() { + refreshTemplates(); + } + + @Override + public void offClusterManager() { + // do nothing + } + + private void refreshTemplates() { if (loaded.compareAndSet(false, true)) { if (SETTING_APPLICATION_BASED_CONFIGURATION_TEMPLATES_LOAD.get(settings) > 0) { - int countOfTemplatesToLoad = SETTING_APPLICATION_BASED_CONFIGURATION_TEMPLATES_LOAD.get(settings); + int countOfTemplatesToLoad = templatesToLoad; int templatesLoaded = 0; int failedLoadingTemplates = 0; int failedLoadingRepositories = 0; diff --git a/server/src/main/java/org/opensearch/common/settings/ClusterSettings.java b/server/src/main/java/org/opensearch/common/settings/ClusterSettings.java index 5dcf23ae52294..8f613b40f2a7b 100644 --- a/server/src/main/java/org/opensearch/common/settings/ClusterSettings.java +++ b/server/src/main/java/org/opensearch/common/settings/ClusterSettings.java @@ -82,6 +82,7 @@ import org.opensearch.cluster.service.ClusterManagerService; import org.opensearch.cluster.service.ClusterManagerTaskThrottler; import org.opensearch.cluster.service.ClusterService; +import org.opensearch.cluster.service.applicationtemplates.SystemTemplatesService; import org.opensearch.common.annotation.PublicApi; import org.opensearch.common.cache.CacheType; import org.opensearch.common.cache.settings.CacheSettings; @@ -758,7 +759,9 @@ public void apply(Settings value, Settings current, Settings previous) { SearchService.CLUSTER_ALLOW_DERIVED_FIELD_SETTING, // Composite index settings - CompositeIndexSettings.STAR_TREE_INDEX_ENABLED_SETTING + CompositeIndexSettings.STAR_TREE_INDEX_ENABLED_SETTING, + + SystemTemplatesService.SETTING_APPLICATION_BASED_CONFIGURATION_TEMPLATES_LOAD ) ) ); diff --git a/server/src/main/java/org/opensearch/node/Node.java b/server/src/main/java/org/opensearch/node/Node.java index 95dcebfc463f7..950558159acec 100644 --- a/server/src/main/java/org/opensearch/node/Node.java +++ b/server/src/main/java/org/opensearch/node/Node.java @@ -85,6 +85,7 @@ import org.opensearch.cluster.routing.allocation.DiskThresholdMonitor; import org.opensearch.cluster.service.ClusterService; import org.opensearch.cluster.service.applicationtemplates.SystemTemplatesPlugin; +import org.opensearch.cluster.service.applicationtemplates.SystemTemplatesService; import org.opensearch.common.SetOnce; import org.opensearch.common.StopWatch; import org.opensearch.common.cache.module.CacheModule; @@ -664,18 +665,20 @@ protected Node( settings, settingsModule.getClusterSettings(), threadPool, - clusterManagerMetrics, - pluginsService.filterPlugins(SystemTemplatesPlugin.class) + clusterManagerMetrics ); clusterService.addStateApplier(scriptService); resourcesToClose.add(clusterService); final Set> consistentSettings = settingsModule.getConsistentSettings(); if (consistentSettings.isEmpty() == false) { - clusterService.addLocalNodeMasterListener( + clusterService.addLocalNodeClusterManagerListener( new ConsistentSettingsService(settings, clusterService, consistentSettings).newHashPublisher() ); } + clusterService.addLocalNodeClusterManagerListener(new SystemTemplatesService(pluginsService.filterPlugins(SystemTemplatesPlugin.class), + clusterService.getClusterSettings(), settings)); + final ClusterInfoService clusterInfoService = newClusterInfoService(settings, clusterService, threadPool, client); final UsageService usageService = new UsageService(); From 1c80d7a5ca64b9c10f21a3d48f6a3eff4dba7030 Mon Sep 17 00:00:00 2001 From: Mohit Godwani Date: Sat, 13 Jul 2024 14:07:54 +0530 Subject: [PATCH 03/10] Address PR comments, add verification logic, refactor classes Signed-off-by: Mohit Godwani --- ...erStateComponentSystemTemplateLoader.java} | 19 +++--- .../applicationtemplates/SystemTemplate.java | 14 ++-- .../SystemTemplateLoader.java} | 7 +- .../SystemTemplateMetadata.java} | 17 +++-- .../SystemTemplateRepository.java} | 18 ++--- .../SystemTemplatesPlugin.java | 9 ++- .../SystemTemplatesService.java | 65 ++++++++++++++----- .../TemplateRepositoryMetadata.java} | 9 ++- .../MetadataIndexTemplateService.java | 23 ++++++- .../common/settings/ClusterSettings.java | 5 +- .../main/java/org/opensearch/node/Node.java | 10 +-- .../TestSystemTemplatesRepositoryPlugin.java | 19 +++--- 12 files changed, 143 insertions(+), 72 deletions(-) rename server/src/main/java/org/opensearch/cluster/{service/applicationtemplates/ClusterStateComponentTemplateLoader.java => applicationtemplates/ClusterStateComponentSystemTemplateLoader.java} (78%) rename server/src/main/java/org/opensearch/cluster/{service => }/applicationtemplates/SystemTemplate.java (65%) rename server/src/main/java/org/opensearch/cluster/{service/applicationtemplates/TemplateLoader.java => applicationtemplates/SystemTemplateLoader.java} (76%) rename server/src/main/java/org/opensearch/cluster/{service/applicationtemplates/SystemTemplateInfo.java => applicationtemplates/SystemTemplateMetadata.java} (54%) rename server/src/main/java/org/opensearch/cluster/{service/applicationtemplates/TemplateRepository.java => applicationtemplates/SystemTemplateRepository.java} (50%) rename server/src/main/java/org/opensearch/cluster/{service => }/applicationtemplates/SystemTemplatesPlugin.java (70%) rename server/src/main/java/org/opensearch/cluster/{service => }/applicationtemplates/SystemTemplatesService.java (62%) rename server/src/main/java/org/opensearch/cluster/{service/applicationtemplates/TemplateRepositoryInfo.java => applicationtemplates/TemplateRepositoryMetadata.java} (68%) diff --git a/server/src/main/java/org/opensearch/cluster/service/applicationtemplates/ClusterStateComponentTemplateLoader.java b/server/src/main/java/org/opensearch/cluster/applicationtemplates/ClusterStateComponentSystemTemplateLoader.java similarity index 78% rename from server/src/main/java/org/opensearch/cluster/service/applicationtemplates/ClusterStateComponentTemplateLoader.java rename to server/src/main/java/org/opensearch/cluster/applicationtemplates/ClusterStateComponentSystemTemplateLoader.java index 75b5ba8ee09f8..0386cdb25d8eb 100644 --- a/server/src/main/java/org/opensearch/cluster/service/applicationtemplates/ClusterStateComponentTemplateLoader.java +++ b/server/src/main/java/org/opensearch/cluster/applicationtemplates/ClusterStateComponentSystemTemplateLoader.java @@ -6,7 +6,7 @@ * compatible open source license. */ -package org.opensearch.cluster.service.applicationtemplates; +package org.opensearch.cluster.applicationtemplates; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; @@ -15,6 +15,7 @@ import org.opensearch.client.OriginSettingClient; import org.opensearch.cluster.ClusterState; import org.opensearch.cluster.metadata.ComponentTemplate; +import org.opensearch.common.annotation.ExperimentalApi; import org.opensearch.common.unit.TimeValue; import org.opensearch.common.xcontent.json.JsonXContent; import org.opensearch.core.xcontent.DeprecationHandler; @@ -23,22 +24,22 @@ import org.opensearch.threadpool.ThreadPool; import java.io.IOException; -import java.nio.charset.StandardCharsets; import java.util.function.Supplier; -public class ClusterStateComponentTemplateLoader implements TemplateLoader { +@ExperimentalApi +public class ClusterStateComponentSystemTemplateLoader implements SystemTemplateLoader { - private Client client; + private final Client client; - private Supplier clusterStateSupplier; + private final Supplier clusterStateSupplier; - private static final Logger logger = LogManager.getLogger(TemplateLoader.class); + private static final Logger logger = LogManager.getLogger(SystemTemplateLoader.class); public static final String TEMPLATE_LOADER_IDENTIFIER = "system_template_loader"; - public ClusterStateComponentTemplateLoader(Client client, - ThreadPool threadPool, - Supplier clusterStateSupplier) { + public ClusterStateComponentSystemTemplateLoader(Client client, + ThreadPool threadPool, + Supplier clusterStateSupplier) { this.client = new OriginSettingClient(client, TEMPLATE_LOADER_IDENTIFIER); this.clusterStateSupplier = clusterStateSupplier; } diff --git a/server/src/main/java/org/opensearch/cluster/service/applicationtemplates/SystemTemplate.java b/server/src/main/java/org/opensearch/cluster/applicationtemplates/SystemTemplate.java similarity index 65% rename from server/src/main/java/org/opensearch/cluster/service/applicationtemplates/SystemTemplate.java rename to server/src/main/java/org/opensearch/cluster/applicationtemplates/SystemTemplate.java index 1baf452768f69..3d99b8597d300 100644 --- a/server/src/main/java/org/opensearch/cluster/service/applicationtemplates/SystemTemplate.java +++ b/server/src/main/java/org/opensearch/cluster/applicationtemplates/SystemTemplate.java @@ -6,22 +6,24 @@ * compatible open source license. */ -package org.opensearch.cluster.service.applicationtemplates; +package org.opensearch.cluster.applicationtemplates; +import org.opensearch.common.annotation.ExperimentalApi; import org.opensearch.core.common.bytes.BytesReference; /** * Encapsulates the information and content about a system template available within a repository. */ +@ExperimentalApi public class SystemTemplate { private final BytesReference templateContent; - private final SystemTemplateInfo templateInfo; + private final SystemTemplateMetadata templateInfo; - private final TemplateRepositoryInfo repositoryInfo; + private final TemplateRepositoryMetadata repositoryInfo; - public SystemTemplate(BytesReference templateContent, SystemTemplateInfo templateInfo, TemplateRepositoryInfo repositoryInfo) { + public SystemTemplate(BytesReference templateContent, SystemTemplateMetadata templateInfo, TemplateRepositoryMetadata repositoryInfo) { this.templateContent = templateContent; this.templateInfo = templateInfo; this.repositoryInfo = repositoryInfo; @@ -31,11 +33,11 @@ public BytesReference templateContent() { return templateContent; } - public SystemTemplateInfo templateInfo() { + public SystemTemplateMetadata templateInfo() { return templateInfo; } - public TemplateRepositoryInfo repositoryInfo() { + public TemplateRepositoryMetadata repositoryInfo() { return repositoryInfo; } } diff --git a/server/src/main/java/org/opensearch/cluster/service/applicationtemplates/TemplateLoader.java b/server/src/main/java/org/opensearch/cluster/applicationtemplates/SystemTemplateLoader.java similarity index 76% rename from server/src/main/java/org/opensearch/cluster/service/applicationtemplates/TemplateLoader.java rename to server/src/main/java/org/opensearch/cluster/applicationtemplates/SystemTemplateLoader.java index 7c43d19ae2526..51fb3bbc2ce15 100644 --- a/server/src/main/java/org/opensearch/cluster/service/applicationtemplates/TemplateLoader.java +++ b/server/src/main/java/org/opensearch/cluster/applicationtemplates/SystemTemplateLoader.java @@ -6,7 +6,9 @@ * compatible open source license. */ -package org.opensearch.cluster.service.applicationtemplates; +package org.opensearch.cluster.applicationtemplates; + +import org.opensearch.common.annotation.ExperimentalApi; import java.io.IOException; @@ -14,7 +16,8 @@ /** * Interface to load template into the OpenSearch runtime. */ -public interface TemplateLoader { +@ExperimentalApi +public interface SystemTemplateLoader { /** * @param template Templated to be loaded diff --git a/server/src/main/java/org/opensearch/cluster/service/applicationtemplates/SystemTemplateInfo.java b/server/src/main/java/org/opensearch/cluster/applicationtemplates/SystemTemplateMetadata.java similarity index 54% rename from server/src/main/java/org/opensearch/cluster/service/applicationtemplates/SystemTemplateInfo.java rename to server/src/main/java/org/opensearch/cluster/applicationtemplates/SystemTemplateMetadata.java index 209fcc885d250..88c008c5422f1 100644 --- a/server/src/main/java/org/opensearch/cluster/service/applicationtemplates/SystemTemplateInfo.java +++ b/server/src/main/java/org/opensearch/cluster/applicationtemplates/SystemTemplateMetadata.java @@ -6,12 +6,15 @@ * compatible open source license. */ -package org.opensearch.cluster.service.applicationtemplates; +package org.opensearch.cluster.applicationtemplates; + +import org.opensearch.common.annotation.ExperimentalApi; /** * Metadata information about a template available in a template repository. */ -public class SystemTemplateInfo { +@ExperimentalApi +public class SystemTemplateMetadata { private final long version; private final String type; @@ -21,7 +24,7 @@ public class SystemTemplateInfo { public static final String COMPONENT_TEMPLATE_TYPE = "@abc_template"; - public SystemTemplateInfo(long version, String type, String name) { + public SystemTemplateMetadata(long version, String type, String name) { this.version = version; this.type = type; this.name = name; @@ -39,12 +42,12 @@ public long version() { return version; } - public static SystemTemplateInfo fromComponentTemplate(String fullyQualifiedName) { - return new SystemTemplateInfo(Long.parseLong(fullyQualifiedName.substring(fullyQualifiedName.lastIndexOf(DELIMITER))), COMPONENT_TEMPLATE_TYPE, fullyQualifiedName.substring(0, fullyQualifiedName.lastIndexOf(DELIMITER))); + public static SystemTemplateMetadata fromComponentTemplate(String fullyQualifiedName) { + return new SystemTemplateMetadata(Long.parseLong(fullyQualifiedName.substring(fullyQualifiedName.lastIndexOf(DELIMITER))), COMPONENT_TEMPLATE_TYPE, fullyQualifiedName.substring(0, fullyQualifiedName.lastIndexOf(DELIMITER))); } - public static SystemTemplateInfo createComponentTemplateInfo(String name, long version) { - return new SystemTemplateInfo(version, COMPONENT_TEMPLATE_TYPE, name); + public static SystemTemplateMetadata createComponentTemplateInfo(String name, long version) { + return new SystemTemplateMetadata(version, COMPONENT_TEMPLATE_TYPE, name); } public final String fullyQualifiedName() { diff --git a/server/src/main/java/org/opensearch/cluster/service/applicationtemplates/TemplateRepository.java b/server/src/main/java/org/opensearch/cluster/applicationtemplates/SystemTemplateRepository.java similarity index 50% rename from server/src/main/java/org/opensearch/cluster/service/applicationtemplates/TemplateRepository.java rename to server/src/main/java/org/opensearch/cluster/applicationtemplates/SystemTemplateRepository.java index 6e968a29e0dfb..9cf302b8874f2 100644 --- a/server/src/main/java/org/opensearch/cluster/service/applicationtemplates/TemplateRepository.java +++ b/server/src/main/java/org/opensearch/cluster/applicationtemplates/SystemTemplateRepository.java @@ -6,30 +6,32 @@ * compatible open source license. */ -package org.opensearch.cluster.service.applicationtemplates; +package org.opensearch.cluster.applicationtemplates; + +import org.opensearch.common.annotation.ExperimentalApi; import java.io.IOException; -import java.util.List; /** * Repository interface around the templates provided by a store (e.g. code repo, remote file store, etc) */ -public interface TemplateRepository extends AutoCloseable { +@ExperimentalApi +public interface SystemTemplateRepository extends AutoCloseable { /** * @return Metadata about the repository */ - TemplateRepositoryInfo info(); + TemplateRepositoryMetadata metadata(); /** * @return Metadata for all available templates */ - Iterable listTemplates() throws IOException; + Iterable listTemplates() throws IOException; /** * - * @param template - * @return + * @param template metadata about template to be fetched + * @return The actual template content */ - SystemTemplate fetchTemplate(SystemTemplateInfo template) throws IOException; + SystemTemplate getTemplate(SystemTemplateMetadata template) throws IOException; } diff --git a/server/src/main/java/org/opensearch/cluster/service/applicationtemplates/SystemTemplatesPlugin.java b/server/src/main/java/org/opensearch/cluster/applicationtemplates/SystemTemplatesPlugin.java similarity index 70% rename from server/src/main/java/org/opensearch/cluster/service/applicationtemplates/SystemTemplatesPlugin.java rename to server/src/main/java/org/opensearch/cluster/applicationtemplates/SystemTemplatesPlugin.java index 3d4d662beff18..54871e6db7010 100644 --- a/server/src/main/java/org/opensearch/cluster/service/applicationtemplates/SystemTemplatesPlugin.java +++ b/server/src/main/java/org/opensearch/cluster/applicationtemplates/SystemTemplatesPlugin.java @@ -6,23 +6,26 @@ * compatible open source license. */ -package org.opensearch.cluster.service.applicationtemplates; +package org.opensearch.cluster.applicationtemplates; + +import org.opensearch.common.annotation.ExperimentalApi; import java.io.IOException; /** * Plugin interface to expose the template maintaining logic. */ +@ExperimentalApi public interface SystemTemplatesPlugin { /** * @return repository implementation from which templates are to be fetched. */ - TemplateRepository loadRepository() throws IOException; + SystemTemplateRepository loadRepository() throws IOException; /** * @param templateInfo Metadata about the template to load * @return Implementation of TemplateLoader which determines how to make the template available at runtime. */ - TemplateLoader loaderFor(SystemTemplateInfo templateInfo); + SystemTemplateLoader loaderFor(SystemTemplateMetadata templateInfo); } diff --git a/server/src/main/java/org/opensearch/cluster/service/applicationtemplates/SystemTemplatesService.java b/server/src/main/java/org/opensearch/cluster/applicationtemplates/SystemTemplatesService.java similarity index 62% rename from server/src/main/java/org/opensearch/cluster/service/applicationtemplates/SystemTemplatesService.java rename to server/src/main/java/org/opensearch/cluster/applicationtemplates/SystemTemplatesService.java index c665fe7421c6c..f0fb031ec4d7c 100644 --- a/server/src/main/java/org/opensearch/cluster/service/applicationtemplates/SystemTemplatesService.java +++ b/server/src/main/java/org/opensearch/cluster/applicationtemplates/SystemTemplatesService.java @@ -6,30 +6,30 @@ * compatible open source license. */ -package org.opensearch.cluster.service.applicationtemplates; +package org.opensearch.cluster.applicationtemplates; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; -import org.opensearch.cluster.ClusterChangedEvent; -import org.opensearch.cluster.ClusterStateListener; import org.opensearch.cluster.LocalNodeClusterManagerListener; +import org.opensearch.common.annotation.ExperimentalApi; import org.opensearch.common.settings.ClusterSettings; import org.opensearch.common.settings.Setting; import org.opensearch.common.settings.Settings; -import java.io.IOException; +import java.util.ArrayList; import java.util.List; import java.util.concurrent.atomic.AtomicBoolean; /** * Service class to orchestrate execution around available templates' management. */ +@ExperimentalApi public class SystemTemplatesService implements LocalNodeClusterManagerListener { public static final int APPLICATION_BASED_CONFIGURATION_TEMPLATES_LOAD_DEFAULT_COUNT = 50; - public static final Setting SETTING_APPLICATION_BASED_CONFIGURATION_TEMPLATES_LOAD = Setting.intSetting( - "cluster.application_templates.load", + public static final Setting SETTING_APPLICATION_BASED_CONFIGURATION_MAX_TEMPLATES_LOAD = Setting.intSetting( + "cluster.application_templates.max_load", APPLICATION_BASED_CONFIGURATION_TEMPLATES_LOAD_DEFAULT_COUNT, 0, 1000, @@ -37,13 +37,22 @@ public class SystemTemplatesService implements LocalNodeClusterManagerListener { Setting.Property.NodeScope ); + public static final Setting SETTING_APPLICATION_BASED_CONFIGURATION_TEMPLATES_ENABLED = Setting.boolSetting( + "cluster.application_templates.enabled", + false, + Setting.Property.Dynamic, + Setting.Property.NodeScope + ); + private final List systemTemplatesPluginList; private final Settings settings; private final AtomicBoolean loaded = new AtomicBoolean(false); - private volatile int templatesToLoad = APPLICATION_BASED_CONFIGURATION_TEMPLATES_LOAD_DEFAULT_COUNT; + private volatile int templatesToLoad; + + private volatile boolean enabledTemplates; private static final Logger logger = LogManager.getLogger(SystemTemplatesService.class); @@ -51,13 +60,16 @@ public SystemTemplatesService(List systemTemplatesPluginL ClusterSettings clusterSettings, Settings settings) { this.systemTemplatesPluginList = systemTemplatesPluginList; - clusterSettings.addSettingsUpdateConsumer(SETTING_APPLICATION_BASED_CONFIGURATION_TEMPLATES_LOAD, this::setTemplatesToLoad); this.settings = settings; + setEnabledTemplates(settings.getAsBoolean(SETTING_APPLICATION_BASED_CONFIGURATION_TEMPLATES_ENABLED.getKey(), false)); + setMaxTemplatesToLoad(settings.getAsInt(SETTING_APPLICATION_BASED_CONFIGURATION_MAX_TEMPLATES_LOAD.getKey(), APPLICATION_BASED_CONFIGURATION_TEMPLATES_LOAD_DEFAULT_COUNT)); + clusterSettings.addSettingsUpdateConsumer(SETTING_APPLICATION_BASED_CONFIGURATION_MAX_TEMPLATES_LOAD, this::setMaxTemplatesToLoad); + clusterSettings.addSettingsUpdateConsumer(SETTING_APPLICATION_BASED_CONFIGURATION_TEMPLATES_ENABLED, this::setEnabledTemplates); } @Override public void onClusterManager() { - refreshTemplates(); + refreshTemplates(false); } @Override @@ -65,19 +77,24 @@ public void offClusterManager() { // do nothing } - private void refreshTemplates() { + public void verifyRepositories() { + refreshTemplates(true); + } + + public void refreshTemplates(boolean verification) { if (loaded.compareAndSet(false, true)) { - if (SETTING_APPLICATION_BASED_CONFIGURATION_TEMPLATES_LOAD.get(settings) > 0) { + if (enabledTemplates && templatesToLoad > 0) { int countOfTemplatesToLoad = templatesToLoad; int templatesLoaded = 0; int failedLoadingTemplates = 0; int failedLoadingRepositories = 0; + List exceptions = new ArrayList<>(); for (SystemTemplatesPlugin plugin : systemTemplatesPluginList) { - try (TemplateRepository repository = plugin.loadRepository()) { - TemplateRepositoryInfo repositoryInfo = repository.info(); + try (SystemTemplateRepository repository = plugin.loadRepository()) { + TemplateRepositoryMetadata repositoryInfo = repository.metadata(); logger.debug("Loading templates from repository: {} at version {}", repositoryInfo.id(), repositoryInfo.version()); - for (SystemTemplateInfo templateInfo : repository.listTemplates()) { + for (SystemTemplateMetadata templateInfo : repository.listTemplates()) { if (templatesLoaded == countOfTemplatesToLoad) { logger.debug("Not loading template: {} from repository: {} as we've breached the max count [{}] allowed", @@ -85,10 +102,15 @@ private void refreshTemplates() { break; } try { - SystemTemplate template = repository.fetchTemplate(templateInfo); - plugin.loaderFor(templateInfo).loadTemplate(template); + SystemTemplate template = repository.getTemplate(templateInfo); + + // Load plugin if not in verification phase. + if (!verification) { + plugin.loaderFor(templateInfo).loadTemplate(template); + } countOfTemplatesToLoad--; } catch (Exception ex) { + exceptions.add(ex); logger.error("Failed loading template {} from repository: {}", templateInfo.fullyQualifiedName(), repositoryInfo.id(), ex); failedLoadingTemplates++; @@ -100,6 +122,7 @@ private void refreshTemplates() { break; } } catch (Exception ex) { + exceptions.add(ex); failedLoadingRepositories++; logger.error("Failed loading repository from plugin: {}", plugin.getClass().getName(), ex); } @@ -107,11 +130,19 @@ private void refreshTemplates() { logger.debug("Stats: Total Loaded Templates: [{}], Failed Loading Templates: [{}], Failed Loading Repositories: [{}]", templatesLoaded, failedLoadingTemplates, failedLoadingRepositories); + + if (verification) { + throw new IllegalStateException("Some of the repositories could not be loaded or are corrupted: " + exceptions); + } } } } - private void setTemplatesToLoad(int templatesToLoad) { + private void setMaxTemplatesToLoad(int templatesToLoad) { this.templatesToLoad = templatesToLoad; } + + private void setEnabledTemplates(boolean enabled) { + enabledTemplates = enabled; + } } diff --git a/server/src/main/java/org/opensearch/cluster/service/applicationtemplates/TemplateRepositoryInfo.java b/server/src/main/java/org/opensearch/cluster/applicationtemplates/TemplateRepositoryMetadata.java similarity index 68% rename from server/src/main/java/org/opensearch/cluster/service/applicationtemplates/TemplateRepositoryInfo.java rename to server/src/main/java/org/opensearch/cluster/applicationtemplates/TemplateRepositoryMetadata.java index 9328cd24c3981..7ab4553aade0e 100644 --- a/server/src/main/java/org/opensearch/cluster/service/applicationtemplates/TemplateRepositoryInfo.java +++ b/server/src/main/java/org/opensearch/cluster/applicationtemplates/TemplateRepositoryMetadata.java @@ -6,17 +6,20 @@ * compatible open source license. */ -package org.opensearch.cluster.service.applicationtemplates; +package org.opensearch.cluster.applicationtemplates; + +import org.opensearch.common.annotation.ExperimentalApi; /** * The information to uniquely identify a template repository. */ -public class TemplateRepositoryInfo { +@ExperimentalApi +public class TemplateRepositoryMetadata { private final String id; private final long version; - public TemplateRepositoryInfo(String id, long version) { + public TemplateRepositoryMetadata(String id, long version) { this.id = id; this.version = version; } diff --git a/server/src/main/java/org/opensearch/cluster/metadata/MetadataIndexTemplateService.java b/server/src/main/java/org/opensearch/cluster/metadata/MetadataIndexTemplateService.java index 5b03d3f7b19ce..76096de7d67dc 100644 --- a/server/src/main/java/org/opensearch/cluster/metadata/MetadataIndexTemplateService.java +++ b/server/src/main/java/org/opensearch/cluster/metadata/MetadataIndexTemplateService.java @@ -45,6 +45,7 @@ import org.opensearch.cluster.service.ClusterManagerTaskKeys; import org.opensearch.cluster.service.ClusterManagerTaskThrottler; import org.opensearch.cluster.service.ClusterService; +import org.opensearch.cluster.applicationtemplates.ClusterStateComponentSystemTemplateLoader; import org.opensearch.common.Nullable; import org.opensearch.common.Priority; import org.opensearch.common.UUIDs; @@ -56,6 +57,7 @@ import org.opensearch.common.settings.IndexScopedSettings; import org.opensearch.common.settings.Settings; import org.opensearch.common.unit.TimeValue; +import org.opensearch.common.util.concurrent.ThreadContext; import org.opensearch.common.util.set.Sets; import org.opensearch.common.xcontent.XContentFactory; import org.opensearch.core.action.ActionListener; @@ -72,6 +74,7 @@ import org.opensearch.indices.IndexTemplateMissingException; import org.opensearch.indices.IndicesService; import org.opensearch.indices.InvalidIndexTemplateException; +import org.opensearch.threadpool.ThreadPool; import java.io.IOException; import java.io.UncheckedIOException; @@ -110,6 +113,7 @@ public class MetadataIndexTemplateService { private final MetadataCreateIndexService metadataCreateIndexService; private final IndexScopedSettings indexScopedSettings; private final NamedXContentRegistry xContentRegistry; + private final ThreadPool threadPool; private final ClusterManagerTaskThrottler.ThrottlingKey createIndexTemplateTaskKey; private final ClusterManagerTaskThrottler.ThrottlingKey createIndexTemplateV2TaskKey; private final ClusterManagerTaskThrottler.ThrottlingKey removeIndexTemplateTaskKey; @@ -124,7 +128,8 @@ public MetadataIndexTemplateService( AliasValidator aliasValidator, IndicesService indicesService, IndexScopedSettings indexScopedSettings, - NamedXContentRegistry xContentRegistry + NamedXContentRegistry xContentRegistry, + ThreadPool threadPool ) { this.clusterService = clusterService; this.aliasValidator = aliasValidator; @@ -132,6 +137,7 @@ public MetadataIndexTemplateService( this.metadataCreateIndexService = metadataCreateIndexService; this.indexScopedSettings = indexScopedSettings; this.xContentRegistry = xContentRegistry; + this.threadPool = threadPool; // Task is onboarded for throttling, it will get retried from associated TransportClusterManagerNodeAction. createIndexTemplateTaskKey = clusterService.registerClusterManagerTask(ClusterManagerTaskKeys.CREATE_INDEX_TEMPLATE_KEY, true); @@ -253,6 +259,13 @@ ClusterState addComponentTemplate( throw new IllegalArgumentException("component template [" + name + "] already exists"); } + if (isSystemComponentTemplate(template) + && ClusterStateComponentSystemTemplateLoader.TEMPLATE_LOADER_IDENTIFIER.equals( + threadPool.getThreadContext().getTransient(ThreadContext.ACTION_ORIGIN_TRANSIENT_NAME))) { + throw new IllegalArgumentException("Users are not permitted to create a system template. " + + "Should be loaded through a repository only."); + }; + CompressedXContent mappings = template.template().mappings(); String stringMappings = mappings == null ? null : mappings.string(); @@ -399,7 +412,8 @@ public ClusterManagerTaskThrottler.ThrottlingKey getClusterManagerThrottlingKey( public ClusterState execute(ClusterState currentState) { Set templateNames = new HashSet<>(); for (String templateName : currentState.metadata().componentTemplates().keySet()) { - if (Regex.simpleMatch(name, templateName)) { + if (Regex.simpleMatch(name, templateName) && + !isSystemComponentTemplate(currentState.metadata().componentTemplates().get(templateName))) { templateNames.add(templateName); } } @@ -412,6 +426,7 @@ public ClusterState execute(ClusterState currentState) { // TODO: perhaps introduce a ComponentTemplateMissingException? throw new IndexTemplateMissingException(name); } + Metadata.Builder metadata = Metadata.builder(currentState.metadata()); for (String templateName : templateNames) { logger.info("removing component template [{}]", templateName); @@ -1558,6 +1573,10 @@ private void validate(String name, @Nullable Settings settings, List ind } } + private boolean isSystemComponentTemplate(ComponentTemplate template) { + return true; + } + /** * Listener for putting metadata in the template * diff --git a/server/src/main/java/org/opensearch/common/settings/ClusterSettings.java b/server/src/main/java/org/opensearch/common/settings/ClusterSettings.java index 8f613b40f2a7b..a4230f42617f0 100644 --- a/server/src/main/java/org/opensearch/common/settings/ClusterSettings.java +++ b/server/src/main/java/org/opensearch/common/settings/ClusterSettings.java @@ -82,7 +82,7 @@ import org.opensearch.cluster.service.ClusterManagerService; import org.opensearch.cluster.service.ClusterManagerTaskThrottler; import org.opensearch.cluster.service.ClusterService; -import org.opensearch.cluster.service.applicationtemplates.SystemTemplatesService; +import org.opensearch.cluster.applicationtemplates.SystemTemplatesService; import org.opensearch.common.annotation.PublicApi; import org.opensearch.common.cache.CacheType; import org.opensearch.common.cache.settings.CacheSettings; @@ -761,7 +761,8 @@ public void apply(Settings value, Settings current, Settings previous) { // Composite index settings CompositeIndexSettings.STAR_TREE_INDEX_ENABLED_SETTING, - SystemTemplatesService.SETTING_APPLICATION_BASED_CONFIGURATION_TEMPLATES_LOAD + SystemTemplatesService.SETTING_APPLICATION_BASED_CONFIGURATION_MAX_TEMPLATES_LOAD, + SystemTemplatesService.SETTING_APPLICATION_BASED_CONFIGURATION_TEMPLATES_ENABLED ) ) ); diff --git a/server/src/main/java/org/opensearch/node/Node.java b/server/src/main/java/org/opensearch/node/Node.java index 950558159acec..eaea78dc8c207 100644 --- a/server/src/main/java/org/opensearch/node/Node.java +++ b/server/src/main/java/org/opensearch/node/Node.java @@ -84,8 +84,8 @@ import org.opensearch.cluster.routing.allocation.AwarenessReplicaBalance; import org.opensearch.cluster.routing.allocation.DiskThresholdMonitor; import org.opensearch.cluster.service.ClusterService; -import org.opensearch.cluster.service.applicationtemplates.SystemTemplatesPlugin; -import org.opensearch.cluster.service.applicationtemplates.SystemTemplatesService; +import org.opensearch.cluster.applicationtemplates.SystemTemplatesPlugin; +import org.opensearch.cluster.applicationtemplates.SystemTemplatesService; import org.opensearch.common.SetOnce; import org.opensearch.common.StopWatch; import org.opensearch.common.cache.module.CacheModule; @@ -676,8 +676,10 @@ protected Node( ); } - clusterService.addLocalNodeClusterManagerListener(new SystemTemplatesService(pluginsService.filterPlugins(SystemTemplatesPlugin.class), - clusterService.getClusterSettings(), settings)); + SystemTemplatesService systemTemplatesService = new SystemTemplatesService(pluginsService.filterPlugins(SystemTemplatesPlugin.class), + clusterService.getClusterSettings(), settings); + systemTemplatesService.verifyRepositories(); + clusterService.addLocalNodeClusterManagerListener(systemTemplatesService); final ClusterInfoService clusterInfoService = newClusterInfoService(settings, clusterService, threadPool, client); final UsageService usageService = new UsageService(); diff --git a/test/framework/src/main/java/org/opensearch/cluster/service/applicationtemplates/TestSystemTemplatesRepositoryPlugin.java b/test/framework/src/main/java/org/opensearch/cluster/service/applicationtemplates/TestSystemTemplatesRepositoryPlugin.java index bb04136adb6da..fd473ec18ccfc 100644 --- a/test/framework/src/main/java/org/opensearch/cluster/service/applicationtemplates/TestSystemTemplatesRepositoryPlugin.java +++ b/test/framework/src/main/java/org/opensearch/cluster/service/applicationtemplates/TestSystemTemplatesRepositoryPlugin.java @@ -8,6 +8,7 @@ package org.opensearch.cluster.service.applicationtemplates; +import org.opensearch.cluster.applicationtemplates.*; import org.opensearch.core.common.bytes.BytesReference; import org.opensearch.plugins.Plugin; @@ -18,27 +19,27 @@ public class TestSystemTemplatesRepositoryPlugin extends Plugin implements SystemTemplatesPlugin { - private SystemTemplateInfo info = SystemTemplateInfo.createComponentTemplateInfo("dummy", 1); + private SystemTemplateMetadata info = SystemTemplateMetadata.createComponentTemplateInfo("dummy", 1); - private TemplateRepositoryInfo repoInfo = new TemplateRepositoryInfo("test", 1); + private TemplateRepositoryMetadata repoInfo = new TemplateRepositoryMetadata("test", 1); private SystemTemplate systemTemplate = new SystemTemplate(BytesReference.fromByteBuffer(ByteBuffer.wrap("content".getBytes(StandardCharsets.UTF_8))), info, repoInfo); @Override - public TemplateRepository loadRepository() throws IOException { - return new TemplateRepository() { + public SystemTemplateRepository loadRepository() throws IOException { + return new SystemTemplateRepository() { @Override - public TemplateRepositoryInfo info() { + public TemplateRepositoryMetadata metadata() { return repoInfo; } @Override - public List listTemplates() throws IOException { + public List listTemplates() throws IOException { return List.of(info); } @Override - public SystemTemplate fetchTemplate(SystemTemplateInfo template) throws IOException { + public SystemTemplate getTemplate(SystemTemplateMetadata template) throws IOException { return systemTemplate; } @@ -49,8 +50,8 @@ public void close() throws Exception { } @Override - public TemplateLoader loaderFor(SystemTemplateInfo templateInfo) { - return new TemplateLoader() { // Asserting Loader + public SystemTemplateLoader loaderFor(SystemTemplateMetadata templateInfo) { + return new SystemTemplateLoader() { // Asserting Loader @Override public void loadTemplate(SystemTemplate template) throws IOException { assert template.templateInfo() == info; From c10f5372a2d0690a7d88ff8bdf507bafa2cd6ba1 Mon Sep 17 00:00:00 2001 From: Mohit Godwani Date: Sat, 13 Jul 2024 14:11:48 +0530 Subject: [PATCH 04/10] Add changelog Signed-off-by: Mohit Godwani --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 14804bbd5974a..695f5c15293ab 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - Add batching supported processor base type AbstractBatchingProcessor ([#14554](https://github.com/opensearch-project/OpenSearch/pull/14554)) - Fix race condition while parsing derived fields from search definition ([14445](https://github.com/opensearch-project/OpenSearch/pull/14445)) - Add allowlist setting for ingest-common and search-pipeline-common processors ([#14439](https://github.com/opensearch-project/OpenSearch/issues/14439)) +- Add Plugin interface for loading application based configuration templates (([#14659](https://github.com/opensearch-project/OpenSearch/issues/14659))) ### Dependencies - Bump `org.gradle.test-retry` from 1.5.8 to 1.5.9 ([#13442](https://github.com/opensearch-project/OpenSearch/pull/13442)) From 2a13e48a62aca3aadece318a5e8ca396dc9b8518 Mon Sep 17 00:00:00 2001 From: Mohit Godwani Date: Sun, 14 Jul 2024 15:35:53 +0530 Subject: [PATCH 05/10] Address PR comments Signed-off-by: Mohit Godwani --- .../ClusterStateComponentSystemTemplateLoader.java | 3 +++ .../cluster/applicationtemplates/package-info.java | 10 ++++++++++ 2 files changed, 13 insertions(+) create mode 100644 server/src/main/java/org/opensearch/cluster/applicationtemplates/package-info.java diff --git a/server/src/main/java/org/opensearch/cluster/applicationtemplates/ClusterStateComponentSystemTemplateLoader.java b/server/src/main/java/org/opensearch/cluster/applicationtemplates/ClusterStateComponentSystemTemplateLoader.java index 0386cdb25d8eb..d3d225cef2370 100644 --- a/server/src/main/java/org/opensearch/cluster/applicationtemplates/ClusterStateComponentSystemTemplateLoader.java +++ b/server/src/main/java/org/opensearch/cluster/applicationtemplates/ClusterStateComponentSystemTemplateLoader.java @@ -26,6 +26,9 @@ import java.io.IOException; import java.util.function.Supplier; +/** + * Class reponsible for loading the component templates provided by a repository into the cluster state. + */ @ExperimentalApi public class ClusterStateComponentSystemTemplateLoader implements SystemTemplateLoader { diff --git a/server/src/main/java/org/opensearch/cluster/applicationtemplates/package-info.java b/server/src/main/java/org/opensearch/cluster/applicationtemplates/package-info.java new file mode 100644 index 0000000000000..3fef2aab07d43 --- /dev/null +++ b/server/src/main/java/org/opensearch/cluster/applicationtemplates/package-info.java @@ -0,0 +1,10 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +/** Core classes responsible for handling all application based configuration templates related operations. */ +package org.opensearch.cluster.applicationtemplates; From 21541ba9892ffecc5db5ca6db4df7a33bf1bd8b6 Mon Sep 17 00:00:00 2001 From: Mohit Godwani Date: Mon, 15 Jul 2024 12:16:57 +0530 Subject: [PATCH 06/10] Add tests, Fix spotless, Modify implementation based on PR Comments, add feature flag Signed-off-by: Mohit Godwani --- ...terStateComponentSystemTemplateLoader.java | 62 +++++-- .../applicationtemplates/SystemTemplate.java | 16 +- .../SystemTemplateLoader.java | 3 +- .../SystemTemplateMetadata.java | 13 +- .../SystemTemplatesService.java | 171 ++++++++++-------- .../MetadataIndexTemplateService.java | 23 +-- .../common/settings/ClusterSettings.java | 3 +- .../common/settings/FeatureFlagSettings.java | 3 +- .../opensearch/common/util/FeatureFlags.java | 14 +- .../main/java/org/opensearch/node/Node.java | 12 +- ...tateComponentSystemTemplateLoaderTest.java | 148 +++++++++++++++ .../SystemTemplatesServiceTest.java | 90 +++++++++ .../TestSystemTemplatesRepositoryPlugin.java | 35 ++-- .../test/OpenSearchSingleNodeTestCase.java | 1 + 14 files changed, 452 insertions(+), 142 deletions(-) create mode 100644 server/src/test/java/org/opensearch/cluster/applicationtemplates/ClusterStateComponentSystemTemplateLoaderTest.java create mode 100644 server/src/test/java/org/opensearch/cluster/applicationtemplates/SystemTemplatesServiceTest.java diff --git a/server/src/main/java/org/opensearch/cluster/applicationtemplates/ClusterStateComponentSystemTemplateLoader.java b/server/src/main/java/org/opensearch/cluster/applicationtemplates/ClusterStateComponentSystemTemplateLoader.java index d3d225cef2370..e13ca8a9ded98 100644 --- a/server/src/main/java/org/opensearch/cluster/applicationtemplates/ClusterStateComponentSystemTemplateLoader.java +++ b/server/src/main/java/org/opensearch/cluster/applicationtemplates/ClusterStateComponentSystemTemplateLoader.java @@ -10,6 +10,7 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; +import org.opensearch.OpenSearchCorruptionException; import org.opensearch.action.admin.indices.template.put.PutComponentTemplateAction; import org.opensearch.client.Client; import org.opensearch.client.OriginSettingClient; @@ -21,13 +22,13 @@ import org.opensearch.core.xcontent.DeprecationHandler; import org.opensearch.core.xcontent.NamedXContentRegistry; import org.opensearch.core.xcontent.XContentParser; -import org.opensearch.threadpool.ThreadPool; import java.io.IOException; +import java.util.Objects; import java.util.function.Supplier; /** - * Class reponsible for loading the component templates provided by a repository into the cluster state. + * Class responsible for loading the component templates provided by a repository into the cluster state. */ @ExperimentalApi public class ClusterStateComponentSystemTemplateLoader implements SystemTemplateLoader { @@ -40,30 +41,61 @@ public class ClusterStateComponentSystemTemplateLoader implements SystemTemplate public static final String TEMPLATE_LOADER_IDENTIFIER = "system_template_loader"; - public ClusterStateComponentSystemTemplateLoader(Client client, - ThreadPool threadPool, - Supplier clusterStateSupplier) { + public ClusterStateComponentSystemTemplateLoader(Client client, Supplier clusterStateSupplier) { this.client = new OriginSettingClient(client, TEMPLATE_LOADER_IDENTIFIER); this.clusterStateSupplier = clusterStateSupplier; } @Override - public void loadTemplate(SystemTemplate template) throws IOException { - ComponentTemplate existingTemplate = clusterStateSupplier.get().metadata().componentTemplates().get(template.templateInfo().fullyQualifiedName()); + public boolean loadTemplate(SystemTemplate template) throws IOException { + final ComponentTemplate existingTemplate = clusterStateSupplier.get() + .metadata() + .componentTemplates() + .get(template.templateMetadata().fullyQualifiedName()); - XContentParser contentParser = JsonXContent.jsonXContent.createParser(NamedXContentRegistry.EMPTY, DeprecationHandler.IGNORE_DEPRECATIONS, - template.templateContent().utf8ToString()); - ComponentTemplate newTemplate = ComponentTemplate.parse(contentParser); + final XContentParser contentParser = JsonXContent.jsonXContent.createParser( + NamedXContentRegistry.EMPTY, + DeprecationHandler.IGNORE_DEPRECATIONS, + template.templateContent().utf8ToString() + ); + final ComponentTemplate newTemplate = ComponentTemplate.parse(contentParser); + + if (!Objects.equals(newTemplate.version(), template.templateMetadata().version())) { + throw new OpenSearchCorruptionException( + "Template version mismatch for " + + template.templateMetadata().name() + + ". Version in metadata: " + + template.templateMetadata().version() + + " , Version in content: " + + newTemplate.version() + ); + } + + if (existingTemplate != null + && !Boolean.parseBoolean(Objects.toString(existingTemplate.metadata().get(SystemTemplateMetadata.COMPONENT_TEMPLATE_TYPE)))) { + throw new OpenSearchCorruptionException( + "Attempting to create " + template.templateMetadata().name() + " which has already been created through some other source." + ); + } if (existingTemplate != null && existingTemplate.version() >= newTemplate.version()) { - logger.debug("Skipping putting template {} as its existing version [{}] is >= fetched version [{}]", template.templateInfo().fullyQualifiedName(), + logger.debug( + "Skipping putting template {} as its existing version [{}] is >= fetched version [{}]", + template.templateMetadata().fullyQualifiedName(), existingTemplate.version(), - newTemplate.version()); + newTemplate.version() + ); + return false; } - PutComponentTemplateAction.Request request = new PutComponentTemplateAction.Request(template.templateInfo().fullyQualifiedName()) - .componentTemplate(newTemplate); + final PutComponentTemplateAction.Request request = new PutComponentTemplateAction.Request( + template.templateMetadata().fullyQualifiedName() + ).componentTemplate(newTemplate); - client.admin().indices().execute(PutComponentTemplateAction.INSTANCE, request).actionGet(TimeValue.timeValueMillis(30000)); + return client.admin() + .indices() + .execute(PutComponentTemplateAction.INSTANCE, request) + .actionGet(TimeValue.timeValueMillis(30000)) + .isAcknowledged(); } } diff --git a/server/src/main/java/org/opensearch/cluster/applicationtemplates/SystemTemplate.java b/server/src/main/java/org/opensearch/cluster/applicationtemplates/SystemTemplate.java index 3d99b8597d300..e11ded7ef5546 100644 --- a/server/src/main/java/org/opensearch/cluster/applicationtemplates/SystemTemplate.java +++ b/server/src/main/java/org/opensearch/cluster/applicationtemplates/SystemTemplate.java @@ -19,25 +19,25 @@ public class SystemTemplate { private final BytesReference templateContent; - private final SystemTemplateMetadata templateInfo; + private final SystemTemplateMetadata templateMetadata; - private final TemplateRepositoryMetadata repositoryInfo; + private final TemplateRepositoryMetadata repositoryMetadata; public SystemTemplate(BytesReference templateContent, SystemTemplateMetadata templateInfo, TemplateRepositoryMetadata repositoryInfo) { this.templateContent = templateContent; - this.templateInfo = templateInfo; - this.repositoryInfo = repositoryInfo; + this.templateMetadata = templateInfo; + this.repositoryMetadata = repositoryInfo; } public BytesReference templateContent() { return templateContent; } - public SystemTemplateMetadata templateInfo() { - return templateInfo; + public SystemTemplateMetadata templateMetadata() { + return templateMetadata; } - public TemplateRepositoryMetadata repositoryInfo() { - return repositoryInfo; + public TemplateRepositoryMetadata repositoryMetadata() { + return repositoryMetadata; } } diff --git a/server/src/main/java/org/opensearch/cluster/applicationtemplates/SystemTemplateLoader.java b/server/src/main/java/org/opensearch/cluster/applicationtemplates/SystemTemplateLoader.java index 51fb3bbc2ce15..077580aed5a64 100644 --- a/server/src/main/java/org/opensearch/cluster/applicationtemplates/SystemTemplateLoader.java +++ b/server/src/main/java/org/opensearch/cluster/applicationtemplates/SystemTemplateLoader.java @@ -12,7 +12,6 @@ import java.io.IOException; - /** * Interface to load template into the OpenSearch runtime. */ @@ -23,5 +22,5 @@ public interface SystemTemplateLoader { * @param template Templated to be loaded * @throws IOException If an exceptional situation is encountered while parsing/loading the template */ - void loadTemplate(SystemTemplate template) throws IOException; + boolean loadTemplate(SystemTemplate template) throws IOException; } diff --git a/server/src/main/java/org/opensearch/cluster/applicationtemplates/SystemTemplateMetadata.java b/server/src/main/java/org/opensearch/cluster/applicationtemplates/SystemTemplateMetadata.java index 88c008c5422f1..19818acee2ecd 100644 --- a/server/src/main/java/org/opensearch/cluster/applicationtemplates/SystemTemplateMetadata.java +++ b/server/src/main/java/org/opensearch/cluster/applicationtemplates/SystemTemplateMetadata.java @@ -43,14 +43,21 @@ public long version() { } public static SystemTemplateMetadata fromComponentTemplate(String fullyQualifiedName) { - return new SystemTemplateMetadata(Long.parseLong(fullyQualifiedName.substring(fullyQualifiedName.lastIndexOf(DELIMITER))), COMPONENT_TEMPLATE_TYPE, fullyQualifiedName.substring(0, fullyQualifiedName.lastIndexOf(DELIMITER))); + assert fullyQualifiedName.length() > 1 : "System template name must have at least one component"; + assert fullyQualifiedName.substring(1, fullyQualifiedName.indexOf(DELIMITER, 1)).equals(COMPONENT_TEMPLATE_TYPE); + + return new SystemTemplateMetadata( + Long.parseLong(fullyQualifiedName.substring(fullyQualifiedName.lastIndexOf(DELIMITER))), + COMPONENT_TEMPLATE_TYPE, + fullyQualifiedName.substring(0, fullyQualifiedName.lastIndexOf(DELIMITER)) + ); } - public static SystemTemplateMetadata createComponentTemplateInfo(String name, long version) { + public static SystemTemplateMetadata fromComponentTemplateInfo(String name, long version) { return new SystemTemplateMetadata(version, COMPONENT_TEMPLATE_TYPE, name); } public final String fullyQualifiedName() { - return name + DELIMITER + version; + return type + DELIMITER + name + DELIMITER + version; } } diff --git a/server/src/main/java/org/opensearch/cluster/applicationtemplates/SystemTemplatesService.java b/server/src/main/java/org/opensearch/cluster/applicationtemplates/SystemTemplatesService.java index f0fb031ec4d7c..0a1f1887d730e 100644 --- a/server/src/main/java/org/opensearch/cluster/applicationtemplates/SystemTemplatesService.java +++ b/server/src/main/java/org/opensearch/cluster/applicationtemplates/SystemTemplatesService.java @@ -15,6 +15,8 @@ import org.opensearch.common.settings.ClusterSettings; import org.opensearch.common.settings.Setting; import org.opensearch.common.settings.Settings; +import org.opensearch.common.util.FeatureFlags; +import org.opensearch.threadpool.ThreadPool; import java.util.ArrayList; import java.util.List; @@ -26,17 +28,6 @@ @ExperimentalApi public class SystemTemplatesService implements LocalNodeClusterManagerListener { - public static final int APPLICATION_BASED_CONFIGURATION_TEMPLATES_LOAD_DEFAULT_COUNT = 50; - - public static final Setting SETTING_APPLICATION_BASED_CONFIGURATION_MAX_TEMPLATES_LOAD = Setting.intSetting( - "cluster.application_templates.max_load", - APPLICATION_BASED_CONFIGURATION_TEMPLATES_LOAD_DEFAULT_COUNT, - 0, - 1000, - Setting.Property.Dynamic, - Setting.Property.NodeScope - ); - public static final Setting SETTING_APPLICATION_BASED_CONFIGURATION_TEMPLATES_ENABLED = Setting.boolSetting( "cluster.application_templates.enabled", false, @@ -45,31 +36,31 @@ public class SystemTemplatesService implements LocalNodeClusterManagerListener { ); private final List systemTemplatesPluginList; - - private final Settings settings; + private final ThreadPool threadPool; private final AtomicBoolean loaded = new AtomicBoolean(false); - private volatile int templatesToLoad; - private volatile boolean enabledTemplates; + private volatile Stats latestStats; + private static final Logger logger = LogManager.getLogger(SystemTemplatesService.class); - public SystemTemplatesService(List systemTemplatesPluginList, - ClusterSettings clusterSettings, - Settings settings) { + public SystemTemplatesService( + List systemTemplatesPluginList, + ThreadPool threadPool, + ClusterSettings clusterSettings, + Settings settings + ) { this.systemTemplatesPluginList = systemTemplatesPluginList; - this.settings = settings; + this.threadPool = threadPool; setEnabledTemplates(settings.getAsBoolean(SETTING_APPLICATION_BASED_CONFIGURATION_TEMPLATES_ENABLED.getKey(), false)); - setMaxTemplatesToLoad(settings.getAsInt(SETTING_APPLICATION_BASED_CONFIGURATION_MAX_TEMPLATES_LOAD.getKey(), APPLICATION_BASED_CONFIGURATION_TEMPLATES_LOAD_DEFAULT_COUNT)); - clusterSettings.addSettingsUpdateConsumer(SETTING_APPLICATION_BASED_CONFIGURATION_MAX_TEMPLATES_LOAD, this::setMaxTemplatesToLoad); clusterSettings.addSettingsUpdateConsumer(SETTING_APPLICATION_BASED_CONFIGURATION_TEMPLATES_ENABLED, this::setEnabledTemplates); } @Override public void onClusterManager() { - refreshTemplates(false); + threadPool.generic().execute(() -> refreshTemplates(false)); } @Override @@ -81,68 +72,104 @@ public void verifyRepositories() { refreshTemplates(true); } - public void refreshTemplates(boolean verification) { - if (loaded.compareAndSet(false, true)) { - if (enabledTemplates && templatesToLoad > 0) { - int countOfTemplatesToLoad = templatesToLoad; - int templatesLoaded = 0; - int failedLoadingTemplates = 0; - int failedLoadingRepositories = 0; - - List exceptions = new ArrayList<>(); - for (SystemTemplatesPlugin plugin : systemTemplatesPluginList) { - try (SystemTemplateRepository repository = plugin.loadRepository()) { - TemplateRepositoryMetadata repositoryInfo = repository.metadata(); - logger.debug("Loading templates from repository: {} at version {}", repositoryInfo.id(), repositoryInfo.version()); - for (SystemTemplateMetadata templateInfo : repository.listTemplates()) { - - if (templatesLoaded == countOfTemplatesToLoad) { - logger.debug("Not loading template: {} from repository: {} as we've breached the max count [{}] allowed", - templateInfo.fullyQualifiedName(), repositoryInfo.id(), countOfTemplatesToLoad); - break; - } - try { - SystemTemplate template = repository.getTemplate(templateInfo); - - // Load plugin if not in verification phase. - if (!verification) { - plugin.loaderFor(templateInfo).loadTemplate(template); - } - countOfTemplatesToLoad--; - } catch (Exception ex) { - exceptions.add(ex); - logger.error("Failed loading template {} from repository: {}", - templateInfo.fullyQualifiedName(), repositoryInfo.id(), ex); - failedLoadingTemplates++; + public Stats stats() { + return latestStats; + } + + void refreshTemplates(boolean verification) { + int templatesLoaded = 0; + int failedLoadingTemplates = 0; + int failedLoadingRepositories = 0; + List exceptions = new ArrayList<>(); + + if (loaded.compareAndSet(false, true) && enabledTemplates) { + for (SystemTemplatesPlugin plugin : systemTemplatesPluginList) { + try (SystemTemplateRepository repository = plugin.loadRepository()) { + + final TemplateRepositoryMetadata repositoryMetadata = repository.metadata(); + logger.debug( + "Loading templates from repository: {} at version {}", + repositoryMetadata.id(), + repositoryMetadata.version() + ); + + for (SystemTemplateMetadata templateMetadata : repository.listTemplates()) { + try { + final SystemTemplate template = repository.getTemplate(templateMetadata); + + // Load plugin if not in verification phase. + if (!verification && plugin.loaderFor(templateMetadata).loadTemplate(template)) { + templatesLoaded++; } - } - if (templatesLoaded == countOfTemplatesToLoad) { - logger.debug("Loaded maximum permitted templates"); - break; + } catch (Exception ex) { + exceptions.add(ex); + logger.error( + "Failed loading template {} from repository: {}", + templateMetadata.fullyQualifiedName(), + repositoryMetadata.id(), + ex + ); + failedLoadingTemplates++; } - } catch (Exception ex) { - exceptions.add(ex); - failedLoadingRepositories++; - logger.error("Failed loading repository from plugin: {}", plugin.getClass().getName(), ex); } + } catch (Exception ex) { + exceptions.add(ex); + failedLoadingRepositories++; + logger.error("Failed loading repository from plugin: {}", plugin.getClass().getName(), ex); } + } - logger.debug("Stats: Total Loaded Templates: [{}], Failed Loading Templates: [{}], Failed Loading Repositories: [{}]", - templatesLoaded, failedLoadingTemplates, failedLoadingRepositories); - - if (verification) { - throw new IllegalStateException("Some of the repositories could not be loaded or are corrupted: " + exceptions); - } + logger.debug( + "Stats: Total Loaded Templates: [{}], Failed Loading Templates: [{}], Failed Loading Repositories: [{}]", + templatesLoaded, + failedLoadingTemplates, + failedLoadingRepositories + ); + + // End exceptionally if invoked in verification context + if (verification && (failedLoadingRepositories > 0 || failedLoadingTemplates > 0)) { + latestStats = new Stats(templatesLoaded, failedLoadingTemplates, failedLoadingRepositories); + throw new IllegalStateException("Some of the repositories could not be loaded or are corrupted: " + exceptions); } } - } - private void setMaxTemplatesToLoad(int templatesToLoad) { - this.templatesToLoad = templatesToLoad; + latestStats = new Stats(templatesLoaded, failedLoadingTemplates, failedLoadingRepositories); } private void setEnabledTemplates(boolean enabled) { + if (!FeatureFlags.isEnabled(FeatureFlags.APPLICATION_BASED_CONFIGURATION_TEMPLATES_SETTING)) { + throw new IllegalArgumentException( + "Application Based Configuration Templates is under an experimental feature and can be activated only by enabling " + + FeatureFlags.APPLICATION_BASED_CONFIGURATION_TEMPLATES_SETTING.getKey() + + " feature flag." + ); + } enabledTemplates = enabled; } + + @ExperimentalApi + public static class Stats { + private final long templatesLoaded; + private final long failedLoadingTemplates; + private final long failedLoadingRepositories; + + public Stats(long templatesLoaded, long failedLoadingTemplates, long failedLoadingRepositories) { + this.templatesLoaded = templatesLoaded; + this.failedLoadingTemplates = failedLoadingTemplates; + this.failedLoadingRepositories = failedLoadingRepositories; + } + + public long getTemplatesLoaded() { + return templatesLoaded; + } + + public long getFailedLoadingTemplates() { + return failedLoadingTemplates; + } + + public long getFailedLoadingRepositories() { + return failedLoadingRepositories; + } + } } diff --git a/server/src/main/java/org/opensearch/cluster/metadata/MetadataIndexTemplateService.java b/server/src/main/java/org/opensearch/cluster/metadata/MetadataIndexTemplateService.java index 76096de7d67dc..5b03d3f7b19ce 100644 --- a/server/src/main/java/org/opensearch/cluster/metadata/MetadataIndexTemplateService.java +++ b/server/src/main/java/org/opensearch/cluster/metadata/MetadataIndexTemplateService.java @@ -45,7 +45,6 @@ import org.opensearch.cluster.service.ClusterManagerTaskKeys; import org.opensearch.cluster.service.ClusterManagerTaskThrottler; import org.opensearch.cluster.service.ClusterService; -import org.opensearch.cluster.applicationtemplates.ClusterStateComponentSystemTemplateLoader; import org.opensearch.common.Nullable; import org.opensearch.common.Priority; import org.opensearch.common.UUIDs; @@ -57,7 +56,6 @@ import org.opensearch.common.settings.IndexScopedSettings; import org.opensearch.common.settings.Settings; import org.opensearch.common.unit.TimeValue; -import org.opensearch.common.util.concurrent.ThreadContext; import org.opensearch.common.util.set.Sets; import org.opensearch.common.xcontent.XContentFactory; import org.opensearch.core.action.ActionListener; @@ -74,7 +72,6 @@ import org.opensearch.indices.IndexTemplateMissingException; import org.opensearch.indices.IndicesService; import org.opensearch.indices.InvalidIndexTemplateException; -import org.opensearch.threadpool.ThreadPool; import java.io.IOException; import java.io.UncheckedIOException; @@ -113,7 +110,6 @@ public class MetadataIndexTemplateService { private final MetadataCreateIndexService metadataCreateIndexService; private final IndexScopedSettings indexScopedSettings; private final NamedXContentRegistry xContentRegistry; - private final ThreadPool threadPool; private final ClusterManagerTaskThrottler.ThrottlingKey createIndexTemplateTaskKey; private final ClusterManagerTaskThrottler.ThrottlingKey createIndexTemplateV2TaskKey; private final ClusterManagerTaskThrottler.ThrottlingKey removeIndexTemplateTaskKey; @@ -128,8 +124,7 @@ public MetadataIndexTemplateService( AliasValidator aliasValidator, IndicesService indicesService, IndexScopedSettings indexScopedSettings, - NamedXContentRegistry xContentRegistry, - ThreadPool threadPool + NamedXContentRegistry xContentRegistry ) { this.clusterService = clusterService; this.aliasValidator = aliasValidator; @@ -137,7 +132,6 @@ public MetadataIndexTemplateService( this.metadataCreateIndexService = metadataCreateIndexService; this.indexScopedSettings = indexScopedSettings; this.xContentRegistry = xContentRegistry; - this.threadPool = threadPool; // Task is onboarded for throttling, it will get retried from associated TransportClusterManagerNodeAction. createIndexTemplateTaskKey = clusterService.registerClusterManagerTask(ClusterManagerTaskKeys.CREATE_INDEX_TEMPLATE_KEY, true); @@ -259,13 +253,6 @@ ClusterState addComponentTemplate( throw new IllegalArgumentException("component template [" + name + "] already exists"); } - if (isSystemComponentTemplate(template) - && ClusterStateComponentSystemTemplateLoader.TEMPLATE_LOADER_IDENTIFIER.equals( - threadPool.getThreadContext().getTransient(ThreadContext.ACTION_ORIGIN_TRANSIENT_NAME))) { - throw new IllegalArgumentException("Users are not permitted to create a system template. " + - "Should be loaded through a repository only."); - }; - CompressedXContent mappings = template.template().mappings(); String stringMappings = mappings == null ? null : mappings.string(); @@ -412,8 +399,7 @@ public ClusterManagerTaskThrottler.ThrottlingKey getClusterManagerThrottlingKey( public ClusterState execute(ClusterState currentState) { Set templateNames = new HashSet<>(); for (String templateName : currentState.metadata().componentTemplates().keySet()) { - if (Regex.simpleMatch(name, templateName) && - !isSystemComponentTemplate(currentState.metadata().componentTemplates().get(templateName))) { + if (Regex.simpleMatch(name, templateName)) { templateNames.add(templateName); } } @@ -426,7 +412,6 @@ public ClusterState execute(ClusterState currentState) { // TODO: perhaps introduce a ComponentTemplateMissingException? throw new IndexTemplateMissingException(name); } - Metadata.Builder metadata = Metadata.builder(currentState.metadata()); for (String templateName : templateNames) { logger.info("removing component template [{}]", templateName); @@ -1573,10 +1558,6 @@ private void validate(String name, @Nullable Settings settings, List ind } } - private boolean isSystemComponentTemplate(ComponentTemplate template) { - return true; - } - /** * Listener for putting metadata in the template * diff --git a/server/src/main/java/org/opensearch/common/settings/ClusterSettings.java b/server/src/main/java/org/opensearch/common/settings/ClusterSettings.java index a4230f42617f0..3ecf88ba6082a 100644 --- a/server/src/main/java/org/opensearch/common/settings/ClusterSettings.java +++ b/server/src/main/java/org/opensearch/common/settings/ClusterSettings.java @@ -49,6 +49,7 @@ import org.opensearch.cluster.NodeConnectionsService; import org.opensearch.cluster.action.index.MappingUpdatedAction; import org.opensearch.cluster.action.shard.ShardStateAction; +import org.opensearch.cluster.applicationtemplates.SystemTemplatesService; import org.opensearch.cluster.coordination.ClusterBootstrapService; import org.opensearch.cluster.coordination.ClusterFormationFailureHelper; import org.opensearch.cluster.coordination.Coordinator; @@ -82,7 +83,6 @@ import org.opensearch.cluster.service.ClusterManagerService; import org.opensearch.cluster.service.ClusterManagerTaskThrottler; import org.opensearch.cluster.service.ClusterService; -import org.opensearch.cluster.applicationtemplates.SystemTemplatesService; import org.opensearch.common.annotation.PublicApi; import org.opensearch.common.cache.CacheType; import org.opensearch.common.cache.settings.CacheSettings; @@ -761,7 +761,6 @@ public void apply(Settings value, Settings current, Settings previous) { // Composite index settings CompositeIndexSettings.STAR_TREE_INDEX_ENABLED_SETTING, - SystemTemplatesService.SETTING_APPLICATION_BASED_CONFIGURATION_MAX_TEMPLATES_LOAD, SystemTemplatesService.SETTING_APPLICATION_BASED_CONFIGURATION_TEMPLATES_ENABLED ) ) diff --git a/server/src/main/java/org/opensearch/common/settings/FeatureFlagSettings.java b/server/src/main/java/org/opensearch/common/settings/FeatureFlagSettings.java index b6166f5d3cce1..d893d8d92be3b 100644 --- a/server/src/main/java/org/opensearch/common/settings/FeatureFlagSettings.java +++ b/server/src/main/java/org/opensearch/common/settings/FeatureFlagSettings.java @@ -38,6 +38,7 @@ protected FeatureFlagSettings( FeatureFlags.REMOTE_STORE_MIGRATION_EXPERIMENTAL_SETTING, FeatureFlags.PLUGGABLE_CACHE_SETTING, FeatureFlags.REMOTE_PUBLICATION_EXPERIMENTAL_SETTING, - FeatureFlags.STAR_TREE_INDEX_SETTING + FeatureFlags.STAR_TREE_INDEX_SETTING, + FeatureFlags.APPLICATION_BASED_CONFIGURATION_TEMPLATES_SETTING ); } diff --git a/server/src/main/java/org/opensearch/common/util/FeatureFlags.java b/server/src/main/java/org/opensearch/common/util/FeatureFlags.java index ceb2559a0e16c..9d57e6939e3ae 100644 --- a/server/src/main/java/org/opensearch/common/util/FeatureFlags.java +++ b/server/src/main/java/org/opensearch/common/util/FeatureFlags.java @@ -107,6 +107,16 @@ public class FeatureFlags { public static final String STAR_TREE_INDEX = "opensearch.experimental.feature.composite_index.star_tree.enabled"; public static final Setting STAR_TREE_INDEX_SETTING = Setting.boolSetting(STAR_TREE_INDEX, false, Property.NodeScope); + /** + * Gates the functionality of application based configuration templates. + */ + public static final String APPLICATION_BASED_CONFIGURATION_TEMPLATES = "opensearch.experimental.feature.application_templates.enabled"; + public static final Setting APPLICATION_BASED_CONFIGURATION_TEMPLATES_SETTING = Setting.boolSetting( + APPLICATION_BASED_CONFIGURATION_TEMPLATES, + false, + Property.NodeScope + ); + private static final List> ALL_FEATURE_FLAG_SETTINGS = List.of( REMOTE_STORE_MIGRATION_EXPERIMENTAL_SETTING, EXTENSIONS_SETTING, @@ -116,8 +126,10 @@ public class FeatureFlags { TIERED_REMOTE_INDEX_SETTING, PLUGGABLE_CACHE_SETTING, REMOTE_PUBLICATION_EXPERIMENTAL_SETTING, - STAR_TREE_INDEX_SETTING + STAR_TREE_INDEX_SETTING, + APPLICATION_BASED_CONFIGURATION_TEMPLATES_SETTING ); + /** * Should store the settings from opensearch.yml. */ diff --git a/server/src/main/java/org/opensearch/node/Node.java b/server/src/main/java/org/opensearch/node/Node.java index a6aba8744e84a..971c70eaf368c 100644 --- a/server/src/main/java/org/opensearch/node/Node.java +++ b/server/src/main/java/org/opensearch/node/Node.java @@ -68,6 +68,8 @@ import org.opensearch.cluster.InternalClusterInfoService; import org.opensearch.cluster.NodeConnectionsService; import org.opensearch.cluster.action.index.MappingUpdatedAction; +import org.opensearch.cluster.applicationtemplates.SystemTemplatesPlugin; +import org.opensearch.cluster.applicationtemplates.SystemTemplatesService; import org.opensearch.cluster.coordination.PersistedStateRegistry; import org.opensearch.cluster.metadata.AliasValidator; import org.opensearch.cluster.metadata.IndexTemplateMetadata; @@ -84,8 +86,6 @@ import org.opensearch.cluster.routing.allocation.AwarenessReplicaBalance; import org.opensearch.cluster.routing.allocation.DiskThresholdMonitor; import org.opensearch.cluster.service.ClusterService; -import org.opensearch.cluster.applicationtemplates.SystemTemplatesPlugin; -import org.opensearch.cluster.applicationtemplates.SystemTemplatesService; import org.opensearch.common.SetOnce; import org.opensearch.common.StopWatch; import org.opensearch.common.cache.module.CacheModule; @@ -676,8 +676,12 @@ protected Node( ); } - SystemTemplatesService systemTemplatesService = new SystemTemplatesService(pluginsService.filterPlugins(SystemTemplatesPlugin.class), - clusterService.getClusterSettings(), settings); + SystemTemplatesService systemTemplatesService = new SystemTemplatesService( + pluginsService.filterPlugins(SystemTemplatesPlugin.class), + threadPool, + clusterService.getClusterSettings(), + settings + ); systemTemplatesService.verifyRepositories(); clusterService.addLocalNodeClusterManagerListener(systemTemplatesService); diff --git a/server/src/test/java/org/opensearch/cluster/applicationtemplates/ClusterStateComponentSystemTemplateLoaderTest.java b/server/src/test/java/org/opensearch/cluster/applicationtemplates/ClusterStateComponentSystemTemplateLoaderTest.java new file mode 100644 index 0000000000000..343dd88d3289b --- /dev/null +++ b/server/src/test/java/org/opensearch/cluster/applicationtemplates/ClusterStateComponentSystemTemplateLoaderTest.java @@ -0,0 +1,148 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.cluster.applicationtemplates; + +import org.opensearch.OpenSearchCorruptionException; +import org.opensearch.cluster.metadata.ComponentTemplate; +import org.opensearch.cluster.service.ClusterService; +import org.opensearch.common.xcontent.json.JsonXContent; +import org.opensearch.core.common.bytes.BytesArray; +import org.opensearch.core.xcontent.DeprecationHandler; +import org.opensearch.core.xcontent.NamedXContentRegistry; +import org.opensearch.core.xcontent.XContentParser; +import org.opensearch.test.OpenSearchSingleNodeTestCase; + +import java.io.IOException; +import java.util.UUID; + +public class ClusterStateComponentSystemTemplateLoaderTest extends OpenSearchSingleNodeTestCase { + + public static final String SAMPLE_TEMPLATE = "{\n" + + " \"template\": {\n" + + " \"settings\": {\n" + + " \"index\": {\n" + + " \"codec\": \"best_compression\",\n" + + " \"merge.policy\": \"log_byte_size\",\n" + + " \"refresh_interval\": \"60s\"\n" + + " }\n" + + " }\n" + + " },\n" + + " \"_meta\": {\n" + + " \"@abc_template\": true,\n" + + " \"_version\": 1\n" + + " },\n" + + " \"version\": 1\n" + + "}"; + + public static final String SAMPLE_TEMPLATE_V2 = "{\n" + + " \"template\": {\n" + + " \"settings\": {\n" + + " \"index\": {\n" + + " \"codec\": \"best_compression\",\n" + + " \"merge.policy\": \"log_byte_size\",\n" + + " \"refresh_interval\": \"60s\"\n" + + " }\n" + + " }\n" + + " },\n" + + " \"_meta\": {\n" + + " \"@abc_template\": true,\n" + + " \"_version\": 2\n" + + " },\n" + + " \"version\": 2\n" + + "}"; + + public void testLoadTemplate() throws IOException { + ClusterStateComponentSystemTemplateLoader loader = new ClusterStateComponentSystemTemplateLoader( + node().client(), + () -> node().injector().getInstance(ClusterService.class).state() + ); + + TemplateRepositoryMetadata repositoryMetadata = new TemplateRepositoryMetadata(UUID.randomUUID().toString(), 1L); + SystemTemplateMetadata metadata = SystemTemplateMetadata.fromComponentTemplateInfo("dummy", 1L); + + // Load for the first time + assertTrue( + loader.loadTemplate( + new SystemTemplate( + new BytesArray(SAMPLE_TEMPLATE), + metadata, + new TemplateRepositoryMetadata(UUID.randomUUID().toString(), 1L) + ) + ) + ); + assertTrue( + node().injector() + .getInstance(ClusterService.class) + .state() + .metadata() + .componentTemplates() + .containsKey(metadata.fullyQualifiedName()) + ); + XContentParser parser = JsonXContent.jsonXContent.createParser( + NamedXContentRegistry.EMPTY, + DeprecationHandler.IGNORE_DEPRECATIONS, + SAMPLE_TEMPLATE + ); + assertEquals( + node().injector().getInstance(ClusterService.class).state().metadata().componentTemplates().get(metadata.fullyQualifiedName()), + ComponentTemplate.parse(parser) + ); + + // Retry and ensure loading does not happen again with same version + assertFalse( + loader.loadTemplate( + new SystemTemplate( + new BytesArray(SAMPLE_TEMPLATE), + metadata, + new TemplateRepositoryMetadata(UUID.randomUUID().toString(), 1L) + ) + ) + ); + + // Retry with new template version + SystemTemplateMetadata newVersionMetadata = SystemTemplateMetadata.fromComponentTemplateInfo("dummy", 2L); + assertTrue(loader.loadTemplate(new SystemTemplate(new BytesArray(SAMPLE_TEMPLATE_V2), newVersionMetadata, repositoryMetadata))); + parser = JsonXContent.jsonXContent.createParser( + NamedXContentRegistry.EMPTY, + DeprecationHandler.IGNORE_DEPRECATIONS, + SAMPLE_TEMPLATE_V2 + ); + assertEquals( + node().injector() + .getInstance(ClusterService.class) + .state() + .metadata() + .componentTemplates() + .get(newVersionMetadata.fullyQualifiedName()), + ComponentTemplate.parse(parser) + ); + } + + public void testLoadTemplateVersionMismatch() throws IOException { + ClusterStateComponentSystemTemplateLoader loader = new ClusterStateComponentSystemTemplateLoader( + node().client(), + () -> node().injector().getInstance(ClusterService.class).state() + ); + + TemplateRepositoryMetadata repositoryMetadata = new TemplateRepositoryMetadata(UUID.randomUUID().toString(), 1L); + SystemTemplateMetadata metadata = SystemTemplateMetadata.fromComponentTemplateInfo("dummy", 2L); + + // Load for the first time + assertThrows( + OpenSearchCorruptionException.class, + () -> loader.loadTemplate( + new SystemTemplate( + new BytesArray(SAMPLE_TEMPLATE), + metadata, + new TemplateRepositoryMetadata(UUID.randomUUID().toString(), 1L) + ) + ) + ); + } +} diff --git a/server/src/test/java/org/opensearch/cluster/applicationtemplates/SystemTemplatesServiceTest.java b/server/src/test/java/org/opensearch/cluster/applicationtemplates/SystemTemplatesServiceTest.java new file mode 100644 index 0000000000000..16f49020629df --- /dev/null +++ b/server/src/test/java/org/opensearch/cluster/applicationtemplates/SystemTemplatesServiceTest.java @@ -0,0 +1,90 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.cluster.applicationtemplates; + +import org.opensearch.cluster.service.applicationtemplates.TestSystemTemplatesRepositoryPlugin; +import org.opensearch.common.settings.ClusterSettings; +import org.opensearch.common.settings.Settings; +import org.opensearch.common.util.FeatureFlags; +import org.opensearch.common.util.concurrent.OpenSearchExecutors; +import org.opensearch.test.OpenSearchTestCase; +import org.opensearch.threadpool.ThreadPool; + +import java.io.IOException; +import java.util.ArrayList; +import java.util.List; + +import org.mockito.Mockito; + +import static org.opensearch.common.settings.ClusterSettings.BUILT_IN_CLUSTER_SETTINGS; +import static org.mockito.Mockito.when; + +public class SystemTemplatesServiceTest extends OpenSearchTestCase { + + private SystemTemplatesService systemTemplatesService; + + public void testSystemTemplatesLoaded() throws IOException { + setupService(true); + + systemTemplatesService.onClusterManager(); + SystemTemplatesService.Stats stats = systemTemplatesService.stats(); + assertNotNull(stats); + assertEquals(stats.getTemplatesLoaded(), 1L); + assertEquals(stats.getFailedLoadingTemplates(), 0L); + assertEquals(stats.getFailedLoadingRepositories(), 1L); + } + + public void testSystemTemplatesVerify() throws IOException { + setupService(false); + + systemTemplatesService.verifyRepositories(); + + SystemTemplatesService.Stats stats = systemTemplatesService.stats(); + assertNotNull(stats); + assertEquals(stats.getTemplatesLoaded(), 0L); + assertEquals(stats.getFailedLoadingTemplates(), 0L); + assertEquals(stats.getFailedLoadingRepositories(), 0L); + } + + public void testSystemTemplatesVerifyWithFailingRepository() throws IOException { + setupService(true); + + assertThrows(IllegalStateException.class, () -> systemTemplatesService.verifyRepositories()); + + SystemTemplatesService.Stats stats = systemTemplatesService.stats(); + assertNotNull(stats); + assertEquals(stats.getTemplatesLoaded(), 0L); + assertEquals(stats.getFailedLoadingTemplates(), 0L); + assertEquals(stats.getFailedLoadingRepositories(), 1L); + } + + void setupService(boolean errorFromMockPlugin) throws IOException { + FeatureFlags.initializeFeatureFlags(Settings.builder().put(FeatureFlags.APPLICATION_BASED_CONFIGURATION_TEMPLATES, true).build()); + + ThreadPool mockPool = Mockito.mock(ThreadPool.class); + when(mockPool.generic()).thenReturn(OpenSearchExecutors.newDirectExecutorService()); + + List plugins = new ArrayList<>(); + plugins.add(new TestSystemTemplatesRepositoryPlugin()); + + if (errorFromMockPlugin) { + SystemTemplatesPlugin mockPlugin = Mockito.mock(SystemTemplatesPlugin.class); + when(mockPlugin.loadRepository()).thenThrow(new IOException()); + plugins.add(mockPlugin); + } + + ClusterSettings mockSettings = new ClusterSettings(Settings.EMPTY, BUILT_IN_CLUSTER_SETTINGS); + systemTemplatesService = new SystemTemplatesService( + plugins, + mockPool, + mockSettings, + Settings.builder().put(SystemTemplatesService.SETTING_APPLICATION_BASED_CONFIGURATION_TEMPLATES_ENABLED.getKey(), true).build() + ); + } +} diff --git a/test/framework/src/main/java/org/opensearch/cluster/service/applicationtemplates/TestSystemTemplatesRepositoryPlugin.java b/test/framework/src/main/java/org/opensearch/cluster/service/applicationtemplates/TestSystemTemplatesRepositoryPlugin.java index fd473ec18ccfc..c5245c7109d8f 100644 --- a/test/framework/src/main/java/org/opensearch/cluster/service/applicationtemplates/TestSystemTemplatesRepositoryPlugin.java +++ b/test/framework/src/main/java/org/opensearch/cluster/service/applicationtemplates/TestSystemTemplatesRepositoryPlugin.java @@ -8,7 +8,12 @@ package org.opensearch.cluster.service.applicationtemplates; -import org.opensearch.cluster.applicationtemplates.*; +import org.opensearch.cluster.applicationtemplates.SystemTemplate; +import org.opensearch.cluster.applicationtemplates.SystemTemplateLoader; +import org.opensearch.cluster.applicationtemplates.SystemTemplateMetadata; +import org.opensearch.cluster.applicationtemplates.SystemTemplateRepository; +import org.opensearch.cluster.applicationtemplates.SystemTemplatesPlugin; +import org.opensearch.cluster.applicationtemplates.TemplateRepositoryMetadata; import org.opensearch.core.common.bytes.BytesReference; import org.opensearch.plugins.Plugin; @@ -19,23 +24,27 @@ public class TestSystemTemplatesRepositoryPlugin extends Plugin implements SystemTemplatesPlugin { - private SystemTemplateMetadata info = SystemTemplateMetadata.createComponentTemplateInfo("dummy", 1); + private final SystemTemplateMetadata templateMetadata = SystemTemplateMetadata.fromComponentTemplateInfo("dummy", 1); - private TemplateRepositoryMetadata repoInfo = new TemplateRepositoryMetadata("test", 1); + private final TemplateRepositoryMetadata repoMetadata = new TemplateRepositoryMetadata("test", 1); - private SystemTemplate systemTemplate = new SystemTemplate(BytesReference.fromByteBuffer(ByteBuffer.wrap("content".getBytes(StandardCharsets.UTF_8))), info, repoInfo); + private final SystemTemplate systemTemplate = new SystemTemplate( + BytesReference.fromByteBuffer(ByteBuffer.wrap("content".getBytes(StandardCharsets.UTF_8))), + templateMetadata, + repoMetadata + ); @Override public SystemTemplateRepository loadRepository() throws IOException { return new SystemTemplateRepository() { @Override public TemplateRepositoryMetadata metadata() { - return repoInfo; + return repoMetadata; } @Override public List listTemplates() throws IOException { - return List.of(info); + return List.of(templateMetadata); } @Override @@ -44,19 +53,19 @@ public SystemTemplate getTemplate(SystemTemplateMetadata template) throws IOExce } @Override - public void close() throws Exception { - } + public void close() throws Exception {} }; } @Override - public SystemTemplateLoader loaderFor(SystemTemplateMetadata templateInfo) { + public SystemTemplateLoader loaderFor(SystemTemplateMetadata templateMetadata) { return new SystemTemplateLoader() { // Asserting Loader @Override - public void loadTemplate(SystemTemplate template) throws IOException { - assert template.templateInfo() == info; - assert template.repositoryInfo() == repoInfo; - assert template.templateContent() == systemTemplate; + public boolean loadTemplate(SystemTemplate template) throws IOException { + assert template.templateMetadata() == TestSystemTemplatesRepositoryPlugin.this.templateMetadata; + assert template.repositoryMetadata() == repoMetadata; + assert template.templateContent() == systemTemplate.templateContent(); + return true; } }; } diff --git a/test/framework/src/main/java/org/opensearch/test/OpenSearchSingleNodeTestCase.java b/test/framework/src/main/java/org/opensearch/test/OpenSearchSingleNodeTestCase.java index 45ea63e862df6..1dfad60c04155 100644 --- a/test/framework/src/main/java/org/opensearch/test/OpenSearchSingleNodeTestCase.java +++ b/test/framework/src/main/java/org/opensearch/test/OpenSearchSingleNodeTestCase.java @@ -438,6 +438,7 @@ protected Settings featureFlagSettings() { featureSettings.put(builtInFlag.getKey(), builtInFlag.getDefaultRaw(Settings.EMPTY)); } featureSettings.put(FeatureFlags.TELEMETRY_SETTING.getKey(), true); + featureSettings.put(FeatureFlags.APPLICATION_BASED_CONFIGURATION_TEMPLATES_SETTING.getKey(), true); return featureSettings.build(); } From d7b35dee693c7cf172c42ec10d91a31310070cff Mon Sep 17 00:00:00 2001 From: Mohit Godwani Date: Mon, 15 Jul 2024 16:22:41 +0530 Subject: [PATCH 07/10] Fix precommit Signed-off-by: Mohit Godwani --- .../SystemTemplatesService.java | 14 ++++++++++---- ...erStateComponentSystemTemplateLoaderTests.java} | 2 +- ...eTest.java => SystemTemplatesServiceTests.java} | 2 +- 3 files changed, 12 insertions(+), 6 deletions(-) rename server/src/test/java/org/opensearch/cluster/applicationtemplates/{ClusterStateComponentSystemTemplateLoaderTest.java => ClusterStateComponentSystemTemplateLoaderTests.java} (98%) rename server/src/test/java/org/opensearch/cluster/applicationtemplates/{SystemTemplatesServiceTest.java => SystemTemplatesServiceTests.java} (98%) diff --git a/server/src/main/java/org/opensearch/cluster/applicationtemplates/SystemTemplatesService.java b/server/src/main/java/org/opensearch/cluster/applicationtemplates/SystemTemplatesService.java index 0a1f1887d730e..6393816212a4b 100644 --- a/server/src/main/java/org/opensearch/cluster/applicationtemplates/SystemTemplatesService.java +++ b/server/src/main/java/org/opensearch/cluster/applicationtemplates/SystemTemplatesService.java @@ -10,6 +10,7 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; +import org.apache.logging.log4j.message.ParameterizedMessage; import org.opensearch.cluster.LocalNodeClusterManagerListener; import org.opensearch.common.annotation.ExperimentalApi; import org.opensearch.common.settings.ClusterSettings; @@ -105,9 +106,11 @@ void refreshTemplates(boolean verification) { } catch (Exception ex) { exceptions.add(ex); logger.error( - "Failed loading template {} from repository: {}", - templateMetadata.fullyQualifiedName(), - repositoryMetadata.id(), + new ParameterizedMessage( + "Failed loading template {} from repository: {}", + templateMetadata.fullyQualifiedName(), + repositoryMetadata.id() + ), ex ); failedLoadingTemplates++; @@ -116,7 +119,7 @@ void refreshTemplates(boolean verification) { } catch (Exception ex) { exceptions.add(ex); failedLoadingRepositories++; - logger.error("Failed loading repository from plugin: {}", plugin.getClass().getName(), ex); + logger.error(new ParameterizedMessage("Failed loading repository from plugin: {}", plugin.getClass().getName()), ex); } } @@ -148,6 +151,9 @@ private void setEnabledTemplates(boolean enabled) { enabledTemplates = enabled; } + /** + * Class to record stats for templates loaded through the listener in a single iteration. + */ @ExperimentalApi public static class Stats { private final long templatesLoaded; diff --git a/server/src/test/java/org/opensearch/cluster/applicationtemplates/ClusterStateComponentSystemTemplateLoaderTest.java b/server/src/test/java/org/opensearch/cluster/applicationtemplates/ClusterStateComponentSystemTemplateLoaderTests.java similarity index 98% rename from server/src/test/java/org/opensearch/cluster/applicationtemplates/ClusterStateComponentSystemTemplateLoaderTest.java rename to server/src/test/java/org/opensearch/cluster/applicationtemplates/ClusterStateComponentSystemTemplateLoaderTests.java index 343dd88d3289b..8e777a2ecf9c9 100644 --- a/server/src/test/java/org/opensearch/cluster/applicationtemplates/ClusterStateComponentSystemTemplateLoaderTest.java +++ b/server/src/test/java/org/opensearch/cluster/applicationtemplates/ClusterStateComponentSystemTemplateLoaderTests.java @@ -21,7 +21,7 @@ import java.io.IOException; import java.util.UUID; -public class ClusterStateComponentSystemTemplateLoaderTest extends OpenSearchSingleNodeTestCase { +public class ClusterStateComponentSystemTemplateLoaderTests extends OpenSearchSingleNodeTestCase { public static final String SAMPLE_TEMPLATE = "{\n" + " \"template\": {\n" diff --git a/server/src/test/java/org/opensearch/cluster/applicationtemplates/SystemTemplatesServiceTest.java b/server/src/test/java/org/opensearch/cluster/applicationtemplates/SystemTemplatesServiceTests.java similarity index 98% rename from server/src/test/java/org/opensearch/cluster/applicationtemplates/SystemTemplatesServiceTest.java rename to server/src/test/java/org/opensearch/cluster/applicationtemplates/SystemTemplatesServiceTests.java index 16f49020629df..4addf3802b40d 100644 --- a/server/src/test/java/org/opensearch/cluster/applicationtemplates/SystemTemplatesServiceTest.java +++ b/server/src/test/java/org/opensearch/cluster/applicationtemplates/SystemTemplatesServiceTests.java @@ -25,7 +25,7 @@ import static org.opensearch.common.settings.ClusterSettings.BUILT_IN_CLUSTER_SETTINGS; import static org.mockito.Mockito.when; -public class SystemTemplatesServiceTest extends OpenSearchTestCase { +public class SystemTemplatesServiceTests extends OpenSearchTestCase { private SystemTemplatesService systemTemplatesService; From 3a9000932d8df6359e7289de2eafa64d92ab0138 Mon Sep 17 00:00:00 2001 From: Mohit Godwani Date: Mon, 15 Jul 2024 16:58:52 +0530 Subject: [PATCH 08/10] Fix node bootup Signed-off-by: Mohit Godwani --- .../cluster/applicationtemplates/SystemTemplatesService.java | 4 +++- .../java/org/opensearch/test/OpenSearchIntegTestCase.java | 1 + 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/server/src/main/java/org/opensearch/cluster/applicationtemplates/SystemTemplatesService.java b/server/src/main/java/org/opensearch/cluster/applicationtemplates/SystemTemplatesService.java index 6393816212a4b..ccb9272fa57b1 100644 --- a/server/src/main/java/org/opensearch/cluster/applicationtemplates/SystemTemplatesService.java +++ b/server/src/main/java/org/opensearch/cluster/applicationtemplates/SystemTemplatesService.java @@ -55,7 +55,9 @@ public SystemTemplatesService( ) { this.systemTemplatesPluginList = systemTemplatesPluginList; this.threadPool = threadPool; - setEnabledTemplates(settings.getAsBoolean(SETTING_APPLICATION_BASED_CONFIGURATION_TEMPLATES_ENABLED.getKey(), false)); + if (settings.getAsBoolean(SETTING_APPLICATION_BASED_CONFIGURATION_TEMPLATES_ENABLED.getKey(), false)) { + setEnabledTemplates(settings.getAsBoolean(SETTING_APPLICATION_BASED_CONFIGURATION_TEMPLATES_ENABLED.getKey(), false)); + } clusterSettings.addSettingsUpdateConsumer(SETTING_APPLICATION_BASED_CONFIGURATION_TEMPLATES_ENABLED, this::setEnabledTemplates); } diff --git a/test/framework/src/main/java/org/opensearch/test/OpenSearchIntegTestCase.java b/test/framework/src/main/java/org/opensearch/test/OpenSearchIntegTestCase.java index 2955c988a12c6..7a50502e418e2 100644 --- a/test/framework/src/main/java/org/opensearch/test/OpenSearchIntegTestCase.java +++ b/test/framework/src/main/java/org/opensearch/test/OpenSearchIntegTestCase.java @@ -683,6 +683,7 @@ protected Settings featureFlagSettings() { } // Enabling Telemetry setting by default featureSettings.put(FeatureFlags.TELEMETRY_SETTING.getKey(), true); + featureSettings.put(FeatureFlags.APPLICATION_BASED_CONFIGURATION_TEMPLATES_SETTING.getKey(), true); return featureSettings.build(); } From 9713c4c9c017c628ab69f18b1466e1f029520d93 Mon Sep 17 00:00:00 2001 From: Mohit Godwani Date: Tue, 16 Jul 2024 19:23:53 +0530 Subject: [PATCH 09/10] Address PR comments Signed-off-by: Mohit Godwani --- ... => ClusterStateSystemTemplateLoader.java} | 47 ++++++++++--------- .../SystemTemplateMetadata.java | 5 ++ ...lusterStateSystemTemplateLoaderTests.java} | 10 ++-- 3 files changed, 35 insertions(+), 27 deletions(-) rename server/src/main/java/org/opensearch/cluster/applicationtemplates/{ClusterStateComponentSystemTemplateLoader.java => ClusterStateSystemTemplateLoader.java} (83%) rename server/src/test/java/org/opensearch/cluster/applicationtemplates/{ClusterStateComponentSystemTemplateLoaderTests.java => ClusterStateSystemTemplateLoaderTests.java} (93%) diff --git a/server/src/main/java/org/opensearch/cluster/applicationtemplates/ClusterStateComponentSystemTemplateLoader.java b/server/src/main/java/org/opensearch/cluster/applicationtemplates/ClusterStateSystemTemplateLoader.java similarity index 83% rename from server/src/main/java/org/opensearch/cluster/applicationtemplates/ClusterStateComponentSystemTemplateLoader.java rename to server/src/main/java/org/opensearch/cluster/applicationtemplates/ClusterStateSystemTemplateLoader.java index e13ca8a9ded98..e274d925ecc9e 100644 --- a/server/src/main/java/org/opensearch/cluster/applicationtemplates/ClusterStateComponentSystemTemplateLoader.java +++ b/server/src/main/java/org/opensearch/cluster/applicationtemplates/ClusterStateSystemTemplateLoader.java @@ -31,7 +31,7 @@ * Class responsible for loading the component templates provided by a repository into the cluster state. */ @ExperimentalApi -public class ClusterStateComponentSystemTemplateLoader implements SystemTemplateLoader { +public class ClusterStateSystemTemplateLoader implements SystemTemplateLoader { private final Client client; @@ -40,8 +40,9 @@ public class ClusterStateComponentSystemTemplateLoader implements SystemTemplate private static final Logger logger = LogManager.getLogger(SystemTemplateLoader.class); public static final String TEMPLATE_LOADER_IDENTIFIER = "system_template_loader"; + public static final String TEMPLATE_TYPE_KEY = "_type"; - public ClusterStateComponentSystemTemplateLoader(Client client, Supplier clusterStateSupplier) { + public ClusterStateSystemTemplateLoader(Client client, Supplier clusterStateSupplier) { this.client = new OriginSettingClient(client, TEMPLATE_LOADER_IDENTIFIER); this.clusterStateSupplier = clusterStateSupplier; } @@ -53,12 +54,31 @@ public boolean loadTemplate(SystemTemplate template) throws IOException { .componentTemplates() .get(template.templateMetadata().fullyQualifiedName()); - final XContentParser contentParser = JsonXContent.jsonXContent.createParser( + if (existingTemplate != null + && !SystemTemplateMetadata.COMPONENT_TEMPLATE_TYPE.equals(Objects.toString(existingTemplate.metadata().get(TEMPLATE_TYPE_KEY)))) { + throw new OpenSearchCorruptionException( + "Attempting to create " + template.templateMetadata().name() + " which has already been created through some other source." + ); + } + + if (existingTemplate != null && existingTemplate.version() >= template.templateMetadata().version()) { + logger.debug( + "Skipping putting template {} as its existing version [{}] is >= fetched version [{}]", + template.templateMetadata().fullyQualifiedName(), + existingTemplate.version(), + template.templateMetadata().version() + ); + return false; + } + + ComponentTemplate newTemplate = null; + try(XContentParser contentParser = JsonXContent.jsonXContent.createParser( NamedXContentRegistry.EMPTY, DeprecationHandler.IGNORE_DEPRECATIONS, template.templateContent().utf8ToString() - ); - final ComponentTemplate newTemplate = ComponentTemplate.parse(contentParser); + )) { + newTemplate = ComponentTemplate.parse(contentParser); + } if (!Objects.equals(newTemplate.version(), template.templateMetadata().version())) { throw new OpenSearchCorruptionException( @@ -71,23 +91,6 @@ public boolean loadTemplate(SystemTemplate template) throws IOException { ); } - if (existingTemplate != null - && !Boolean.parseBoolean(Objects.toString(existingTemplate.metadata().get(SystemTemplateMetadata.COMPONENT_TEMPLATE_TYPE)))) { - throw new OpenSearchCorruptionException( - "Attempting to create " + template.templateMetadata().name() + " which has already been created through some other source." - ); - } - - if (existingTemplate != null && existingTemplate.version() >= newTemplate.version()) { - logger.debug( - "Skipping putting template {} as its existing version [{}] is >= fetched version [{}]", - template.templateMetadata().fullyQualifiedName(), - existingTemplate.version(), - newTemplate.version() - ); - return false; - } - final PutComponentTemplateAction.Request request = new PutComponentTemplateAction.Request( template.templateMetadata().fullyQualifiedName() ).componentTemplate(newTemplate); diff --git a/server/src/main/java/org/opensearch/cluster/applicationtemplates/SystemTemplateMetadata.java b/server/src/main/java/org/opensearch/cluster/applicationtemplates/SystemTemplateMetadata.java index 19818acee2ecd..9bbe27ac0e281 100644 --- a/server/src/main/java/org/opensearch/cluster/applicationtemplates/SystemTemplateMetadata.java +++ b/server/src/main/java/org/opensearch/cluster/applicationtemplates/SystemTemplateMetadata.java @@ -42,6 +42,11 @@ public long version() { return version; } + /** + * Gets the metadata using fully qualified name for the template + * @param fullyQualifiedName (e.g. @abc_template@logs@1) + * @return Metadata object based on name + */ public static SystemTemplateMetadata fromComponentTemplate(String fullyQualifiedName) { assert fullyQualifiedName.length() > 1 : "System template name must have at least one component"; assert fullyQualifiedName.substring(1, fullyQualifiedName.indexOf(DELIMITER, 1)).equals(COMPONENT_TEMPLATE_TYPE); diff --git a/server/src/test/java/org/opensearch/cluster/applicationtemplates/ClusterStateComponentSystemTemplateLoaderTests.java b/server/src/test/java/org/opensearch/cluster/applicationtemplates/ClusterStateSystemTemplateLoaderTests.java similarity index 93% rename from server/src/test/java/org/opensearch/cluster/applicationtemplates/ClusterStateComponentSystemTemplateLoaderTests.java rename to server/src/test/java/org/opensearch/cluster/applicationtemplates/ClusterStateSystemTemplateLoaderTests.java index 8e777a2ecf9c9..63caccc87e67a 100644 --- a/server/src/test/java/org/opensearch/cluster/applicationtemplates/ClusterStateComponentSystemTemplateLoaderTests.java +++ b/server/src/test/java/org/opensearch/cluster/applicationtemplates/ClusterStateSystemTemplateLoaderTests.java @@ -21,7 +21,7 @@ import java.io.IOException; import java.util.UUID; -public class ClusterStateComponentSystemTemplateLoaderTests extends OpenSearchSingleNodeTestCase { +public class ClusterStateSystemTemplateLoaderTests extends OpenSearchSingleNodeTestCase { public static final String SAMPLE_TEMPLATE = "{\n" + " \"template\": {\n" @@ -34,7 +34,7 @@ public class ClusterStateComponentSystemTemplateLoaderTests extends OpenSearchSi + " }\n" + " },\n" + " \"_meta\": {\n" - + " \"@abc_template\": true,\n" + + " \"_type\": \"@abc_template\",\n" + " \"_version\": 1\n" + " },\n" + " \"version\": 1\n" @@ -51,14 +51,14 @@ public class ClusterStateComponentSystemTemplateLoaderTests extends OpenSearchSi + " }\n" + " },\n" + " \"_meta\": {\n" - + " \"@abc_template\": true,\n" + + " \"_type\": \"@abc_template\",\n" + " \"_version\": 2\n" + " },\n" + " \"version\": 2\n" + "}"; public void testLoadTemplate() throws IOException { - ClusterStateComponentSystemTemplateLoader loader = new ClusterStateComponentSystemTemplateLoader( + ClusterStateSystemTemplateLoader loader = new ClusterStateSystemTemplateLoader( node().client(), () -> node().injector().getInstance(ClusterService.class).state() ); @@ -125,7 +125,7 @@ public void testLoadTemplate() throws IOException { } public void testLoadTemplateVersionMismatch() throws IOException { - ClusterStateComponentSystemTemplateLoader loader = new ClusterStateComponentSystemTemplateLoader( + ClusterStateSystemTemplateLoader loader = new ClusterStateSystemTemplateLoader( node().client(), () -> node().injector().getInstance(ClusterService.class).state() ); From bbb166768cd7f3c16b5d710e1048918d2095ce36 Mon Sep 17 00:00:00 2001 From: Mohit Godwani Date: Tue, 16 Jul 2024 19:26:00 +0530 Subject: [PATCH 10/10] Apply spotless Signed-off-by: Mohit Godwani --- .../ClusterStateSystemTemplateLoader.java | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/server/src/main/java/org/opensearch/cluster/applicationtemplates/ClusterStateSystemTemplateLoader.java b/server/src/main/java/org/opensearch/cluster/applicationtemplates/ClusterStateSystemTemplateLoader.java index e274d925ecc9e..332960ef49064 100644 --- a/server/src/main/java/org/opensearch/cluster/applicationtemplates/ClusterStateSystemTemplateLoader.java +++ b/server/src/main/java/org/opensearch/cluster/applicationtemplates/ClusterStateSystemTemplateLoader.java @@ -55,7 +55,9 @@ public boolean loadTemplate(SystemTemplate template) throws IOException { .get(template.templateMetadata().fullyQualifiedName()); if (existingTemplate != null - && !SystemTemplateMetadata.COMPONENT_TEMPLATE_TYPE.equals(Objects.toString(existingTemplate.metadata().get(TEMPLATE_TYPE_KEY)))) { + && !SystemTemplateMetadata.COMPONENT_TEMPLATE_TYPE.equals( + Objects.toString(existingTemplate.metadata().get(TEMPLATE_TYPE_KEY)) + )) { throw new OpenSearchCorruptionException( "Attempting to create " + template.templateMetadata().name() + " which has already been created through some other source." ); @@ -72,11 +74,13 @@ public boolean loadTemplate(SystemTemplate template) throws IOException { } ComponentTemplate newTemplate = null; - try(XContentParser contentParser = JsonXContent.jsonXContent.createParser( - NamedXContentRegistry.EMPTY, - DeprecationHandler.IGNORE_DEPRECATIONS, - template.templateContent().utf8ToString() - )) { + try ( + XContentParser contentParser = JsonXContent.jsonXContent.createParser( + NamedXContentRegistry.EMPTY, + DeprecationHandler.IGNORE_DEPRECATIONS, + template.templateContent().utf8ToString() + ) + ) { newTemplate = ComponentTemplate.parse(contentParser); }