From a22f2853ba37e3fe023b14c92abd2808e03c3f28 Mon Sep 17 00:00:00 2001 From: Alison Goryachev Date: Thu, 7 Jan 2021 14:08:46 -0500 Subject: [PATCH] [ILM] Fix hot phase serialization (#87213) --- .../components/phases/hot_phase/hot_phase.tsx | 272 +++++++++--------- .../sections/edit_policy/constants.ts | 2 +- .../form/configuration_issues_context.tsx | 14 +- .../sections/edit_policy/form/deserializer.ts | 8 +- .../form/deserializer_and_serializer.test.ts | 32 ++- .../sections/edit_policy/form/schema.ts | 24 +- .../edit_policy/form/serializer/serializer.ts | 119 ++++++-- .../application/sections/edit_policy/types.ts | 18 +- 8 files changed, 308 insertions(+), 181 deletions(-) diff --git a/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/components/phases/hot_phase/hot_phase.tsx b/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/components/phases/hot_phase/hot_phase.tsx index d9976605393c7..a777f30fd2e42 100644 --- a/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/components/phases/hot_phase/hot_phase.tsx +++ b/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/components/phases/hot_phase/hot_phase.tsx @@ -52,7 +52,7 @@ export const HotPhase: FunctionComponent = () => { watch: isUsingDefaultRolloverPath, }); const { isUsingRollover } = useConfigurationIssues(); - const isUsingDefaultRollover = get(formData, isUsingDefaultRolloverPath); + const isUsingDefaultRollover: boolean = get(formData, isUsingDefaultRolloverPath); const [showEmptyRolloverFieldsError, setShowEmptyRolloverFieldsError] = useState(false); return ( @@ -145,145 +145,145 @@ export const HotPhase: FunctionComponent = () => { } fullWidth > -
- path="_meta.hot.useRollover"> - {(field) => ( - <> - field.setValue(e.target.checked)} - data-test-subj="rolloverSwitch" - /> -   - + path="_meta.hot.customRollover.enabled"> + {(field) => ( + <> + field.setValue(e.target.checked)} + data-test-subj="rolloverSwitch" + /> +   + + } + /> + + )} + + {isUsingRollover && ( + <> + + {showEmptyRolloverFieldsError && ( + <> + +
{i18nTexts.editPolicy.errors.rollOverConfigurationCallout.body}
+
+ + + )} + + + + {(field) => { + const showErrorCallout = field.errors.some( + (e) => e.code === ROLLOVER_EMPTY_VALIDATION + ); + if (showErrorCallout !== showEmptyRolloverFieldsError) { + setShowEmptyRolloverFieldsError(showErrorCallout); + } + return ( + + ); + }} + + + + - } - /> + + + + + + + + + + + + + + + + + )} - - {isUsingRollover && ( - <> - - {showEmptyRolloverFieldsError && ( - <> - -
{i18nTexts.editPolicy.errors.rollOverConfigurationCallout.body}
-
- - - )} - - - - {(field) => { - const showErrorCallout = field.errors.some( - (e) => e.code === ROLLOVER_EMPTY_VALIDATION - ); - if (showErrorCallout !== showEmptyRolloverFieldsError) { - setShowEmptyRolloverFieldsError(showErrorCallout); - } - return ( - - ); - }} - - - - - - - - - - - - - - - - - - - - - - - )} -
+ + ) : ( +
+ )} {isUsingRollover && ( <> diff --git a/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/constants.ts b/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/constants.ts index 48ed38fc8a0d7..af59aa4b9323a 100644 --- a/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/constants.ts +++ b/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/constants.ts @@ -4,7 +4,7 @@ * you may not use this file except in compliance with the Elastic License. */ -export const useRolloverPath = '_meta.hot.useRollover'; +export const isUsingCustomRolloverPath = '_meta.hot.customRollover.enabled'; export const isUsingDefaultRolloverPath = '_meta.hot.isUsingDefaultRollover'; diff --git a/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/form/configuration_issues_context.tsx b/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/form/configuration_issues_context.tsx index 3a66abebccc1a..4ddb85899f3ac 100644 --- a/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/form/configuration_issues_context.tsx +++ b/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/form/configuration_issues_context.tsx @@ -9,7 +9,7 @@ import React, { FunctionComponent, createContext, useContext } from 'react'; import { useFormData } from '../../../../shared_imports'; -import { isUsingDefaultRolloverPath, useRolloverPath } from '../constants'; +import { isUsingDefaultRolloverPath, isUsingCustomRolloverPath } from '../constants'; export interface ConfigurationIssues { /** @@ -33,14 +33,20 @@ const pathToHotPhaseSearchableSnapshot = export const ConfigurationIssuesProvider: FunctionComponent = ({ children }) => { const [formData] = useFormData({ - watch: [pathToHotPhaseSearchableSnapshot, useRolloverPath, isUsingDefaultRolloverPath], + watch: [ + pathToHotPhaseSearchableSnapshot, + isUsingCustomRolloverPath, + isUsingDefaultRolloverPath, + ], }); const isUsingDefaultRollover = get(formData, isUsingDefaultRolloverPath); - const rolloverSwitchEnabled = get(formData, useRolloverPath); + // Provide default value, as path may become undefined if removed from the DOM + const isUsingCustomRollover = get(formData, isUsingCustomRolloverPath, true); + return ( { const _meta: FormInternal['_meta'] = { hot: { - useRollover: Boolean(hot?.actions?.rollover), isUsingDefaultRollover: isUsingDefaultRollover(policy), + customRollover: { + enabled: Boolean(hot?.actions?.rollover), + }, bestCompression: hot?.actions?.forcemerge?.index_codec === 'best_compression', readonlyEnabled: Boolean(hot?.actions?.readonly), }, @@ -53,13 +55,13 @@ export const deserializer = (policy: SerializedPolicy): FormInternal => { if (draft.phases.hot.actions.rollover.max_size) { const maxSize = splitSizeAndUnits(draft.phases.hot.actions.rollover.max_size); draft.phases.hot.actions.rollover.max_size = maxSize.size; - draft._meta.hot.maxStorageSizeUnit = maxSize.units; + draft._meta.hot.customRollover.maxStorageSizeUnit = maxSize.units; } if (draft.phases.hot.actions.rollover.max_age) { const maxAge = splitSizeAndUnits(draft.phases.hot.actions.rollover.max_age); draft.phases.hot.actions.rollover.max_age = maxAge.size; - draft._meta.hot.maxAgeUnit = maxAge.units; + draft._meta.hot.customRollover.maxAgeUnit = maxAge.units; } } diff --git a/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/form/deserializer_and_serializer.test.ts b/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/form/deserializer_and_serializer.test.ts index d72dbb38f6c95..b5abf51c29028 100644 --- a/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/form/deserializer_and_serializer.test.ts +++ b/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/form/deserializer_and_serializer.test.ts @@ -7,6 +7,7 @@ import { setAutoFreeze } from 'immer'; import { cloneDeep } from 'lodash'; import { SerializedPolicy } from '../../../../../common/types'; +import { defaultRolloverAction } from '../../../constants'; import { deserializer } from './deserializer'; import { createSerializer } from './serializer'; import { FormInternal } from '../types'; @@ -202,6 +203,18 @@ describe('deserializer and serializer', () => { expect(result.phases.warm!.actions.readonly).toBeUndefined(); }); + it('allows force merge and readonly actions to be configured in hot with default rollover enabled', () => { + formInternal._meta.hot.isUsingDefaultRollover = true; + formInternal._meta.hot.bestCompression = false; + formInternal.phases.hot!.actions.forcemerge = undefined; + formInternal._meta.hot.readonlyEnabled = false; + + const result = serializer(formInternal); + + expect(result.phases.hot!.actions.readonly).toBeUndefined(); + expect(result.phases.hot!.actions.forcemerge).toBeUndefined(); + }); + it('removes set priority if it is disabled in the form', () => { delete formInternal.phases.hot!.actions.set_priority; delete formInternal.phases.warm!.actions.set_priority; @@ -234,17 +247,21 @@ describe('deserializer and serializer', () => { expect(result.phases.cold!.actions.allocate!.exclude).toBeUndefined(); }); - it('removes forcemerge and rollover config when rollover is disabled in hot phase', () => { - formInternal._meta.hot.useRollover = false; + it('removes forcemerge, readonly, and rollover config when rollover is disabled in hot phase', () => { + // These two toggles jointly control whether rollover is enabled since the default is + // for rollover to be enabled. + formInternal._meta.hot.isUsingDefaultRollover = false; + formInternal._meta.hot.customRollover.enabled = false; const result = serializer(formInternal); expect(result.phases.hot!.actions.rollover).toBeUndefined(); expect(result.phases.hot!.actions.forcemerge).toBeUndefined(); + expect(result.phases.hot!.actions.readonly).toBeUndefined(); }); it('removes min_age from warm when rollover is enabled', () => { - formInternal._meta.hot.useRollover = true; + formInternal._meta.hot.customRollover.enabled = true; formInternal._meta.warm.warmPhaseOnRollover = true; const result = serializer(formInternal); @@ -252,6 +269,15 @@ describe('deserializer and serializer', () => { expect(result.phases.warm!.min_age).toBeUndefined(); }); + it('adds default rollover configuration when enabled, but previously not configured', () => { + delete formInternal.phases.hot!.actions.rollover; + formInternal._meta.hot.isUsingDefaultRollover = true; + + const result = serializer(formInternal); + + expect(result.phases.hot!.actions.rollover).toEqual(defaultRolloverAction); + }); + it('removes snapshot_repository when it is unset', () => { delete formInternal.phases.hot!.actions.searchable_snapshot; delete formInternal.phases.cold!.actions.searchable_snapshot; diff --git a/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/form/schema.ts b/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/form/schema.ts index ae2432971059c..4bdf902d27b6d 100644 --- a/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/form/schema.ts +++ b/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/form/schema.ts @@ -32,23 +32,25 @@ const serializers = { export const schema: FormSchema = { _meta: { hot: { - useRollover: { - defaultValue: true, - label: i18n.translate('xpack.indexLifecycleMgmt.hotPhase.enableRolloverLabel', { - defaultMessage: 'Enable rollover', - }), - }, isUsingDefaultRollover: { defaultValue: true, label: i18n.translate('xpack.indexLifecycleMgmt.hotPhase.isUsingDefaultRollover', { defaultMessage: 'Use recommended defaults', }), }, - maxStorageSizeUnit: { - defaultValue: 'gb', - }, - maxAgeUnit: { - defaultValue: 'd', + customRollover: { + enabled: { + defaultValue: true, + label: i18n.translate('xpack.indexLifecycleMgmt.hotPhase.enableRolloverLabel', { + defaultMessage: 'Enable rollover', + }), + }, + maxStorageSizeUnit: { + defaultValue: 'gb', + }, + maxAgeUnit: { + defaultValue: 'd', + }, }, bestCompression: { label: i18nTexts.editPolicy.bestCompressionFieldLabel, diff --git a/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/form/serializer/serializer.ts b/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/form/serializer/serializer.ts index 2a7c109fec950..f718073afa352 100644 --- a/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/form/serializer/serializer.ts +++ b/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/form/serializer/serializer.ts @@ -5,7 +5,6 @@ */ import { produce } from 'immer'; - import { merge, cloneDeep } from 'lodash'; import { SerializedPolicy } from '../../../../../../common/types'; @@ -29,6 +28,13 @@ export const createSerializer = (originalPolicy?: SerializedPolicy) => ( // Copy over all updated fields merge(draft, updatedPolicy); + /** + * Important shared values for serialization + */ + const isUsingRollover = Boolean( + _meta.hot.isUsingDefaultRollover || _meta.hot.customRollover.enabled + ); + // Next copy over all meta fields and delete any fields that have been removed // by fields exposed in the form. It is very important that we do not delete // data that the form does not control! E.g., unfollow action in hot phase. @@ -42,25 +48,40 @@ export const createSerializer = (originalPolicy?: SerializedPolicy) => ( if (draft.phases.hot?.actions) { const hotPhaseActions = draft.phases.hot.actions; - if (_meta.hot.isUsingDefaultRollover) { - hotPhaseActions.rollover = cloneDeep(defaultRolloverAction); - } else if (hotPhaseActions.rollover && _meta.hot.useRollover) { - if (updatedPolicy.phases.hot!.actions.rollover?.max_age) { - hotPhaseActions.rollover.max_age = `${hotPhaseActions.rollover.max_age}${_meta.hot.maxAgeUnit}`; - } else { - delete hotPhaseActions.rollover.max_age; - } - if (typeof updatedPolicy.phases.hot!.actions.rollover?.max_docs !== 'number') { - delete hotPhaseActions.rollover.max_docs; - } - - if (updatedPolicy.phases.hot!.actions.rollover?.max_size) { - hotPhaseActions.rollover.max_size = `${hotPhaseActions.rollover.max_size}${_meta.hot.maxStorageSizeUnit}`; + /** + * HOT PHASE ROLLOVER + */ + if (isUsingRollover) { + if (_meta.hot.isUsingDefaultRollover) { + hotPhaseActions.rollover = cloneDeep(defaultRolloverAction); } else { - delete hotPhaseActions.rollover.max_size; + // Rollover may not exist if editing an existing policy with initially no rollover configured + if (!hotPhaseActions.rollover) { + hotPhaseActions.rollover = {}; + } + + // We are using user-defined, custom rollover settings. + if (updatedPolicy.phases.hot!.actions.rollover?.max_age) { + hotPhaseActions.rollover.max_age = `${hotPhaseActions.rollover.max_age}${_meta.hot.customRollover.maxAgeUnit}`; + } else { + delete hotPhaseActions.rollover.max_age; + } + + if (typeof updatedPolicy.phases.hot!.actions.rollover?.max_docs !== 'number') { + delete hotPhaseActions.rollover.max_docs; + } + + if (updatedPolicy.phases.hot!.actions.rollover?.max_size) { + hotPhaseActions.rollover.max_size = `${hotPhaseActions.rollover.max_size}${_meta.hot.customRollover.maxStorageSizeUnit}`; + } else { + delete hotPhaseActions.rollover.max_size; + } } + /** + * HOT PHASE FORCEMERGE + */ if (!updatedPolicy.phases.hot!.actions?.forcemerge) { delete hotPhaseActions.forcemerge; } else if (_meta.hot.bestCompression) { @@ -73,6 +94,9 @@ export const createSerializer = (originalPolicy?: SerializedPolicy) => ( hotPhaseActions.forcemerge.index_codec = 'best_compression'; } + /** + * HOT PHASE READ-ONLY + */ if (_meta.hot.readonlyEnabled) { hotPhaseActions.readonly = hotPhaseActions.readonly ?? {}; } else { @@ -84,14 +108,23 @@ export const createSerializer = (originalPolicy?: SerializedPolicy) => ( delete hotPhaseActions.readonly; } + /** + * HOT PHASE SET PRIORITY + */ if (!updatedPolicy.phases.hot!.actions?.set_priority) { delete hotPhaseActions.set_priority; } + /** + * HOT PHASE SHRINK + */ if (!updatedPolicy.phases.hot?.actions?.shrink) { delete hotPhaseActions.shrink; } + /** + * HOT PHASE SEARCHABLE SNAPSHOT + */ if (!updatedPolicy.phases.hot!.actions?.searchable_snapshot) { delete hotPhaseActions.searchable_snapshot; } @@ -103,11 +136,16 @@ export const createSerializer = (originalPolicy?: SerializedPolicy) => ( if (_meta.warm.enabled) { draft.phases.warm!.actions = draft.phases.warm?.actions ?? {}; const warmPhase = draft.phases.warm!; - // If warm phase on rollover is enabled, delete min age field - // An index lifecycle switches to warm phase when rollover occurs, so you cannot specify a warm phase time - // They are mutually exclusive + + /** + * WARM PHASE MIN AGE + * + * If warm phase on rollover is enabled, delete min age field + * An index lifecycle switches to warm phase when rollover occurs, so you cannot specify a warm phase time + * They are mutually exclusive + */ if ( - (!_meta.hot.useRollover || !_meta.warm.warmPhaseOnRollover) && + (!isUsingRollover || !_meta.warm.warmPhaseOnRollover) && updatedPolicy.phases.warm?.min_age ) { warmPhase.min_age = `${updatedPolicy.phases.warm!.min_age}${_meta.warm.minAgeUnit}`; @@ -115,6 +153,9 @@ export const createSerializer = (originalPolicy?: SerializedPolicy) => ( delete warmPhase.min_age; } + /** + * WARM PHASE DATA ALLOCATION + */ warmPhase.actions = serializeMigrateAndAllocateActions( _meta.warm, warmPhase.actions, @@ -122,6 +163,9 @@ export const createSerializer = (originalPolicy?: SerializedPolicy) => ( updatedPolicy.phases.warm?.actions?.allocate?.number_of_replicas ); + /** + * WARM PHASE FORCEMERGE + */ if (!updatedPolicy.phases.warm?.actions?.forcemerge) { delete warmPhase.actions.forcemerge; } else if (_meta.warm.bestCompression) { @@ -130,16 +174,25 @@ export const createSerializer = (originalPolicy?: SerializedPolicy) => ( delete warmPhase.actions.forcemerge!.index_codec; } + /** + * WARM PHASE READ ONLY + */ if (_meta.warm.readonlyEnabled) { warmPhase.actions.readonly = warmPhase.actions.readonly ?? {}; } else { delete warmPhase.actions.readonly; } + /** + * WARM PHASE SET PRIORITY + */ if (!updatedPolicy.phases.warm?.actions?.set_priority) { delete warmPhase.actions.set_priority; } + /** + * WARM PHASE SHRINK + */ if (!updatedPolicy.phases.warm?.actions?.shrink) { delete warmPhase.actions.shrink; } @@ -154,10 +207,16 @@ export const createSerializer = (originalPolicy?: SerializedPolicy) => ( draft.phases.cold!.actions = draft.phases.cold?.actions ?? {}; const coldPhase = draft.phases.cold!; + /** + * COLD PHASE MIN AGE + */ if (updatedPolicy.phases.cold?.min_age) { coldPhase.min_age = `${updatedPolicy.phases.cold!.min_age}${_meta.cold.minAgeUnit}`; } + /** + * COLD PHASE DATA ALLOCATION + */ coldPhase.actions = serializeMigrateAndAllocateActions( _meta.cold, coldPhase.actions, @@ -165,16 +224,25 @@ export const createSerializer = (originalPolicy?: SerializedPolicy) => ( updatedPolicy.phases.cold?.actions?.allocate?.number_of_replicas ); + /** + * COLD PHASE FREEZE + */ if (_meta.cold.freezeEnabled) { coldPhase.actions.freeze = coldPhase.actions.freeze ?? {}; } else { delete coldPhase.actions.freeze; } + /** + * COLD PHASE SET PRIORITY + */ if (!updatedPolicy.phases.cold?.actions?.set_priority) { delete coldPhase.actions.set_priority; } + /** + * COLD PHASE SEARCHABLE SNAPSHOT + */ if (!updatedPolicy.phases.cold?.actions?.searchable_snapshot) { delete coldPhase.actions.searchable_snapshot; } @@ -187,12 +255,23 @@ export const createSerializer = (originalPolicy?: SerializedPolicy) => ( */ if (_meta.delete.enabled) { const deletePhase = draft.phases.delete!; + + /** + * DELETE PHASE DELETE + */ deletePhase.actions = deletePhase.actions ?? {}; deletePhase.actions.delete = deletePhase.actions.delete ?? {}; + + /** + * DELETE PHASE SEARCHABLE SNAPSHOT + */ if (updatedPolicy.phases.delete?.min_age) { deletePhase.min_age = `${updatedPolicy.phases.delete!.min_age}${_meta.delete.minAgeUnit}`; } + /** + * DELETE PHASE WAIT FOR SNAPSHOT + */ if (!updatedPolicy.phases.delete?.actions?.wait_for_snapshot) { delete deletePhase.actions.wait_for_snapshot; } diff --git a/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/types.ts b/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/types.ts index 4dfd7503b9973..247f607106216 100644 --- a/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/types.ts +++ b/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/types.ts @@ -22,11 +22,23 @@ export interface ForcemergeFields { } interface HotPhaseMetaFields extends ForcemergeFields { - useRollover: boolean; + /** + * By default rollover is enabled with set values for max age, max size and max docs. In this policy form + * opting in to default rollover overrides custom rollover values. + */ isUsingDefaultRollover: boolean; - maxStorageSizeUnit?: string; - maxAgeUnit?: string; + readonlyEnabled: boolean; + + /** + * If a policy has defined values other than the default rollover {@link defaultRolloverAction}, we store + * them here. + */ + customRollover: { + enabled: boolean; + maxStorageSizeUnit?: string; + maxAgeUnit?: string; + }; } interface WarmPhaseMetaFields extends DataAllocationMetaFields, MinAgeField, ForcemergeFields {