Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ILM] Fix set replicas serialization and validation #85233

Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -141,9 +141,8 @@ export const ColdPhase: FunctionComponent = () => {
'xpack.indexLifecycleMgmt.editPolicy.coldPhase.numberOfReplicas.switchLabel',
{ defaultMessage: 'Set replicas' }
),
initialValue: Boolean(
policy.phases.cold?.actions?.allocate?.number_of_replicas
),
initialValue:
policy.phases.cold?.actions?.allocate?.number_of_replicas != null,
}}
fullWidth
>
Original file line number Diff line number Diff line change
@@ -24,7 +24,7 @@ export const ForcemergeField: React.FunctionComponent<Props> = ({ phase }) => {
const { policy } = useEditPolicyContext();

const initialToggleValue = useMemo<boolean>(() => {
return Boolean(policy.phases[phase]?.actions?.forcemerge);
return policy.phases[phase]?.actions?.forcemerge != null;
}, [policy, phase]);

return (
Original file line number Diff line number Diff line change
@@ -162,7 +162,7 @@ export const WarmPhase: FunctionComponent = () => {
'xpack.indexLifecycleMgmt.editPolicy.warmPhase.numberOfReplicas.switchLabel',
{ defaultMessage: 'Set replicas' }
),
initialValue: Boolean(policy.phases.warm?.actions?.allocate?.number_of_replicas),
initialValue: policy.phases.warm?.actions?.allocate?.number_of_replicas != null,
}}
fullWidth
>
@@ -203,7 +203,7 @@ export const WarmPhase: FunctionComponent = () => {
'data-test-subj': 'shrinkSwitch',
label: i18nTexts.shrinkLabel,
'aria-label': i18nTexts.shrinkLabel,
initialValue: Boolean(policy.phases.warm?.actions?.shrink),
initialValue: policy.phases.warm?.actions?.shrink != null,
}}
fullWidth
>
Original file line number Diff line number Diff line change
@@ -56,11 +56,17 @@ const originalPolicy: SerializedPolicy = {
shrink: { number_of_shards: 12 },
allocate: {
number_of_replicas: 3,
include: {
some: 'value',
},
exclude: {
some: 'value',
},
},
set_priority: {
priority: 10,
},
migrate: { enabled: false },
migrate: { enabled: true },
},
},
cold: {
@@ -133,7 +139,8 @@ describe('deserializer and serializer', () => {

const copyOfThisTestPolicy = cloneDeep(thisTestPolicy);

expect(serializer(deserializer(thisTestPolicy))).toEqual(thisTestPolicy);
const _formInternal = deserializer(thisTestPolicy);
expect(serializer(_formInternal)).toEqual(thisTestPolicy);

// Assert that the policy we passed in is unaltered after deserialization and serialization
expect(thisTestPolicy).not.toBe(copyOfThisTestPolicy);
@@ -247,4 +254,40 @@ describe('deserializer and serializer', () => {
},
});
});

it('sets all known allocate options correctly', () => {
formInternal.phases.warm!.actions.allocate!.number_of_replicas = 0;
formInternal._meta.warm.dataTierAllocationType = 'node_attrs';
formInternal._meta.warm.allocationNodeAttribute = 'some:value';

expect(serializer(formInternal).phases.warm!.actions.allocate).toEqual({
number_of_replicas: 0,
require: {
some: 'value',
},
include: {
some: 'value',
},
exclude: {
some: 'value',
},
});
});

it('sets allocate and migrate actions when defined together', () => {
formInternal.phases.warm!.actions.allocate!.number_of_replicas = 0;
formInternal._meta.warm.dataTierAllocationType = 'none';
// This should not be set...
formInternal._meta.warm.allocationNodeAttribute = 'some:value';

const result = serializer(formInternal);

expect(result.phases.warm!.actions.allocate).toEqual({
number_of_replicas: 0,
});

expect(result.phases.warm!.actions.migrate).toEqual({
enabled: false,
});
});
});
Original file line number Diff line number Diff line change
@@ -202,7 +202,7 @@ export const schema: FormSchema<FormInternal> = {
}),
validations: [
{
validator: ifExistsNumberGreaterThanZero,
validator: ifExistsNumberNonNegative,
},
],
serializer: serializers.stringToNumber,
@@ -273,7 +273,7 @@ export const schema: FormSchema<FormInternal> = {
}),
validations: [
{
validator: ifExistsNumberGreaterThanZero,
validator: ifExistsNumberNonNegative,
},
],
serializer: serializers.stringToNumber,
Original file line number Diff line number Diff line change
@@ -11,18 +11,33 @@ import { SerializedActionWithAllocation } from '../../../../../../common/types';
import { DataAllocationMetaFields } from '../../types';

export const serializeMigrateAndAllocateActions = (
/**
* Form metadata about what tier allocation strategy to use and custom node
* allocation information.
*/
{ dataTierAllocationType, allocationNodeAttribute }: DataAllocationMetaFields,
newActions: SerializedActionWithAllocation = {},
originalActions: SerializedActionWithAllocation = {}
/**
* The new configuration merged with old configuration to ensure we don't lose
* any fields.
*/
mergedActions: SerializedActionWithAllocation = {},
/**
* The actions from the policy for a given phase when it was loaded.
*/
originalActions: SerializedActionWithAllocation = {},
/**
* The number of replicas value to set in the allocate action.
*/
numberOfReplicas?: number
): SerializedActionWithAllocation => {
const { allocate, migrate, ...otherActions } = newActions;
const { allocate, migrate, ...otherActions } = mergedActions;

// First copy over all non-allocate and migrate actions.
const actions: SerializedActionWithAllocation = { ...otherActions };

// The UI only knows about include, exclude and require, so copy over all other values.
// The UI only knows about include, exclude, require and number_of_replicas so copy over all other values.
if (allocate) {
const { include, exclude, require, ...otherSettings } = allocate;
const { include, exclude, require, number_of_replicas: __, ...otherSettings } = allocate;
if (!isEmpty(otherSettings)) {
actions.allocate = { ...otherSettings };
}
@@ -69,5 +84,13 @@ export const serializeMigrateAndAllocateActions = (
break;
default:
}

if (numberOfReplicas != null) {
actions.allocate = {
...actions.allocate,
number_of_replicas: numberOfReplicas,
};
}

return actions;
};
Original file line number Diff line number Diff line change
@@ -95,7 +95,8 @@ export const createSerializer = (originalPolicy?: SerializedPolicy) => (
warmPhase.actions = serializeMigrateAndAllocateActions(
_meta.warm,
warmPhase.actions,
originalPolicy?.phases.warm?.actions
originalPolicy?.phases.warm?.actions,
updatedPolicy.phases.warm?.actions?.allocate?.number_of_replicas
);

if (!updatedPolicy.phases.warm?.actions?.forcemerge) {
@@ -129,7 +130,8 @@ export const createSerializer = (originalPolicy?: SerializedPolicy) => (
coldPhase.actions = serializeMigrateAndAllocateActions(
_meta.cold,
coldPhase.actions,
originalPolicy?.phases.cold?.actions
originalPolicy?.phases.cold?.actions,
updatedPolicy.phases.cold?.actions?.allocate?.number_of_replicas
);

if (_meta.cold.freezeEnabled) {