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

LeaderElection logic stops after a leader failed to update a lock #1155

Closed
aldlfkahs opened this issue Aug 30, 2022 · 10 comments
Closed

LeaderElection logic stops after a leader failed to update a lock #1155

aldlfkahs opened this issue Aug 30, 2022 · 10 comments
Labels
lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed.

Comments

@aldlfkahs
Copy link

I imported client-go v0.22.2 and used leader election logic, but I only deployed one pod.
When the pod failed to update the lock for whatever reason, the pod does not run OnStartedLeading() callback forever even though that the pod still has 'lease' resource.

My leader election code is below.

func runLeaderElection(lock *resourcelock.LeaseLock, ctx context.Context, id string) {

	leaderelection.RunOrDie(ctx, leaderelection.LeaderElectionConfig{
		Lock:            lock,
		ReleaseOnCancel: true,
		LeaseDuration:   10 * time.Second,
		RenewDeadline:   5 * time.Second,
		RetryPeriod:     2 * time.Second,
		Callbacks: leaderelection.LeaderCallbacks{
			OnStartedLeading: func(c context.Context) {
				// do something
			},
			OnStoppedLeading: func() {
				klog.V(3).Info("no longer the leader, staying inactive and stop metering & event logging")
				// do something
			},
			OnNewLeader: func(current_id string) {
				if current_id == id {
					klog.V(3).Info("still the leader!")
					return
				}
				klog.V(3).Info("new leader is ", current_id)
			},
		},
	})
}

Logs printed like below.

I0825 13:31:24.323896       1 leaderelection.go:248] attempting to acquire leader lease hypercloud5-system/hypercloud5-api-server...
I0825 13:31:24.524691       1 main.go:677] new leader is hypercloud5-api-server-5648fb66bf-vdp8w
I0825 13:31:37.133639       1 leaderelection.go:258] successfully acquired lease hypercloud5-system/hypercloud5-api-server
I0825 13:31:37.133731       1 main.go:674] still the leader!
E0825 13:59:46.770035       1 leaderelection.go:367] Failed to update lock: Put "[https://10.121.0.1:443/apis/coordination.k8s.io/v1/namespaces/hypercloud5-system/leases/hypercloud5-api-server](https://10.121.0.1/apis/coordination.k8s.io/v1/namespaces/hypercloud5-system/leases/hypercloud5-api-server)": context deadline exceeded
I0825 13:59:46.770085       1 leaderelection.go:283] failed to renew lease hypercloud5-system/hypercloud5-api-server: timed out waiting for the condition
E0825 13:59:53.375311       1 leaderelection.go:306] Failed to release lock: Operation cannot be fulfilled on leases.coordination.k8s.io "hypercloud5-api-server": the object has been modified; please apply your changes to the latest version and try again
I0825 13:59:53.375334       1 main.go:669] no longer the leader, staying inactive and stop metering service

You can definitely see that OnStoppedLeading() callback is called.
However, there is no new leader election after above logs.
Interesting point is that the pod still has lease!

I doubt Update() function in leaderelection.go.

// release attempts to release the leader lease if we have acquired it.
func (le *LeaderElector) release() bool {
	if !le.IsLeader() {
		return true
	}
	now := metav1.Now()
	leaderElectionRecord := rl.LeaderElectionRecord{
		LeaderTransitions:    le.observedRecord.LeaderTransitions,
		LeaseDurationSeconds: 1,
		RenewTime:            now,
		AcquireTime:          now,
	}
	if err := le.config.Lock.Update(context.TODO(), leaderElectionRecord); err != nil {  // <--- HERE
		klog.Errorf("Failed to release lock: %v", err)
		return false
	}

	le.setObservedRecord(&leaderElectionRecord)
	return true
}

If the leader fails to Update() in release() function, I think it just return false and got stuck.

What is the problem? How can I fix it?

@linsite
Copy link

linsite commented Nov 26, 2022

I've hit the same error. My situation is that I want to use leader election as kind of a cluster lock, where the election duration is quite short. After serveral turns of lock/unlock actions, the same error " the object has been modified; please apply your changes to the latest version and try again" was found.

I believe it can be caused by the ctx in 'tryAcquireOrRenew' is canceled by caller before the Update call returns, while the 'release' call sees a different version of the lease resource, makes the release call to fail.

@aimuz
@aojea

@linsite
Copy link

linsite commented Nov 26, 2022

I update the example to reproduce these issue.

package main

import (
	"context"
	"flag"
	"log"
	"os"
	"os/signal"
	"syscall"
	"time"

	"github.com/google/uuid"
	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
	clientset "k8s.io/client-go/kubernetes"
	"k8s.io/client-go/rest"
	"k8s.io/client-go/tools/clientcmd"
	"k8s.io/client-go/tools/leaderelection"
	"k8s.io/client-go/tools/leaderelection/resourcelock"
	"k8s.io/klog/v2"
)

func buildConfig(kubeconfig string) (*rest.Config, error) {
	if kubeconfig != "" {
		cfg, err := clientcmd.BuildConfigFromFlags("", kubeconfig)
		if err != nil {
			return nil, err
		}
		return cfg, nil
	}

	cfg, err := rest.InClusterConfig()
	if err != nil {
		return nil, err
	}
	return cfg, nil
}

func main() {
	klog.InitFlags(nil)

	var kubeconfig string
	var leaseLockName string
	var leaseLockNamespace string
	var id string

	flag.StringVar(&kubeconfig, "kubeconfig", "", "absolute path to the kubeconfig file")
	flag.StringVar(&id, "id", uuid.New().String(), "the holder identity name")
	flag.StringVar(&leaseLockName, "lease-lock-name", "", "the lease lock resource name")
	flag.StringVar(&leaseLockNamespace, "lease-lock-namespace", "", "the lease lock resource namespace")
	flag.Parse()

	if leaseLockName == "" {
		klog.Fatal("unable to get lease lock resource name (missing lease-lock-name flag).")
	}
	if leaseLockNamespace == "" {
		klog.Fatal("unable to get lease lock resource namespace (missing lease-lock-namespace flag).")
	}

	// leader election uses the Kubernetes API by writing to a
	// lock object, which can be a LeaseLock object (preferred),
	// a ConfigMap, or an Endpoints (deprecated) object.
	// Conflicting writes are detected and each client handles those actions
	// independently.
	config, err := buildConfig(kubeconfig)
	if err != nil {
		klog.Fatal(err)
	}
	client := clientset.NewForConfigOrDie(config)

	run := func(ctx context.Context) {
		// complete your controller loop here
		klog.Info("Controller loop...")

		select {}
	}

	// use a Go context so we can tell the leaderelection code when we
	// want to step down
	ctx, cancel := context.WithCancel(context.Background())
	defer cancel()

	// listen for interrupts or the Linux SIGTERM signal and cancel
	// our context, which the leader election code will observe and
	// step down
	ch := make(chan os.Signal, 1)
	signal.Notify(ch, os.Interrupt, syscall.SIGTERM)
	go func() {
		<-ch
		klog.Info("Received termination, signaling shutdown")
		cancel()
	}()

	// we use the Lease lock type since edits to Leases are less common
	// and fewer objects in the cluster watch "all Leases".
	lock := &resourcelock.LeaseLock{
		LeaseMeta: metav1.ObjectMeta{
			Name:      leaseLockName,
			Namespace: leaseLockNamespace,
		},
		Client: client.CoordinationV1(),
		LockConfig: resourcelock.ResourceLockConfig{
			Identity: id,
		},
	}

	// start the leader election code loop
	wait := make(chan struct{}, 1)
	go func() {
		leaderelection.RunOrDie(ctx, leaderelection.LeaderElectionConfig{
			Lock: lock,
			// IMPORTANT: you MUST ensure that any code you have that
			// is protected by the lease must terminate **before**
			// you call cancel. Otherwise, you could have a background
			// loop still running and another process could
			// get elected before your background loop finished, violating
			// the stated goal of the lease.
			ReleaseOnCancel: true,
			LeaseDuration:   60 * time.Second,
			RenewDeadline:   15 * time.Second,
			RetryPeriod:     5 * time.Second,
			Callbacks: leaderelection.LeaderCallbacks{
				OnStartedLeading: func(ctx context.Context) {
					// we're notified when we start - this is where you would
					// usually put your code
					run(ctx)
				},
				OnStoppedLeading: func() {
					// we can do cleanup here
					klog.Infof("leader lost: %s", id)
				},
				OnNewLeader: func(identity string) {
					// we're notified when new leader elected
					if identity == id {
						// I just got the lock
						log.Print("Got the lock")
						wait <- struct{}{}
						return
					}
					klog.Infof("new leader elected: %s", identity)
				},
			},
		})
	}()
	<-wait
        // Sleep a while depends on the network, 1 node cluster needn't this.
        time.Sleep(3 * time.Millisecond)
	cancel()
        // sleep to give `release` a chance to run.
	time.Sleep(300 * time.Millisecond)
}

With below command in a k8s node, 1 out of 5 can fail.

./poc  -lease-lock-name test -lease-lock-namespace default -kubeconfig $HOME/.kube/config -v 5

@aojea
Copy link
Member

aojea commented Nov 26, 2022

Let me see if I understand your use case, you want to get the Leader as a Lock and then release it quickly, so you use the context to do that, but when you use the context it also cancels the subsequent operations , is that right?

@linsite
Copy link

linsite commented Nov 26, 2022

Let me see if I understand your use case, you want to get the Leader as a Lock and then release it quickly, so you use the context to do that, but when you use the context it also cancels the subsequent operations , is that right?

Yeah, pretty so. The real action protected by the lock is done in the leader callback with another context. I just cancel the ctx used by leaderelection.RunOrDie to release the leadership,I've added some logic to ensure that action is done before canceling the ctx.

Maybe it's not a normal case where the leaderelection package used. If so, it's O.K. not to treat it as a problem, since all the kubernetes components which are using the package haven't reported any related issue yet.

@aojea
Copy link
Member

aojea commented Nov 26, 2022

Maybe it's not a normal case where the leaderelection package used. If so, it's O.K. not to treat it as a problem, since all the kubernetes components which are using the package haven't reported any related issue yet.

It doesn't look like the normal use case, basically it seems that what you want is a new method StopLeading() instead of using the context , right?

I think that what you want can be done better using the Lease API https://kubernetes.io/docs/reference/kubernetes-api/cluster-resources/lease-v1/, but maybe having a method to stop leading without killing yourself in the process by cancelling the context is something we need 🤔

Let me ask @deads2k @wojtek-t and @liggitt , they have more historical context and it will be interesting to hear their thoughts

@aldlfkahs
Copy link
Author

I am happy that this issue is finally progressed.

It's not a fundamental solution, but I share my temporary method for someone who needs to fix this situation urgently or cannot change package version.

You can re-leader-election using defer func() like below.

func main() {
	...
	ctx, cancel = context.WithCancel(context.Background())
	podName, _ := os.Hostname()
	lock := getNewLock("my-lock", podName, "my-namespace")
	go runLeaderElection(lock, ctx, podName)
	...
}
func runLeaderElection(lock *resourcelock.LeaseLock, ctx context.Context, id string) {

	defer func() {
		if r := recover(); r != nil {
			if err, ok := r.(error); ok {
				klog.V(1).Infoln("Panic =  " + err.Error())
			} else {
				klog.V(1).Infof("Panic happened with %v", r)
				klog.V(1).Infoln()
			}
		} else {
			klog.V(1).Infoln("leader election just downed...")
		}
		go runLeaderElection(lock, ctx, id) // <-- restart leaderelection
	}()

	leaderelection.RunOrDie(ctx, leaderelection.LeaderElectionConfig{
		Lock:            lock,
		ReleaseOnCancel: true,
		LeaseDuration:   15 * time.Second,
		RenewDeadline:   10 * time.Second,
		RetryPeriod:     2 * time.Second,
		Callbacks: leaderelection.LeaderCallbacks{
			OnStartedLeading: func(c context.Context) {
				// do something
			},
			OnStoppedLeading: func() {
				// do something
			},
			OnNewLeader: func(current_id string) {
				if current_id == id {
					klog.V(3).Info("still the leader!")
					return
				}
				klog.V(3).Info("new leader is ", current_id)
			},
		},
	})
}

Make function that covers leaderelection.RunOrDie(runLeaderElection in above example) with defer func(). In defer func(), run the function itself again. This can help someone who needs temporary solution.

@k8s-triage-robot
Copy link

The Kubernetes project currently lacks enough contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Mark this issue as fresh with /remove-lifecycle stale
  • Close this issue with /close
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jul 9, 2023
@k8s-triage-robot
Copy link

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Mark this issue as fresh with /remove-lifecycle rotten
  • Close this issue with /close
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle rotten

@k8s-ci-robot k8s-ci-robot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Jan 19, 2024
@k8s-triage-robot
Copy link

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages issues according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Reopen this issue with /reopen
  • Mark this issue as fresh with /remove-lifecycle rotten
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/close not-planned

@k8s-ci-robot
Copy link
Contributor

@k8s-triage-robot: Closing this issue, marking it as "Not Planned".

In response to this:

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages issues according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Reopen this issue with /reopen
  • Mark this issue as fresh with /remove-lifecycle rotten
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/close not-planned

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@k8s-ci-robot k8s-ci-robot closed this as not planned Won't fix, can't repro, duplicate, stale Feb 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants