Skip to content
This repository has been archived by the owner on Aug 19, 2020. It is now read-only.

Pod labeler #115

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Conversation

josh-ferrell-sas
Copy link

A while back I wrote a sidecar that would patch the pod with either vip-holder true or vip-holder false depending on the keepalived state. I decided to write the code into the upstream project as part of the /health endpoint.

@claassistantio
Copy link

claassistantio commented Jan 20, 2020

CLA assistant check
All committers have signed the CLA.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.8%) to 14.211% when pulling 891659a on josh-ferrell:pod-labeler into 121232b on aledbf:master.

}

for k,v := range labels {
pod.ObjectMeta.Labels[k] = v
Copy link
Contributor

Choose a reason for hiding this comment

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

This looks like it replaces all labels instead of just adding vip-holder if it doesn't exist.

@@ -195,6 +195,20 @@ func (k *keepalived) IsRunning() bool {
return true
}

func (k *keepalived) isMaster() bool {
Copy link
Contributor

Choose a reason for hiding this comment

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

This method should be called by Healthy() to reduce duplication.

@@ -466,6 +466,19 @@ func NewIPVSController(kubeClient *kubernetes.Clientset, namespace string, useUn
return
}

state := ipvsc.keepalived.isMaster()
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think it's appropriate that invoking the health check endpoint can somehow trigger pod labels to be changed. This logic should live elsewhere.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants