search

kube-aws / kube-spot-termination-notice-handler

A Kubernetes DaemonSet to gracefully delete pods 2 minutes before an EC2 Spot Instance gets terminated
Apache License 2.0
378 stars 77 forks source link

Added gmail notification handler #21

Closed omerxx closed 5 years ago

omerxx commented 5 years ago

Hi,

I added a gmail handler and I'll open a PR to the stable/helm repo to make use of the same parameters. I also ran all the shellcheck.net rejections and fixed basic bash stuff.

Hope you'll like it.

omerxx commented 5 years ago

@egeland @mumoshu Can you guys check this out?

mumoshu commented 5 years ago

@omerxx Hey! Thanks for the PR!

I don't have an immediate test environment for this feature, so I appreciate your help. Did you get a chance to verify this to work in a real cluster?

omerxx commented 5 years ago

Hi @mumoshu, yes I've created it for my company and we're now testing in production. I also have changes ready for helm/stable which we use to install this and use the gmail handler. I'll create a PR form them once this one is merged.

Thanks!

mumoshu commented 5 years ago

Ah, now merging is blocked due to unsigned commits. Would you mind signing according to https://help.github.com/articles/about-gpg/ ?

omerxx commented 5 years ago

@mumoshu I will, and also checking the questions by @Multiply

omerxx commented 5 years ago

@mumoshu Squashed and signed my commits. About @Multiply I had another thinking and I actually find it ok. Any problem you see?

Multiply commented 5 years ago

@omerxx The output of the echo command is wrong, and is missing the hint about which variable is missing. Have you seen my comment above, with an explanation of what happens?

omerxx commented 5 years ago

@mumoshu Fixed @Multiply comment too.

omerxx commented 5 years ago

@mumoshu Can we merge?

mumoshu commented 5 years ago

@omerxx @Multiply Thanks for a lot for your supports ☺️

egeland commented 5 years ago

Nice feature addition. 👍

Would have preferred to see the linting stuff (shellcheck) in a separate PR to a new feature - please keep in mind for next time. 😁

omerxx commented 5 years ago

@egeland Understood! Thanks!