Add resource to manage ML datafeed state #1422
Conversation
nick-benoit
left a comment
nick-benoit
left a comment
There was a problem hiding this comment.
Nice. Having the resource to manage the state is a cool idea. Just a few optional questions / thoughts.
| "github.com/hashicorp/terraform-plugin-testing/helper/resource" | ||
| "github.com/hashicorp/terraform-plugin-testing/terraform" | ||
| ) | ||
|
|
There was a problem hiding this comment.
Is there a good way to add test coverage for datafeed_timeout and timeouts? I guess since they aren't written to any particular resource we read back from ES it might be a bit tricky. It would be nice to have some test coverage though.
There was a problem hiding this comment.
Yea, good question. I've added a test to verify the timeouts on the job state resource, but I can't think of a way to do the same on the datafeed (i.e artificially block the datafeed from starting).
ES doesn't return the size as provided, so ensure 1024mb = 1024m = 1g etc
* origin/main: First take at documented coding standards (#1429) chore(deps): update golangci/golangci-lint-action action to v9 (#1431) chore(deps): update golang:1.25.4 docker digest to 6ca9eb0 (#1432) chore(deps): update kibana-openapi-spec digest to 96ffcd7 (#1430) chore(deps): update kibana-openapi-spec digest to a8e3b64 (#1428) chore(deps): update golang docker tag to v1.25.4 (#1423)
There was a problem hiding this comment.
Pull Request Overview
This PR adds a new resource elasticstack_elasticsearch_ml_datafeed_state to manage the operational state (started/stopped) of existing Elasticsearch ML datafeeds. The implementation includes comprehensive testing, timeout handling, and a custom memory size type for improved ML job configuration.
Key changes:
- Added new
elasticstack_elasticsearch_ml_datafeed_stateresource for managing datafeed states - Introduced custom
MemorySizetype with semantic equality checking for ML memory limits - Enhanced timeout handling with descriptive error messages for ML operations
- Refactored datafeed state utilities for reusability
Reviewed Changes
Copilot reviewed 36 out of 37 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| provider/plugin_framework.go | Registered new datafeed state resource |
| internal/elasticsearch/ml/datafeed_state/*.go | Core implementation of datafeed state resource including CRUD operations |
| internal/utils/customtypes/memory_size*.go | New custom type for memory size values with validation and semantic equality |
| internal/models/ml.go | Added datafeed timing stats and search interval models, moved ML job models |
| internal/elasticsearch/ml/datafeed/*.go | Refactored state utilities to be reusable by datafeed state resource |
| internal/elasticsearch/ml/job_state/*.go | Enhanced timeout error messages and refactored state transition logic |
| internal/elasticsearch/ml/anomaly_detection_job/*.go | Updated to use new MemorySize custom type |
| docker-compose.yml, .github/workflows/test.yml | Configured ML memory limits for testing |
| examples/, docs/ | Added documentation and examples for new resource |
| Node *DatafeedNode `json:"node,omitempty"` | ||
| AssignmentExplanation *string `json:"assignment_explanation,omitempty"` | ||
| RunningState *DatafeedRunning `json:"running_state,omitempty"` | ||
| TimingStats *DatafeedTimingStats `json:"timing_stats"` |
There was a problem hiding this comment.
The TimingStats field is marked as a pointer but doesn't have omitempty in the JSON tag. This is inconsistent with other pointer fields in the struct (like Node, AssignmentExplanation, RunningState) which all have omitempty. This could cause issues when marshaling/unmarshaling JSON if the field is nil. Add omitempty to the JSON tag.
| TimingStats *DatafeedTimingStats `json:"timing_stats"` | |
| TimingStats *DatafeedTimingStats `json:"timing_stats,omitempty"` |
nick-benoit
left a comment
nick-benoit
left a comment
There was a problem hiding this comment.
Changes from my comments look good 👍 Pending you resolve the couple of new copilot comments.
Sidenote, it seems to me this is a notably more useful review than we were seeing before.
* origin/main: (90 commits) Add resource to manage ML datafeed state (#1422) Extract agent policy test config (#1439) chore(deps): update golang:1.25.4 docker digest to e68f6a0 (#1437) chore(deps): update kibana-openapi-spec digest to 6a867d8 (#1435) chore(deps): update docker.elastic.co/kibana/kibana docker tag to v9.2.1 (#1434) chore(deps): update docker.elastic.co/elasticsearch/elasticsearch docker tag to v9.2.1 (#1433) First take at documented coding standards (#1429) chore(deps): update golangci/golangci-lint-action action to v9 (#1431) chore(deps): update golang:1.25.4 docker digest to 6ca9eb0 (#1432) chore(deps): update kibana-openapi-spec digest to 96ffcd7 (#1430) chore(deps): update kibana-openapi-spec digest to a8e3b64 (#1428) chore(deps): update golang docker tag to v1.25.4 (#1423) Update the system_user acceptance tests to utilise the config directory pattern (#1404) Add an issue template for covering new stack features (#1418) chore(deps): update golang:1.25.3 docker digest to 6d4e5e7 (#1421) Update provider description (#1405) chore(deps): update golang:1.25.3 docker digest to 0afe9b5 (#1416) chore(deps): update module github.com/golangci/golangci-lint to v2.6.1 (#1417) Added space awareness to Agent Policies (#1390) Fix data view color field format params not computed in Terraform state (#1414) ...
…4f89-4206-9e30-02dc9febbc48 * origin/main: (45 commits) Add resource to manage ML datafeed state (#1422) Extract agent policy test config (#1439) chore(deps): update golang:1.25.4 docker digest to e68f6a0 (#1437) chore(deps): update kibana-openapi-spec digest to 6a867d8 (#1435) chore(deps): update docker.elastic.co/kibana/kibana docker tag to v9.2.1 (#1434) chore(deps): update docker.elastic.co/elasticsearch/elasticsearch docker tag to v9.2.1 (#1433) First take at documented coding standards (#1429) chore(deps): update golangci/golangci-lint-action action to v9 (#1431) chore(deps): update golang:1.25.4 docker digest to 6ca9eb0 (#1432) chore(deps): update kibana-openapi-spec digest to 96ffcd7 (#1430) chore(deps): update kibana-openapi-spec digest to a8e3b64 (#1428) chore(deps): update golang docker tag to v1.25.4 (#1423) Update the system_user acceptance tests to utilise the config directory pattern (#1404) Add an issue template for covering new stack features (#1418) chore(deps): update golang:1.25.3 docker digest to 6d4e5e7 (#1421) Update provider description (#1405) chore(deps): update golang:1.25.3 docker digest to 0afe9b5 (#1416) chore(deps): update module github.com/golangci/golangci-lint to v2.6.1 (#1417) Added space awareness to Agent Policies (#1390) Fix data view color field format params not computed in Terraform state (#1414) ...
Fixes #813