Delete old shutdowns before creating new ones

[pebrc]

Fixes #5628

Moves the cleanup at the beginning of the upgrade function to make sure we don't end up with left over shutdowns and then don't get to clean them up in time because we exit early after node deletions.

[pebrc]

run/e2e-tests tags=es

[pebrc]

I need to clean up the tests but I think the principle works. Just wanted to raise this already to run the e2e tests.

[pebrc]

Test failed due to an issue with the new pipeline. Will try to run manual tests for the time being.

[thbkrkr]

run/e2e-tests tags=es

[pebrc]

Test run of all es e2e test was successful. There is a more ambitious version of this PR that simply moves the maybeCompleteNodeUpgrades function to the same position as the call to nodeShutdown.Clear. It has the advantage of keeping the clean up code together. I am still testing this locally but the current change set would be a minimal fix IMO.

[naemono]

Running test locally, but can't see a scenario where clearing old shutdowns where the node is back in the cluster at the start of any upgrade loop would cause future problems.

[barkbay]

LGTM 👍
I did some manual testing while keeping an eye on /_nodes/shutdown to ensure that there was at most one shutdown record. Maybe this is something we could include in a test, ideally in a unit test or/and maybe along the continuous health check. (in another PR ofc)

[pebrc]

Thanks @barkbay and @naemono for the quick reviews!

Maybe this is something we could include in a test, ideally in a unit test or/and maybe along the continuous health check. (in another PR ofc)

Yes I think I am going to follow up on this PR with something a bit more comprehensive targeting the main branch/next release and will include improved e2e testing as well. I think your idea of an additional continuous health check is really good.

[pebrc]

💚 All backports created successfully

Status	Branch	Result
✅	2.2

Questions ?

Please refer to the Backport tool documentation