Skip to content

Commit ebf0dd4

Browse files
colings86colings86
authored
Adds a check to only fail policy update if unsafe action is changed (#32002)
1 parent 89e8f9e commit ebf0dd4

File tree

2 files changed

+134
-3
lines changed

2 files changed

+134
-3
lines changed

‎x-pack/plugin/index-lifecycle/src/main/java/org/elasticsearch/xpack/indexlifecycle/IndexLifecycleRunner.java‎

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,10 @@
2828
import org.elasticsearch.xpack.core.indexlifecycle.ClusterStateWaitStep;
2929
import org.elasticsearch.xpack.core.indexlifecycle.ErrorStep;
3030
import org.elasticsearch.xpack.core.indexlifecycle.IndexLifecycleMetadata;
31+
import org.elasticsearch.xpack.core.indexlifecycle.LifecycleAction;
3132
import org.elasticsearch.xpack.core.indexlifecycle.LifecyclePolicy;
3233
import org.elasticsearch.xpack.core.indexlifecycle.LifecycleSettings;
34+
import org.elasticsearch.xpack.core.indexlifecycle.Phase;
3335
import org.elasticsearch.xpack.core.indexlifecycle.RolloverAction;
3436
import org.elasticsearch.xpack.core.indexlifecycle.Step;
3537
import org.elasticsearch.xpack.core.indexlifecycle.Step.StepKey;
@@ -340,15 +342,33 @@ private static boolean canSetPolicy(StepKey currentStepKey, LifecyclePolicy curr
340342
if (currentPolicy.isActionSafe(currentStepKey)) {
341343
return true;
342344
} else {
343-
// Index is in the shrink action so fail it
344-
// NORELEASE also need to check if the shrink action has changed between oldPolicy and newPolicy
345-
return false;
345+
// Index is in an unsafe action so fail it if the action has changed between oldPolicy and newPolicy
346+
return isActionChanged(currentStepKey, currentPolicy, newPolicy) == false;
346347
}
347348
} else {
348349
// Index not previously managed by ILM so safe to change policy
349350
return true;
350351
}
351352
}
353+
354+
private static boolean isActionChanged(StepKey stepKey, LifecyclePolicy currentPolicy, LifecyclePolicy newPolicy) {
355+
LifecycleAction currentAction = getActionFromPolicy(currentPolicy, stepKey.getPhase(), stepKey.getAction());
356+
LifecycleAction newAction = getActionFromPolicy(newPolicy, stepKey.getPhase(), stepKey.getAction());
357+
if (newAction == null) {
358+
return true;
359+
} else {
360+
return currentAction.equals(newAction) == false;
361+
}
362+
}
363+
364+
private static LifecycleAction getActionFromPolicy(LifecyclePolicy policy, String phaseName, String actionName) {
365+
Phase phase = policy.getPhases().get(phaseName);
366+
if (phase != null) {
367+
return phase.getActions().get(actionName);
368+
} else {
369+
return null;
370+
}
371+
}
352372

353373
/**
354374
* Returns <code>true</code> if the provided policy is allowed to be updated

‎x-pack/plugin/index-lifecycle/src/test/java/org/elasticsearch/xpack/indexlifecycle/IndexLifecycleRunnerTests.java‎

Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -938,6 +938,67 @@ public void testSetPolicyForIndexIndexInUnsafe() {
938938
assertSame(clusterState, newClusterState);
939939
}
940940

941+
public void testSetPolicyForIndexIndexInUnsafeActionUnchanged() {
942+
long now = randomNonNegativeLong();
943+
String indexName = randomAlphaOfLength(10);
944+
String oldPolicyName = "old_policy";
945+
String newPolicyName = "new_policy";
946+
StepKey currentStep = new StepKey(randomAlphaOfLength(10), MockAction.NAME, randomAlphaOfLength(10));
947+
LifecyclePolicy oldPolicy = createPolicy(oldPolicyName, null, currentStep);
948+
LifecyclePolicy newPolicy = createPolicy(newPolicyName, null, currentStep);
949+
Settings.Builder indexSettingsBuilder = Settings.builder().put(LifecycleSettings.LIFECYCLE_NAME, oldPolicyName)
950+
.put(LifecycleSettings.LIFECYCLE_PHASE, currentStep.getPhase())
951+
.put(LifecycleSettings.LIFECYCLE_ACTION, currentStep.getAction())
952+
.put(LifecycleSettings.LIFECYCLE_STEP, currentStep.getName()).put(LifecycleSettings.LIFECYCLE_SKIP, true);
953+
List<LifecyclePolicyMetadata> policyMetadatas = new ArrayList<>();
954+
policyMetadatas.add(new LifecyclePolicyMetadata(oldPolicy, Collections.emptyMap()));
955+
ClusterState clusterState = buildClusterState(indexName, indexSettingsBuilder, policyMetadatas);
956+
Index index = clusterState.metaData().index(indexName).getIndex();
957+
Index[] indices = new Index[] { index };
958+
List<String> failedIndexes = new ArrayList<>();
959+
960+
ClusterState newClusterState = IndexLifecycleRunner.setPolicyForIndexes(newPolicyName, indices, clusterState, newPolicy,
961+
failedIndexes);
962+
963+
assertTrue(failedIndexes.isEmpty());
964+
assertClusterStateOnPolicy(clusterState, index, newPolicyName, currentStep, currentStep, newClusterState, now);
965+
}
966+
967+
public void testSetPolicyForIndexIndexInUnsafeActionChanged() {
968+
String indexName = randomAlphaOfLength(10);
969+
String oldPolicyName = "old_policy";
970+
String newPolicyName = "new_policy";
971+
StepKey currentStep = new StepKey(randomAlphaOfLength(10), MockAction.NAME, randomAlphaOfLength(10));
972+
LifecyclePolicy oldPolicy = createPolicy(oldPolicyName, null, currentStep);
973+
974+
// change the current action so its not equal to the old one by adding a step
975+
Map<String, Phase> phases = new HashMap<>();
976+
Map<String, LifecycleAction> actions = new HashMap<>();
977+
MockAction unsafeAction = new MockAction(Collections.singletonList(new MockStep(currentStep, null)), false);
978+
actions.put(unsafeAction.getWriteableName(), unsafeAction);
979+
Phase phase = new Phase(currentStep.getPhase(), TimeValue.timeValueMillis(0), actions);
980+
phases.put(phase.getName(), phase);
981+
LifecyclePolicy newPolicy = new LifecyclePolicy(TestLifecycleType.INSTANCE, newPolicyName, phases);
982+
983+
Settings.Builder indexSettingsBuilder = Settings.builder().put(LifecycleSettings.LIFECYCLE_NAME, oldPolicyName)
984+
.put(LifecycleSettings.LIFECYCLE_PHASE, currentStep.getPhase())
985+
.put(LifecycleSettings.LIFECYCLE_ACTION, currentStep.getAction())
986+
.put(LifecycleSettings.LIFECYCLE_STEP, currentStep.getName()).put(LifecycleSettings.LIFECYCLE_SKIP, true);
987+
List<LifecyclePolicyMetadata> policyMetadatas = new ArrayList<>();
988+
policyMetadatas.add(new LifecyclePolicyMetadata(oldPolicy, Collections.emptyMap()));
989+
ClusterState clusterState = buildClusterState(indexName, indexSettingsBuilder, policyMetadatas);
990+
Index index = clusterState.metaData().index(indexName).getIndex();
991+
Index[] indices = new Index[] { index };
992+
List<String> failedIndexes = new ArrayList<>();
993+
994+
ClusterState newClusterState = IndexLifecycleRunner.setPolicyForIndexes(newPolicyName, indices, clusterState, newPolicy,
995+
failedIndexes);
996+
997+
assertEquals(1, failedIndexes.size());
998+
assertEquals(index.getName(), failedIndexes.get(0));
999+
assertSame(clusterState, newClusterState);
1000+
}
1001+
9411002
private static LifecyclePolicy createPolicy(String policyName, StepKey safeStep, StepKey unsafeStep) {
9421003
Map<String, Phase> phases = new HashMap<>();
9431004
if (safeStep != null) {
@@ -1002,6 +1063,56 @@ public void testCanUpdatePolicyIndexInUnsafe() {
10021063
assertFalse(canUpdatePolicy);
10031064
}
10041065

1066+
public void testCanUpdatePolicyIndexInUnsafeActionUnchanged() {
1067+
String indexName = randomAlphaOfLength(10);
1068+
String oldPolicyName = "old_policy";
1069+
String newPolicyName = "new_policy";
1070+
StepKey currentStep = new StepKey(randomAlphaOfLength(10), MockAction.NAME, randomAlphaOfLength(10));
1071+
LifecyclePolicy oldPolicy = createPolicy(oldPolicyName, null, currentStep);
1072+
LifecyclePolicy newPolicy = createPolicy(newPolicyName,
1073+
new StepKey(randomAlphaOfLength(10), MockAction.NAME, randomAlphaOfLength(10)), currentStep);
1074+
Settings.Builder indexSettingsBuilder = Settings.builder().put(LifecycleSettings.LIFECYCLE_NAME, oldPolicyName)
1075+
.put(LifecycleSettings.LIFECYCLE_PHASE, currentStep.getPhase())
1076+
.put(LifecycleSettings.LIFECYCLE_ACTION, currentStep.getAction())
1077+
.put(LifecycleSettings.LIFECYCLE_STEP, currentStep.getName()).put(LifecycleSettings.LIFECYCLE_SKIP, true);
1078+
List<LifecyclePolicyMetadata> policyMetadatas = new ArrayList<>();
1079+
policyMetadatas.add(new LifecyclePolicyMetadata(oldPolicy, Collections.emptyMap()));
1080+
ClusterState clusterState = buildClusterState(indexName, indexSettingsBuilder, policyMetadatas);
1081+
1082+
boolean canUpdatePolicy = IndexLifecycleRunner.canUpdatePolicy(oldPolicyName, newPolicy, clusterState);
1083+
1084+
assertTrue(canUpdatePolicy);
1085+
}
1086+
1087+
public void testCanUpdatePolicyIndexInUnsafeActionChanged() {
1088+
String indexName = randomAlphaOfLength(10);
1089+
String oldPolicyName = "old_policy";
1090+
String newPolicyName = "new_policy";
1091+
StepKey currentStep = new StepKey(randomAlphaOfLength(10), MockAction.NAME, randomAlphaOfLength(10));
1092+
LifecyclePolicy oldPolicy = createPolicy(oldPolicyName, null, currentStep);
1093+
1094+
// change the current action so its not equal to the old one by adding a step
1095+
Map<String, Phase> phases = new HashMap<>();
1096+
Map<String, LifecycleAction> actions = new HashMap<>();
1097+
MockAction unsafeAction = new MockAction(Collections.singletonList(new MockStep(currentStep, null)), false);
1098+
actions.put(unsafeAction.getWriteableName(), unsafeAction);
1099+
Phase phase = new Phase(currentStep.getPhase(), TimeValue.timeValueMillis(0), actions);
1100+
phases.put(phase.getName(), phase);
1101+
LifecyclePolicy newPolicy = new LifecyclePolicy(TestLifecycleType.INSTANCE, newPolicyName, phases);
1102+
1103+
Settings.Builder indexSettingsBuilder = Settings.builder().put(LifecycleSettings.LIFECYCLE_NAME, oldPolicyName)
1104+
.put(LifecycleSettings.LIFECYCLE_PHASE, currentStep.getPhase())
1105+
.put(LifecycleSettings.LIFECYCLE_ACTION, currentStep.getAction())
1106+
.put(LifecycleSettings.LIFECYCLE_STEP, currentStep.getName()).put(LifecycleSettings.LIFECYCLE_SKIP, true);
1107+
List<LifecyclePolicyMetadata> policyMetadatas = new ArrayList<>();
1108+
policyMetadatas.add(new LifecyclePolicyMetadata(oldPolicy, Collections.emptyMap()));
1109+
ClusterState clusterState = buildClusterState(indexName, indexSettingsBuilder, policyMetadatas);
1110+
1111+
boolean canUpdatePolicy = IndexLifecycleRunner.canUpdatePolicy(oldPolicyName, newPolicy, clusterState);
1112+
1113+
assertFalse(canUpdatePolicy);
1114+
}
1115+
10051116
public void testCanUpdatePolicyIndexNotManaged() {
10061117
String indexName = randomAlphaOfLength(10);
10071118
String oldPolicyName = "old_policy";

0 commit comments

Comments
 (0)