-
Notifications
You must be signed in to change notification settings - Fork 96
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
Add max_primary_shard_docs
condition to ILM rollover
#845
Add max_primary_shard_docs
condition to ILM rollover
#845
Conversation
💚 CLA has been signed |
* Add `max_primary_shard_docs` condition to rollover * Update test for rollover `max_primary_shard_docs` condition * Specify min version in the description * Update CHANGELOG.md
eaf341f
to
f16e3b4
Compare
internal/elasticsearch/index/ilm.go
Outdated
"priority": {skipEmptyCheck: true}, | ||
"max_primary_shard_docs": {minVersion: version.Must(version.NewVersion("8.2.0"))}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not sure about what should be put there as this property is optional. Maybe a skipEmptyCheck should be added there.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this should be fine, skipEmptyCheck
is around dealing with how the TF SDK represents unset attributes as Golang zero values.
* Add `max_primary_shard_docs` condition to rollover * Update test for rollover `max_primary_shard_docs` condition * Specify min version in the description * Update CHANGELOG.md
f16e3b4
to
b97f1c7
Compare
* Add `max_primary_shard_docs` condition to rollover * Update test for rollover `max_primary_shard_docs` condition * Specify min version in the description * Update CHANGELOG.md
b97f1c7
to
3136fbc
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for adding this in, we'll want to update the tests to only cover this attribute when it's supported.
@@ -273,12 +274,13 @@ resource "elasticstack_elasticsearch_index_lifecycle" "test_rollover" { | |||
max_age = "7d" | |||
max_docs = 10000 | |||
max_size = "100gb" | |||
max_primary_shard_docs = 5000 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We'll want to update the tests to only specify this when it's supported by the backing cluster. That means adding a case covering this attribute which is skipped when it's not supported (example)
* Add `max_primary_shard_docs` condition to rollover * Update test for rollover `max_primary_shard_docs` condition * Specify min version in the description * Update CHANGELOG.md
3e169ec
to
2430e93
Compare
Add
max_primary_shard_docs
condition to rollover (closes [Bug] max_primary_shard_docs variable is not supported in elasticstack_elasticsearch_index_lifecycle.hot.rollover #794)Update test for rollover
max_primary_shard_docs
conditionSpecify min version in the description
Update CHANGELOG.md