Skip to content

Commit

Permalink
[Transform] Improve transform upgrader (elastic#79601)
Browse files Browse the repository at this point in the history
This changes fixes some issues with the upgrader. It inlines the index name resolver as wildcard
deletes are not supported if action.destructive_requires_name is set to true(default). It changes
response to always return all counters in order to avoid an empty response. After documentation has
been created, it switches the deprecation URL to the upgrade API docs.
  • Loading branch information
Hendrik Muhs committed Oct 25, 2021
1 parent 5d06001 commit e8b2dca
Show file tree
Hide file tree
Showing 13 changed files with 177 additions and 58 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -37,15 +37,9 @@ private static UpgradeTransformsResponse createTestInstance() {

private static void toXContent(UpgradeTransformsResponse response, XContentBuilder builder) throws IOException {
builder.startObject();
if (response.getUpdated() != 0) {
builder.field("updated", response.getUpdated());
}
if (response.getNoAction() != 0) {
builder.field("no_action", response.getNoAction());
}
if (response.getNeedsUpdate() != 0) {
builder.field("needs_update", response.getNeedsUpdate());
}
builder.field("updated", response.getUpdated());
builder.field("no_action", response.getNoAction());
builder.field("needs_update", response.getNeedsUpdate());
builder.endObject();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,21 @@
package org.elasticsearch.xpack.core.transform;

public class TransformDeprecations {

public static final String UPGRADE_TRANSFORM_URL = "https://ela.st/es-8-upgrade-transforms";

// breaking changes base url for the _next_ major release
public static final String BREAKING_CHANGES_BASE_URL =
"https://www.elastic.co/guide/en/elasticsearch/reference/master/migrating-8.0.html";

public static final String QUERY_BREAKING_CHANGES_URL = BREAKING_CHANGES_BASE_URL + "#breaking_80_search_changes";

public static final String AGGS_BREAKING_CHANGES_URL = BREAKING_CHANGES_BASE_URL + "#breaking_80_aggregations_changes";

public static final String ACTION_UPGRADE_TRANSFORMS_API = "Use the upgrade transforms API to fix your transforms.";

public static final String ACTION_MAX_PAGE_SEARCH_SIZE_IS_DEPRECATED =
"[max_page_search_size] is deprecated inside pivot. Use settings instead.";

private TransformDeprecations() {}
}
Original file line number Diff line number Diff line change
Expand Up @@ -125,15 +125,9 @@ public void writeTo(StreamOutput out) throws IOException {
@Override
public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
builder.startObject();
if (updated != 0L) {
builder.field("updated", updated);
}
if (noAction != 0L) {
builder.field("no_action", noAction);
}
if (needsUpdate != 0L) {
builder.field("needs_update", needsUpdate);
}
builder.field("updated", updated);
builder.field("no_action", noAction);
builder.field("needs_update", needsUpdate);
builder.endObject();
return builder;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,15 +14,15 @@
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.io.stream.StreamOutput;
import org.elasticsearch.common.io.stream.Writeable;
import org.elasticsearch.core.Nullable;
import org.elasticsearch.core.TimeValue;
import org.elasticsearch.xcontent.ConstructingObjectParser;
import org.elasticsearch.xcontent.NamedXContentRegistry;
import org.elasticsearch.xcontent.ObjectParser;
import org.elasticsearch.xcontent.ParseField;
import org.elasticsearch.xcontent.ToXContentObject;
import org.elasticsearch.xcontent.XContentBuilder;
import org.elasticsearch.xcontent.XContentParser;
import org.elasticsearch.core.Nullable;
import org.elasticsearch.core.TimeValue;
import org.elasticsearch.xpack.core.common.time.TimeUtils;
import org.elasticsearch.xpack.core.common.validation.SourceDestValidator;
import org.elasticsearch.xpack.core.common.validation.SourceDestValidator.SourceDestValidation;
Expand Down Expand Up @@ -402,8 +402,8 @@ public List<DeprecationIssue> checkForDeprecations(NamedXContentRegistry namedXC
new DeprecationIssue(
Level.CRITICAL,
"Transform [" + id + "] is too old",
TransformDeprecations.BREAKING_CHANGES_BASE_URL,
"The configuration uses an old format, you can use [_update] or [_upgrade] to update",
TransformDeprecations.UPGRADE_TRANSFORM_URL,
TransformDeprecations.ACTION_UPGRADE_TRANSFORMS_API,
false,
null
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,13 @@
import org.elasticsearch.common.io.stream.Writeable;
import org.elasticsearch.common.logging.DeprecationCategory;
import org.elasticsearch.common.logging.DeprecationLogger;
import org.elasticsearch.core.Nullable;
import org.elasticsearch.search.aggregations.MultiBucketConsumerService;
import org.elasticsearch.xcontent.ConstructingObjectParser;
import org.elasticsearch.xcontent.NamedXContentRegistry;
import org.elasticsearch.xcontent.ToXContentObject;
import org.elasticsearch.xcontent.XContentBuilder;
import org.elasticsearch.xcontent.XContentParser;
import org.elasticsearch.core.Nullable;
import org.elasticsearch.search.aggregations.MultiBucketConsumerService;
import org.elasticsearch.xpack.core.deprecation.DeprecationIssue;
import org.elasticsearch.xpack.core.deprecation.DeprecationIssue.Level;
import org.elasticsearch.xpack.core.transform.TransformDeprecations;
Expand Down Expand Up @@ -91,7 +91,7 @@ public PivotConfig(final GroupConfig groups, final AggregationConfig aggregation
deprecationLogger.critical(
DeprecationCategory.API,
TransformField.MAX_PAGE_SEARCH_SIZE.getPreferredName(),
"[max_page_search_size] is deprecated inside pivot please use settings instead"
TransformDeprecations.ACTION_MAX_PAGE_SEARCH_SIZE_IS_DEPRECATED
);
}
}
Expand Down Expand Up @@ -187,7 +187,7 @@ public void checkForDeprecations(String id, NamedXContentRegistry namedXContentR
Level.WARNING,
"Transform [" + id + "] uses deprecated max_page_search_size",
TransformDeprecations.BREAKING_CHANGES_BASE_URL,
"[max_page_search_size] is deprecated inside pivot please use settings instead",
TransformDeprecations.ACTION_MAX_PAGE_SEARCH_SIZE_IS_DEPRECATED,
false,
null
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import org.elasticsearch.xpack.core.deprecation.DeprecationIssue;
import org.elasticsearch.xpack.core.deprecation.DeprecationIssue.Level;
import org.elasticsearch.xpack.core.transform.AbstractSerializingTransformTestCase;
import org.elasticsearch.xpack.core.transform.TransformDeprecations;
import org.elasticsearch.xpack.core.transform.transforms.latest.LatestConfig;
import org.elasticsearch.xpack.core.transform.transforms.latest.LatestConfigTests;
import org.elasticsearch.xpack.core.transform.transforms.pivot.PivotConfig;
Expand Down Expand Up @@ -510,7 +511,7 @@ public void testRewriteForUpdate() throws IOException {
assertTrue(transformConfigRewritten.getSettings().getDatesAsEpochMillis());
assertFalse(transformConfigRewritten.getSettings().getAlignCheckpoints());

assertWarnings("[max_page_search_size] is deprecated inside pivot please use settings instead");
assertWarnings(TransformDeprecations.ACTION_MAX_PAGE_SEARCH_SIZE_IS_DEPRECATED);
assertEquals(Version.CURRENT, transformConfigRewritten.getVersion());
}

Expand Down Expand Up @@ -583,7 +584,7 @@ public void testRewriteForUpdateMaxPageSizeSearchConflicting() throws IOExceptio
assertNotNull(transformConfigRewritten.getSettings().getMaxPageSearchSize());
assertEquals(555L, transformConfigRewritten.getSettings().getMaxPageSearchSize().longValue());
assertEquals(Version.CURRENT, transformConfigRewritten.getVersion());
assertWarnings("[max_page_search_size] is deprecated inside pivot please use settings instead");
assertWarnings(TransformDeprecations.ACTION_MAX_PAGE_SEARCH_SIZE_IS_DEPRECATED);
}

public void testRewriteForBWCOfDateNormalization() throws IOException {
Expand Down Expand Up @@ -739,7 +740,7 @@ public void testCheckForDeprecations() {
TransformConfig deprecatedConfig = randomTransformConfigWithDeprecatedFields(id, Version.CURRENT);

// check _and_ clear warnings
assertWarnings("[max_page_search_size] is deprecated inside pivot please use settings instead");
assertWarnings(TransformDeprecations.ACTION_MAX_PAGE_SEARCH_SIZE_IS_DEPRECATED);

// important: checkForDeprecations does _not_ create new deprecation warnings
assertThat(
Expand All @@ -749,8 +750,8 @@ public void testCheckForDeprecations() {
new DeprecationIssue(
Level.WARNING,
"Transform [" + id + "] uses deprecated max_page_search_size",
"https://www.elastic.co/guide/en/elasticsearch/reference/master/migrating-8.0.html",
"[max_page_search_size] is deprecated inside pivot please use settings instead",
TransformDeprecations.BREAKING_CHANGES_BASE_URL,
TransformDeprecations.ACTION_MAX_PAGE_SEARCH_SIZE_IS_DEPRECATED,
false,
null
)
Expand All @@ -761,7 +762,7 @@ public void testCheckForDeprecations() {
deprecatedConfig = randomTransformConfigWithDeprecatedFields(id, Version.V_7_10_0);

// check _and_ clear warnings
assertWarnings("[max_page_search_size] is deprecated inside pivot please use settings instead");
assertWarnings(TransformDeprecations.ACTION_MAX_PAGE_SEARCH_SIZE_IS_DEPRECATED);

// important: checkForDeprecations does _not_ create new deprecation warnings
assertThat(
Expand All @@ -771,8 +772,8 @@ public void testCheckForDeprecations() {
new DeprecationIssue(
Level.WARNING,
"Transform [" + id + "] uses deprecated max_page_search_size",
"https://www.elastic.co/guide/en/elasticsearch/reference/master/migrating-8.0.html",
"[max_page_search_size] is deprecated inside pivot please use settings instead",
TransformDeprecations.BREAKING_CHANGES_BASE_URL,
TransformDeprecations.ACTION_MAX_PAGE_SEARCH_SIZE_IS_DEPRECATED,
false,
null
)
Expand All @@ -783,7 +784,7 @@ public void testCheckForDeprecations() {
deprecatedConfig = randomTransformConfigWithDeprecatedFields(id, Version.V_7_4_0);

// check _and_ clear warnings
assertWarnings("[max_page_search_size] is deprecated inside pivot please use settings instead");
assertWarnings(TransformDeprecations.ACTION_MAX_PAGE_SEARCH_SIZE_IS_DEPRECATED);

// important: checkForDeprecations does _not_ create new deprecation warnings
assertThat(
Expand All @@ -793,16 +794,16 @@ public void testCheckForDeprecations() {
new DeprecationIssue(
Level.CRITICAL,
"Transform [" + id + "] is too old",
"https://www.elastic.co/guide/en/elasticsearch/reference/master/migrating-8.0.html",
"The configuration uses an old format, you can use [_update] or [_upgrade] to update",
TransformDeprecations.UPGRADE_TRANSFORM_URL,
TransformDeprecations.ACTION_UPGRADE_TRANSFORMS_API,
false,
null
),
new DeprecationIssue(
Level.WARNING,
"Transform [" + id + "] uses deprecated max_page_search_size",
"https://www.elastic.co/guide/en/elasticsearch/reference/master/migrating-8.0.html",
"[max_page_search_size] is deprecated inside pivot please use settings instead",
TransformDeprecations.BREAKING_CHANGES_BASE_URL,
TransformDeprecations.ACTION_MAX_PAGE_SEARCH_SIZE_IS_DEPRECATED,
false,
null
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
import org.elasticsearch.xcontent.XContentParser;
import org.elasticsearch.xcontent.XContentType;
import org.elasticsearch.xpack.core.transform.AbstractSerializingTransformTestCase;
import org.elasticsearch.xpack.core.transform.TransformDeprecations;

import java.io.IOException;
import java.util.Arrays;
Expand Down Expand Up @@ -437,7 +438,7 @@ public void testAggNameValidationsWithInvalidFieldnames() {

public void testDeprecation() {
PivotConfig pivotConfig = randomPivotConfigWithDeprecatedFields();
assertWarnings("[max_page_search_size] is deprecated inside pivot please use settings instead");
assertWarnings(TransformDeprecations.ACTION_MAX_PAGE_SEARCH_SIZE_IS_DEPRECATED);
}

private static String dotJoin(String... fields) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
import org.elasticsearch.index.shard.ShardPath;
import org.elasticsearch.index.store.StoreStats;
import org.elasticsearch.index.warmer.WarmerStats;
import org.elasticsearch.indices.TestIndexNameExpressionResolver;
import org.elasticsearch.search.suggest.completion.CompletionStats;
import org.elasticsearch.test.client.NoOpClient;
import org.elasticsearch.xpack.core.transform.transforms.TransformCheckpoint;
Expand Down Expand Up @@ -138,8 +139,13 @@ public void createComponents() {
if (mockClientForCheckpointing == null) {
mockClientForCheckpointing = new MockClientForCheckpointing("TransformCheckpointServiceNodeTests");
}

transformsConfigManager = new IndexBasedTransformConfigManager(client(), xContentRegistry());
ClusterService clusterService = mock(ClusterService.class);
transformsConfigManager = new IndexBasedTransformConfigManager(
clusterService,
TestIndexNameExpressionResolver.newInstance(),
client(),
xContentRegistry()
);

// use a mock for the checkpoint service
TransformAuditor mockAuditor = mock(TransformAuditor.class);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,30 @@

import org.elasticsearch.ResourceAlreadyExistsException;
import org.elasticsearch.ResourceNotFoundException;
import org.elasticsearch.Version;
import org.elasticsearch.action.admin.indices.create.CreateIndexRequest;
import org.elasticsearch.action.admin.indices.refresh.RefreshRequest;
import org.elasticsearch.action.get.GetRequest;
import org.elasticsearch.action.index.IndexRequest;
import org.elasticsearch.action.support.WriteRequest;
import org.elasticsearch.cluster.ClusterName;
import org.elasticsearch.cluster.ClusterState;
import org.elasticsearch.cluster.metadata.IndexMetadata;
import org.elasticsearch.cluster.metadata.Metadata;
import org.elasticsearch.cluster.routing.IndexRoutingTable;
import org.elasticsearch.cluster.routing.RoutingTable;
import org.elasticsearch.cluster.routing.ShardRoutingState;
import org.elasticsearch.cluster.routing.TestShardRouting;
import org.elasticsearch.cluster.service.ClusterService;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.UUIDs;
import org.elasticsearch.common.collect.ImmutableOpenMap;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.core.Tuple;
import org.elasticsearch.index.Index;
import org.elasticsearch.index.IndexNotFoundException;
import org.elasticsearch.index.mapper.MapperService;
import org.elasticsearch.indices.TestIndexNameExpressionResolver;
import org.elasticsearch.xcontent.ToXContent;
import org.elasticsearch.xcontent.XContentBuilder;
import org.elasticsearch.xcontent.XContentFactory;
Expand All @@ -33,6 +49,7 @@
import org.elasticsearch.xpack.transform.TransformSingleNodeTestCase;
import org.junit.Before;

import java.io.IOException;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
Expand All @@ -50,14 +67,23 @@
import static org.hamcrest.CoreMatchers.is;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.instanceOf;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;

public class TransformConfigManagerTests extends TransformSingleNodeTestCase {

private IndexBasedTransformConfigManager transformConfigManager;
private ClusterService clusterService;

@Before
public void createComponents() {
transformConfigManager = new IndexBasedTransformConfigManager(client(), xContentRegistry());
clusterService = mock(ClusterService.class);
transformConfigManager = new IndexBasedTransformConfigManager(
clusterService,
TestIndexNameExpressionResolver.newInstance(),
client(),
xContentRegistry()
);
}

public void testGetMissingTransform() throws InterruptedException {
Expand Down Expand Up @@ -706,6 +732,10 @@ public void testDeleteOldIndices() throws Exception {
assertAsync(listener -> transformConfigManager.getTransformConfiguration(transformId, listener), transformConfigNew, null, null);

// delete old indices
when(clusterService.state()).thenReturn(
createClusterStateWithTransformIndex(oldIndex, TransformInternalIndexConstants.LATEST_INDEX_NAME)
);

assertAsync(listener -> transformConfigManager.deleteOldIndices(listener), true, null, null);

// the config should still be there
Expand All @@ -724,4 +754,32 @@ public void testDeleteOldIndices() throws Exception {
);
}

private static ClusterState createClusterStateWithTransformIndex(String... indexes) throws IOException {
ImmutableOpenMap.Builder<String, IndexMetadata> indexMapBuilder = ImmutableOpenMap.builder();
Metadata.Builder metaBuilder = Metadata.builder();
ClusterState.Builder csBuilder = ClusterState.builder(ClusterName.DEFAULT);
RoutingTable.Builder routingTableBuilder = RoutingTable.builder();

for (String index : indexes) {
IndexMetadata.Builder builder = new IndexMetadata.Builder(index).settings(
Settings.builder()
.put(TransformInternalIndex.settings())
.put(IndexMetadata.SETTING_INDEX_VERSION_CREATED.getKey(), Version.CURRENT)
.build()
).numberOfReplicas(0).numberOfShards(1).putMapping(Strings.toString(TransformInternalIndex.mappings()));
indexMapBuilder.put(index, builder.build());

routingTableBuilder.add(
IndexRoutingTable.builder(new Index(index, UUIDs.randomBase64UUID()))
.addShard(TestShardRouting.newShardRouting(index, 0, "node_a", null, true, ShardRoutingState.STARTED))
.build()
);

}
csBuilder.routingTable(routingTableBuilder.build());
metaBuilder.indices(indexMapBuilder.build());
csBuilder.metadata(metaBuilder.build());

return csBuilder.build();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -318,7 +318,12 @@ public Collection<Object> createComponents(
return emptyList();
}

TransformConfigManager configManager = new IndexBasedTransformConfigManager(client, xContentRegistry);
TransformConfigManager configManager = new IndexBasedTransformConfigManager(
clusterService,
expressionResolver,
client,
xContentRegistry
);
TransformAuditor auditor = new TransformAuditor(client, clusterService.getNodeName(), clusterService);
TransformCheckpointService checkpointService = new TransformCheckpointService(
Clock.systemUTC(),
Expand Down
Loading

0 comments on commit e8b2dca

Please sign in to comment.