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 bug when clearing priority field #70154

Merged

Conversation

alisonelizabeth
Copy link
Contributor

@alisonelizabeth alisonelizabeth commented Jun 28, 2020

Fixes #67968

The issue above describes how to test/reproduce the bug.

It looks like the code was always falling back to the default priority value when the input was cleared. Per the ES docs, the priority field should be set to null to remove it.

Release note

This fixes a bug in Index Lifecycle Management where it was not possible to clear the index priority value when saving a policy.

@alisonelizabeth alisonelizabeth marked this pull request as ready for review June 29, 2020 01:23
@alisonelizabeth alisonelizabeth requested a review from a team as a code owner June 29, 2020 01:23
@elasticmachine
Copy link
Contributor

Pinging @elastic/es-ui (Team:Elasticsearch UI)

const { priority } = actions.set_priority;

if (priority) {
policy[PHASE_INDEX_PRIORITY] = priority;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: this is a good use case for policy[PHASE_INDEX_PRIORITY] = priority ?? '' and remove the if else statement.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Good point. Thanks for the reminder about this!

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Build metrics

✅ unchanged

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

Copy link
Contributor

@cjcenizal cjcenizal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested locally, code LGTM. Thanks for fixing this!!

@cjcenizal
Copy link
Contributor

@alisonelizabeth Can we backport this to 7.8, since it's a bugfix?

@alisonelizabeth alisonelizabeth merged commit 771f3ae into elastic:master Jun 30, 2020
@alisonelizabeth alisonelizabeth deleted the bugfix/ilm_priority_field branch June 30, 2020 00:34
alisonelizabeth added a commit to alisonelizabeth/kibana that referenced this pull request Jun 30, 2020
# Conflicts:
#	x-pack/plugins/index_lifecycle_management/__jest__/client_integration/edit_policy/constants.ts
#	x-pack/plugins/index_lifecycle_management/__jest__/client_integration/edit_policy/edit_policy.helpers.tsx
#	x-pack/plugins/index_lifecycle_management/__jest__/client_integration/edit_policy/edit_policy.test.ts
#	x-pack/plugins/index_lifecycle_management/public/application/store/selectors/policies.js
jloleysens added a commit to jloleysens/kibana that referenced this pull request Jun 30, 2020
…ata-streams

* 'master' of github.com:elastic/kibana: (50 commits)
  [Logs UI] [Alerting] "Group by" functionality (elastic#68250)
  [Discover] Deangularize Skip to bottom button (elastic#69811)
  Implement recursive plugin discovery (elastic#68811)
  Use ts-expect-error in platform code (elastic#69883)
  [SIEM][Detection Engine][Lists] Moves getQueryFilter to common folder for use by both front and backend
  [Ingest Manager][SECURITY SOLUTION] adjust config reassign link and add roundtrip to Reassignment flow (elastic#70208)
  [Security][Lists] Add API functions and react hooks for value list APIs (elastic#69603)
  [ILM] Fix bug when clearing priority field (elastic#70154)
  [Platform][Security] Updates cluster_manager ignorePaths to include security scripts (elastic#70139)
  [IngestManager] Allow to filter agent by packages (elastic#69731)
  [code coverage] exclude folders: test_helpers, tests_bundle (elastic#70199)
  [Metrics UI] UX improvements for saved views (elastic#69910)
  [APM] docs: unique transaction troubleshooting (elastic#69831)
  Cross cluster search functional test with minimun privileges assigned to the test_user (elastic#70007)
  [Maps] choropleth layer wizard (elastic#69699)
  Make custom errors by extending Error (elastic#69966)
  [Ingest Manager] Support updated package output structure (elastic#69864)
  Resolver test coverage (elastic#70246)
  Async Discover search test (elastic#64388)
  [ui-shared-deps] include styled-components (elastic#69322)
  ...

# Conflicts:
#	x-pack/plugins/snapshot_restore/server/types.ts
gmmorris added a commit to gmmorris/kibana that referenced this pull request Jun 30, 2020
…bana into alerting/consumer-based-rbac

* 'alerting/consumer-based-rbac' of github.com:gmmorris/kibana: (49 commits)
  [Discover] Deangularize Skip to bottom button (elastic#69811)
  Implement recursive plugin discovery (elastic#68811)
  Use ts-expect-error in platform code (elastic#69883)
  [SIEM][Detection Engine][Lists] Moves getQueryFilter to common folder for use by both front and backend
  [Ingest Manager][SECURITY SOLUTION] adjust config reassign link and add roundtrip to Reassignment flow (elastic#70208)
  [Security][Lists] Add API functions and react hooks for value list APIs (elastic#69603)
  [ILM] Fix bug when clearing priority field (elastic#70154)
  [Platform][Security] Updates cluster_manager ignorePaths to include security scripts (elastic#70139)
  [IngestManager] Allow to filter agent by packages (elastic#69731)
  [code coverage] exclude folders: test_helpers, tests_bundle (elastic#70199)
  [Metrics UI] UX improvements for saved views (elastic#69910)
  [APM] docs: unique transaction troubleshooting (elastic#69831)
  Cross cluster search functional test with minimun privileges assigned to the test_user (elastic#70007)
  [Maps] choropleth layer wizard (elastic#69699)
  Make custom errors by extending Error (elastic#69966)
  [Ingest Manager] Support updated package output structure (elastic#69864)
  Resolver test coverage (elastic#70246)
  Async Discover search test (elastic#64388)
  [ui-shared-deps] include styled-components (elastic#69322)
  SECURITY-ENDPOINT: add host properties (elastic#70238)
  ...
Bamieh pushed a commit to Bamieh/kibana that referenced this pull request Jul 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:ILM release_note:fix Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more v7.8.1 v7.9.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[ILM] Unable to remove Index priority value on existing index lifecycle policy
5 participants