Skip to content

Commit f68aef6

Browse files
joegallojoegallo
authored
Validate that snapshot repository exists for ILM policies during GenerateSnapshotNameStep (#77657)
1 parent eed2c81 commit f68aef6

File tree

2 files changed

+118
-27
lines changed

2 files changed

+118
-27
lines changed

‎x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/GenerateSnapshotNameStep.java‎

Lines changed: 31 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,10 @@
1313
import org.elasticsearch.cluster.metadata.IndexMetadata;
1414
import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver;
1515
import org.elasticsearch.cluster.metadata.Metadata;
16-
import org.elasticsearch.core.Nullable;
16+
import org.elasticsearch.cluster.metadata.RepositoriesMetadata;
1717
import org.elasticsearch.common.Strings;
1818
import org.elasticsearch.common.UUIDs;
19+
import org.elasticsearch.core.Nullable;
1920
import org.elasticsearch.index.Index;
2021

2122
import java.util.Collections;
@@ -61,29 +62,39 @@ public ClusterState performAction(Index index, ClusterState clusterState) {
6162
return clusterState;
6263
}
6364

64-
ClusterState.Builder newClusterStateBuilder = ClusterState.builder(clusterState);
65-
66-
LifecycleExecutionState lifecycleState = fromIndexMetadata(indexMetaData);
67-
assert lifecycleState.getSnapshotName() == null : "index " + index.getName() + " should not have a snapshot generated by " +
68-
"the ilm policy but has " + lifecycleState.getSnapshotName();
69-
LifecycleExecutionState.Builder newCustomData = LifecycleExecutionState.builder(lifecycleState);
7065
String policy = indexMetaData.getSettings().get(LifecycleSettings.LIFECYCLE_NAME);
71-
String snapshotNamePrefix = ("<{now/d}-" + index.getName() + "-" + policy + ">").toLowerCase(Locale.ROOT);
72-
String snapshotName = generateSnapshotName(snapshotNamePrefix);
73-
ActionRequestValidationException validationException = validateGeneratedSnapshotName(snapshotNamePrefix, snapshotName);
74-
if (validationException != null) {
75-
logger.warn("unable to generate a snapshot name as part of policy [{}] for index [{}] due to [{}]",
76-
policy, index.getName(), validationException.getMessage());
77-
throw validationException;
66+
LifecycleExecutionState lifecycleState = fromIndexMetadata(indexMetaData);
67+
68+
// validate that the snapshot repository exists -- because policies are refreshed on later retries, and because
69+
// this fails prior to the snapshot repository being recorded in the ilm metadata, the policy can just be corrected
70+
// and everything will pass on the subsequent retry
71+
if (clusterState.metadata().custom(RepositoriesMetadata.TYPE, RepositoriesMetadata.EMPTY).repository(snapshotRepository) == null) {
72+
throw new IllegalStateException("repository [" + snapshotRepository + "] is missing. [" + policy + "] policy for " +
73+
"index [" + index.getName() + "] cannot continue until the repository is created or the policy is changed");
7874
}
79-
newCustomData.setSnapshotName(snapshotName);
80-
newCustomData.setSnapshotRepository(snapshotRepository);
75+
76+
LifecycleExecutionState.Builder newCustomData = LifecycleExecutionState.builder(lifecycleState);
8177
newCustomData.setSnapshotIndexName(index.getName());
78+
newCustomData.setSnapshotRepository(snapshotRepository);
79+
if (lifecycleState.getSnapshotName() == null) {
80+
// generate and validate the snapshotName
81+
String snapshotNamePrefix = ("<{now/d}-" + index.getName() + "-" + policy + ">").toLowerCase(Locale.ROOT);
82+
String snapshotName = generateSnapshotName(snapshotNamePrefix);
83+
ActionRequestValidationException validationException = validateGeneratedSnapshotName(snapshotNamePrefix, snapshotName);
84+
if (validationException != null) {
85+
logger.warn("unable to generate a snapshot name as part of policy [{}] for index [{}] due to [{}]",
86+
policy, index.getName(), validationException.getMessage());
87+
throw validationException;
88+
}
89+
90+
newCustomData.setSnapshotName(snapshotName);
91+
}
8292

83-
IndexMetadata.Builder indexMetadataBuilder = IndexMetadata.builder(indexMetaData);
84-
indexMetadataBuilder.putCustom(ILM_CUSTOM_METADATA_KEY, newCustomData.build().asMap());
85-
newClusterStateBuilder.metadata(Metadata.builder(clusterState.getMetadata()).put(indexMetadataBuilder));
86-
return newClusterStateBuilder.build();
93+
return ClusterState.builder(clusterState)
94+
.metadata(Metadata.builder(clusterState.getMetadata())
95+
.put(IndexMetadata.builder(indexMetaData)
96+
.putCustom(ILM_CUSTOM_METADATA_KEY, newCustomData.build().asMap())))
97+
.build();
8798
}
8899

89100
@Override

‎x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/GenerateSnapshotNameStepTests.java‎

Lines changed: 87 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -12,12 +12,17 @@
1212
import org.elasticsearch.cluster.metadata.IndexMetadata;
1313
import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver;
1414
import org.elasticsearch.cluster.metadata.Metadata;
15+
import org.elasticsearch.cluster.metadata.RepositoriesMetadata;
16+
import org.elasticsearch.cluster.metadata.RepositoryMetadata;
1517
import org.elasticsearch.common.Strings;
18+
import org.elasticsearch.common.settings.Settings;
1619

20+
import java.util.Collections;
1721
import java.util.Locale;
1822

1923
import static org.elasticsearch.xpack.core.ilm.GenerateSnapshotNameStep.generateSnapshotName;
2024
import static org.elasticsearch.xpack.core.ilm.GenerateSnapshotNameStep.validateGeneratedSnapshotName;
25+
import static org.elasticsearch.xpack.core.ilm.LifecycleExecutionState.ILM_CUSTOM_METADATA_KEY;
2126
import static org.hamcrest.Matchers.containsInAnyOrder;
2227
import static org.hamcrest.Matchers.containsString;
2328
import static org.hamcrest.Matchers.greaterThan;
@@ -63,23 +68,98 @@ protected GenerateSnapshotNameStep copyInstance(GenerateSnapshotNameStep instanc
6368
public void testPerformAction() {
6469
String indexName = randomAlphaOfLength(10);
6570
String policyName = "test-ilm-policy";
66-
IndexMetadata.Builder indexMetadataBuilder =
67-
IndexMetadata.builder(indexName).settings(settings(Version.CURRENT).put(LifecycleSettings.LIFECYCLE_NAME, policyName))
68-
.numberOfShards(randomIntBetween(1, 5)).numberOfReplicas(randomIntBetween(0, 5));
71+
final IndexMetadata indexMetadata = IndexMetadata.builder(indexName)
72+
.settings(settings(Version.CURRENT).put(LifecycleSettings.LIFECYCLE_NAME, policyName))
73+
.numberOfShards(randomIntBetween(1, 5))
74+
.numberOfReplicas(randomIntBetween(0, 5))
75+
.build();
76+
77+
GenerateSnapshotNameStep generateSnapshotNameStep = createRandomInstance();
78+
79+
// generate a snapshot repository with the expected name
80+
RepositoryMetadata repo = new RepositoryMetadata(generateSnapshotNameStep.getSnapshotRepository(), "fs", Settings.EMPTY);
6981

70-
final IndexMetadata indexMetadata = indexMetadataBuilder.build();
7182
ClusterState clusterState = ClusterState.builder(emptyClusterState())
72-
.metadata(Metadata.builder().put(indexMetadata, false).build()).build();
83+
.metadata(Metadata.builder()
84+
.put(indexMetadata, false)
85+
.putCustom(RepositoriesMetadata.TYPE, new RepositoriesMetadata(Collections.singletonList(repo)))
86+
.build()).build();
7387

74-
GenerateSnapshotNameStep generateSnapshotNameStep = createRandomInstance();
75-
ClusterState newClusterState = generateSnapshotNameStep.performAction(indexMetadata.getIndex(), clusterState);
88+
ClusterState newClusterState;
7689

90+
// the snapshot index name, snapshot repository, and snapshot name are generated as expected
91+
newClusterState = generateSnapshotNameStep.performAction(indexMetadata.getIndex(), clusterState);
7792
LifecycleExecutionState executionState = LifecycleExecutionState.fromIndexMetadata(newClusterState.metadata().index(indexName));
93+
assertThat(executionState.getSnapshotIndexName(), is(indexName));
7894
assertThat("the " + GenerateSnapshotNameStep.NAME + " step must generate a snapshot name", executionState.getSnapshotName(),
7995
notNullValue());
8096
assertThat(executionState.getSnapshotRepository(), is(generateSnapshotNameStep.getSnapshotRepository()));
8197
assertThat(executionState.getSnapshotName(), containsString(indexName.toLowerCase(Locale.ROOT)));
8298
assertThat(executionState.getSnapshotName(), containsString(policyName.toLowerCase(Locale.ROOT)));
99+
100+
// re-running this step results in no change to the important outputs
101+
newClusterState = generateSnapshotNameStep.performAction(indexMetadata.getIndex(), newClusterState);
102+
LifecycleExecutionState repeatedState = LifecycleExecutionState.fromIndexMetadata(newClusterState.metadata().index(indexName));
103+
assertThat(repeatedState.getSnapshotIndexName(), is(executionState.getSnapshotIndexName()));
104+
assertThat(repeatedState.getSnapshotRepository(), is(executionState.getSnapshotRepository()));
105+
assertThat(repeatedState.getSnapshotName(), is(executionState.getSnapshotName()));
106+
}
107+
108+
public void testPerformActionRejectsNonexistentRepository() {
109+
String indexName = randomAlphaOfLength(10);
110+
String policyName = "test-ilm-policy";
111+
final IndexMetadata indexMetadata = IndexMetadata.builder(indexName)
112+
.settings(settings(Version.CURRENT).put(LifecycleSettings.LIFECYCLE_NAME, policyName))
113+
.numberOfShards(randomIntBetween(1, 5))
114+
.numberOfReplicas(randomIntBetween(0, 5))
115+
.build();
116+
117+
GenerateSnapshotNameStep generateSnapshotNameStep = createRandomInstance();
118+
119+
ClusterState clusterState = ClusterState.builder(emptyClusterState())
120+
.metadata(Metadata.builder()
121+
.put(indexMetadata, false)
122+
.putCustom(RepositoriesMetadata.TYPE, RepositoriesMetadata.EMPTY)
123+
.build()).build();
124+
125+
IllegalStateException illegalStateException = expectThrows(IllegalStateException.class,
126+
() -> generateSnapshotNameStep.performAction(indexMetadata.getIndex(), clusterState));
127+
assertThat(illegalStateException.getMessage(), is("repository [" + generateSnapshotNameStep.getSnapshotRepository() + "] " +
128+
"is missing. [test-ilm-policy] policy for index [" + indexName + "] cannot continue until the repository " +
129+
"is created or the policy is changed"));
130+
}
131+
132+
public void testPerformActionWillOverwriteCachedRepository() {
133+
String indexName = randomAlphaOfLength(10);
134+
String policyName = "test-ilm-policy";
135+
136+
LifecycleExecutionState.Builder newCustomData = LifecycleExecutionState.builder();
137+
newCustomData.setSnapshotName("snapshot-name-is-not-touched");
138+
newCustomData.setSnapshotRepository("snapshot-repository-will-be-reset");
139+
140+
final IndexMetadata indexMetadata = IndexMetadata.builder(indexName)
141+
.settings(settings(Version.CURRENT).put(LifecycleSettings.LIFECYCLE_NAME, policyName))
142+
.numberOfShards(randomIntBetween(1, 5))
143+
.numberOfReplicas(randomIntBetween(0, 5))
144+
.putCustom(ILM_CUSTOM_METADATA_KEY, newCustomData.build().asMap())
145+
.build();
146+
147+
GenerateSnapshotNameStep generateSnapshotNameStep = createRandomInstance();
148+
149+
// generate a snapshot repository with the expected name
150+
RepositoryMetadata repo = new RepositoryMetadata(generateSnapshotNameStep.getSnapshotRepository(), "fs", Settings.EMPTY);
151+
152+
ClusterState clusterState = ClusterState.builder(emptyClusterState())
153+
.metadata(Metadata.builder()
154+
.put(indexMetadata, false)
155+
.putCustom(RepositoriesMetadata.TYPE, new RepositoriesMetadata(Collections.singletonList(repo)))
156+
.build()).build();
157+
158+
ClusterState newClusterState = generateSnapshotNameStep.performAction(indexMetadata.getIndex(), clusterState);
159+
160+
LifecycleExecutionState executionState = LifecycleExecutionState.fromIndexMetadata(newClusterState.metadata().index(indexName));
161+
assertThat(executionState.getSnapshotName(), is("snapshot-name-is-not-touched"));
162+
assertThat(executionState.getSnapshotRepository(), is(generateSnapshotNameStep.getSnapshotRepository()));
83163
}
84164

85165
public void testNameGeneration() {

0 commit comments

Comments
 (0)