fix(notready): condition failed

[redref]

Reworked condition around "NotReady" node.

Workflow :

• Node goes to NotReady
• At some point, pod is deleted by apiserver as node is NotReady for a long time
• PVC got delete and reschedule as skipLv cannot find matching POD even if annotation is not there

Should not impact any other usecase. Only default when pod is not found.

[redref]

/assign @antmoveh

[antmoveh]

Your design modifies the conditions under which pods are rebuilt. Only those with special annotations are not allowed to migrate.

https://github.com/carina-io/carina/blob/main/docs/manual_zh/failover.md#%E8%8A%82%E7%82%B9notready%E5%AE%B9%E5%99%A8%E8%BF%81%E7%A7%BB

This can cause some unnecessary trouble, such as node Notready, Pod migration pvc is rebuilt data loss

In most cases, data migration is performed in the database master/slave cluster

Data loss occurs when a single-master database is migrated

[redref]

Your design modifies the conditions under which pods are rebuilt. Only those with special annotations are not allowed to migrate.

https://github.com/carina-io/carina/blob/main/docs/manual_zh/failover.md#%E8%8A%82%E7%82%B9notready%E5%AE%B9%E5%99%A8%E8%BF%81%E7%A7%BB

This can cause some unnecessary trouble, such as node Notready, Pod migration pvc is rebuilt data loss

In most cases, data migration is performed in the database master/slave cluster

Data loss occurs when a single-master database is migrated

Hello @antmoveh ,

My understanding is on the contrary, it make the default to skip if error or annotation not found.

Here the call where we skip clearPod and LV clearing. Return true or error will not take any action, and keep Volume where it is according to function naming "skipLv". https://github.com/carina-io/carina/blob/main/controllers
/node_controller.go#L196`

Now my modifications :

• https://github.com/carina-io/carina/pull/197/files#diff-cd5408109e3244decb24045e493a573fb2202876abd89bbb52a4f86646c16143R361 should not have any impact as err is already not null
• https://github.com/carina-io/carina/pull/197/files#diff-cd5408109e3244decb24045e493a573fb2202876abd89bbb52a4f86646c16143R381 effectively fix the default behavior in case we do not find the pod in the pod.items list (my real world use case triggering this MR, I was having data loss with node NotReady and pod stopped)
• https://github.com/carina-io/carina/pull/197/files#diff-cd5408109e3244decb24045e493a573fb2202876abd89bbb52a4f86646c16143R376 handle the migration behavior when annotation is not false

I might fix the last line to be triggered only when annotation is true (instead of not false), but to my understanding, other lines are fine and fix behavior.

Thanks for reading and please advise in case I missed something.

[antmoveh]

/approve

[antmoveh]

/lgtm

[carina-ci-bot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: antmoveh, redref

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [antmoveh]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment