[RS-1818] Update ILM policy when warm index readonly setting changes #3336
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
peterkellydev
approved these changes
Apr 30, 2024
electricjesus
approved these changes
Apr 30, 2024
Brian-McM
reviewed
Apr 30, 2024
| } | ||
|
|
||
| currentMaxAge := existingPolicy.Phases.Hot.Actions.Rollover.MaxAge | ||
| currentMaxSize := existingPolicy.Phases.Hot.Actions.Rollover.MaxSize | ||
| currentMinAge := existingPolicy.Phases.Delete.MinAge | ||
| return currentMaxAge, currentMaxSize, currentMinAge, nil | ||
| readOnlyAfterRollover := existingPolicy.Phases.Warm.Actions.Readonly != nil | ||
| return currentMaxAge, currentMaxSize, currentMinAge, readOnlyAfterRollover, nil |
There was a problem hiding this comment.
Do you know why we don't just do a rollover if anything has changed, i.e. do some sort of deep equal?
There was a problem hiding this comment.
That was my thought too when I first saw this, seems like it would be a lot simpler and better long term. But a bigger change could expose me to more unforeseen issues and wanted to keep the change straightforward.
The main issue I could foresee would be sensitivity to json formatting or ordering mismatch could cause the comparison to "fail" and the policy to be continuously updated. This could be avoided with proper testing.
Would be happy to clean that up further if you think that's a good change?
Brian-McM
approved these changes
Apr 30, 2024
gantony
pushed a commit
to gantony/operator
that referenced
this pull request
May 1, 2024
…s-1.34 [RS-1818] Update ILM policy when warm index readonly setting changes
5 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Further testing around #3325 showed that the operator does not update the ILM policy when the value of
readOnlyAfterRolloverchanges.The code responsible for updating the ILM policy only do so if there is a change warranting an update. That check does not generically compare the old policy to the new one, but instead only looks at a handful of relevant parameters and tirggers an update only when one of these relevant parameters changes. This PR adds
readOnlyAfterRolloverto that logic and updates the tests to cover that logic.For PR author
make gen-filesmake gen-versionsFor PR reviewers
A note for code reviewers - all pull requests must have the following:
kind/bugif this is a bugfix.kind/enhancementif this is a a new feature.enterpriseif this PR applies to Calico Enterprise only.