From b308a4a47184da4d2947d4b719734ec6840617ac Mon Sep 17 00:00:00 2001 From: Colin Goodheart-Smithe Date: Thu, 24 May 2018 14:01:41 +0100 Subject: [PATCH] Changes PhaseAfterStep to take the name of the previous phase (#30756) * Changes PhaseAfterStep to take the name of the previous phase This changes the way the phase after step is built so its key has the phase name of the phase that preceeds it rather than the phase that follows it. This is more intuitive to the user since the index is in the warm phase until the after condition for the cold phase is met. * Fixes REST tests x-pack/plugin/src/test/resources/rest-api-spec/test/index_lifecycle/20_m ove_to_step.yml x-pack/plugin/src/test/resources/rest-api-spec/test/index_lifecycle/20_m ove_to_step.yml --- .../core/indexlifecycle/LifecyclePolicy.java | 11 ++++++++--- .../core/indexlifecycle/PhaseAfterStep.java | 1 + .../indexlifecycle/LifecyclePolicyTests.java | 7 ++++--- .../test/index_lifecycle/20_move_to_step.yml | 16 ++++++++-------- 4 files changed, 21 insertions(+), 14 deletions(-) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/indexlifecycle/LifecyclePolicy.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/indexlifecycle/LifecyclePolicy.java index 379cd6f62f7..07c5c8c87d4 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/indexlifecycle/LifecyclePolicy.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/indexlifecycle/LifecyclePolicy.java @@ -178,15 +178,19 @@ public class LifecyclePolicy extends AbstractDiffable // add steps for each phase, in reverse while (phaseIterator.hasPrevious()) { + Phase previousPhase = phaseIterator.previous(); + // add `after` step for phase before next if (phase != null) { - Step.StepKey afterStepKey = new Step.StepKey(phase.getName(), "pre-" + lastStepKey.getAction(), "after"); + // after step should have the name of the previous phase since the index is still in the + // previous phase until the after condition is reached + Step.StepKey afterStepKey = new Step.StepKey(previousPhase.getName(), PhaseAfterStep.NAME, PhaseAfterStep.NAME); Step phaseAfterStep = new PhaseAfterStep(nowSupplier, phase.getAfter(), afterStepKey, lastStepKey); steps.add(phaseAfterStep); lastStepKey = phaseAfterStep.getKey(); } - phase = phaseIterator.previous(); + phase = previousPhase; List orderedActions = type.getOrderedActions(phase); ListIterator actionIterator = orderedActions.listIterator(orderedActions.size()); // add steps for each action, in reverse @@ -203,7 +207,8 @@ public class LifecyclePolicy extends AbstractDiffable } if (phase != null) { - Step.StepKey afterStepKey = new Step.StepKey(phase.getName(), "pre-" + lastStepKey.getAction(), "after"); + // The very first after step is in a phase before the hot phase so call this "new" + Step.StepKey afterStepKey = new Step.StepKey("new", PhaseAfterStep.NAME, PhaseAfterStep.NAME); Step phaseAfterStep = new PhaseAfterStep(nowSupplier, phase.getAfter(), afterStepKey, lastStepKey); steps.add(phaseAfterStep); lastStepKey = phaseAfterStep.getKey(); diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/indexlifecycle/PhaseAfterStep.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/indexlifecycle/PhaseAfterStep.java index 0802f8c0fb7..3cc090b106f 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/indexlifecycle/PhaseAfterStep.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/indexlifecycle/PhaseAfterStep.java @@ -14,6 +14,7 @@ import java.util.Objects; import java.util.function.LongSupplier; public class PhaseAfterStep extends ClusterStateWaitStep { + public static final String NAME = "after"; private final TimeValue after; private final LongSupplier nowSupplier; diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/indexlifecycle/LifecyclePolicyTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/indexlifecycle/LifecyclePolicyTests.java index 40c69e13b71..12fe09b24b3 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/indexlifecycle/LifecyclePolicyTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/indexlifecycle/LifecyclePolicyTests.java @@ -135,7 +135,7 @@ public class LifecyclePolicyTests extends AbstractSerializingTestCase steps = policy.toSteps(client, nowSupplier); assertThat(steps.size(), equalTo(4)); assertSame(steps.get(0).getKey(), firstStepKey); @@ -151,10 +151,11 @@ public class LifecyclePolicyTests extends AbstractSerializingTestCase 0L; MockStep secondActionStep = new MockStep(new StepKey("second_phase", "test2", "test"), TerminalPolicyStep.KEY); - MockStep secondAfter = new MockStep(new StepKey("second_phase", "pre-test2", "after"), secondActionStep.getKey()); + MockStep secondAfter = new MockStep(new StepKey("first_phase", PhaseAfterStep.NAME, PhaseAfterStep.NAME), + secondActionStep.getKey()); MockStep firstActionAnotherStep = new MockStep(new StepKey("first_phase", "test", "bar"), secondAfter.getKey()); MockStep firstActionStep = new MockStep(new StepKey("first_phase", "test", "foo"), firstActionAnotherStep.getKey()); - MockStep firstAfter = new MockStep(new StepKey("first_phase", "pre-test", "after"), firstActionStep.getKey()); + MockStep firstAfter = new MockStep(new StepKey("new", PhaseAfterStep.NAME, PhaseAfterStep.NAME), firstActionStep.getKey()); MockStep init = new MockStep(InitializePolicyContextStep.KEY, firstAfter.getKey()); lifecycleName = randomAlphaOfLengthBetween(1, 20); diff --git a/x-pack/plugin/src/test/resources/rest-api-spec/test/index_lifecycle/20_move_to_step.yml b/x-pack/plugin/src/test/resources/rest-api-spec/test/index_lifecycle/20_move_to_step.yml index 22b2d4dd7f3..ed04a029bae 100644 --- a/x-pack/plugin/src/test/resources/rest-api-spec/test/index_lifecycle/20_move_to_step.yml +++ b/x-pack/plugin/src/test/resources/rest-api-spec/test/index_lifecycle/20_move_to_step.yml @@ -76,8 +76,8 @@ teardown: index: "my_index" body: current_step: - phase: "hot" - action: "pre-pre-readonly" + phase: "new" + action: "after" name: "after" next_step: phase: "warm" @@ -118,8 +118,8 @@ teardown: index: my_index - match: { my_index.settings.index.lifecycle.name: "my_moveable_timeseries_lifecycle" } - match: { my_index.settings.index.lifecycle.step: "after" } - - match: { my_index.settings.index.lifecycle.action: "pre-pre-readonly" } - - match: { my_index.settings.index.lifecycle.phase: "hot" } + - match: { my_index.settings.index.lifecycle.action: "after" } + - match: { my_index.settings.index.lifecycle.phase: "new" } --- "Test Invalid Move To Step With Invalid Next Step": @@ -130,8 +130,8 @@ teardown: index: "my_index" body: current_step: - phase: "hot" - action: "pre-pre-readonly" + phase: "new" + action: "after" name: "after" next_step: phase: "invalid" @@ -146,8 +146,8 @@ teardown: index: my_index - match: { my_index.settings.index.lifecycle.name: "my_moveable_timeseries_lifecycle" } - match: { my_index.settings.index.lifecycle.step: "after" } - - match: { my_index.settings.index.lifecycle.action: "pre-pre-readonly" } - - match: { my_index.settings.index.lifecycle.phase: "hot" } + - match: { my_index.settings.index.lifecycle.action: "after" } + - match: { my_index.settings.index.lifecycle.phase: "new" } --- "Test Invalid Move To Step With Invalid Policy":