Add deleteLocalData flag, basic logging, and cluster-autoscaler annotation

[eterna2]

Quick workaround for #19 #21.

Originally, wanted to refine the fix abit (use proper logging, better handle the node annotation, etc), but I just couldn't find the time, so here is the PR - not ideal, but it is working in my cluster.

Changes

• add deleteLocalData flag for node draining because some of my pods uses the local data volume mount
• add some really basic logging because I needed them to debug and monitor aws-asg-roller state
• add basic func to set node annotation in k8s to prevent cluster-autoscaler to scale down the nodes, and removing the annotations when update is done

Important Note

summary of how annotation is handled:

• if asg needs update, annotate all old nodes with flag to prevent scaling down
• if asg does not need update, remove flag from all nodes in asg

Hence if aws-asg-roller is improperly killed or removed without restart or redeployment, the old instances will be annotated with the no scale down flag!

[deitch]

Thanks @eterna2 ; looking right now...

[deitch]

Overall pretty good, thank you for doing this. As you said, nice and quick, but that doesn't make it bad. The logging could be better overall, but again, not something to fix in this PR.

I made a minor suggestion or two, a question, and could you please add documentation to the README:

• the new env var supported and why
• how the interaction between CAPI and this work

Thanks!

[deitch]

Also, should rebase, as I just switched it over to GitHub actions.

[eterna2]

@deitch

Updated as requested.

Just to add:
There is still a issue somewhere. Most likely a logical issue, haven't got time to properly think it over. U might want to think if u have time.

Because based on my observations, depending on the timing of the check, u might end up with a desired == original when needs update > 0 and no new nodes are in progress of creation.

This results in an infinite loop, cuz no new nodes will be created, and no nodes will be terminated.

I suspect some count somewhere went wrong, when cluster-autoscaler creates even more nodes due to demand, or when some nodes are in progress of being scaled-down when the check is been done.

I have to restart aws-asg-roller to get the update running again.

[eterna2]

Forgot to add. I made some additional changes. Changed ROLLER_DELETE_LOCAL_DATA default to false to maintain backward compatibility (instead of true which I am currently using for my cluster).

[deitch]

This looks pretty good. Thanks for the updates. Ready to merge it in?

There is still a issue somewhere. Most likely a logical issue, haven't got time to properly think it over.

Does this block us?

[eterna2]

Nope. It is working but might fail during edge cases.

Will require more work.

Issue is not directly related to the new features. More like this PR is unable to completely resolve all the issues between aws-asg-roller and cluster-autoscaler, because the current update strategy did not account of asg modifications by cluster autoscaler.

[deitch]

OK, I will hold off then.

[eterna2]

Erm. I won't be working on it for the moment thou.

What I mean is that this PR

• adds the support for localData flag for node draining, and
• solve the issue of cluster-autoscaler spinning down nodes raised by aws-asg-roller.

I don't I have the bandwidth to completely solve the edge cases with CAS. And shld be done in a separate PR by someone?

[deitch]

I don't I have the bandwidth to completely solve the edge cases with CAS

Heh, me neither for now. Hoping to have more OSS time by end of year. I am bogged down on diskfs as well. I really need extra days in my week.

And should be done in a separate PR by someone?

I am fine with that. If you are comfortable that this PR is an improvement over current state, without solving everything, I am fine merging it in and then treating that as a separate issue.

Your thoughts?

[eterna2]

Yup, we can do that. Raise an issue on that.

[deitch]

Done. Now we find out if the Github Actions deploy process to docker hub actually works...

[eterna2]

I just had a great idea on a quick workaround.

Cuz the issue I observe in production is that the loop get stuck in an infinite loop becuz the desired and current is equal and yet there are no new pending nodes.

And how I workaround this in production is to monitor and restart the aws-asg-roller pod when this happens.

We could probably fix this by having a check for this condition:

If (desired == original) and (num_ready_nodes = desired) and (num_old_node > 0) and (time since last increase > x minutes) then (++desired)

What do u think?

[deitch]

I think I don't properly understood the problem. :-)

Do you mind opening a new issue for this with a fresh explanation? And then your proposed workaround as a separate comment?