Skip to content

Commit

Permalink
fix leadership label removed after twice winning election (#209)
Browse files Browse the repository at this point in the history
* fix leadership label removed after twice winning election

Currently, due to faulty labeling logic, if a pod won leadership twice it will loose it's leadership label.
This commit fixes that, and also adds unit tests to prevent this from happening.

Signed-off-by: Ram Lavi <[email protected]>

* adding update label unit tests

Signed-off-by: Ram Lavi <[email protected]>
  • Loading branch information
RamLavi authored Jun 25, 2020
1 parent f491b8a commit 77858f3
Show file tree
Hide file tree
Showing 7 changed files with 632 additions and 30 deletions.
60 changes: 30 additions & 30 deletions pkg/manager/leaderelection.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ func (k *KubeMacPoolManager) waitToStartLeading(poolManger *poolmanager.PoolMana
return errors.Wrap(err, "failed to start pool manager routines")
}

err = k.UpdateLeaderLabel()
err = updateLeaderLabel(k.runtimeManager.GetClient(), k.podName, k.podNamespace)
if err != nil {
return errors.Wrap(err, "failed marking pod as leader")
}
Expand All @@ -40,68 +40,68 @@ func (k *KubeMacPoolManager) waitToStartLeading(poolManger *poolmanager.PoolMana
return nil
}

// Adds the leader label to elected pod and removes it from all the other pods, if exists
func (k *KubeMacPoolManager) UpdateLeaderLabel() error {
logger := logf.Log.WithName("UpdateLeaderLabel")
// By setting this status to true in all pods, we declare the kubemacpool as ready and allow the webhooks to start running.
func (k *KubeMacPoolManager) setLeadershipConditions(status corev1.ConditionStatus) error {
podList := corev1.PodList{}
err := k.runtimeManager.GetClient().List(context.TODO(), &podList, &client.ListOptions{Namespace: k.podNamespace})
if err != nil {
return errors.Wrap(err, "failed to list kubemacpool manager pods")
}

for _, pod := range podList.Items {
err = retry.RetryOnConflict(retry.DefaultRetry, func() error {
podKey := types.NamespacedName{Namespace: k.podNamespace, Name: pod.Name}
err := k.runtimeManager.GetClient().Get(context.TODO(), podKey, &pod)
if err != nil {
return errors.Wrap(err, "failed to get kubemacpool manager pod")
return errors.Wrap(err, "failed to get kubemacpool manager pods")
}

_, exist := pod.Labels[names.LEADER_LABEL]
if pod.Name == k.podName && !exist {
logger.V(1).Info("add the label to the elected leader", "Pod Name", pod.Name)
pod.Labels[names.LEADER_LABEL] = "true"
} else if exist {
logger.V(1).Info("deleting leader label from old leader", "Pod Name", pod.Name)
delete(pod.Labels, names.LEADER_LABEL)
} else {
return nil
}
pod.Status.Conditions = append(pod.Status.Conditions, corev1.PodCondition{Type: names.LEADER_READY_CONDITION_TYPE, Status: status, LastProbeTime: metav1.Time{}})

return k.runtimeManager.GetClient().Status().Update(context.TODO(), &pod)
err = k.runtimeManager.GetClient().Status().Update(context.TODO(), &pod)
return err
})

if err != nil {
return errors.Wrap(err, fmt.Sprintf("failed to updating kubemacpool leader label in pod %s", pod.Name))
return errors.Wrap(err, "failed to update Leadership readiness gate status to kubemacpool manager pods")
}
}

return nil
}

// By setting this status to true in all pods, we declare the kubemacpool as ready and allow the webhooks to start running.
func (k *KubeMacPoolManager) setLeadershipConditions(status corev1.ConditionStatus) error {
// Adds the leader label to elected pod and removes it from all the other pods, if exists
func updateLeaderLabel(kubeClient client.Client, leaderPodName, managerNamespace string) error {
logger := logf.Log.WithName("UpdateLeaderLabel")
podList := corev1.PodList{}
err := k.runtimeManager.GetClient().List(context.TODO(), &podList, &client.ListOptions{Namespace: k.podNamespace})
err := kubeClient.List(context.TODO(), &podList, &client.ListOptions{Namespace: managerNamespace})
if err != nil {
return errors.Wrap(err, "failed to list kubemacpool manager pods")
}

for _, pod := range podList.Items {
err = retry.RetryOnConflict(retry.DefaultRetry, func() error {
podKey := types.NamespacedName{Namespace: k.podNamespace, Name: pod.Name}
err := k.runtimeManager.GetClient().Get(context.TODO(), podKey, &pod)
podKey := types.NamespacedName{Namespace: managerNamespace, Name: pod.Name}
err := kubeClient.Get(context.TODO(), podKey, &pod)
if err != nil {
return errors.Wrap(err, "failed to get kubemacpool manager pods")
return errors.Wrap(err, "failed to get kubemacpool manager pod")
}

pod.Status.Conditions = append(pod.Status.Conditions, corev1.PodCondition{Type: names.LEADER_READY_CONDITION_TYPE, Status: status, LastProbeTime: metav1.Time{}})
if pod.Name == leaderPodName {
logger.Info("add the label to the elected leader", "Pod Name", pod.Name)
if len(pod.Labels) == 0 {
pod.Labels = make(map[string]string)
}
pod.Labels[names.LEADER_LABEL] = "true"
} else {
logger.Info("deleting leader label if exists", "Pod Name", pod.Name)
delete(pod.Labels, names.LEADER_LABEL)
}

err = k.runtimeManager.GetClient().Status().Update(context.TODO(), &pod)
return err
return kubeClient.Status().Update(context.TODO(), &pod)
})

if err != nil {
return errors.Wrap(err, "failed to update Leadership readiness gate status to kubemacpool manager pods")
return errors.Wrap(err, fmt.Sprintf("failed to updating kubemacpool leader label in pod %s", pod.Name))
}
}

return nil
}
29 changes: 29 additions & 0 deletions pkg/manager/manager_suite_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
/*
Copyright 2019 The KubeMacPool Authors.
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 manager

import (
"testing"

. "github.com/onsi/ginkgo"
. "github.com/onsi/gomega"
)

func TestPoolManager(t *testing.T) {
RegisterFailHandler(Fail)
RunSpecs(t, "Manager Suite")
}
91 changes: 91 additions & 0 deletions pkg/manager/manager_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
/*
Copyright 2019 The KubeMacPool Authors.
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 manager

import (
"context"

. "github.com/onsi/ginkgo"
. "github.com/onsi/gomega"

"github.com/onsi/ginkgo/extensions/table"
v1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/types"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/client/fake"

"github.com/k8snetworkplumbingwg/kubemacpool/pkg/names"
)

var _ = Describe("leader election", func() {
leaderPodName := "leaderPod"
loosingPodName := "loosingPod"
leaderLabelValue := "true"

leaderPod := v1.Pod{ObjectMeta: metav1.ObjectMeta{Name: leaderPodName, Namespace: names.MANAGER_NAMESPACE}}
looserPod := v1.Pod{ObjectMeta: metav1.ObjectMeta{Name: loosingPodName, Namespace: names.MANAGER_NAMESPACE}}

createEnvironment := func(fakeObjectsForClient ...runtime.Object) client.Client {
fakeClient := fake.NewFakeClient(fakeObjectsForClient...)

return fakeClient
}

Describe("Internal Functions", func() {
Context("When leader Pod is passed for leader label update", func() {
table.DescribeTable("Should update the leader label in all pods", func(leaderPodFormerLabels, looserPodFormerLabels string) {
By("Adding the initial label state of the pods prior to winning the election")
initiatePodLabels(&leaderPod, leaderPodFormerLabels, leaderLabelValue)
initiatePodLabels(&looserPod, looserPodFormerLabels, leaderLabelValue)

By("Initiating the Environment")
kubeClient := createEnvironment(&leaderPod, &looserPod)

By("running label update method")
err := updateLeaderLabel(kubeClient, leaderPodName, names.MANAGER_NAMESPACE)
Expect(err).ToNot(HaveOccurred(), "should successfully update kubemacpool leader labels")

By("checking the leader pod has the leader label")
checkLeaderPod := v1.Pod{}
err = kubeClient.Get(context.TODO(), types.NamespacedName{Namespace: names.MANAGER_NAMESPACE, Name: leaderPodName}, &checkLeaderPod)
Expect(err).ToNot(HaveOccurred(), "should successfully get the kubemacpool leader pod")
Expect(checkLeaderPod.Labels).To(HaveLen(1), "leader pod should have only 1 label")
Expect(checkLeaderPod.Labels[names.LEADER_LABEL]).To(Equal(leaderLabelValue), "leader pod should have the leader label value")

By("checking the non-leader pod has no leader label")
checkLooserPod := v1.Pod{}
err = kubeClient.Get(context.TODO(), types.NamespacedName{Namespace: names.MANAGER_NAMESPACE, Name: loosingPodName}, &checkLooserPod)
Expect(err).ToNot(HaveOccurred(), "should successfully get the kubemacpool non-leader pod")
Expect(checkLooserPod.Labels).To(HaveLen(0), "non-leader pod should not have any labels")
},
table.Entry("all pods don't have a former leader label", "", ""),
table.Entry("leader pod already has leader label from former election", names.LEADER_LABEL, ""),
table.Entry("looser pod already has leader label from former election", "", names.LEADER_LABEL),
table.Entry("all pods have a former leader label", names.LEADER_LABEL, names.LEADER_LABEL),
)
})
})
})

func initiatePodLabels(pod *v1.Pod, label string, labelValue string) {
if len(label) != 0 {
pod.Labels = make(map[string]string)
pod.Labels[label] = labelValue
}
}
2 changes: 2 additions & 0 deletions vendor/modules.txt
Original file line number Diff line number Diff line change
Expand Up @@ -501,6 +501,7 @@ sigs.k8s.io/controller-runtime/pkg/cache/internal
sigs.k8s.io/controller-runtime/pkg/client
sigs.k8s.io/controller-runtime/pkg/client/apiutil
sigs.k8s.io/controller-runtime/pkg/client/config
sigs.k8s.io/controller-runtime/pkg/client/fake
sigs.k8s.io/controller-runtime/pkg/controller
sigs.k8s.io/controller-runtime/pkg/envtest
sigs.k8s.io/controller-runtime/pkg/event
Expand All @@ -509,6 +510,7 @@ sigs.k8s.io/controller-runtime/pkg/healthz
sigs.k8s.io/controller-runtime/pkg/internal/controller
sigs.k8s.io/controller-runtime/pkg/internal/controller/metrics
sigs.k8s.io/controller-runtime/pkg/internal/log
sigs.k8s.io/controller-runtime/pkg/internal/objectutil
sigs.k8s.io/controller-runtime/pkg/internal/recorder
sigs.k8s.io/controller-runtime/pkg/internal/testing/integration
sigs.k8s.io/controller-runtime/pkg/internal/testing/integration/addr
Expand Down
Loading

0 comments on commit 77858f3

Please sign in to comment.