maxmustermann/kibana
1
0
Fork 0
Code Issues Pull requests Projects Releases Wiki Activity

[ILM] Fix set replicas serialization and validation (#85233)

Browse source 
* added fix for serializing replicas and updated validation to correctly check for non negative nr

* added tests and fixed incorrect use of warm phase setting in cold phase

* remove unused import

* clean up use of Boolean and rename nrOfReplicas -> numberOfReplicas

* fix comment
This commit is contained in:
Jean-Louis Leysens 2020-12-08 16:33:07 +01:00 committed by GitHub
parent 6ff71992a3
commit 8eae30c82f
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
7 changed files with 84 additions and 17 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

5
x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/components/phases/cold_phase/cold_phase.tsx
View file

@ -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
>

2
x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/components/phases/shared_fields/forcemerge_field.tsx
View file

@ -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 (

4
x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/components/phases/warm_phase/warm_phase.tsx
View file

@ -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
>

47
x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/form/deserializer_and_serializer.test.ts
View file

@ -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,
});
});
});

4
x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/form/schema.ts
View file

@ -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,

33
x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/form/serializer/serialize_migrate_and_allocate_actions.ts
View file

@ -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;
};

6
x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/form/serializer/serializer.ts
View file

@ -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) {