Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

simple client library that implements leader election for a service #16830

Merged
merged 1 commit into from Dec 8, 2015
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
281 changes: 281 additions & 0 deletions pkg/client/leaderelection/leaderelection.go
@@ -0,0 +1,281 @@
/*
Copyright 2015 The Kubernetes Authors All rights reserved.

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

// Package leaderelection implements leader election of a set of endpoints.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should probably denote that the life of this library is subject to change and should be leveraged only by certain components.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SGTM

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

// It uses an annotation in the endpoints object to store the record of the
// election state.
//
// This implementation does not guarantee that only one client is acting as a
// leader (a.k.a. fencing). A client observes timestamps captured locally to
// infer the state of the leader election. Thus the implementation is tolerant
// to arbitrary clock skew, but is not tolerant to arbitrary clock skew rate.
//
// However the level of tolerance to skew rate can be configured by setting
// RenewDeadline and LeaseDuration appropriately. The tolerance expressed as a
// maximum tolerated ratio of time passed on the fastest node to time passed on
// the slowest node can be approximately achieved with a configuration that sets
// the same ratio of LeaseDuration to RenewDeadline. For example if a user wanted
// to tolerate some nodes progressing forward in time twice as fast as other nodes,
// the user could set LeaseDuration to 60 seconds and RenewDeadline to 30 seconds.
//
// While not required, some method of clock synchronization between nodes in the
// cluster is highly recommended. It's important to keep in mind when configuring
// this client that the tolerance to skew rate varies inversely to master
// availability.
//
// Larger clusters often have a more lenient SLA for API latency. This should be
// taken into account when configuring the client. The rate of leader transistions
// should be monitored and RetryPeriod and LeaseDuration should be increased
// until the rate is stable and acceptably low. It's important to keep in mind
// when configuring this client that the tolerance to API latency varies inversely
// to master availability.
//
// DISCLAIMER: this is an alpha API. This library will likely change significantly
// or even be removed entirely in subsequent releases. Depend on this API at
// your own risk.

package leaderelection

import (
"encoding/json"
"fmt"
"reflect"
"time"

"github.com/golang/glog"

"k8s.io/kubernetes/pkg/api"
"k8s.io/kubernetes/pkg/api/errors"
"k8s.io/kubernetes/pkg/api/unversioned"
"k8s.io/kubernetes/pkg/client/record"
client "k8s.io/kubernetes/pkg/client/unversioned"
"k8s.io/kubernetes/pkg/util"
"k8s.io/kubernetes/pkg/util/wait"
)

const (
JitterFactor = 1.2

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove blank.

LeaderElectionRecordAnnotationKey = "control-plane.alpha.kubernetes.io/leader"
)

// NewLeadereElector creates a LeaderElector from a LeaderElecitionConfig
func NewLeaderElector(lec LeaderElectionConfig) (*LeaderElector, error) {
if lec.LeaseDuration <= lec.RenewDeadline {
return nil, fmt.Errorf("leaseDuration must be greater than renewDeadline")
}
if lec.RenewDeadline <= time.Duration(JitterFactor*float64(lec.RetryPeriod)) {
return nil, fmt.Errorf("renewDeadline must be greater than retryPeriod*JitterFactor")
}
if lec.Client == nil {
return nil, fmt.Errorf("Client must not be nil.")
}
if lec.EventRecorder == nil {
return nil, fmt.Errorf("EventRecorder must not be nil.")
}
return &LeaderElector{
config: lec,
}, nil
}

type LeaderElectionConfig struct {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These fields deserve comments.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

// EndpointsMeta should contain a Name and a Namespace of an
// Endpoints object that the LeaderElector will attempt to lead.
EndpointsMeta api.ObjectMeta
// Identity is a unique identifier of the leader elector.
Identity string

Client client.Interface
EventRecorder record.EventRecorder

// LeaseDuration is the duration that non-leader candidates will
// wait to force acquire leadership. This is measured against time of
// last observed ack.
LeaseDuration time.Duration
// RenewDeadline is the duration that the acting master will retry
// refreshing leadership before giving up.
RenewDeadline time.Duration
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: would this be better named RenewPeriod? it's not really a deadline (timestamp)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is a deadline. If the LeaderElector is unable to renew within the deadline, it fails and gives up it's leader slot. Can you explain what you mean a bit more?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"deadline", to me, identifies a specific point in time. this duration doesn't do that, it only defines a the size of a time window. not a big deal, just think that Deadline suffix is somewhat misleading.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We use deadline in the API in the same way that we use it here. The type (duration as opposed to time) should signify the meaning.

https://github.com/kubernetes/kubernetes/blob/master/pkg/api/types.go#L988

https://github.com/kubernetes/kubernetes/blob/master/docs/devel/api-conventions.md#naming-conventions

// RetryPeriod is the duration the LeaderElector clients should wait
// between tries of actions.
RetryPeriod time.Duration

// Callbacks are callbacks that are triggered during certain lifecycle
// events of the LeaderElector
Callbacks LeaderCallbacks
}

// LeaderCallbacks are callbacks that are triggered during certain
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like these may be invoked concurrently. Can we document that fact?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They are invoked asynchronously

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

// lifecycle events of the LeaderElector. These are invoked asynchronously.
//
// possible future callbacks:
// * OnChallenge()
// * OnNewLeader()
type LeaderCallbacks struct {
// OnStartedLeading is called when a LeaderElector client starts leading
OnStartedLeading func(stop <-chan struct{})
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is there a reason for a struct in the channel, it looks like it's only signaling as if it were a bool.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ahh nvmd looks like a constraint of the until functions.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We mostly use a struct chan for stop signal

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how does closure of the stop chan here differ from OnStoppedLeading? can we document what it's actually signalling?

// OnStoppedLeading is called when a LeaderElector client stops leading
OnStoppedLeading func()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should be default to fatal honestly, as you enter into unsafe territory if a controller tries to do something during a rectification loop.

Is there a legit reason you would not want to fail immediately?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. A client might want to execute a shutdown routine or even transition to a candidate. It should be the responsibility of the library user to decide what happens here

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In leader election "fail fast" applies because you widen the window for a split brain. Optimistic concurrency can't save you if your controllers are in flux.

I think the recommendation would be to immediate fatal, which was specified in the original document.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, it's a reasonable default. In response to "Is there a legit reason you would not want to fail immediately?" I think the answer is yes.

}

// LeaderElector is a leader election client.
//
// possible future methods:
// * (le *LeaderElector) IsLeader()
// * (le *LeaderElector) GetLeader()
type LeaderElector struct {
config LeaderElectionConfig
// internal bookkeeping
observedRecord LeaderElectionRecord
observedTime time.Time
}

// LeaderElectionRecord is the record that is stored in the leader election annotation.
// This information should be used for observational purposes only and could be replaced
// with a random string (e.g. UUID) with only slight modification of this code.
// TODO(mikedanese): this should potentially be versioned
type LeaderElectionRecord struct {
HolderIdentity string `json:"holderIdentity"`
LeaseDurationSeconds int `json:"leaseDurationSeconds"`
AcquireTime unversioned.Time `json:"acquireTime"`
RenewTime unversioned.Time `json:"renewTime"`
}

// Run starts the leader election loop
func (le *LeaderElector) Run() {
defer func() {
util.HandleCrash()
le.config.Callbacks.OnStoppedLeading()
}()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure I follow the logic behind this panic handler.
If the panic is, say, due to being out of memory, then the OnStoppedLeading callback may not be able to do anything useful.

Not sure if this is needed for correct behavior or just an "optimization"?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is an optimization. I would prefer an OOM kill to an out of memory panic here... We should catch a nil pointer in the aquire though. e.g.:

OnStartedLeading: func() {
    go controller.Run()
    var bad map[string]string
    // nil deref
    a := bad["a"]
}

What i want here is for Run to call OnStoppLeading before it returns. It won't unless I handle crash.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seems like somewhat duplicated effort given the stop chan, which could signal the same thing (if it was closed via defer as I suggested). the difference being that a client won't have a reference to stop unless they've received the start callback.

I think the API would be more clear if "started" and "stopped" were NOT callbacks, but instead were exposed as latches:

// Started returns a chan that closes upon being elected for leadership. No objects are ever sent over the chan.
func (le *LeaderElector) Started() <-chan struct{} {... }
// Stopped returns a chan that closes upon being removed from leadership. No objects are ever sent over the chan.
func (le *LeaderElector) Stopped() <-chan struct{} {... }

.. and get rid of callbacks all together.

le.acquire()
stop := make(chan struct{})
go le.config.Callbacks.OnStartedLeading(stop)
le.renew()
close(stop)
}

// acquire loops calling tryAcquireOrRenew and returns immediately when tryAcquireOrRenew succeeds.
func (le *LeaderElector) acquire() {
stop := make(chan struct{})
util.Until(func() {
succeeded := le.tryAcquireOrRenew()
if !succeeded {
glog.V(4).Infof("failed to renew lease %v/%v", le.config.EndpointsMeta.Namespace, le.config.EndpointsMeta.Name)
time.Sleep(wait.Jitter(le.config.RetryPeriod, JitterFactor))
return
}
le.config.EventRecorder.Eventf(&api.Endpoints{ObjectMeta: le.config.EndpointsMeta}, api.EventTypeNormal, "%v became leader", le.config.Identity)
glog.Infof("sucessfully acquired lease %v/%v", le.config.EndpointsMeta.Namespace, le.config.EndpointsMeta.Name)
close(stop)
}, 0, stop)
}

// renew loops calling tryAcquireOrRenew and returns immediately when tryAcquireOrRenew fails.
func (le *LeaderElector) renew() {
stop := make(chan struct{})
util.Until(func() {
err := wait.Poll(le.config.RetryPeriod, le.config.RenewDeadline, func() (bool, error) {
return le.tryAcquireOrRenew(), nil
})
if err == nil {
glog.V(4).Infof("succesfully renewed lease %v/%v", le.config.EndpointsMeta.Namespace, le.config.EndpointsMeta.Name)
return
}
le.config.EventRecorder.Eventf(&api.Endpoints{ObjectMeta: le.config.EndpointsMeta}, api.EventTypeNormal, "%v stopped leading", le.config.Identity)
glog.Infof("failed to renew lease %v/%v", le.config.EndpointsMeta.Namespace, le.config.EndpointsMeta.Name)
close(stop)
}, 0, stop)
}

// tryAcquireOrRenew tries to acquire a leader lease if it is not already acquired,
// else it tries to renew the lease if it has already been acquired. Returns true
// on success else returns false.
func (le *LeaderElector) tryAcquireOrRenew() bool {
now := unversioned.Now()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about clock-skew...?

Before we relied on TTL from write, but now you're relying on what is essentially relative time.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We rely only on timestamps captured locally to function correctly.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a pretty dangerous supposition to rely on, that forces hard NTP requirements. Even then, there is a good reason why both ZK and etcd provide the consensus on variable state.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does it force hard NTP requirements? Machines could have hours or days of skew and this would work.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

b/c your using relative local times, there exists the condition where leader (X) has accurate time and sets duration of say 1 hour, then immediately crashes. Now standby (Y) has a slow clock b/c lets say it's running on a VM in a overly provisioned environment. For (Y) 1 hour wall clock duration from "now" could be 2 hours or 3 days, you don't really know. This leaves you headless for that period of time. For (Y) it only thinks 1 hour has passed.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This approach is tolerant of arbitrary clock skew, but not arbitrary skew rate. You can configure your skew rate tolerance by setting RenewDeadline and LeaseDuration appropriately. The problem described will be a problem for any system that doesn't apply fencing during write transactions, including podmaster.go. We said we aren't going to fence. The limitations of this approach are discussed above, and well understood. This approach works well in practice and it's battle tested. I'm not sure what else we can do.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure what else we can do.

Documentation, and I would indeed recommend some form of clock synchronization. I probably would in any case, but denoting this would be good, along with extra comments in the code to make it clear to other readers.

leaderElectionRecord := LeaderElectionRecord{
HolderIdentity: le.config.Identity,
LeaseDurationSeconds: int(le.config.LeaseDuration / time.Second),
RenewTime: now,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are marshalling time.Now() using the default formatter, which I think prints out the time in local time zone. But nodes may be in different time zones. Are two nodes guaranteed to be able to parse time zones used by other zones? Seems like it depends on ZONEINFO and such. I know nodes in a cluster are supposed to have synchronized clocks, but I don't know what syncs time zone info. Suggest either format time in UTC or document this requirement.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess this is safe because the times are just used for debugging and not for correctness?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should serialize with unversioned.Time but I wouldn't want to take a
dependency on that package.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess this is safe because the times are just used for debugging and not for correctness?

Yes.

We should serialize with unversioned.Time but I wouldn't want to take a
dependency on that package.

We already depend on it indirectly since we are depending on pkg/api.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using unversioned.Time now

AcquireTime: now,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Slight problem here is that on 1k node cluster "now" could be upwards of a second from the time you actually set the value.

e.g. you're relying on some level of SLA so windows should be large.

what if I had a cluster that spanned multiple availability zones.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are the knobs in the config object not sufficiently flexible to fit slower SLA envs? We need to send "now" with the request and it's for observational purposes only...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should document to denote.

}

e, err := le.config.Client.Endpoints(le.config.EndpointsMeta.Namespace).Get(le.config.EndpointsMeta.Name)
if err != nil {
if !errors.IsNotFound(err) {
return false
}

leaderElectionRecordBytes, err := json.Marshal(leaderElectionRecord)
if err != nil {
return false
}
_, err = le.config.Client.Endpoints(le.config.EndpointsMeta.Namespace).Create(&api.Endpoints{
ObjectMeta: api.ObjectMeta{
Name: le.config.EndpointsMeta.Name,
Namespace: le.config.EndpointsMeta.Namespace,
Annotations: map[string]string{
LeaderElectionRecordAnnotationKey: string(leaderElectionRecordBytes),
},
},
})
if err != nil {
glog.Errorf("error initially creating endpoints: %v", err)
return false
}
le.observedRecord = leaderElectionRecord
le.observedTime = time.Now()
return true
}

if e.Annotations == nil {
e.Annotations = make(map[string]string)
}

if oldLeaderElectionRecordBytes, found := e.Annotations[LeaderElectionRecordAnnotationKey]; found {
var oldLeaderElectionRecord LeaderElectionRecord
if err := json.Unmarshal([]byte(oldLeaderElectionRecordBytes), &oldLeaderElectionRecord); err != nil {
glog.Errorf("error unmarshaling leader election record: %v", err)
return false
}
if !reflect.DeepEqual(le.observedRecord, oldLeaderElectionRecord) {
le.observedRecord = oldLeaderElectionRecord
le.observedTime = time.Now()
}
if oldLeaderElectionRecord.HolderIdentity == le.config.Identity {
leaderElectionRecord.AcquireTime = oldLeaderElectionRecord.AcquireTime
}
if le.observedTime.Add(le.config.LeaseDuration).After(now.Time) &&
oldLeaderElectionRecord.HolderIdentity != le.config.Identity {
glog.Infof("lock is held by %v and has not yet expired", oldLeaderElectionRecord.HolderIdentity)
return false
}
}

leaderElectionRecordBytes, err := json.Marshal(leaderElectionRecord)
if err != nil {
glog.Errorf("err marshaling leader election record: %v", err)
return false
}
e.Annotations[LeaderElectionRecordAnnotationKey] = string(leaderElectionRecordBytes)

_, err = le.config.Client.Endpoints(le.config.EndpointsMeta.Namespace).Update(e)
if err != nil {
glog.Errorf("err: %v", err)
return false
}
le.observedRecord = leaderElectionRecord
le.observedTime = time.Now()
return true
}