Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix ILM to DSL migration test for BA. #106054

Merged
merged 3 commits into from
Mar 7, 2024
Merged

Fix ILM to DSL migration test for BA. #106054

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b4c1ab0
Fix ILM to DSL migration test for BA.
afoucret Mar 7, 2024
80e1ead
Apply suggestions from code review
afoucret Mar 7, 2024
b70b474
Fix version check
afoucret Mar 7, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 45 additions & 8 deletions ...start/src/javaRestTest/java/org/elasticsearch/xpack/application/FullClusterRestartIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import org.elasticsearch.Version;
import org.elasticsearch.client.Request;
import org.elasticsearch.client.Response;
import org.elasticsearch.client.ResponseException;
import org.elasticsearch.test.cluster.ElasticsearchCluster;
import org.elasticsearch.test.cluster.local.distribution.DistributionType;
import org.elasticsearch.test.rest.ObjectPath;
Expand All @@ -22,12 +23,19 @@
import java.util.List;

import static org.elasticsearch.Version.V_8_12_0;
import static org.hamcrest.Matchers.greaterThanOrEqualTo;

public class FullClusterRestartIT extends ParameterizedFullClusterRestartTestCase {

// DSL was introduced with version 8.12.0 of ES.
private static final Version DSL_DEFAULT_RETENTION_VERSION = V_8_12_0;

// DSL was introduced with the version 3 of the registry.
private static final int DSL_REGISTRY_VERSION = 3;

// Legacy name we used for ILM policy configuration in versions prior to 8.12.0.
private static final String EVENT_DATA_STREAM_LEGACY_TEMPLATE_NAME = "behavioral_analytics-events-default";

// Event data streams template name.
private static final String EVENT_DATA_STREAM_LEGACY_ILM_POLICY_NAME = "behavioral_analytics-events-default_policy";

@ClassRule
Expand All @@ -36,6 +44,7 @@ public class FullClusterRestartIT extends ParameterizedFullClusterRestartTestCas
.version(getOldClusterTestVersion())
.nodes(2)
.setting("xpack.security.enabled", "false")
.setting("xpack.license.self_generated.type", "trial")
.module("x-pack-ent-search")
.build();

Expand All @@ -48,7 +57,6 @@ protected ElasticsearchCluster getUpgradeCluster() {
return cluster;
}

@AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/104470")
public void testBehavioralAnalyticsDataRetention() throws Exception {
assumeTrue(
"Data retention changed by default to DSL in " + DSL_DEFAULT_RETENTION_VERSION,
Expand All @@ -59,26 +67,32 @@ public void testBehavioralAnalyticsDataRetention() throws Exception {
String newAnalyticsCollectionName = "newstuff";

if (isRunningAgainstOldCluster()) {
// Ensure index template is installed before executing the tests.
assertBusy(() -> assertDataStreamTemplateExists(EVENT_DATA_STREAM_LEGACY_TEMPLATE_NAME));

// Create an analytics collection
Request legacyPutRequest = new Request("PUT", "_application/analytics/" + legacyAnalyticsCollectionName);
assertOK(client().performRequest(legacyPutRequest));

// Validate that ILM lifecycle is in place
assertBusy(() -> assertLegacyDataRetentionPolicy(legacyAnalyticsCollectionName));
assertBusy(() -> assertUsingLegacyDataRetentionPolicy(legacyAnalyticsCollectionName));
} else {
// Ensure index template is updated to version 3 before executing the tests.
assertBusy(() -> assertDataStreamTemplateExists(EVENT_DATA_STREAM_LEGACY_TEMPLATE_NAME, DSL_REGISTRY_VERSION));

// Create a new analytics collection
Request putRequest = new Request("PUT", "_application/analytics/" + newAnalyticsCollectionName);
assertOK(client().performRequest(putRequest));

// Validate that NO ILM lifecycle is in place and we are using DLS instead.
assertBusy(() -> assertDslDataRetention(newAnalyticsCollectionName));
assertBusy(() -> assertUsingDslDataRetention(newAnalyticsCollectionName));

// Validate that the existing analytics collection created with an older version is still using ILM
assertBusy(() -> assertLegacyDataRetentionPolicy(legacyAnalyticsCollectionName));
assertBusy(() -> assertUsingLegacyDataRetentionPolicy(legacyAnalyticsCollectionName));
}
}

private void assertLegacyDataRetentionPolicy(String analyticsCollectionName) throws IOException {
private void assertUsingLegacyDataRetentionPolicy(String analyticsCollectionName) throws IOException {
String dataStreamName = "behavioral_analytics-events-" + analyticsCollectionName;
Request getDataStreamRequest = new Request("GET", "_data_stream/" + dataStreamName);
Response response = client().performRequest(getDataStreamRequest);
Expand All @@ -93,7 +107,7 @@ private void assertLegacyDataRetentionPolicy(String analyticsCollectionName) thr
assertNotNull(policy.evaluate(EVENT_DATA_STREAM_LEGACY_ILM_POLICY_NAME));
}

private void assertDslDataRetention(String analyticsCollectionName) throws IOException {
private void assertUsingDslDataRetention(String analyticsCollectionName) throws IOException {
String dataStreamName = "behavioral_analytics-events-" + analyticsCollectionName;
Request getDataStreamRequest = new Request("GET", "_data_stream/" + dataStreamName);
Response response = client().performRequest(getDataStreamRequest);
Expand All @@ -105,13 +119,36 @@ private void assertDslDataRetention(String analyticsCollectionName) throws IOExc
for (Object dataStreamObj : dataStreams) {
ObjectPath dataStream = new ObjectPath(dataStreamObj);
if (dataStreamName.equals(dataStream.evaluate("name"))) {
assertNull(dataStream.evaluate("ilm_policy"));
assertEquals(true, dataStream.evaluate("lifecycle.enabled"));
assertEquals("180d", dataStream.evaluate("lifecycle.data_retention"));
assertEquals("Data stream lifecycle", dataStream.evaluate("next_generation_managed_by"));
assertEquals(false, dataStream.evaluate("prefer_ilm"));
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess these checks are better indicators than ilm_policy being null?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The test was not correct and in fact ilm_policy is not null but the policy is not used since prefer_ilm is false.

We was not able to see it before since the test were not executed.

My plan is to open a tech debt issue to check is we can completely remove the ILM policy since it is not used anymore.

Copy link
Member

Choose a reason for hiding this comment

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

At the time I believe we had to keep it for BWC reasons?

Copy link
Contributor Author

@afoucret afoucret Mar 7, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I have never been fully convince that we have to keep it.
I mean

  • Clusters created before 8.12 will have the ILM policy created before they update and data streams created before the update will use it.
  • The registry update does not cleanup useless policies

I created atech debt issue to evaluate if we can remove it.

evaluatedNewDataStream = true;
}
}
assertTrue(evaluatedNewDataStream);
}

private void assertDataStreamTemplateExists(String templateName) throws IOException {
assertDataStreamTemplateExists(templateName, null);
}

private void assertDataStreamTemplateExists(String templateName, Integer minVersion) throws IOException {
try {
Request getIndexTemplateRequest = new Request("GET", "_index_template/" + templateName);
Response response = client().performRequest(getIndexTemplateRequest);
assertOK(response);
if (minVersion != null) {
String pathToVersion = "index_templates.0.index_template.version";
ObjectPath indexTemplatesResponse = ObjectPath.createFromResponse(response);
assertThat(indexTemplatesResponse.evaluate(pathToVersion), greaterThanOrEqualTo(minVersion));
}
} catch (ResponseException e) {
int status = e.getResponse().getStatusLine().getStatusCode();
if (status == 404) {
throw new AssertionError("Waiting for the template to be created");
}
throw e;
}
}
}