Browse Source

[Profiling] Tighten resource creation check (#99873)

Index management in the Universal Profiling plugin manages different
types of resources:

1. Component templates, composable templates and ILM policies
2. Indices and data streams

The latter require the former to be present so they can be created
properly. This is achieved by checking that the predicate
`isAllResourcesCreated` in `ProfilingIndexTemplateRegistry` returns
`true`. While this predicate checks that all its managed resources are
present, it does not check whether their version is correct. This can
lead to a race condition where an older version of e.g. an index
template is present and an index that matches this index template is
rolled over but the new index still uses the old index template.

With this commit we tighten the check so that the predicate returns
`true` if and only if all its managed resources are present *and* the
version is not outdated. This avoids that race condition as the creation
of indices and data streams will only proceed after all preconditions
are met.
Daniel Mitterdorfer 2 years ago
parent
commit
602aa013ab

+ 5 - 0
docs/changelog/99873.yaml

@@ -0,0 +1,5 @@
+pr: 99873
+summary: "[Profiling] Tighten resource creation check"
+area: Application
+type: bug
+issues: []

+ 21 - 7
x-pack/plugin/profiling/src/main/java/org/elasticsearch/xpack/profiling/ProfilingIndexTemplateRegistry.java

@@ -282,7 +282,7 @@ public class ProfilingIndexTemplateRegistry extends IndexTemplateRegistry {
         }
     }
 
-    private int getVersion(LifecyclePolicy policy, String logicalVersion) {
+    private static int getVersion(LifecyclePolicy policy, String logicalVersion) {
         Map<String, Object> meta = policy.getMetadata();
         try {
             return meta != null ? Integer.parseInt(meta.getOrDefault("version", Integer.MIN_VALUE).toString()) : Integer.MIN_VALUE;
@@ -294,21 +294,35 @@ public class ProfilingIndexTemplateRegistry extends IndexTemplateRegistry {
         }
     }
 
+    /**
+     * Determines whether all resources (component templates, composable templates and lifecycle policies) have been created. This method
+     * also checks whether resources have been created for the expected version.
+     *
+     * @param state Current cluster state.
+     * @param settings Current cluster settings.
+     * @return <code>true</code> if and only if all resources managed by this registry have been created and are current.
+     */
     public static boolean isAllResourcesCreated(ClusterState state, Settings settings) {
-        for (String componentTemplate : COMPONENT_TEMPLATE_CONFIGS.keySet()) {
-            if (state.metadata().componentTemplates().containsKey(componentTemplate) == false) {
+        for (String name : COMPONENT_TEMPLATE_CONFIGS.keySet()) {
+            ComponentTemplate componentTemplate = state.metadata().componentTemplates().get(name);
+            if (componentTemplate == null || componentTemplate.version() < INDEX_TEMPLATE_VERSION) {
                 return false;
             }
         }
-        for (String composableTemplate : COMPOSABLE_INDEX_TEMPLATE_CONFIGS.keySet()) {
-            if (state.metadata().templatesV2().containsKey(composableTemplate) == false) {
+        for (String name : COMPOSABLE_INDEX_TEMPLATE_CONFIGS.keySet()) {
+            ComposableIndexTemplate composableIndexTemplate = state.metadata().templatesV2().get(name);
+            if (composableIndexTemplate == null || composableIndexTemplate.version() < INDEX_TEMPLATE_VERSION) {
                 return false;
             }
         }
         if (isDataStreamsLifecycleOnlyMode(settings) == false) {
+            IndexLifecycleMetadata ilmMetadata = state.metadata().custom(IndexLifecycleMetadata.TYPE);
+            if (ilmMetadata == null) {
+                return false;
+            }
             for (LifecyclePolicyConfig lifecyclePolicy : LIFECYCLE_POLICY_CONFIGS) {
-                IndexLifecycleMetadata ilmMetadata = state.metadata().custom(IndexLifecycleMetadata.TYPE);
-                if (ilmMetadata == null || ilmMetadata.getPolicies().containsKey(lifecyclePolicy.getPolicyName()) == false) {
+                LifecyclePolicy existingPolicy = ilmMetadata.getPolicies().get(lifecyclePolicy.getPolicyName());
+                if (existingPolicy == null || getVersion(existingPolicy, "current") < INDEX_TEMPLATE_VERSION) {
                     return false;
                 }
             }

+ 75 - 0
x-pack/plugin/profiling/src/test/java/org/elasticsearch/xpack/profiling/ProfilingIndexTemplateRegistryTests.java

@@ -309,6 +309,81 @@ public class ProfilingIndexTemplateRegistryTests extends ESTestCase {
         }
     }
 
+    public void testAllResourcesPresentButOutdated() {
+        DiscoveryNode node = DiscoveryNodeUtils.create("node");
+        DiscoveryNodes nodes = DiscoveryNodes.builder().localNodeId("node").masterNodeId("node").add(node).build();
+        Map<String, Integer> componentTemplates = new HashMap<>();
+        Map<String, Integer> composableTemplates = new HashMap<>();
+        Map<String, LifecyclePolicy> policies = new HashMap<>();
+        for (String templateName : registry.getComponentTemplateConfigs().keySet()) {
+            // outdated (or missing) version
+            componentTemplates.put(templateName, frequently() ? ProfilingIndexTemplateRegistry.INDEX_TEMPLATE_VERSION - 1 : null);
+        }
+        for (String templateName : registry.getComposableTemplateConfigs().keySet()) {
+            // outdated (or missing) version
+            composableTemplates.put(templateName, frequently() ? ProfilingIndexTemplateRegistry.INDEX_TEMPLATE_VERSION - 1 : null);
+        }
+        for (LifecyclePolicy policy : registry.getLifecyclePolicies()) {
+            // make a copy as we're modifying the version mapping
+            Map<String, Object> metadata = new HashMap<>(policy.getMetadata());
+            if (frequently()) {
+                metadata.put("version", ProfilingIndexTemplateRegistry.INDEX_TEMPLATE_VERSION - 1);
+            } else {
+                metadata.remove("version");
+            }
+            policies.put(policy.getName(), new LifecyclePolicy(policy.getName(), policy.getPhases(), metadata));
+        }
+        ClusterState clusterState = createClusterState(Settings.EMPTY, componentTemplates, composableTemplates, policies, nodes);
+
+        assertFalse(ProfilingIndexTemplateRegistry.isAllResourcesCreated(clusterState, Settings.EMPTY));
+    }
+
+    public void testAllResourcesPresentAndCurrent() {
+        DiscoveryNode node = DiscoveryNodeUtils.create("node");
+        DiscoveryNodes nodes = DiscoveryNodes.builder().localNodeId("node").masterNodeId("node").add(node).build();
+        Map<String, Integer> componentTemplates = new HashMap<>();
+        Map<String, Integer> composableTemplates = new HashMap<>();
+        Map<String, LifecyclePolicy> policies = new HashMap<>();
+        for (String templateName : registry.getComponentTemplateConfigs().keySet()) {
+            componentTemplates.put(templateName, ProfilingIndexTemplateRegistry.INDEX_TEMPLATE_VERSION);
+        }
+        for (String templateName : registry.getComposableTemplateConfigs().keySet()) {
+            composableTemplates.put(templateName, ProfilingIndexTemplateRegistry.INDEX_TEMPLATE_VERSION);
+        }
+        for (LifecyclePolicy policy : registry.getLifecyclePolicies()) {
+            policies.put(policy.getName(), policy);
+        }
+        ClusterState clusterState = createClusterState(Settings.EMPTY, componentTemplates, composableTemplates, policies, nodes);
+
+        assertTrue(ProfilingIndexTemplateRegistry.isAllResourcesCreated(clusterState, Settings.EMPTY));
+    }
+
+    public void testSomeResourcesMissing() {
+        DiscoveryNode node = DiscoveryNodeUtils.create("node");
+        DiscoveryNodes nodes = DiscoveryNodes.builder().localNodeId("node").masterNodeId("node").add(node).build();
+        Map<String, Integer> componentTemplates = new HashMap<>();
+        Map<String, Integer> composableTemplates = new HashMap<>();
+        Map<String, LifecyclePolicy> policies = new HashMap<>();
+        for (String templateName : registry.getComponentTemplateConfigs().keySet()) {
+            if (rarely()) {
+                componentTemplates.put(templateName, ProfilingIndexTemplateRegistry.INDEX_TEMPLATE_VERSION);
+            }
+        }
+        for (String templateName : registry.getComposableTemplateConfigs().keySet()) {
+            if (rarely()) {
+                composableTemplates.put(templateName, ProfilingIndexTemplateRegistry.INDEX_TEMPLATE_VERSION);
+            }
+        }
+        for (LifecyclePolicy policy : registry.getLifecyclePolicies()) {
+            if (rarely()) {
+                policies.put(policy.getName(), policy);
+            }
+        }
+        ClusterState clusterState = createClusterState(Settings.EMPTY, componentTemplates, composableTemplates, policies, nodes);
+
+        assertFalse(ProfilingIndexTemplateRegistry.isAllResourcesCreated(clusterState, Settings.EMPTY));
+    }
+
     private ActionResponse verifyComposableTemplateInstalled(
         AtomicInteger calledTimes,
         ActionType<?> action,