The handler sometimes doesn't cordon any node

[someone-stole-my-name]

Describe the bug

The getRollingUpdateTimestampsFromNode(node) conditions used throughout the code to check if a node should be cordoned or not have a flaw. If something or someone decides to stop the rolling update, and manually uncordon the nodes, the handler on next start will happily evict pods in that node without actually cordoning anything. It is specially problematic when using the eager cordoning feature, since it leads to an upgrade that can never end.

What do you see?

No response

What do you expect to see?

No response

List the steps that must be taken to reproduce this issue

1. Start the handler
2. Let it cordon a node
3. Stop the handler
4. Uncordon the node manually
5. Start the handler again

Version

No response

Additional information

No response

[zaafar]

Is there any workaround for this bug?

[someone-stole-my-name]

Is there any workaround for this bug?

Manually removing the annotations.

[zaafar]

Just to be clear, that wouldn't redo the EAGER_CORDONING, it will only redo the regular cordoning. Is there any hack/trick to redo the EAGER_CORDONING after this bug?

[TwiN]

Seems like this bug was introduced in #42. Versions prior to v1.7.0 didn't have the aws-eks-asg-rolling-update-handler.twin.sh/cordoned-at annotations, and I believe I had specifically avoided adding this annotation just in case something like this would happen.

I'll see if I can do something 🤔

[TwiN]

@zaafar @someone-stole-my-name Hey folks, I think #117 fixed this, but I'd appreciate if you could both give it a try.

As soon as https://github.com/TwiN/aws-eks-asg-rolling-update-handler/actions/runs/5172952729 finishes, it should be available with the latest tag (twinproduction/aws-eks-asg-rolling-update-handler:latest)

[zaafar]

sure, i am going to test it and get back to you.

[zaafar]

looks good to me.

[zaafar]

please whenever you get some time, make a release.

[TwiN]

Just released the fix in v1.8.1