From ae262067d6418dcca107b7839d581e9e549f14cd Mon Sep 17 00:00:00 2001 From: William Brafford Date: Wed, 29 Sep 2021 16:28:09 -0400 Subject: [PATCH 01/11] Implement and test get feature upgrade status API --- .../system/indices/FeatureUpgradeApiIT.java | 73 +++++++++++++++++++ .../migration/FeatureUpgradeIT.java | 40 ++++++++++ .../GetFeatureUpgradeStatusResponse.java | 26 +++++++ ...ransportGetFeatureUpgradeStatusAction.java | 50 ++++++++++--- 4 files changed, 180 insertions(+), 9 deletions(-) create mode 100644 qa/system-indices/src/javaRestTest/java/org/elasticsearch/system/indices/FeatureUpgradeApiIT.java create mode 100644 server/src/internalClusterTest/java/org/elasticsearch/migration/FeatureUpgradeIT.java diff --git a/qa/system-indices/src/javaRestTest/java/org/elasticsearch/system/indices/FeatureUpgradeApiIT.java b/qa/system-indices/src/javaRestTest/java/org/elasticsearch/system/indices/FeatureUpgradeApiIT.java new file mode 100644 index 0000000000000..a9fc84e2e8b2b --- /dev/null +++ b/qa/system-indices/src/javaRestTest/java/org/elasticsearch/system/indices/FeatureUpgradeApiIT.java @@ -0,0 +1,73 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +package org.elasticsearch.system.indices; + +import org.apache.http.util.EntityUtils; +import org.elasticsearch.Version; +import org.elasticsearch.client.Request; +import org.elasticsearch.client.Response; +import org.elasticsearch.common.settings.SecureString; +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.common.util.concurrent.ThreadContext; +import org.elasticsearch.common.xcontent.XContentUtils; +import org.elasticsearch.test.XContentTestUtils; +import org.elasticsearch.test.rest.ESRestTestCase; +import org.junit.After; +import org.junit.Before; + +import java.util.Collections; +import java.util.List; +import java.util.Map; + +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.hasSize; +import static org.hamcrest.Matchers.instanceOf; +import static org.hamcrest.Matchers.is; + +public class FeatureUpgradeApiIT extends ESRestTestCase { + + static final String BASIC_AUTH_VALUE = basicAuthHeaderValue("rest_user", new SecureString("rest-user-password".toCharArray())); + + @Override + protected Settings restClientSettings() { + return Settings.builder().put(ThreadContext.PREFIX + ".Authorization", BASIC_AUTH_VALUE).build(); + } + + @Before + public void testCreatingSystemIndex() throws Exception { + Response response = client().performRequest(new Request("PUT", "/_net_new_sys_index/_create")); + assertThat(response.getStatusLine().getStatusCode(), is(200)); + } + + @SuppressWarnings("unchecked") + public void testGetFeatureUpgradedStatuses() throws Exception { + Response response = client().performRequest(new Request("GET", "/_migration/system_features")); + assertThat(response.getStatusLine().getStatusCode(), is(200)); + XContentTestUtils.JsonMapView view = XContentTestUtils.createJsonMapView(response.getEntity().getContent()); + String upgradeStatus = view.get("upgrade_status"); + assertThat(upgradeStatus, equalTo("NO_UPGRADE_NEEDED")); + List> features = view.get("features"); + Map testFeature = features.stream() + .filter(feature -> "system indices qa".equals(feature.get("feature_name"))) + .findFirst() + .orElse(Collections.emptyMap()); + + assertThat(testFeature.size(), equalTo(4)); + assertThat(testFeature.get("minimum_index_version"), equalTo(Version.CURRENT.toString())); + assertThat(testFeature.get("upgrade_status"), equalTo("NO_UPGRADE_NEEDED")); + assertThat(testFeature.get("indices"), instanceOf(List.class)); + + assertThat((List) testFeature.get("indices"), hasSize(1)); + } + + @After + public void resetFeatures() throws Exception { + client().performRequest(new Request("POST", "/_features/_reset")); + } +} diff --git a/server/src/internalClusterTest/java/org/elasticsearch/migration/FeatureUpgradeIT.java b/server/src/internalClusterTest/java/org/elasticsearch/migration/FeatureUpgradeIT.java new file mode 100644 index 0000000000000..78ad436577711 --- /dev/null +++ b/server/src/internalClusterTest/java/org/elasticsearch/migration/FeatureUpgradeIT.java @@ -0,0 +1,40 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +package org.elasticsearch.migration; + +import org.elasticsearch.Version; +import org.elasticsearch.action.admin.cluster.migration.GetFeatureUpgradeStatusAction; +import org.elasticsearch.action.admin.cluster.migration.GetFeatureUpgradeStatusRequest; +import org.elasticsearch.action.admin.cluster.migration.GetFeatureUpgradeStatusResponse; +import org.elasticsearch.test.ESIntegTestCase; + +import java.util.List; + +import static org.hamcrest.Matchers.equalTo; + +public class FeatureUpgradeIT extends ESIntegTestCase { + + public void testGetFeatureUpgradeStatus() throws Exception { + GetFeatureUpgradeStatusResponse apiResponse = client().execute( + GetFeatureUpgradeStatusAction.INSTANCE, new GetFeatureUpgradeStatusRequest()) + .get(); + + GetFeatureUpgradeStatusResponse.FeatureUpgradeStatus status = new GetFeatureUpgradeStatusResponse.FeatureUpgradeStatus( + "tasks", + Version.CURRENT.toString(), + "NO_UPGRADE_NEEDED", + List.of() + ); + assertThat(apiResponse, equalTo( + new GetFeatureUpgradeStatusResponse( + List.of(status), + "NO_UPGRADE_NEEDED" + ))); + } +} diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/migration/GetFeatureUpgradeStatusResponse.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/migration/GetFeatureUpgradeStatusResponse.java index 63e0fd451fad4..c95d756ad09ca 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/migration/GetFeatureUpgradeStatusResponse.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/migration/GetFeatureUpgradeStatusResponse.java @@ -88,6 +88,14 @@ public int hashCode() { return Objects.hash(featureUpgradeStatuses, upgradeStatus); } + @Override + public String toString() { + return "GetFeatureUpgradeStatusResponse{" + + "featureUpgradeStatuses=" + featureUpgradeStatuses + + ", upgradeStatus='" + upgradeStatus + '\'' + + '}'; + } + /** * A class for a particular feature, showing whether it needs to be upgraded and the earliest * Elasticsearch version used to create one of this feature's system indices. @@ -177,6 +185,16 @@ public boolean equals(Object o) { public int hashCode() { return Objects.hash(featureName, minimumIndexVersion, upgradeStatus, indexVersions); } + + @Override + public String toString() { + return "FeatureUpgradeStatus{" + + "featureName='" + featureName + '\'' + + ", minimumIndexVersion='" + minimumIndexVersion + '\'' + + ", upgradeStatus='" + upgradeStatus + '\'' + + ", indexVersions=" + indexVersions + + '}'; + } } /** @@ -239,5 +257,13 @@ public boolean equals(Object o) { public int hashCode() { return Objects.hash(indexName, version); } + + @Override + public String toString() { + return "IndexVersion{" + + "indexName='" + indexName + '\'' + + ", version='" + version + '\'' + + '}'; + } } } diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/migration/TransportGetFeatureUpgradeStatusAction.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/migration/TransportGetFeatureUpgradeStatusAction.java index 77261d9463304..7197f0b525534 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/migration/TransportGetFeatureUpgradeStatusAction.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/migration/TransportGetFeatureUpgradeStatusAction.java @@ -8,15 +8,18 @@ package org.elasticsearch.action.admin.cluster.migration; +import org.elasticsearch.Version; import org.elasticsearch.action.ActionListener; import org.elasticsearch.action.support.ActionFilters; import org.elasticsearch.action.support.master.TransportMasterNodeAction; import org.elasticsearch.cluster.ClusterState; import org.elasticsearch.cluster.block.ClusterBlockException; import org.elasticsearch.cluster.block.ClusterBlockLevel; +import org.elasticsearch.cluster.metadata.IndexMetadata; import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver; import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.inject.Inject; +import org.elasticsearch.indices.IndexPatternMatcher; import org.elasticsearch.indices.SystemIndices; import org.elasticsearch.tasks.Task; import org.elasticsearch.threadpool.ThreadPool; @@ -24,6 +27,9 @@ import java.util.ArrayList; import java.util.List; +import java.util.Map; +import java.util.stream.Collectors; +import java.util.stream.Stream; /** * Transport class for the get feature upgrade status action @@ -60,16 +66,42 @@ public TransportGetFeatureUpgradeStatusAction( @Override protected void masterOperation(Task task, GetFeatureUpgradeStatusRequest request, ClusterState state, ActionListener listener) throws Exception { - List indexVersions = new ArrayList<>(); - indexVersions.add(new GetFeatureUpgradeStatusResponse.IndexVersion(".security-7", "7.1.1")); + List features = new ArrayList<>(); - features.add(new GetFeatureUpgradeStatusResponse.FeatureUpgradeStatus( - "security", - "7.1.1", - "UPGRADE_NEEDED", - indexVersions - )); - listener.onResponse(new GetFeatureUpgradeStatusResponse(features, "UPGRADE_NEEDED")); + boolean isUpgradeNeeded = false; + + for (Map.Entry featureEntry : systemIndices.getFeatures().entrySet()) { + String featureName = featureEntry.getKey(); + SystemIndices.Feature feature = featureEntry.getValue(); + + Version minimumVersion = Version.CURRENT; + + List indexVersions = new ArrayList<>(); + List descriptors = Stream.concat(feature.getIndexDescriptors().stream(), + feature.getAssociatedIndexDescriptors().stream()).collect(Collectors.toList()); + for (IndexPatternMatcher descriptor : descriptors) { + List concreteIndices = descriptor.getMatchingIndices(state.metadata()); + for (String index : concreteIndices) { + IndexMetadata metadata = state.metadata().index(index); + indexVersions.add(new GetFeatureUpgradeStatusResponse.IndexVersion(index, metadata.getCreationVersion().toString())); + minimumVersion = Version.min(metadata.getCreationVersion(), minimumVersion); + } + } + + if (minimumVersion.before(Version.V_7_0_0)) { + isUpgradeNeeded = true; + } + + features.add(new GetFeatureUpgradeStatusResponse.FeatureUpgradeStatus( + featureName, + minimumVersion.toString(), + minimumVersion.before(Version.V_7_0_0) ? "UPGRADE_NEEDED" : "NO_UPGRADE_NEEDED", + indexVersions + )); + } + + + listener.onResponse(new GetFeatureUpgradeStatusResponse(features, isUpgradeNeeded ? "UPGRADE_NEEDED" : "NO_UPGRADE_NEEDED")); } @Override From f5d141426d99a6bf6a6a00e7357acf8b19f0821b Mon Sep 17 00:00:00 2001 From: William Brafford Date: Thu, 30 Sep 2021 16:56:15 -0400 Subject: [PATCH 02/11] Use Version object instead of string versions --- .../org/elasticsearch/client/MigrationIT.java | 25 +++++++++++++------ .../GetFeatureUpgradeStatusResponseTests.java | 9 ++++--- .../system/indices/FeatureUpgradeApiIT.java | 2 -- .../migration/FeatureUpgradeIT.java | 2 +- .../GetFeatureUpgradeStatusResponse.java | 25 ++++++++++--------- ...ransportGetFeatureUpgradeStatusAction.java | 4 +-- .../GetFeatureUpgradeStatusResponseTests.java | 5 ++-- 7 files changed, 41 insertions(+), 31 deletions(-) diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/MigrationIT.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/MigrationIT.java index 219e41ade3980..06ee1c97039f6 100644 --- a/client/rest-high-level/src/test/java/org/elasticsearch/client/MigrationIT.java +++ b/client/rest-high-level/src/test/java/org/elasticsearch/client/MigrationIT.java @@ -8,6 +8,7 @@ package org.elasticsearch.client; +import org.elasticsearch.Version; import org.elasticsearch.client.migration.DeprecationInfoRequest; import org.elasticsearch.client.migration.DeprecationInfoResponse; import org.elasticsearch.client.migration.GetFeatureUpgradeStatusRequest; @@ -18,8 +19,11 @@ import java.io.IOException; import java.util.Collections; +import java.util.Optional; import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.greaterThanOrEqualTo; +import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.nullValue; public class MigrationIT extends ESRestHighLevelClientTestCase { @@ -38,19 +42,24 @@ public void testGetDeprecationInfo() throws IOException { public void testGetFeatureUpgradeStatus() throws IOException { GetFeatureUpgradeStatusRequest request = new GetFeatureUpgradeStatusRequest(); GetFeatureUpgradeStatusResponse response = highLevelClient().migration().getFeatureUpgradeStatus(request, RequestOptions.DEFAULT); - assertThat(response.getUpgradeStatus(), equalTo("UPGRADE_NEEDED")); - assertThat(response.getFeatureUpgradeStatuses().size(), equalTo(1)); - GetFeatureUpgradeStatusResponse.FeatureUpgradeStatus status = response.getFeatureUpgradeStatuses().get(0); - assertThat(status.getUpgradeStatus(), equalTo("UPGRADE_NEEDED")); - assertThat(status.getMinimumIndexVersion(), equalTo("7.1.1")); - assertThat(status.getFeatureName(), equalTo("security")); - assertThat(status.getIndexVersions().size(), equalTo(1)); + assertThat(response.getUpgradeStatus(), equalTo("NO_UPGRADE_NEEDED")); + assertThat(response.getFeatureUpgradeStatuses().size(), greaterThanOrEqualTo(1)); + Optional optionalTasksStatus = response.getFeatureUpgradeStatuses().stream() + .filter(status -> "tasks".equals(status.getFeatureName())) + .findFirst(); + + assertThat(optionalTasksStatus.isPresent(), is(true)); + + GetFeatureUpgradeStatusResponse.FeatureUpgradeStatus tasksStatus = optionalTasksStatus.get(); + + assertThat(tasksStatus.getUpgradeStatus(), equalTo("NO_UPGRADE_NEEDED")); + assertThat(tasksStatus.getMinimumIndexVersion(), equalTo(Version.CURRENT.toString())); + assertThat(tasksStatus.getFeatureName(), equalTo("tasks")); } public void testPostFeatureUpgradeStatus() throws IOException { PostFeatureUpgradeRequest request = new PostFeatureUpgradeRequest(); PostFeatureUpgradeResponse response = highLevelClient().migration().postFeatureUpgrade(request, RequestOptions.DEFAULT); - // a test like this cannot test actual deprecations assertThat(response.isAccepted(), equalTo(true)); assertThat(response.getFeatures().size(), equalTo(1)); PostFeatureUpgradeResponse.Feature feature = response.getFeatures().get(0); diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/migration/GetFeatureUpgradeStatusResponseTests.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/migration/GetFeatureUpgradeStatusResponseTests.java index 9c9c4ef32b514..d177dea776bb0 100644 --- a/client/rest-high-level/src/test/java/org/elasticsearch/client/migration/GetFeatureUpgradeStatusResponseTests.java +++ b/client/rest-high-level/src/test/java/org/elasticsearch/client/migration/GetFeatureUpgradeStatusResponseTests.java @@ -8,6 +8,7 @@ package org.elasticsearch.client.migration; +import org.elasticsearch.Version; import org.elasticsearch.client.AbstractResponseTestCase; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.common.xcontent.XContentType; @@ -37,12 +38,12 @@ protected org.elasticsearch.action.admin.cluster.migration.GetFeatureUpgradeStat randomList(5, () -> new org.elasticsearch.action.admin.cluster.migration.GetFeatureUpgradeStatusResponse.FeatureUpgradeStatus( randomAlphaOfLengthBetween(3, 20), - randomAlphaOfLengthBetween(5, 9), + randomFrom(Version.CURRENT, Version.CURRENT.minimumCompatibilityVersion()), randomAlphaOfLengthBetween(4, 16), randomList(4, () -> new org.elasticsearch.action.admin.cluster.migration.GetFeatureUpgradeStatusResponse.IndexVersion( randomAlphaOfLengthBetween(3, 20), - randomAlphaOfLengthBetween(5, 9))) + randomFrom(Version.CURRENT, Version.CURRENT.minimumCompatibilityVersion()))) )), randomAlphaOfLength(5) ); @@ -71,7 +72,7 @@ protected void assertInstances( GetFeatureUpgradeStatusResponse.FeatureUpgradeStatus clientStatus = clientInstance.getFeatureUpgradeStatuses().get(i); assertThat(clientStatus.getFeatureName(), equalTo(serverTestStatus.getFeatureName())); - assertThat(clientStatus.getMinimumIndexVersion(), equalTo(serverTestStatus.getMinimumIndexVersion())); + assertThat(clientStatus.getMinimumIndexVersion(), equalTo(serverTestStatus.getMinimumIndexVersion().toString())); assertThat(clientStatus.getUpgradeStatus(), equalTo(serverTestStatus.getUpgradeStatus())); assertThat(clientStatus.getIndexVersions(), hasSize(serverTestStatus.getIndexVersions().size())); @@ -82,7 +83,7 @@ protected void assertInstances( GetFeatureUpgradeStatusResponse.IndexVersion clientIndexVersion = clientStatus.getIndexVersions().get(j); assertThat(clientIndexVersion.getIndexName(), equalTo(serverIndexVersion.getIndexName())); - assertThat(clientIndexVersion.getVersion(), equalTo(serverIndexVersion.getVersion())); + assertThat(clientIndexVersion.getVersion(), equalTo(serverIndexVersion.getVersion().toString())); } } } diff --git a/qa/system-indices/src/javaRestTest/java/org/elasticsearch/system/indices/FeatureUpgradeApiIT.java b/qa/system-indices/src/javaRestTest/java/org/elasticsearch/system/indices/FeatureUpgradeApiIT.java index a9fc84e2e8b2b..598bc22d52237 100644 --- a/qa/system-indices/src/javaRestTest/java/org/elasticsearch/system/indices/FeatureUpgradeApiIT.java +++ b/qa/system-indices/src/javaRestTest/java/org/elasticsearch/system/indices/FeatureUpgradeApiIT.java @@ -8,14 +8,12 @@ package org.elasticsearch.system.indices; -import org.apache.http.util.EntityUtils; import org.elasticsearch.Version; import org.elasticsearch.client.Request; import org.elasticsearch.client.Response; import org.elasticsearch.common.settings.SecureString; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.util.concurrent.ThreadContext; -import org.elasticsearch.common.xcontent.XContentUtils; import org.elasticsearch.test.XContentTestUtils; import org.elasticsearch.test.rest.ESRestTestCase; import org.junit.After; diff --git a/server/src/internalClusterTest/java/org/elasticsearch/migration/FeatureUpgradeIT.java b/server/src/internalClusterTest/java/org/elasticsearch/migration/FeatureUpgradeIT.java index 78ad436577711..0c3084f73794d 100644 --- a/server/src/internalClusterTest/java/org/elasticsearch/migration/FeatureUpgradeIT.java +++ b/server/src/internalClusterTest/java/org/elasticsearch/migration/FeatureUpgradeIT.java @@ -27,7 +27,7 @@ GetFeatureUpgradeStatusAction.INSTANCE, new GetFeatureUpgradeStatusRequest()) GetFeatureUpgradeStatusResponse.FeatureUpgradeStatus status = new GetFeatureUpgradeStatusResponse.FeatureUpgradeStatus( "tasks", - Version.CURRENT.toString(), + Version.CURRENT, "NO_UPGRADE_NEEDED", List.of() ); diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/migration/GetFeatureUpgradeStatusResponse.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/migration/GetFeatureUpgradeStatusResponse.java index c95d756ad09ca..1a2585de9756a 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/migration/GetFeatureUpgradeStatusResponse.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/migration/GetFeatureUpgradeStatusResponse.java @@ -8,6 +8,7 @@ package org.elasticsearch.action.admin.cluster.migration; +import org.elasticsearch.Version; import org.elasticsearch.action.ActionResponse; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; @@ -102,7 +103,7 @@ public String toString() { */ public static class FeatureUpgradeStatus implements Writeable, ToXContentObject { private final String featureName; - private final String minimumIndexVersion; + private final Version minimumIndexVersion; private final String upgradeStatus; private final List indexVersions; @@ -112,7 +113,7 @@ public static class FeatureUpgradeStatus implements Writeable, ToXContentObject * @param upgradeStatus Whether the feature needs to be upgraded * @param indexVersions A list of this feature's concrete indices and the Elasticsearch version that created them */ - public FeatureUpgradeStatus(String featureName, String minimumIndexVersion, + public FeatureUpgradeStatus(String featureName, Version minimumIndexVersion, String upgradeStatus, List indexVersions) { this.featureName = featureName; this.minimumIndexVersion = minimumIndexVersion; @@ -126,7 +127,7 @@ public FeatureUpgradeStatus(String featureName, String minimumIndexVersion, */ public FeatureUpgradeStatus(StreamInput in) throws IOException { this.featureName = in.readString(); - this.minimumIndexVersion = in.readString(); + this.minimumIndexVersion = Version.readVersion(in); this.upgradeStatus = in.readString(); this.indexVersions = in.readList(IndexVersion::new); } @@ -135,7 +136,7 @@ public String getFeatureName() { return this.featureName; } - public String getMinimumIndexVersion() { + public Version getMinimumIndexVersion() { return this.minimumIndexVersion; } @@ -150,7 +151,7 @@ public List getIndexVersions() { @Override public void writeTo(StreamOutput out) throws IOException { out.writeString(this.featureName); - out.writeString(this.minimumIndexVersion); + Version.writeVersion(this.minimumIndexVersion, out); out.writeString(this.upgradeStatus); out.writeList(this.indexVersions); } @@ -159,7 +160,7 @@ public void writeTo(StreamOutput out) throws IOException { public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { builder.startObject(); builder.field("feature_name", this.featureName); - builder.field("minimum_index_version", this.minimumIndexVersion); + builder.field("minimum_index_version", this.minimumIndexVersion.toString()); builder.field("upgrade_status", this.upgradeStatus); builder.startArray("indices"); for (IndexVersion version : this.indexVersions) { @@ -202,13 +203,13 @@ public String toString() { */ public static class IndexVersion implements Writeable, ToXContentObject { private final String indexName; - private final String version; + private final Version version; /** * @param indexName Name of the index * @param version Version of Elasticsearch that created the index */ - public IndexVersion(String indexName, String version) { + public IndexVersion(String indexName, Version version) { this.indexName = indexName; this.version = version; } @@ -219,28 +220,28 @@ public IndexVersion(String indexName, String version) { */ public IndexVersion(StreamInput in) throws IOException { this.indexName = in.readString(); - this.version = in.readString(); + this.version = Version.readVersion(in); } public String getIndexName() { return this.indexName; } - public String getVersion() { + public Version getVersion() { return this.version; } @Override public void writeTo(StreamOutput out) throws IOException { out.writeString(this.indexName); - out.writeString(this.version); + Version.writeVersion(this.version, out); } @Override public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { builder.startObject(); builder.field("index", this.indexName); - builder.field("version", this.version); + builder.field("version", this.version.toString()); builder.endObject(); return builder; } diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/migration/TransportGetFeatureUpgradeStatusAction.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/migration/TransportGetFeatureUpgradeStatusAction.java index 7197f0b525534..5069cbd13f5f3 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/migration/TransportGetFeatureUpgradeStatusAction.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/migration/TransportGetFeatureUpgradeStatusAction.java @@ -83,7 +83,7 @@ protected void masterOperation(Task task, GetFeatureUpgradeStatusRequest request List concreteIndices = descriptor.getMatchingIndices(state.metadata()); for (String index : concreteIndices) { IndexMetadata metadata = state.metadata().index(index); - indexVersions.add(new GetFeatureUpgradeStatusResponse.IndexVersion(index, metadata.getCreationVersion().toString())); + indexVersions.add(new GetFeatureUpgradeStatusResponse.IndexVersion(index, metadata.getCreationVersion())); minimumVersion = Version.min(metadata.getCreationVersion(), minimumVersion); } } @@ -94,7 +94,7 @@ protected void masterOperation(Task task, GetFeatureUpgradeStatusRequest request features.add(new GetFeatureUpgradeStatusResponse.FeatureUpgradeStatus( featureName, - minimumVersion.toString(), + minimumVersion, minimumVersion.before(Version.V_7_0_0) ? "UPGRADE_NEEDED" : "NO_UPGRADE_NEEDED", indexVersions )); diff --git a/server/src/test/java/org/elasticsearch/action/admin/cluster/migration/GetFeatureUpgradeStatusResponseTests.java b/server/src/test/java/org/elasticsearch/action/admin/cluster/migration/GetFeatureUpgradeStatusResponseTests.java index 58483df600b72..58cc92237c993 100644 --- a/server/src/test/java/org/elasticsearch/action/admin/cluster/migration/GetFeatureUpgradeStatusResponseTests.java +++ b/server/src/test/java/org/elasticsearch/action/admin/cluster/migration/GetFeatureUpgradeStatusResponseTests.java @@ -8,6 +8,7 @@ package org.elasticsearch.action.admin.cluster.migration; +import org.elasticsearch.Version; import org.elasticsearch.common.io.stream.Writeable; import org.elasticsearch.test.AbstractWireSerializingTestCase; @@ -55,7 +56,7 @@ public void testConstructorHandlesNullLists() { private static GetFeatureUpgradeStatusResponse.FeatureUpgradeStatus createFeatureStatus() { return new GetFeatureUpgradeStatusResponse.FeatureUpgradeStatus( randomAlphaOfLengthBetween(3, 20), - randomAlphaOfLengthBetween(5, 9), + randomFrom(Version.CURRENT, Version.CURRENT.minimumCompatibilityVersion()), randomAlphaOfLengthBetween(4, 16), randomList(4, GetFeatureUpgradeStatusResponseTests::getIndexVersion) ); @@ -64,7 +65,7 @@ private static GetFeatureUpgradeStatusResponse.FeatureUpgradeStatus createFeatur private static GetFeatureUpgradeStatusResponse.IndexVersion getIndexVersion() { return new GetFeatureUpgradeStatusResponse.IndexVersion( randomAlphaOfLengthBetween(3, 20), - randomAlphaOfLengthBetween(5, 9) + randomFrom(Version.CURRENT, Version.CURRENT.minimumCompatibilityVersion()) ); } } From d609f7ecebfc9017edad48c7e490e6104c2c664f Mon Sep 17 00:00:00 2001 From: William Brafford Date: Thu, 30 Sep 2021 17:37:23 -0400 Subject: [PATCH 03/11] Refactor for streams --- .../system/indices/FeatureUpgradeApiIT.java | 3 +- ...ransportGetFeatureUpgradeStatusAction.java | 68 ++++++++++--------- 2 files changed, 38 insertions(+), 33 deletions(-) diff --git a/qa/system-indices/src/javaRestTest/java/org/elasticsearch/system/indices/FeatureUpgradeApiIT.java b/qa/system-indices/src/javaRestTest/java/org/elasticsearch/system/indices/FeatureUpgradeApiIT.java index 598bc22d52237..0fa4731368b25 100644 --- a/qa/system-indices/src/javaRestTest/java/org/elasticsearch/system/indices/FeatureUpgradeApiIT.java +++ b/qa/system-indices/src/javaRestTest/java/org/elasticsearch/system/indices/FeatureUpgradeApiIT.java @@ -17,7 +17,6 @@ import org.elasticsearch.test.XContentTestUtils; import org.elasticsearch.test.rest.ESRestTestCase; import org.junit.After; -import org.junit.Before; import java.util.Collections; import java.util.List; @@ -37,7 +36,6 @@ protected Settings restClientSettings() { return Settings.builder().put(ThreadContext.PREFIX + ".Authorization", BASIC_AUTH_VALUE).build(); } - @Before public void testCreatingSystemIndex() throws Exception { Response response = client().performRequest(new Request("PUT", "/_net_new_sys_index/_create")); assertThat(response.getStatusLine().getStatusCode(), is(200)); @@ -45,6 +43,7 @@ public void testCreatingSystemIndex() throws Exception { @SuppressWarnings("unchecked") public void testGetFeatureUpgradedStatuses() throws Exception { + client().performRequest(new Request("PUT", "/_net_new_sys_index/_create")); Response response = client().performRequest(new Request("GET", "/_migration/system_features")); assertThat(response.getStatusLine().getStatusCode(), is(200)); XContentTestUtils.JsonMapView view = XContentTestUtils.createJsonMapView(response.getEntity().getContent()); diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/migration/TransportGetFeatureUpgradeStatusAction.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/migration/TransportGetFeatureUpgradeStatusAction.java index 5069cbd13f5f3..7d68528263e7b 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/migration/TransportGetFeatureUpgradeStatusAction.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/migration/TransportGetFeatureUpgradeStatusAction.java @@ -15,17 +15,15 @@ import org.elasticsearch.cluster.ClusterState; import org.elasticsearch.cluster.block.ClusterBlockException; import org.elasticsearch.cluster.block.ClusterBlockLevel; -import org.elasticsearch.cluster.metadata.IndexMetadata; import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver; import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.inject.Inject; -import org.elasticsearch.indices.IndexPatternMatcher; import org.elasticsearch.indices.SystemIndices; import org.elasticsearch.tasks.Task; import org.elasticsearch.threadpool.ThreadPool; import org.elasticsearch.transport.TransportService; -import java.util.ArrayList; +import java.util.Collection; import java.util.List; import java.util.Map; import java.util.stream.Collectors; @@ -67,41 +65,49 @@ public TransportGetFeatureUpgradeStatusAction( protected void masterOperation(Task task, GetFeatureUpgradeStatusRequest request, ClusterState state, ActionListener listener) throws Exception { - List features = new ArrayList<>(); - boolean isUpgradeNeeded = false; + List features = systemIndices.getFeatures().entrySet().stream() + .map(entry -> getFeatureUpgradeStatus(state, entry)) + .collect(Collectors.toList()); - for (Map.Entry featureEntry : systemIndices.getFeatures().entrySet()) { - String featureName = featureEntry.getKey(); - SystemIndices.Feature feature = featureEntry.getValue(); + boolean isUpgradeNeeded = features.stream() + .map(GetFeatureUpgradeStatusResponse.FeatureUpgradeStatus::getMinimumIndexVersion) + .min(Version::compareTo) + .orElse(Version.CURRENT) + .before(Version.V_7_0_0); - Version minimumVersion = Version.CURRENT; + listener.onResponse(new GetFeatureUpgradeStatusResponse(features, isUpgradeNeeded ? "UPGRADE_NEEDED" : "NO_UPGRADE_NEEDED")); + } - List indexVersions = new ArrayList<>(); - List descriptors = Stream.concat(feature.getIndexDescriptors().stream(), - feature.getAssociatedIndexDescriptors().stream()).collect(Collectors.toList()); - for (IndexPatternMatcher descriptor : descriptors) { - List concreteIndices = descriptor.getMatchingIndices(state.metadata()); - for (String index : concreteIndices) { - IndexMetadata metadata = state.metadata().index(index); - indexVersions.add(new GetFeatureUpgradeStatusResponse.IndexVersion(index, metadata.getCreationVersion())); - minimumVersion = Version.min(metadata.getCreationVersion(), minimumVersion); - } - } + private GetFeatureUpgradeStatusResponse.FeatureUpgradeStatus getFeatureUpgradeStatus( + ClusterState state, Map.Entry entry) { - if (minimumVersion.before(Version.V_7_0_0)) { - isUpgradeNeeded = true; - } + String featureName = entry.getKey(); + SystemIndices.Feature feature = entry.getValue(); - features.add(new GetFeatureUpgradeStatusResponse.FeatureUpgradeStatus( - featureName, - minimumVersion, - minimumVersion.before(Version.V_7_0_0) ? "UPGRADE_NEEDED" : "NO_UPGRADE_NEEDED", - indexVersions - )); - } + List indexVersions = getIndexVersions(state, feature); + Version minimumVersion = indexVersions.stream() + .map(GetFeatureUpgradeStatusResponse.IndexVersion::getVersion) + .min(Version::compareTo) + .orElse(Version.CURRENT); - listener.onResponse(new GetFeatureUpgradeStatusResponse(features, isUpgradeNeeded ? "UPGRADE_NEEDED" : "NO_UPGRADE_NEEDED")); + return new GetFeatureUpgradeStatusResponse.FeatureUpgradeStatus( + featureName, + minimumVersion, + minimumVersion.before(Version.V_7_0_0) ? "UPGRADE_NEEDED" : "NO_UPGRADE_NEEDED", + indexVersions + ); + } + + private List getIndexVersions(ClusterState state, SystemIndices.Feature feature) { + return Stream.of(feature.getIndexDescriptors(), feature.getAssociatedIndexDescriptors()) + .flatMap(Collection::stream) + .flatMap(descriptor -> descriptor.getMatchingIndices(state.metadata()).stream()) + .map(index -> state.metadata().index(index)) + .map(indexMetadata -> new GetFeatureUpgradeStatusResponse.IndexVersion( + indexMetadata.getIndex().getName(), + indexMetadata.getCreationVersion())) + .collect(Collectors.toList()); } @Override From 4db32663c1e19e97a571e7eb694680ec63367bd2 Mon Sep 17 00:00:00 2001 From: William Brafford Date: Mon, 4 Oct 2021 15:27:55 -0400 Subject: [PATCH 04/11] Add integration test for feature upgrade endpoint --- .../upgrades/FeatureUpgradeIT.java | 91 +++++++++++++++++++ 1 file changed, 91 insertions(+) create mode 100644 qa/rolling-upgrade/src/test/java/org/elasticsearch/upgrades/FeatureUpgradeIT.java diff --git a/qa/rolling-upgrade/src/test/java/org/elasticsearch/upgrades/FeatureUpgradeIT.java b/qa/rolling-upgrade/src/test/java/org/elasticsearch/upgrades/FeatureUpgradeIT.java new file mode 100644 index 0000000000000..de1de6ec9f158 --- /dev/null +++ b/qa/rolling-upgrade/src/test/java/org/elasticsearch/upgrades/FeatureUpgradeIT.java @@ -0,0 +1,91 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +package org.elasticsearch.upgrades; + +import org.elasticsearch.client.Request; +import org.elasticsearch.client.ResponseException; +import org.elasticsearch.test.XContentTestUtils; + +import java.util.Collections; +import java.util.List; +import java.util.Map; + +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.is; + +public class FeatureUpgradeIT extends AbstractRollingTestCase { + + @SuppressWarnings("unchecked") + public void testGetFeatureUpgradeStatus() throws Exception { + + final String systemIndexWarning = "this request accesses system indices: [.watches], but in a future major version, direct " + + "access to system indices will be prevented by default"; + if (CLUSTER_TYPE == ClusterType.OLD) { + // setup - put something in the watcher index + Request createWatch = new Request("PUT", "/_watcher/watch/1"); + createWatch.addParameter("active", "false"); + createWatch.setJsonEntity("{\n" + + " \"trigger\" : {\n" + + " \"schedule\" : { \"interval\" : \"10s\" } \n" + + " },\n" + + " \"input\" : {\n" + + " \"search\" : {\n" + + " \"request\" : {\n" + + " \"indices\" : [ \"logs\" ],\n" + + " \"body\" : {\n" + + " \"query\" : {\n" + + " \"match\" : { \"message\": \"error\" }\n" + + " }\n" + + " }\n" + + " }\n" + + " }\n" + + " }\n" + + "}"); + + client().performRequest(createWatch); + + Request getTasksIndex = new Request("GET", "/.watches"); + getTasksIndex.setOptions(expectVersionSpecificWarnings(v -> { + v.current(systemIndexWarning); + v.compatible(systemIndexWarning); + })); + getTasksIndex.addParameter("allow_no_indices", "false"); + + getTasksIndex.setOptions(expectVersionSpecificWarnings(v -> { + v.current(systemIndexWarning); + v.compatible(systemIndexWarning); + })); + assertBusy(() -> { + try { + assertThat(client().performRequest(getTasksIndex).getStatusLine().getStatusCode(), is(200)); + } catch (ResponseException e) { + throw new AssertionError(".watcher index does not exist yet"); + } + }); + + } else if (CLUSTER_TYPE == ClusterType.UPGRADED) { + // check results + assertBusy(() -> { + Request clusterStateRequest = new Request("GET", "/_migration/system_features"); + XContentTestUtils.JsonMapView view = new XContentTestUtils.JsonMapView( + entityAsMap(client().performRequest(clusterStateRequest))); + + List> features = view.get("features"); + Map feature = features.stream() + .filter(e -> "watcher".equals(e.get("feature_name"))) + .findFirst() + .orElse(Collections.emptyMap()); + + assertThat(feature.size(), equalTo(4)); + assertThat(feature.get("minimum_index_version"), equalTo(UPGRADE_FROM_VERSION.toString())); + }); + } + } + +} From 6bfe376e21aeea18e5429df2c3a12085f35b0fbc Mon Sep 17 00:00:00 2001 From: William Brafford Date: Mon, 4 Oct 2021 16:19:47 -0400 Subject: [PATCH 05/11] Use tasks instead of watcher to avoid license stuff in testing --- .../upgrades/FeatureUpgradeIT.java | 62 +++++++++++-------- 1 file changed, 36 insertions(+), 26 deletions(-) diff --git a/qa/rolling-upgrade/src/test/java/org/elasticsearch/upgrades/FeatureUpgradeIT.java b/qa/rolling-upgrade/src/test/java/org/elasticsearch/upgrades/FeatureUpgradeIT.java index de1de6ec9f158..91697b32bebc2 100644 --- a/qa/rolling-upgrade/src/test/java/org/elasticsearch/upgrades/FeatureUpgradeIT.java +++ b/qa/rolling-upgrade/src/test/java/org/elasticsearch/upgrades/FeatureUpgradeIT.java @@ -24,33 +24,43 @@ public class FeatureUpgradeIT extends AbstractRollingTestCase { @SuppressWarnings("unchecked") public void testGetFeatureUpgradeStatus() throws Exception { - final String systemIndexWarning = "this request accesses system indices: [.watches], but in a future major version, direct " + + final String systemIndexWarning = "this request accesses system indices: [.tasks], but in a future major version, direct " + "access to system indices will be prevented by default"; if (CLUSTER_TYPE == ClusterType.OLD) { // setup - put something in the watcher index - Request createWatch = new Request("PUT", "/_watcher/watch/1"); - createWatch.addParameter("active", "false"); - createWatch.setJsonEntity("{\n" + - " \"trigger\" : {\n" + - " \"schedule\" : { \"interval\" : \"10s\" } \n" + - " },\n" + - " \"input\" : {\n" + - " \"search\" : {\n" + - " \"request\" : {\n" + - " \"indices\" : [ \"logs\" ],\n" + - " \"body\" : {\n" + - " \"query\" : {\n" + - " \"match\" : { \"message\": \"error\" }\n" + - " }\n" + - " }\n" + - " }\n" + - " }\n" + - " }\n" + - "}"); - - client().performRequest(createWatch); - - Request getTasksIndex = new Request("GET", "/.watches"); + // create index + Request createTestIndex = new Request("PUT", "/test_index_old"); + createTestIndex.setJsonEntity("{\"settings\": {\"index.number_of_replicas\": 0}}"); + client().performRequest(createTestIndex); + + Request bulk = new Request("POST", "/_bulk"); + bulk.addParameter("refresh", "true"); + bulk.setJsonEntity("{\"index\": {\"_index\": \"test_index_old\"}}\n" + + "{\"f1\": \"v1\", \"f2\": \"v2\"}\n"); + client().performRequest(bulk); + + // start a async reindex job + Request reindex = new Request("POST", "/_reindex"); + reindex.setJsonEntity( + "{\n" + + " \"source\":{\n" + + " \"index\":\"test_index_old\"\n" + + " },\n" + + " \"dest\":{\n" + + " \"index\":\"test_index_reindex\"\n" + + " }\n" + + "}"); + reindex.addParameter("wait_for_completion", "false"); + Map response = entityAsMap(client().performRequest(reindex)); + String taskId = (String) response.get("task"); + + // wait for task + Request getTask = new Request("GET", "/_tasks/" + taskId); + getTask.addParameter("wait_for_completion", "true"); + client().performRequest(getTask); + + // make sure .tasks index exists + Request getTasksIndex = new Request("GET", "/.tasks"); getTasksIndex.setOptions(expectVersionSpecificWarnings(v -> { v.current(systemIndexWarning); v.compatible(systemIndexWarning); @@ -65,7 +75,7 @@ public void testGetFeatureUpgradeStatus() throws Exception { try { assertThat(client().performRequest(getTasksIndex).getStatusLine().getStatusCode(), is(200)); } catch (ResponseException e) { - throw new AssertionError(".watcher index does not exist yet"); + throw new AssertionError(".tasks index does not exist yet"); } }); @@ -78,7 +88,7 @@ public void testGetFeatureUpgradeStatus() throws Exception { List> features = view.get("features"); Map feature = features.stream() - .filter(e -> "watcher".equals(e.get("feature_name"))) + .filter(e -> "tasks".equals(e.get("feature_name"))) .findFirst() .orElse(Collections.emptyMap()); From 8fe49bfcc15c953c2e19ef119c1586746018039e Mon Sep 17 00:00:00 2001 From: William Brafford Date: Tue, 5 Oct 2021 14:10:35 -0400 Subject: [PATCH 06/11] Add sort order on response and fix docs test --- .../migration/apis/feature_upgrade.asciidoc | 79 ++++++++++++++++--- ...ransportGetFeatureUpgradeStatusAction.java | 3 + 2 files changed, 73 insertions(+), 9 deletions(-) diff --git a/docs/reference/migration/apis/feature_upgrade.asciidoc b/docs/reference/migration/apis/feature_upgrade.asciidoc index 7c50460e8b27d..88cd5d477f4d2 100644 --- a/docs/reference/migration/apis/feature_upgrade.asciidoc +++ b/docs/reference/migration/apis/feature_upgrade.asciidoc @@ -43,19 +43,80 @@ Example response: -------------------------------------------------- { "features" : [ + { + "feature_name" : "async_search", + "minimum_index_version" : "8.0.0", + "upgrade_status" : "NO_UPGRADE_NEEDED", + "indices" : [ ] + }, + { + "feature_name" : "enrich", + "minimum_index_version" : "8.0.0", + "upgrade_status" : "NO_UPGRADE_NEEDED", + "indices" : [ ] + }, + { + "feature_name" : "fleet", + "minimum_index_version" : "8.0.0", + "upgrade_status" : "NO_UPGRADE_NEEDED", + "indices" : [ ] + }, + { + "feature_name" : "geoip", + "minimum_index_version" : "8.0.0", + "upgrade_status" : "NO_UPGRADE_NEEDED", + "indices" : [ ] + }, + { + "feature_name" : "kibana", + "minimum_index_version" : "8.0.0", + "upgrade_status" : "NO_UPGRADE_NEEDED", + "indices" : [ ] + }, + { + "feature_name" : "logstash_management", + "minimum_index_version" : "8.0.0", + "upgrade_status" : "NO_UPGRADE_NEEDED", + "indices" : [ ] + }, + { + "feature_name" : "machine_learning", + "minimum_index_version" : "8.0.0", + "upgrade_status" : "NO_UPGRADE_NEEDED", + "indices" : [ ] + }, + { + "feature_name" : "searchable_snapshots", + "minimum_index_version" : "8.0.0", + "upgrade_status" : "NO_UPGRADE_NEEDED", + "indices" : [ ] + }, { "feature_name" : "security", - "minimum_index_version" : "7.1.1", - "upgrade_status" : "UPGRADE_NEEDED", - "indices" : [ - { - "index" : ".security-7", - "version" : "7.1.1" - } - ] + "minimum_index_version" : "8.0.0", + "upgrade_status" : "NO_UPGRADE_NEEDED", + "indices" : [ ] + }, + { + "feature_name" : "tasks", + "minimum_index_version" : "8.0.0", + "upgrade_status" : "NO_UPGRADE_NEEDED", + "indices" : [ ] + }, + { + "feature_name" : "transform", + "minimum_index_version" : "8.0.0", + "upgrade_status" : "NO_UPGRADE_NEEDED", + "indices" : [ ] + }, + { + "feature_name" : "watcher", + "minimum_index_version" : "8.0.0", + "upgrade_status" : "NO_UPGRADE_NEEDED", + "indices" : [ ] } ], - "upgrade_status" : "UPGRADE_NEEDED" + "upgrade_status" : "NO_UPGRADE_NEEDED" } -------------------------------------------------- diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/migration/TransportGetFeatureUpgradeStatusAction.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/migration/TransportGetFeatureUpgradeStatusAction.java index 7d68528263e7b..06c54bc70afb9 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/migration/TransportGetFeatureUpgradeStatusAction.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/migration/TransportGetFeatureUpgradeStatusAction.java @@ -24,6 +24,7 @@ import org.elasticsearch.transport.TransportService; import java.util.Collection; +import java.util.Comparator; import java.util.List; import java.util.Map; import java.util.stream.Collectors; @@ -66,6 +67,7 @@ protected void masterOperation(Task task, GetFeatureUpgradeStatusRequest request ActionListener listener) throws Exception { List features = systemIndices.getFeatures().entrySet().stream() + .sorted(Map.Entry.comparingByKey()) .map(entry -> getFeatureUpgradeStatus(state, entry)) .collect(Collectors.toList()); @@ -103,6 +105,7 @@ private List getIndexVersions(Clus return Stream.of(feature.getIndexDescriptors(), feature.getAssociatedIndexDescriptors()) .flatMap(Collection::stream) .flatMap(descriptor -> descriptor.getMatchingIndices(state.metadata()).stream()) + .sorted(String::compareTo) .map(index -> state.metadata().index(index)) .map(indexMetadata -> new GetFeatureUpgradeStatusResponse.IndexVersion( indexMetadata.getIndex().getName(), From 555abaa5f18126257671612e6995603f11da5668 Mon Sep 17 00:00:00 2001 From: William Brafford Date: Tue, 5 Oct 2021 14:48:22 -0400 Subject: [PATCH 07/11] Use different index to avoid conflict with other system test --- .../java/org/elasticsearch/upgrades/FeatureUpgradeIT.java | 8 ++++---- .../migration/TransportGetFeatureUpgradeStatusAction.java | 1 - 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/qa/rolling-upgrade/src/test/java/org/elasticsearch/upgrades/FeatureUpgradeIT.java b/qa/rolling-upgrade/src/test/java/org/elasticsearch/upgrades/FeatureUpgradeIT.java index 91697b32bebc2..7944aaa010282 100644 --- a/qa/rolling-upgrade/src/test/java/org/elasticsearch/upgrades/FeatureUpgradeIT.java +++ b/qa/rolling-upgrade/src/test/java/org/elasticsearch/upgrades/FeatureUpgradeIT.java @@ -29,13 +29,13 @@ public void testGetFeatureUpgradeStatus() throws Exception { if (CLUSTER_TYPE == ClusterType.OLD) { // setup - put something in the watcher index // create index - Request createTestIndex = new Request("PUT", "/test_index_old"); + Request createTestIndex = new Request("PUT", "/feature_test_index_old"); createTestIndex.setJsonEntity("{\"settings\": {\"index.number_of_replicas\": 0}}"); client().performRequest(createTestIndex); Request bulk = new Request("POST", "/_bulk"); bulk.addParameter("refresh", "true"); - bulk.setJsonEntity("{\"index\": {\"_index\": \"test_index_old\"}}\n" + + bulk.setJsonEntity("{\"index\": {\"_index\": \"feature_test_index_old\"}}\n" + "{\"f1\": \"v1\", \"f2\": \"v2\"}\n"); client().performRequest(bulk); @@ -44,10 +44,10 @@ public void testGetFeatureUpgradeStatus() throws Exception { reindex.setJsonEntity( "{\n" + " \"source\":{\n" + - " \"index\":\"test_index_old\"\n" + + " \"index\":\"feature_test_index_old\"\n" + " },\n" + " \"dest\":{\n" + - " \"index\":\"test_index_reindex\"\n" + + " \"index\":\"feature_test_index_reindex\"\n" + " }\n" + "}"); reindex.addParameter("wait_for_completion", "false"); diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/migration/TransportGetFeatureUpgradeStatusAction.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/migration/TransportGetFeatureUpgradeStatusAction.java index 06c54bc70afb9..87619d836075d 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/migration/TransportGetFeatureUpgradeStatusAction.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/migration/TransportGetFeatureUpgradeStatusAction.java @@ -24,7 +24,6 @@ import org.elasticsearch.transport.TransportService; import java.util.Collection; -import java.util.Comparator; import java.util.List; import java.util.Map; import java.util.stream.Collectors; From 4b9e57c839fcdb5eb93632920ecb246d79964a06 Mon Sep 17 00:00:00 2001 From: William Brafford Date: Tue, 5 Oct 2021 15:35:42 -0400 Subject: [PATCH 08/11] Use constant enum for statuses --- .../GetFeatureUpgradeStatusResponseTests.java | 10 +++++--- .../migration/FeatureUpgradeIT.java | 5 ++-- .../GetFeatureUpgradeStatusResponse.java | 25 +++++++++++-------- ...ransportGetFeatureUpgradeStatusAction.java | 7 ++++-- .../GetFeatureUpgradeStatusResponseTests.java | 10 +++++--- 5 files changed, 35 insertions(+), 22 deletions(-) diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/migration/GetFeatureUpgradeStatusResponseTests.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/migration/GetFeatureUpgradeStatusResponseTests.java index d177dea776bb0..a483366d2c2d8 100644 --- a/client/rest-high-level/src/test/java/org/elasticsearch/client/migration/GetFeatureUpgradeStatusResponseTests.java +++ b/client/rest-high-level/src/test/java/org/elasticsearch/client/migration/GetFeatureUpgradeStatusResponseTests.java @@ -16,6 +16,8 @@ import java.io.IOException; import java.util.Collections; +import static org.elasticsearch.action.admin.cluster.migration.GetFeatureUpgradeStatusResponse.UpgradeStatus.NO_UPGRADE_NEEDED; +import static org.elasticsearch.action.admin.cluster.migration.GetFeatureUpgradeStatusResponse.UpgradeStatus.UPGRADE_NEEDED; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.hasSize; import static org.hamcrest.Matchers.notNullValue; @@ -39,13 +41,13 @@ protected org.elasticsearch.action.admin.cluster.migration.GetFeatureUpgradeStat () -> new org.elasticsearch.action.admin.cluster.migration.GetFeatureUpgradeStatusResponse.FeatureUpgradeStatus( randomAlphaOfLengthBetween(3, 20), randomFrom(Version.CURRENT, Version.CURRENT.minimumCompatibilityVersion()), - randomAlphaOfLengthBetween(4, 16), + randomFrom(UPGRADE_NEEDED, NO_UPGRADE_NEEDED), randomList(4, () -> new org.elasticsearch.action.admin.cluster.migration.GetFeatureUpgradeStatusResponse.IndexVersion( randomAlphaOfLengthBetween(3, 20), randomFrom(Version.CURRENT, Version.CURRENT.minimumCompatibilityVersion()))) )), - randomAlphaOfLength(5) + randomFrom(UPGRADE_NEEDED, NO_UPGRADE_NEEDED) ); } @@ -59,7 +61,7 @@ protected void assertInstances( org.elasticsearch.action.admin.cluster.migration.GetFeatureUpgradeStatusResponse serverTestInstance, GetFeatureUpgradeStatusResponse clientInstance) { - assertThat(clientInstance.getUpgradeStatus(), equalTo(serverTestInstance.getUpgradeStatus())); + assertThat(clientInstance.getUpgradeStatus(), equalTo(serverTestInstance.getUpgradeStatus().toString())); assertNotNull(serverTestInstance.getFeatureUpgradeStatuses()); assertNotNull(clientInstance.getFeatureUpgradeStatuses()); @@ -73,7 +75,7 @@ protected void assertInstances( assertThat(clientStatus.getFeatureName(), equalTo(serverTestStatus.getFeatureName())); assertThat(clientStatus.getMinimumIndexVersion(), equalTo(serverTestStatus.getMinimumIndexVersion().toString())); - assertThat(clientStatus.getUpgradeStatus(), equalTo(serverTestStatus.getUpgradeStatus())); + assertThat(clientStatus.getUpgradeStatus(), equalTo(serverTestStatus.getUpgradeStatus().toString())); assertThat(clientStatus.getIndexVersions(), hasSize(serverTestStatus.getIndexVersions().size())); diff --git a/server/src/internalClusterTest/java/org/elasticsearch/migration/FeatureUpgradeIT.java b/server/src/internalClusterTest/java/org/elasticsearch/migration/FeatureUpgradeIT.java index 0c3084f73794d..2d110958486d6 100644 --- a/server/src/internalClusterTest/java/org/elasticsearch/migration/FeatureUpgradeIT.java +++ b/server/src/internalClusterTest/java/org/elasticsearch/migration/FeatureUpgradeIT.java @@ -16,6 +16,7 @@ import java.util.List; +import static org.elasticsearch.action.admin.cluster.migration.GetFeatureUpgradeStatusResponse.UpgradeStatus.NO_UPGRADE_NEEDED; import static org.hamcrest.Matchers.equalTo; public class FeatureUpgradeIT extends ESIntegTestCase { @@ -28,13 +29,13 @@ GetFeatureUpgradeStatusAction.INSTANCE, new GetFeatureUpgradeStatusRequest()) GetFeatureUpgradeStatusResponse.FeatureUpgradeStatus status = new GetFeatureUpgradeStatusResponse.FeatureUpgradeStatus( "tasks", Version.CURRENT, - "NO_UPGRADE_NEEDED", + NO_UPGRADE_NEEDED, List.of() ); assertThat(apiResponse, equalTo( new GetFeatureUpgradeStatusResponse( List.of(status), - "NO_UPGRADE_NEEDED" + NO_UPGRADE_NEEDED ))); } } diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/migration/GetFeatureUpgradeStatusResponse.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/migration/GetFeatureUpgradeStatusResponse.java index 1a2585de9756a..7031432d04f6d 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/migration/GetFeatureUpgradeStatusResponse.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/migration/GetFeatureUpgradeStatusResponse.java @@ -28,13 +28,13 @@ public class GetFeatureUpgradeStatusResponse extends ActionResponse implements ToXContentObject { private final List featureUpgradeStatuses; - private final String upgradeStatus; + private final UpgradeStatus upgradeStatus; /** * @param statuses A list of feature statuses * @param upgradeStatus Whether system features need to be upgraded */ - public GetFeatureUpgradeStatusResponse(List statuses, String upgradeStatus) { + public GetFeatureUpgradeStatusResponse(List statuses, UpgradeStatus upgradeStatus) { this.featureUpgradeStatuses = Objects.nonNull(statuses) ? statuses : Collections.emptyList(); this.upgradeStatus = upgradeStatus; } @@ -46,7 +46,7 @@ public GetFeatureUpgradeStatusResponse(List statuses, Stri public GetFeatureUpgradeStatusResponse(StreamInput in) throws IOException { super(in); this.featureUpgradeStatuses = in.readList(FeatureUpgradeStatus::new); - this.upgradeStatus = in.readString(); + this.upgradeStatus = in.readEnum(UpgradeStatus.class); } @Override @@ -65,14 +65,14 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws @Override public void writeTo(StreamOutput out) throws IOException { out.writeList(this.featureUpgradeStatuses); - out.writeString(upgradeStatus); + out.writeEnum(upgradeStatus); } public List getFeatureUpgradeStatuses() { return featureUpgradeStatuses; } - public String getUpgradeStatus() { + public UpgradeStatus getUpgradeStatus() { return upgradeStatus; } @@ -97,6 +97,11 @@ public String toString() { '}'; } + public enum UpgradeStatus { + UPGRADE_NEEDED, + NO_UPGRADE_NEEDED + } + /** * A class for a particular feature, showing whether it needs to be upgraded and the earliest * Elasticsearch version used to create one of this feature's system indices. @@ -104,7 +109,7 @@ public String toString() { public static class FeatureUpgradeStatus implements Writeable, ToXContentObject { private final String featureName; private final Version minimumIndexVersion; - private final String upgradeStatus; + private final UpgradeStatus upgradeStatus; private final List indexVersions; /** @@ -114,7 +119,7 @@ public static class FeatureUpgradeStatus implements Writeable, ToXContentObject * @param indexVersions A list of this feature's concrete indices and the Elasticsearch version that created them */ public FeatureUpgradeStatus(String featureName, Version minimumIndexVersion, - String upgradeStatus, List indexVersions) { + UpgradeStatus upgradeStatus, List indexVersions) { this.featureName = featureName; this.minimumIndexVersion = minimumIndexVersion; this.upgradeStatus = upgradeStatus; @@ -128,7 +133,7 @@ public FeatureUpgradeStatus(String featureName, Version minimumIndexVersion, public FeatureUpgradeStatus(StreamInput in) throws IOException { this.featureName = in.readString(); this.minimumIndexVersion = Version.readVersion(in); - this.upgradeStatus = in.readString(); + this.upgradeStatus = in.readEnum(UpgradeStatus.class); this.indexVersions = in.readList(IndexVersion::new); } @@ -140,7 +145,7 @@ public Version getMinimumIndexVersion() { return this.minimumIndexVersion; } - public String getUpgradeStatus() { + public UpgradeStatus getUpgradeStatus() { return this.upgradeStatus; } @@ -152,7 +157,7 @@ public List getIndexVersions() { public void writeTo(StreamOutput out) throws IOException { out.writeString(this.featureName); Version.writeVersion(this.minimumIndexVersion, out); - out.writeString(this.upgradeStatus); + out.writeEnum(this.upgradeStatus); out.writeList(this.indexVersions); } diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/migration/TransportGetFeatureUpgradeStatusAction.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/migration/TransportGetFeatureUpgradeStatusAction.java index 87619d836075d..72b1bd8e6ced2 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/migration/TransportGetFeatureUpgradeStatusAction.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/migration/TransportGetFeatureUpgradeStatusAction.java @@ -29,6 +29,9 @@ import java.util.stream.Collectors; import java.util.stream.Stream; +import static org.elasticsearch.action.admin.cluster.migration.GetFeatureUpgradeStatusResponse.UpgradeStatus.NO_UPGRADE_NEEDED; +import static org.elasticsearch.action.admin.cluster.migration.GetFeatureUpgradeStatusResponse.UpgradeStatus.UPGRADE_NEEDED; + /** * Transport class for the get feature upgrade status action */ @@ -76,7 +79,7 @@ protected void masterOperation(Task task, GetFeatureUpgradeStatusRequest request .orElse(Version.CURRENT) .before(Version.V_7_0_0); - listener.onResponse(new GetFeatureUpgradeStatusResponse(features, isUpgradeNeeded ? "UPGRADE_NEEDED" : "NO_UPGRADE_NEEDED")); + listener.onResponse(new GetFeatureUpgradeStatusResponse(features, isUpgradeNeeded ? UPGRADE_NEEDED : NO_UPGRADE_NEEDED)); } private GetFeatureUpgradeStatusResponse.FeatureUpgradeStatus getFeatureUpgradeStatus( @@ -95,7 +98,7 @@ private GetFeatureUpgradeStatusResponse.FeatureUpgradeStatus getFeatureUpgradeSt return new GetFeatureUpgradeStatusResponse.FeatureUpgradeStatus( featureName, minimumVersion, - minimumVersion.before(Version.V_7_0_0) ? "UPGRADE_NEEDED" : "NO_UPGRADE_NEEDED", + minimumVersion.before(Version.V_7_0_0) ? UPGRADE_NEEDED : NO_UPGRADE_NEEDED, indexVersions ); } diff --git a/server/src/test/java/org/elasticsearch/action/admin/cluster/migration/GetFeatureUpgradeStatusResponseTests.java b/server/src/test/java/org/elasticsearch/action/admin/cluster/migration/GetFeatureUpgradeStatusResponseTests.java index 58cc92237c993..2430b40c76ef8 100644 --- a/server/src/test/java/org/elasticsearch/action/admin/cluster/migration/GetFeatureUpgradeStatusResponseTests.java +++ b/server/src/test/java/org/elasticsearch/action/admin/cluster/migration/GetFeatureUpgradeStatusResponseTests.java @@ -15,6 +15,8 @@ import java.io.IOException; import java.util.Collections; +import static org.elasticsearch.action.admin.cluster.migration.GetFeatureUpgradeStatusResponse.UpgradeStatus.NO_UPGRADE_NEEDED; +import static org.elasticsearch.action.admin.cluster.migration.GetFeatureUpgradeStatusResponse.UpgradeStatus.UPGRADE_NEEDED; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.notNullValue; @@ -32,7 +34,7 @@ protected Writeable.Reader instanceReader() { protected GetFeatureUpgradeStatusResponse createTestInstance() { return new GetFeatureUpgradeStatusResponse( randomList(8, GetFeatureUpgradeStatusResponseTests::createFeatureStatus), - randomAlphaOfLengthBetween(4, 16) + randomFrom(UPGRADE_NEEDED, NO_UPGRADE_NEEDED) ); } @@ -42,13 +44,13 @@ protected GetFeatureUpgradeStatusResponse mutateInstance(GetFeatureUpgradeStatus randomList(8, () -> randomValueOtherThanMany(instance.getFeatureUpgradeStatuses()::contains, GetFeatureUpgradeStatusResponseTests::createFeatureStatus)), - randomValueOtherThan(instance.getUpgradeStatus(), () -> randomAlphaOfLengthBetween(4, 16)) + randomValueOtherThan(instance.getUpgradeStatus(), () -> randomFrom(UPGRADE_NEEDED, NO_UPGRADE_NEEDED)) ); } /** If constructor is called with null for a list, we just use an empty list */ public void testConstructorHandlesNullLists() { - GetFeatureUpgradeStatusResponse response = new GetFeatureUpgradeStatusResponse(null, "status"); + GetFeatureUpgradeStatusResponse response = new GetFeatureUpgradeStatusResponse(null, UPGRADE_NEEDED); assertThat(response.getFeatureUpgradeStatuses(), notNullValue()); assertThat(response.getFeatureUpgradeStatuses(), equalTo(Collections.emptyList())); } @@ -57,7 +59,7 @@ private static GetFeatureUpgradeStatusResponse.FeatureUpgradeStatus createFeatur return new GetFeatureUpgradeStatusResponse.FeatureUpgradeStatus( randomAlphaOfLengthBetween(3, 20), randomFrom(Version.CURRENT, Version.CURRENT.minimumCompatibilityVersion()), - randomAlphaOfLengthBetween(4, 16), + randomFrom(UPGRADE_NEEDED, NO_UPGRADE_NEEDED), randomList(4, GetFeatureUpgradeStatusResponseTests::getIndexVersion) ); } From 0a91f1dc246638b8a0b2aa3ff4ad6c4056284902 Mon Sep 17 00:00:00 2001 From: William Brafford Date: Tue, 5 Oct 2021 16:05:56 -0400 Subject: [PATCH 09/11] PR feedback --- .../upgrades/FeatureUpgradeIT.java | 12 +++--- .../system/indices/FeatureUpgradeApiIT.java | 10 ++--- .../migration/FeatureUpgradeIT.java | 41 ------------------- 3 files changed, 12 insertions(+), 51 deletions(-) delete mode 100644 server/src/internalClusterTest/java/org/elasticsearch/migration/FeatureUpgradeIT.java diff --git a/qa/rolling-upgrade/src/test/java/org/elasticsearch/upgrades/FeatureUpgradeIT.java b/qa/rolling-upgrade/src/test/java/org/elasticsearch/upgrades/FeatureUpgradeIT.java index 7944aaa010282..8bba5325cec8d 100644 --- a/qa/rolling-upgrade/src/test/java/org/elasticsearch/upgrades/FeatureUpgradeIT.java +++ b/qa/rolling-upgrade/src/test/java/org/elasticsearch/upgrades/FeatureUpgradeIT.java @@ -8,6 +8,7 @@ package org.elasticsearch.upgrades; +import org.elasticsearch.Version; import org.elasticsearch.client.Request; import org.elasticsearch.client.ResponseException; import org.elasticsearch.test.XContentTestUtils; @@ -27,7 +28,7 @@ public void testGetFeatureUpgradeStatus() throws Exception { final String systemIndexWarning = "this request accesses system indices: [.tasks], but in a future major version, direct " + "access to system indices will be prevented by default"; if (CLUSTER_TYPE == ClusterType.OLD) { - // setup - put something in the watcher index + // setup - put something in the tasks index // create index Request createTestIndex = new Request("PUT", "/feature_test_index_old"); createTestIndex.setJsonEntity("{\"settings\": {\"index.number_of_replicas\": 0}}"); @@ -67,10 +68,6 @@ public void testGetFeatureUpgradeStatus() throws Exception { })); getTasksIndex.addParameter("allow_no_indices", "false"); - getTasksIndex.setOptions(expectVersionSpecificWarnings(v -> { - v.current(systemIndexWarning); - v.compatible(systemIndexWarning); - })); assertBusy(() -> { try { assertThat(client().performRequest(getTasksIndex).getStatusLine().getStatusCode(), is(200)); @@ -94,6 +91,11 @@ public void testGetFeatureUpgradeStatus() throws Exception { assertThat(feature.size(), equalTo(4)); assertThat(feature.get("minimum_index_version"), equalTo(UPGRADE_FROM_VERSION.toString())); + if (UPGRADE_FROM_VERSION.before(Version.CURRENT.minimumIndexCompatibilityVersion())) { + assertThat(feature.get("upgrade_status"), equalTo("UPGRADE_NEEDED")); + } else { + assertThat(feature.get("upgrade_status"), equalTo("NO_UPGRADE_NEEDED")); + } }); } } diff --git a/qa/system-indices/src/javaRestTest/java/org/elasticsearch/system/indices/FeatureUpgradeApiIT.java b/qa/system-indices/src/javaRestTest/java/org/elasticsearch/system/indices/FeatureUpgradeApiIT.java index 0fa4731368b25..af9839a772a10 100644 --- a/qa/system-indices/src/javaRestTest/java/org/elasticsearch/system/indices/FeatureUpgradeApiIT.java +++ b/qa/system-indices/src/javaRestTest/java/org/elasticsearch/system/indices/FeatureUpgradeApiIT.java @@ -31,6 +31,11 @@ public class FeatureUpgradeApiIT extends ESRestTestCase { static final String BASIC_AUTH_VALUE = basicAuthHeaderValue("rest_user", new SecureString("rest-user-password".toCharArray())); + @After + public void resetFeatures() throws Exception { + client().performRequest(new Request("POST", "/_features/_reset")); + } + @Override protected Settings restClientSettings() { return Settings.builder().put(ThreadContext.PREFIX + ".Authorization", BASIC_AUTH_VALUE).build(); @@ -62,9 +67,4 @@ public void testGetFeatureUpgradedStatuses() throws Exception { assertThat((List) testFeature.get("indices"), hasSize(1)); } - - @After - public void resetFeatures() throws Exception { - client().performRequest(new Request("POST", "/_features/_reset")); - } } diff --git a/server/src/internalClusterTest/java/org/elasticsearch/migration/FeatureUpgradeIT.java b/server/src/internalClusterTest/java/org/elasticsearch/migration/FeatureUpgradeIT.java deleted file mode 100644 index 2d110958486d6..0000000000000 --- a/server/src/internalClusterTest/java/org/elasticsearch/migration/FeatureUpgradeIT.java +++ /dev/null @@ -1,41 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the Elastic License - * 2.0 and the Server Side Public License, v 1; you may not use this file except - * in compliance with, at your election, the Elastic License 2.0 or the Server - * Side Public License, v 1. - */ - -package org.elasticsearch.migration; - -import org.elasticsearch.Version; -import org.elasticsearch.action.admin.cluster.migration.GetFeatureUpgradeStatusAction; -import org.elasticsearch.action.admin.cluster.migration.GetFeatureUpgradeStatusRequest; -import org.elasticsearch.action.admin.cluster.migration.GetFeatureUpgradeStatusResponse; -import org.elasticsearch.test.ESIntegTestCase; - -import java.util.List; - -import static org.elasticsearch.action.admin.cluster.migration.GetFeatureUpgradeStatusResponse.UpgradeStatus.NO_UPGRADE_NEEDED; -import static org.hamcrest.Matchers.equalTo; - -public class FeatureUpgradeIT extends ESIntegTestCase { - - public void testGetFeatureUpgradeStatus() throws Exception { - GetFeatureUpgradeStatusResponse apiResponse = client().execute( - GetFeatureUpgradeStatusAction.INSTANCE, new GetFeatureUpgradeStatusRequest()) - .get(); - - GetFeatureUpgradeStatusResponse.FeatureUpgradeStatus status = new GetFeatureUpgradeStatusResponse.FeatureUpgradeStatus( - "tasks", - Version.CURRENT, - NO_UPGRADE_NEEDED, - List.of() - ); - assertThat(apiResponse, equalTo( - new GetFeatureUpgradeStatusResponse( - List.of(status), - NO_UPGRADE_NEEDED - ))); - } -} From e73f65f14aa02184417b24fb948a86688c0f008a Mon Sep 17 00:00:00 2001 From: William Brafford Date: Tue, 5 Oct 2021 17:44:42 -0400 Subject: [PATCH 10/11] Add unit tests for transport class methods --- ...ransportGetFeatureUpgradeStatusAction.java | 6 +- ...ortGetFeatureUpgradeStatusActionTests.java | 101 ++++++++++++++++++ 2 files changed, 105 insertions(+), 2 deletions(-) create mode 100644 server/src/test/java/org/elasticsearch/action/admin/cluster/migration/TransportGetFeatureUpgradeStatusActionTests.java diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/migration/TransportGetFeatureUpgradeStatusAction.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/migration/TransportGetFeatureUpgradeStatusAction.java index 72b1bd8e6ced2..b8777d91437e1 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/migration/TransportGetFeatureUpgradeStatusAction.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/migration/TransportGetFeatureUpgradeStatusAction.java @@ -82,7 +82,8 @@ protected void masterOperation(Task task, GetFeatureUpgradeStatusRequest request listener.onResponse(new GetFeatureUpgradeStatusResponse(features, isUpgradeNeeded ? UPGRADE_NEEDED : NO_UPGRADE_NEEDED)); } - private GetFeatureUpgradeStatusResponse.FeatureUpgradeStatus getFeatureUpgradeStatus( + // visible for testing + static GetFeatureUpgradeStatusResponse.FeatureUpgradeStatus getFeatureUpgradeStatus( ClusterState state, Map.Entry entry) { String featureName = entry.getKey(); @@ -103,7 +104,8 @@ private GetFeatureUpgradeStatusResponse.FeatureUpgradeStatus getFeatureUpgradeSt ); } - private List getIndexVersions(ClusterState state, SystemIndices.Feature feature) { + // visible for testing + static List getIndexVersions(ClusterState state, SystemIndices.Feature feature) { return Stream.of(feature.getIndexDescriptors(), feature.getAssociatedIndexDescriptors()) .flatMap(Collection::stream) .flatMap(descriptor -> descriptor.getMatchingIndices(state.metadata()).stream()) diff --git a/server/src/test/java/org/elasticsearch/action/admin/cluster/migration/TransportGetFeatureUpgradeStatusActionTests.java b/server/src/test/java/org/elasticsearch/action/admin/cluster/migration/TransportGetFeatureUpgradeStatusActionTests.java new file mode 100644 index 0000000000000..5fb9cba2b4b18 --- /dev/null +++ b/server/src/test/java/org/elasticsearch/action/admin/cluster/migration/TransportGetFeatureUpgradeStatusActionTests.java @@ -0,0 +1,101 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +package org.elasticsearch.action.admin.cluster.migration; + +import org.elasticsearch.Version; +import org.elasticsearch.cluster.ClusterState; +import org.elasticsearch.cluster.metadata.IndexMetadata; +import org.elasticsearch.cluster.metadata.Metadata; +import org.elasticsearch.common.collect.ImmutableOpenMap; +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.indices.SystemIndexDescriptor; +import org.elasticsearch.indices.SystemIndices; +import org.elasticsearch.test.ESTestCase; + +import java.util.ArrayList; +import java.util.List; +import java.util.Map; + +import static org.elasticsearch.action.admin.cluster.migration.GetFeatureUpgradeStatusResponse.UpgradeStatus.NO_UPGRADE_NEEDED; +import static org.hamcrest.Matchers.equalTo; + +public class TransportGetFeatureUpgradeStatusActionTests extends ESTestCase { + + public static String TEST_SYSTEM_INDEX_PATTERN = ".test*"; + private static final ClusterState CLUSTER_STATE = getClusterState(); + private static final SystemIndices.Feature FEATURE = getFeature(); + + public void testGetFeatureStatus() { + GetFeatureUpgradeStatusResponse.FeatureUpgradeStatus status = + TransportGetFeatureUpgradeStatusAction.getFeatureUpgradeStatus( + CLUSTER_STATE, + Map.entry("test-feature", FEATURE)); + + assertThat(status.getUpgradeStatus(), equalTo(NO_UPGRADE_NEEDED)); + assertThat(status.getFeatureName(), equalTo("test-feature")); + assertThat(status.getMinimumIndexVersion(), equalTo(Version.V_7_0_0)); + assertThat(status.getIndexVersions().size(), equalTo(2)); // additional testing below + } + + public void testGetIndexVersion() { + List versions = + TransportGetFeatureUpgradeStatusAction.getIndexVersions(CLUSTER_STATE, FEATURE); + + assertThat(versions.size(), equalTo(2)); + + { + GetFeatureUpgradeStatusResponse.IndexVersion version = versions.get(0); + assertThat(version.getVersion(), equalTo(Version.CURRENT)); + assertThat(version.getIndexName(), equalTo(".test-index-1")); + } + { + GetFeatureUpgradeStatusResponse.IndexVersion version = versions.get(1); + assertThat(version.getVersion(), equalTo(Version.V_7_0_0)); + assertThat(version.getIndexName(), equalTo(".test-index-2")); + } + } + + private static SystemIndices.Feature getFeature() { + SystemIndexDescriptor descriptor = new SystemIndexDescriptor(TEST_SYSTEM_INDEX_PATTERN, "descriptor for tests"); + + List descriptors = new ArrayList<>(); + descriptors.add(descriptor); + + // system indices feature object + SystemIndices.Feature feature = new SystemIndices.Feature( + "test-feature", + "feature for tests", + descriptors); + return feature; + } + + private static ClusterState getClusterState() { + IndexMetadata indexMetadata1 = IndexMetadata.builder(".test-index-1") + .settings(Settings.builder().put("index.version.created", Version.CURRENT).build()) + .numberOfShards(1) + .numberOfReplicas(0) + .build(); + + IndexMetadata indexMetadata2 = IndexMetadata.builder(".test-index-2") + .settings(Settings.builder().put("index.version.created", Version.V_7_0_0).build()) + .numberOfShards(1) + .numberOfReplicas(0) + .build(); + + ClusterState clusterState = new ClusterState.Builder(ClusterState.EMPTY_STATE) + .metadata(new Metadata.Builder() + .indices(ImmutableOpenMap.builder() + .fPut(".test-index-1", indexMetadata1) + .fPut(".test-index-2", indexMetadata2) + .build()) + .build()) + .build(); + return clusterState; + } +} From a8f4d09cbab6debaf2d0283e5d1389c8b72dbc81 Mon Sep 17 00:00:00 2001 From: William Brafford Date: Wed, 6 Oct 2021 12:35:25 -0400 Subject: [PATCH 11/11] Add IN_PROGRESS upgrade status --- .../GetFeatureUpgradeStatusResponseTests.java | 6 ++---- .../migration/GetFeatureUpgradeStatusResponse.java | 3 ++- .../GetFeatureUpgradeStatusResponseTests.java | 10 +++++----- 3 files changed, 9 insertions(+), 10 deletions(-) diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/migration/GetFeatureUpgradeStatusResponseTests.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/migration/GetFeatureUpgradeStatusResponseTests.java index a483366d2c2d8..e478a24442978 100644 --- a/client/rest-high-level/src/test/java/org/elasticsearch/client/migration/GetFeatureUpgradeStatusResponseTests.java +++ b/client/rest-high-level/src/test/java/org/elasticsearch/client/migration/GetFeatureUpgradeStatusResponseTests.java @@ -16,8 +16,6 @@ import java.io.IOException; import java.util.Collections; -import static org.elasticsearch.action.admin.cluster.migration.GetFeatureUpgradeStatusResponse.UpgradeStatus.NO_UPGRADE_NEEDED; -import static org.elasticsearch.action.admin.cluster.migration.GetFeatureUpgradeStatusResponse.UpgradeStatus.UPGRADE_NEEDED; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.hasSize; import static org.hamcrest.Matchers.notNullValue; @@ -41,13 +39,13 @@ protected org.elasticsearch.action.admin.cluster.migration.GetFeatureUpgradeStat () -> new org.elasticsearch.action.admin.cluster.migration.GetFeatureUpgradeStatusResponse.FeatureUpgradeStatus( randomAlphaOfLengthBetween(3, 20), randomFrom(Version.CURRENT, Version.CURRENT.minimumCompatibilityVersion()), - randomFrom(UPGRADE_NEEDED, NO_UPGRADE_NEEDED), + randomFrom(org.elasticsearch.action.admin.cluster.migration.GetFeatureUpgradeStatusResponse.UpgradeStatus.values()), randomList(4, () -> new org.elasticsearch.action.admin.cluster.migration.GetFeatureUpgradeStatusResponse.IndexVersion( randomAlphaOfLengthBetween(3, 20), randomFrom(Version.CURRENT, Version.CURRENT.minimumCompatibilityVersion()))) )), - randomFrom(UPGRADE_NEEDED, NO_UPGRADE_NEEDED) + randomFrom(org.elasticsearch.action.admin.cluster.migration.GetFeatureUpgradeStatusResponse.UpgradeStatus.values()) ); } diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/migration/GetFeatureUpgradeStatusResponse.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/migration/GetFeatureUpgradeStatusResponse.java index 7031432d04f6d..3e46b4d4b6fa3 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/migration/GetFeatureUpgradeStatusResponse.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/migration/GetFeatureUpgradeStatusResponse.java @@ -99,7 +99,8 @@ public String toString() { public enum UpgradeStatus { UPGRADE_NEEDED, - NO_UPGRADE_NEEDED + NO_UPGRADE_NEEDED, + IN_PROGRESS } /** diff --git a/server/src/test/java/org/elasticsearch/action/admin/cluster/migration/GetFeatureUpgradeStatusResponseTests.java b/server/src/test/java/org/elasticsearch/action/admin/cluster/migration/GetFeatureUpgradeStatusResponseTests.java index 2430b40c76ef8..7966c2171ee87 100644 --- a/server/src/test/java/org/elasticsearch/action/admin/cluster/migration/GetFeatureUpgradeStatusResponseTests.java +++ b/server/src/test/java/org/elasticsearch/action/admin/cluster/migration/GetFeatureUpgradeStatusResponseTests.java @@ -15,7 +15,6 @@ import java.io.IOException; import java.util.Collections; -import static org.elasticsearch.action.admin.cluster.migration.GetFeatureUpgradeStatusResponse.UpgradeStatus.NO_UPGRADE_NEEDED; import static org.elasticsearch.action.admin.cluster.migration.GetFeatureUpgradeStatusResponse.UpgradeStatus.UPGRADE_NEEDED; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.notNullValue; @@ -34,7 +33,7 @@ protected Writeable.Reader instanceReader() { protected GetFeatureUpgradeStatusResponse createTestInstance() { return new GetFeatureUpgradeStatusResponse( randomList(8, GetFeatureUpgradeStatusResponseTests::createFeatureStatus), - randomFrom(UPGRADE_NEEDED, NO_UPGRADE_NEEDED) + randomFrom(org.elasticsearch.action.admin.cluster.migration.GetFeatureUpgradeStatusResponse.UpgradeStatus.values()) ); } @@ -44,8 +43,9 @@ protected GetFeatureUpgradeStatusResponse mutateInstance(GetFeatureUpgradeStatus randomList(8, () -> randomValueOtherThanMany(instance.getFeatureUpgradeStatuses()::contains, GetFeatureUpgradeStatusResponseTests::createFeatureStatus)), - randomValueOtherThan(instance.getUpgradeStatus(), () -> randomFrom(UPGRADE_NEEDED, NO_UPGRADE_NEEDED)) - ); + randomValueOtherThan(instance.getUpgradeStatus(), () -> + randomFrom(org.elasticsearch.action.admin.cluster.migration.GetFeatureUpgradeStatusResponse.UpgradeStatus.values()))); + } /** If constructor is called with null for a list, we just use an empty list */ @@ -59,7 +59,7 @@ private static GetFeatureUpgradeStatusResponse.FeatureUpgradeStatus createFeatur return new GetFeatureUpgradeStatusResponse.FeatureUpgradeStatus( randomAlphaOfLengthBetween(3, 20), randomFrom(Version.CURRENT, Version.CURRENT.minimumCompatibilityVersion()), - randomFrom(UPGRADE_NEEDED, NO_UPGRADE_NEEDED), + randomFrom(org.elasticsearch.action.admin.cluster.migration.GetFeatureUpgradeStatusResponse.UpgradeStatus.values()), randomList(4, GetFeatureUpgradeStatusResponseTests::getIndexVersion) ); }