Support restart task for StatefulSet with updateStrategy: OnDelete

[timothysmith0609]

Builds on #836 by handling the special case of a StatefulSet having updatStrategy: OnDelete. In this case, we need to find all pods with an owner reference to the targeted StatefulSet: this is accomplished using the UID of the StatefulSet.

More info on owner references can be found here.

[peiranliushop]

Need to bump krane version as as well.

[johnjmartin]

is record.metadata.uid something that identifies individual statefulsets?

[timothysmith0609]

It uniquely identifies the resource throughout the lifetime of the cluster. That is, no 2 resources in a cluster will ever have the same UID

[timothysmith0609]

CI is showing a flaky error that I want to get to the bottom of before merging this. It appears it can treat the restart as successful even if multiple replicas of the statefulset have not yet reached the desired level

[kgalieva]

Hey @timothysmith0609! Any updates on this PR?

[timothysmith0609]

I completely forgot this PR existed. I just rebased on latest master and will see what the state of that CI flakiness is

[timothysmith0609]

This is flaky due to transparently returning true for StatefulSet#deploy_succeeded?. Per c8cea8d, it seems reasonable to check the normal success criteria (that used for rollingUpdate strategy) even when deploying a stateful set with onDelete. We would still like to know if the underlying resources are healthy and that there are enough of them running, no?

[timothysmith0609]

@peiranliushop are you ok with the change I made in c8cea8d?

[johnjmartin]

We would still like to know if the underlying resources are healthy and that there are enough of them running, no?

Definitely 👍. We expect pods to come up healthy shortly after they're deleted in this particular deploy strategy

[timothysmith0609]

We expect pods to come up healthy shortly after they're deleted in this particular deploy strategy

This is a little nuanced, actually. When deploying, OnDelete means the underlying pods will not be deleted (this must be done manually by the user). In contrast, when we want to restart the pods (the feature being implemented, here), we need to wait for the underlying pods to restart, even if OnDelete is the rollout strategy.

[johnjmartin]

When deploying, OnDelete means the underlying pods will not be deleted (this must be done manually by the user). In contrast, when we want to restart the pods (the feature being implemented, here), we need to wait for the underlying pods to restart, even if OnDelete is the rollout strategy.

Ah yes. In our case, the way we plan to use the feature is to run a production-platform-next apply (which with OnDelete will not trigger a restart of our pods), then follow that with a production-platform-next restart. This way we can have the affect of restarting all of our StatefulSet pods in one smooth go on each deploy.

[timothysmith0609]

🤦 I forgot the pods would be named the same even after being deleted. I'll fix this tomorrow and get the tests passing before merging

[JamesOwenHall]

I forgot the pods would be named the same even after being deleted.

Ah true. You can probably just swap name for UID; that ought to fix it.