Skip to content

Commit

Permalink
Update leader label only at pods with label 'app=kubemacpool' (#212)
Browse files Browse the repository at this point in the history
Otherwise it iterate over all the pods at the same namespace, this is
not a critical bug is kind of cosmetics but it reduce the time to update leader label.

Signed-off-by: Quique Llorente <[email protected]>
  • Loading branch information
qinqon authored Jun 28, 2020
1 parent 77858f3 commit 68a684e
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 5 deletions.
8 changes: 7 additions & 1 deletion pkg/manager/leaderelection.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,13 @@ func (k *KubeMacPoolManager) setLeadershipConditions(status corev1.ConditionStat
func updateLeaderLabel(kubeClient client.Client, leaderPodName, managerNamespace string) error {
logger := logf.Log.WithName("UpdateLeaderLabel")
podList := corev1.PodList{}
err := kubeClient.List(context.TODO(), &podList, &client.ListOptions{Namespace: managerNamespace})

byNamespaceAndApp := &client.ListOptions{Namespace: managerNamespace}
client.MatchingLabels{
"app": "kubemacpool",
}.ApplyToList(byNamespaceAndApp)

err := kubeClient.List(context.TODO(), &podList, byNamespaceAndApp)
if err != nil {
return errors.Wrap(err, "failed to list kubemacpool manager pods")
}
Expand Down
11 changes: 7 additions & 4 deletions pkg/manager/manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,9 @@ var _ = Describe("leader election", 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(&leaderPod, "app", "kubemacpool")
initiatePodLabels(&looserPod, looserPodFormerLabels, leaderLabelValue)
initiatePodLabels(&looserPod, "app", "kubemacpool")

By("Initiating the Environment")
kubeClient := createEnvironment(&leaderPod, &looserPod)
Expand All @@ -65,14 +67,13 @@ var _ = Describe("leader election", func() {
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")
Expect(checkLeaderPod.Labels).To(HaveKeyWithValue(names.LEADER_LABEL, 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")
Expect(checkLooserPod.Labels).ToNot(HaveKeyWithValue(names.LEADER_LABEL, leaderLabelValue), "non leader pod should not have the leader label value")
},
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, ""),
Expand All @@ -85,7 +86,9 @@ var _ = Describe("leader election", func() {

func initiatePodLabels(pod *v1.Pod, label string, labelValue string) {
if len(label) != 0 {
pod.Labels = make(map[string]string)
if len(pod.Labels) == 0 {
pod.Labels = make(map[string]string)
}
pod.Labels[label] = labelValue
}
}

0 comments on commit 68a684e

Please sign in to comment.