ELB IP changes can bring the cluster down

[danielfm]

I ran into kubernetes/kubernetes#41916 twice in the last 3 days in my production cluster, with almost 50% of worker nodes transitioning to NotReady state almost simultaneously in both days, causing a brief downtime in critical services due to Kubernetes default (and agressive) eviction policy for failing nodes.

I just contacted AWS support to validate the hypothesis of the ELB changing IPs at the time of both incidents, and the answer was yes.

My configuration (multi-node control plane with ELB) matches exactly the one in that issue, and probably most kube-aws users are subject to this.

Have anyone else ran into this at some point?

[mumoshu]

@danielfm Thanks for the report.

I think I have never encountered the issue myself for my production cluster but anyway, I believe this is a BIG issue.

For me, the issue seems to be composed of two parts: one is a long TTL other than ELB's(for example a CNAME record's TTL in kube-aws, which points to ELB's DNS name) and the another is an issue in ELB and/or kubelet's which prevents kubelet to detect broken connection to apiservers.

Is my assumption correct?

Anyway, once recordSetTTL in cluster.yaml is set to considerably lower than 60s(=ELB's default TTL), kubelet should detect any dead long-polling connection to one of API servers (via an ELB CNAME -> ELB instances) and then re-discover ELB instances via DNS and reconnect to one of them.

However I'm happy to work-around the issue in kube-aws.

Possible work-arounds

Perhaps:

• periodically monitoring the list of ips an route53 alias record for an ELB and then
• restarting kubelet whenever the monitor detects the list to be changed

so that kubelet can reconnect to one of active ELB ips before it marks the k8s node NotReady?

If we go that way, the monitor should be executed periodically with an interval of (<node-monitor-grace-period> - <amount of time required for kubelet to start> - <amount of time required for each monitor run>) / <a heuristic value: N> - <amount of time required to report node status after kubelet started> at least, so that we can provide kubelet at most N chances to successfully report its status to possibly available ELB.

Or we can implement a DNS round robin with a health-checking mechanism for serving k8s API endpoints like suggested and described in #281 #373

[mumoshu]

This hash changes only when backend IPs of an ELB has been changed.

$ dig my.k8s.cluster.example.com +noall +answer +short | sort | sha256sum | awk '{print $1}'
96ff696e9a0845dd56b1764dd3cfaa1426cdfbd7510bab64983e97268f8e0bc4

$ dig my.k8s.cluster.example.com +noall +answer +short | sort
52.192.165.96
52.199.129.34
<elb id>.ap-northeast-1.elb.amazonaws.com.

[mumoshu]

@danielfm I guess setting recordSetTTL in cluster.yaml(which sets the TTL of a CNAME record associated to a controller ELB) to be a value considerably lower than that of ELB's DNS A records TTL(=60sec) and kubelet's --node-monitor-grace-period and running the following service would alleviate the issue.

/etc/systemd/system/elb-ip-change-watcher.service:

[Service]
Type=notify
Restart=on-failure
Environment=API_ENDPOINT_DNS_NAME={{.APIEndpoint.DNSName}}
ExecStart=/opt/bin/elb-ip-change-watcher

/opt/bin/elb-ip-change-watcher:

#!/usr/bin/env bash

set -vxe

current_elb_backends_version() {
  dig ${API_ENDPOINT_DNS_NAME:?Missing required env var API_ENDPOINT_DNS_NAME} +noall +answer +short | \
    # take into account only ips even if dig returned a CNAME answer(when API_ENDPOINT_DNS_NAME is a CNAME rather than an A(or Route 53's "Alias") record
    grep -o '[0-9]\{1,3\}\.[0-9]\{1,3\}\.[0-9]\{1,3\}\.[0-9]\{1,3\}' | \
    # sort IPs so that DNS round-robin doesn't unexpectedly trigger a kubelet restart
    sort | \
    sha256sum | \
    # sha256sum returns outputs like "<sha256 hash value> -". We only need the hash value excluding the trailing hyphen
    awk '{print $1}'
}

run_once() {
  local file=$ELB_BACKENDS_VERSION_FILE
  prev_ver=$(cat $file || echo)
  current_ver=$(current_elb_backends_version)
  echo comparing the previous version "$prev_ver" and the current version "$current_ver"
  if [ "$prev_ver" == "" -o "$prev_ver" == "$current_ver" ]; then
    echo the version has not changed. no need to restart kubelet.
    if [ "$KUBELET_RESTART_STRATEGY" == "watchdog" ]; then
      echo "notifying kubelet's watchdog not to trigger a restart of kubelet..."
      local kubelet_pid
      kubelet_pid=$(systemctl show $KUBELET_SYSTEMD_UNIT_NAME -p MainPID | cut -d '=' -f 2)
      systemd-notify --pid=$kubelet_pid WATCHDOG=1
    fi
  else
    echo the version has been changed. need to restart kubelet.
    if [ "$KUBELET_RESTART_STRATEGY" == "systemctl" ]; then
      systemctl restart $KUBELET_SYSTEMD_UNIT_NAME
    fi
  fi
  echo writing $current_ver to $file
  echo "$current_ver" > $file
}

ELB_BACKENDS_VERSION_FILE=${ELB_BACKENDS_VERSION_FILE:-/var/run/coreos/elb-backends-version}
KUBELET_SYSTEMD_UNIT_NAME=${KUBELET_SYSTEMD_UNIT_NAME:-kubelet.service}
KUBELET_RESTART_STRATEGY=${KUBELET_RESTART_STRATEGY:-systemctl}
WATCH_INTERVAL_SEC=${WATCH_INTERVAL_SEC:-3}

systemd-notify --ready
while true; do
  systemd-notify --status "determining if there're changes in elb ips"
  run_once
  systemd-notify --status "sleeping for $WATCH_INTERVAL_SEC seconds"
  sleep $WATCH_INTERVAL_SEC
done

[mumoshu]

Also - I have never realized that but we're creating CNAME records - which has its own TTL(=300 seconds by default in kube-aws) other than ELB's A records' TTL - for controller ELBs.
https://github.com/kubernetes-incubator/kube-aws/blob/master/core/controlplane/config/templates/stack-template.json#L778
We'd better make them Route53 Alias records so that there will be only one TTL.

[mumoshu]

Updated my assumption on the issue in my first comment #598 (comment)

[mumoshu]

Hmm, a TTL for a CNAME record associated to ELB's DNS name seems to be capped at 60s, even though kube-aws' default recordSetTTL is set to 300s.
Then, we won't need to care about the balance of two TTLs(kube-aws CNAME and ELB's A) that much.

[mumoshu]

Anyway, to forcibly restart kubelet to reconnect apiserver(s) when necessary(=elb ips changed) would still be important.
The script and the systemd unit I've suggested in #598 (comment) would be useful to achieve it.

[redbaron]

@mumoshu does it mean that going ELB-less mode with just CNAME record in Route53 pointing to controller nodes is considered dangerous now?

[mumoshu]

@redbaron Do you mean a single CNAME record for your k8s API endpoint which points to just one controller node's DNS name?

I've been assuming that you would have a DNS name associated to one or more A records(rather than CNAME) each is associated to one of controller nodes public/private IP if you'd go without an ELB.

[redbaron]

Doesn't change the fact that final set of A records returned from DNS request change when you trigger controllers ASG update, right? From what I read here it matches the case when ELBs change their IPs

[mumoshu]

@redbaron Ah, so you have a CNAME record which points to a DNS record containing multiple A records each is associated to one of controller nodes, right?

If so, no, as long as you have low TTLs for your DNS records.

AFAIK, someone in the upstream issue said that it becomes an issue only when ELB is involved. Perhaps ELB doesn't immediately shutdown an unnecessary instance and doesn't send FIN? If that's the case, kubelet would be unable to detect broken connections immediately.

When your controllers ASG is updated, old, unnecessary nodes would be terminated before it becomes non-functional like ELB's instances.

So your ELB-less mode with CNAME+A records would be safe as long as you have health checks to update the route 53 record set to eventually return A records "only for healthy controller nodes", and you have a TTL lower than --node-monitor-grace-period

[redbaron]

ELB-less mode I refer to is a recent feature in kube-aws, I didn't check which records it creates exactly, just wanted to verify that is still a safe option to do considering this bug report.

[mumoshu]

@redbaron If you're referring to the DNS round-robin for API endpoints, it isn't implemented yet.

[redbaron]

I wonder if using ALB can help here. L7 load balancer precisely knows all incoming/outgoing requests and can forcibly yet safely close HTTP connection when ELB scales/changes IP addresses.

[mumoshu]

@redbaron Thanks, I never realized that ALBs may help. I wish they can help, too.
Although I'd like to hear from contributors in the upstream issue to kindly clarify whether ALBs help or not,
shall we take this opportunity to add support for ALBs into kube-aws anyway? 😃