-
Notifications
You must be signed in to change notification settings - Fork 37
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
Adds reprovision API to support updating search pipelines, ingest pipelines index settings #804
Conversation
…param for RestCreateWorkflowAction, creates and registers Update Ingest/Search pipeline steps in WorkflowResources, registers update steps in WorkflowStepFactory Signed-off-by: Joshua Palis <[email protected]>
…tep, improved WorkflowProcessSorter.createReprovisionSequence Signed-off-by: Joshua Palis <[email protected]>
…fies updating resource created script to remove error if any Signed-off-by: Joshua Palis <[email protected]>
Signed-off-by: Joshua Palis <[email protected]>
Signed-off-by: Joshua Palis <[email protected]>
Signed-off-by: Joshua Palis <[email protected]>
Signed-off-by: Joshua Palis <[email protected]>
…rtAction tests. Adding check for reprovision without workflowID. Signed-off-by: Joshua Palis <[email protected]>
Signed-off-by: Joshua Palis <[email protected]>
Signed-off-by: Joshua Palis <[email protected]>
Signed-off-by: Joshua Palis <[email protected]>
Signed-off-by: Joshua Palis <[email protected]>
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.
Mostly LGTM with a few nits. Some blockers:
- WorkflowRequest serialization breaks BWC
- Possible race condition with resetting error field
src/main/java/org/opensearch/flowframework/common/WorkflowResources.java
Outdated
Show resolved
Hide resolved
src/main/java/org/opensearch/flowframework/common/WorkflowResources.java
Show resolved
Hide resolved
src/main/java/org/opensearch/flowframework/indices/FlowFrameworkIndicesHandler.java
Outdated
Show resolved
Hide resolved
src/main/java/org/opensearch/flowframework/transport/CreateWorkflowTransportAction.java
Outdated
Show resolved
Hide resolved
src/main/java/org/opensearch/flowframework/workflow/GetResourceStep.java
Outdated
Show resolved
Hide resolved
src/main/java/org/opensearch/flowframework/workflow/GetResourceStep.java
Outdated
Show resolved
Hide resolved
src/main/java/org/opensearch/flowframework/workflow/UpdateIndexStep.java
Outdated
Show resolved
Hide resolved
src/main/java/org/opensearch/flowframework/workflow/UpdateIndexStep.java
Outdated
Show resolved
Hide resolved
src/main/java/org/opensearch/flowframework/workflow/WorkflowStepFactory.java
Show resolved
Hide resolved
One initial question, we are calling this a reprovision API but we are adding a new query param to the create/update API. Should we just have a new api that is a reprovision API. The provision query param we have added to the create API was to combine both APIs into one but here we are juts adding a query param to the update API. |
On this example you give you say you are updating but you made a create_index step:
Was this a mistake or we are creating the index again when updating? |
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.
Will continue the review
src/main/java/org/opensearch/flowframework/common/WorkflowResources.java
Outdated
Show resolved
Hide resolved
src/main/java/org/opensearch/flowframework/indices/FlowFrameworkIndicesHandler.java
Outdated
Show resolved
Hide resolved
src/main/java/org/opensearch/flowframework/rest/RestCreateWorkflowAction.java
Show resolved
Hide resolved
Great question, it is intentional. The user will always only be submitting a template with create steps to preserve idempotency. This way a single template can be used in any cluster to produce the same configuration. In the case of reprovisioning, we determine which |
Signed-off-by: Joshua Palis <[email protected]>
src/main/java/org/opensearch/flowframework/rest/RestCreateWorkflowAction.java
Show resolved
Hide resolved
src/main/java/org/opensearch/flowframework/transport/CreateWorkflowTransportAction.java
Show resolved
Hide resolved
src/main/java/org/opensearch/flowframework/transport/ReprovisionWorkflowRequest.java
Show resolved
Hide resolved
src/main/java/org/opensearch/flowframework/workflow/UpdateIndexStep.java
Outdated
Show resolved
Hide resolved
Signed-off-by: Joshua Palis <[email protected]>
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.
Great work in implementing such design @joshpalis .
src/main/java/org/opensearch/flowframework/workflow/UpdateIndexStep.java
Outdated
Show resolved
Hide resolved
src/main/java/org/opensearch/flowframework/workflow/WorkflowProcessSorter.java
Show resolved
Hide resolved
src/main/java/org/opensearch/flowframework/workflow/WorkflowProcessSorter.java
Outdated
Show resolved
Hide resolved
Signed-off-by: Joshua Palis <[email protected]>
…or field Signed-off-by: Joshua Palis <[email protected]>
Signed-off-by: Joshua Palis <[email protected]>
Signed-off-by: Joshua Palis <[email protected]>
Signed-off-by: Joshua Palis <[email protected]>
Signed-off-by: Joshua Palis <[email protected]>
src/main/java/org/opensearch/flowframework/workflow/UpdateIndexStep.java
Outdated
Show resolved
Hide resolved
Signed-off-by: Joshua Palis <[email protected]>
…ests Signed-off-by: Joshua Palis <[email protected]>
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.
LGTM with one small change needed.
src/main/java/org/opensearch/flowframework/transport/WorkflowRequest.java
Outdated
Show resolved
Hide resolved
Signed-off-by: Joshua Palis <[email protected]>
Signed-off-by: Joshua Palis <[email protected]>
Signed-off-by: Joshua Palis <[email protected]>
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.
LGTM
…elines index settings (#804) * Initial commit, Adds ReprovisionWorkflowTransportAction, reprovision param for RestCreateWorkflowAction, creates and registers Update Ingest/Search pipeline steps in WorkflowResources, registers update steps in WorkflowStepFactory Signed-off-by: Joshua Palis <[email protected]> * Initial reprovisiontransportaction implementation, Added UpdateIndexStep, improved WorkflowProcessSorter.createReprovisionSequence Signed-off-by: Joshua Palis <[email protected]> * Implements Update index Step to support updating index settings, modifies updating resource created script to remove error if any Signed-off-by: Joshua Palis <[email protected]> * Improves workflow node comparision Signed-off-by: Joshua Palis <[email protected]> * Adding comments Signed-off-by: Joshua Palis <[email protected]> * Fixing tests, adding javadocs Signed-off-by: Joshua Palis <[email protected]> * Adding changelog Signed-off-by: Joshua Palis <[email protected]> * Updating parse utils, RestCreateWorkflowAction, CreateWorkflowTransportAction tests. Adding check for reprovision without workflowID. Signed-off-by: Joshua Palis <[email protected]> * Adding update step and get resource step tests Signed-off-by: Joshua Palis <[email protected]> * Adding check for filtered setting list size Signed-off-by: Joshua Palis <[email protected]> * Addign reprovision workflow transport action tests Signed-off-by: Joshua Palis <[email protected]> * Adding tests for reprovision sequence creation Signed-off-by: Joshua Palis <[email protected]> * Addressing comments Signed-off-by: Joshua Palis <[email protected]> * Changing GetResourceStep to WorkflowDataStep Signed-off-by: Joshua Palis <[email protected]> * Addressing PR comments Signed-off-by: Joshua Palis <[email protected]> * Fixing state check for reprovision transport action Signed-off-by: Joshua Palis <[email protected]> * Adding state eror check to reprovision transport action to remove error field Signed-off-by: Joshua Palis <[email protected]> * removing error check from flowframeworkindices handler Signed-off-by: Joshua Palis <[email protected]> * Adding check for no updated settings Signed-off-by: Joshua Palis <[email protected]> * refactor reprovision sequence creation Signed-off-by: Joshua Palis <[email protected]> * Fixing workflowrequest serialization Signed-off-by: Joshua Palis <[email protected]> * Addressing PR comments Signed-off-by: Joshua Palis <[email protected]> * Moving flattenSettings method to ParseUtils, added flatten settings tests Signed-off-by: Joshua Palis <[email protected]> * updating workflowrequest Signed-off-by: Joshua Palis <[email protected]> * fixing workflowrequest Signed-off-by: Joshua Palis <[email protected]> * spotlessApply Signed-off-by: Joshua Palis <[email protected]> --------- Signed-off-by: Joshua Palis <[email protected]> (cherry picked from commit 48c7019) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
…nes, ingest pipelines index settings (#824) Adds reprovision API to support updating search pipelines, ingest pipelines index settings (#804) * Initial commit, Adds ReprovisionWorkflowTransportAction, reprovision param for RestCreateWorkflowAction, creates and registers Update Ingest/Search pipeline steps in WorkflowResources, registers update steps in WorkflowStepFactory * Initial reprovisiontransportaction implementation, Added UpdateIndexStep, improved WorkflowProcessSorter.createReprovisionSequence * Implements Update index Step to support updating index settings, modifies updating resource created script to remove error if any * Improves workflow node comparision * Adding comments * Fixing tests, adding javadocs * Adding changelog * Updating parse utils, RestCreateWorkflowAction, CreateWorkflowTransportAction tests. Adding check for reprovision without workflowID. * Adding update step and get resource step tests * Adding check for filtered setting list size * Addign reprovision workflow transport action tests * Adding tests for reprovision sequence creation * Addressing comments * Changing GetResourceStep to WorkflowDataStep * Addressing PR comments * Fixing state check for reprovision transport action * Adding state eror check to reprovision transport action to remove error field * removing error check from flowframeworkindices handler * Adding check for no updated settings * refactor reprovision sequence creation * Fixing workflowrequest serialization * Addressing PR comments * Moving flattenSettings method to ParseUtils, added flatten settings tests * updating workflowrequest * fixing workflowrequest * spotlessApply --------- (cherry picked from commit 48c7019) Signed-off-by: Joshua Palis <[email protected]> Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Description
Adds fine-grained provisioning API (reprovision) to allow users to iteratively update already provisioned templates without having to first deprovision their created resources. The diagram below illustrates the execution flow of a reprovision request.
URI
PUT /_plugins/_flow_framework/workflow/<workflow_id>?reprovision=true
Capabilities
reprovision=true
parameterSupported Update Steps
Note : Attempts to update non-supported steps will result in an error message
Examples :
Iteratively adding new nodes and updating nodes
default_pipeline
The workflow state is updated for each new resource added :
default_pipeline
. Index settings can be flattened or expandedRetrieving the index shows the change has been applied to the index settings :
Adding new predecessor nodes to existing nodes (Mid-node addition)
Default pipeline is attached to index :
Resources created :
Related Issues
Part of #717
Check List
--signoff
.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.