Skip to content

Support "dry run" mode for updating Desired Nodes#88305

Merged
arteam merged 26 commits intoelastic:mainfrom
arteam:desired-nodes-dry-run
Jul 26, 2022
Merged

Support "dry run" mode for updating Desired Nodes#88305
arteam merged 26 commits intoelastic:mainfrom
arteam:desired-nodes-dry-run

Conversation

@arteam
Copy link
Copy Markdown
Contributor

@arteam arteam commented Jul 6, 2022 •
edited
Loading

Add the dry_run query parameter to support simulating of updating of desired nodes. The update request will be validated, but no cluster state updates will be performed. In order to indicate that the response was a result of a dry run, we add the dry_run run field to the JSON representation of a response.

See #82975

@arteam arteam force-pushed the desired-nodes-dry-run branch 4 times, most recently from b3eacbe to 198a5f9 Compare July 13, 2022 08:55
@arteam arteam force-pushed the desired-nodes-dry-run branch from 198a5f9 to c611576 Compare July 13, 2022 14:29
@arteam arteam changed the title Support dry run mode for updating Desired Nodes Support a dry run mode for updating Desired Nodes Jul 13, 2022
@arteam arteam changed the title Support a dry run mode for updating Desired Nodes Support "dry run" mode for updating Desired Nodes Jul 18, 2022
@arteam arteam added :Distributed/Autoscaling Automatically adding or removing nodes in a cluster :Distributed/Distributed A catch all label for anything in the Distributed Area. Please avoid if you can. >enhancement and removed :Distributed/Autoscaling Automatically adding or removing nodes in a cluster labels Jul 18, 2022
@elasticsearchmachine
Copy link
Copy Markdown
Collaborator

elasticsearchmachine commented Jul 18, 2022

Hi @arteam, I've created a changelog YAML for you.

@arteam
Copy link
Copy Markdown
Contributor Author

arteam commented Jul 18, 2022

@elasticmachine run elasticsearch-ci/part-2

@arteam arteam marked this pull request as ready for review July 18, 2022 19:02
@elasticmachine elasticmachine added the Team:Distributed Meta label for distributed team. label Jul 18, 2022
@elasticmachine
Copy link
Copy Markdown
Collaborator

elasticmachine commented Jul 18, 2022

Pinging @elastic/es-distributed (Team:Distributed)

@arteam arteam requested a review from fcofdez July 18, 2022 19:02
@elasticsearchmachine elasticsearchmachine removed the Team:Clients Meta label for clients team label Jul 22, 2022
@arteam arteam force-pushed the desired-nodes-dry-run branch 2 times, most recently from 6e8e939 to 4932772 Compare July 22, 2022 11:39
@arteam
Copy link
Copy Markdown
Contributor Author

arteam commented Jul 22, 2022

@elasticmachine update branch

@elasticsearchmachine elasticsearchmachine changed the base branch from master to main July 22, 2022 23:05
@arteam
Copy link
Copy Markdown
Contributor Author

arteam commented Jul 25, 2022

@elasticmachine update branch

@arteam arteam requested a review from fcofdez July 25, 2022 08:10
fcofdez
fcofdez reviewed Jul 25, 2022
View reviewed changes
Copy link
Copy Markdown
Contributor

@fcofdez fcofdez left a comment

Choose a reason for hiding this comment

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

Looks good, I left a minor comment about the documentation.

docs/reference/cluster/update-desired-nodes.asciidoc Outdated
able to take better decisions.

It's possible to run the update in "dry run" mode by adding the
`?dry_run` query parameter, or by passing `"dry_run": true` in the request
Copy link
Copy Markdown
Contributor

@fcofdez fcofdez Jul 25, 2022

Choose a reason for hiding this comment

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

or by passing "dry_run": true in the request body

I think this is incorrect, we only support the URI query parameter and I would prefer to keep it that way.

Copy link
Copy Markdown
Contributor Author

@arteam arteam Jul 25, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Good catch! To be honest, we accept it in both forms I thought that we accept it in both forms.

fcofdez
fcofdez approved these changes Jul 25, 2022
View reviewed changes
Copy link
Copy Markdown
Contributor

@fcofdez fcofdez left a comment

Choose a reason for hiding this comment

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

LGTM, there's an assertion missing but you can address it and merge without another review round. Thanks! 👍

@arteam
Add additional get desired nodes assertion
762bcc1 
Verify that the cluster state hasn't been updated
@arteam
Copy link
Copy Markdown
Contributor Author

arteam commented Jul 25, 2022

@elasticmachine run elasticsearch-ci/part-2

@arteam arteam merged commit 72a6fdc into elastic:main Jul 26, 2022
@arteam
Copy link
Copy Markdown
Contributor Author

arteam commented Jul 26, 2022

Thank you @fcofdez!

@arteam arteam deleted the desired-nodes-dry-run branch July 26, 2022 07:03
@arteam arteam mentioned this pull request Jul 26, 2022
Merged
@fcofdez fcofdez mentioned this pull request Jul 28, 2022
Closed
@arteam arteam linked an issue Jul 28, 2022 that may be closed by this pull request
Add the ability to run the Desired Nodes API in dry mode #87781
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@fcofdez fcofdez fcofdez approved these changes

Assignees

No one assigned

Labels

:Distributed/Distributed A catch all label for anything in the Distributed Area. Please avoid if you can. >enhancement Team:Distributed Meta label for distributed team. v8.4.0

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Add the ability to run the Desired Nodes API in dry mode

5 participants

@arteam @elasticsearchmachine @elasticmachine @fcofdez @sethmlarson