airship/kubernetes-entrypoint
Code Issues Proposed changes

Add requireSameNode field for pod dependencies

Browse Source 
Previously pod dependencies always required the pod to be on the same
node.  Though this was documented it often did not match user intuition
as merely depending on a pod shouldn't require anything more than it
existing somewhere in the cluster.  This adds a field to pod
dependencies to specify whether the pod is required to be on the same
node.  This defaults to false for the reasons above, which is a breaking
change.
This commit is contained in:
Sean Eagan 2018-03-21 14:12:03 -05:00
parent e8a23f28f4
commit 8719e95d78
6 changed files with 53 additions and 34 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
README.md
View File

@ -88,14 +88,16 @@ Example:
`DEPENDENCY_SOCKET=/var/run/openvswitch/ovs.socket`
### Pod
Checks if at least one pod matching the specified labels is already running on the same host.
Checks if at least one pod matching the specified labels is already running, by
default anywhere in the cluster, or use `"requireSameNode": true` to require a
a pod on the same node.
In contrast to other dependencies, the syntax uses json in order to avoid inventing a new
format to specify labels and the parsing complexities that would come with that.
This dependency requires a `POD_NAME` env which can be easily passed through the
[downward api](http://kubernetes.io/docs/user-guide/downward-api/). The `POD_NAME` variable is mandatory and is used to resolve dependencies.
Example:
`DEPENDENCY_POD="[{\"namespace\": \"foo\", \"labels\": {\"k1\": \"v1\", \"k2\": \"v2\"}}, {\"labels\": {\"k1\": \"v1\", \"k2\": \"v2\"}}]"`
`DEPENDENCY_POD="[{\"namespace\": \"foo\", \"labels\": {\"k1\": \"v1\", \"k2\": \"v2\"}}, {\"labels\": {\"k1\": \"v1\", \"k2\": \"v2\"}, \"requireSameNode\": true}]"`
## Image

36
dependencies/pod/pod.go vendored
View File

@ -18,16 +18,17 @@ const (
)
type Pod struct {
namespace string
labels map[string]string
podName string
namespace string
labels map[string]string
requireSameNode bool
podName string
}
func init() {
podEnv := fmt.Sprintf("%sPOD", entry.DependencyPrefix)
if podDeps := env.SplitPodEnvToDeps(podEnv); podDeps != nil {
for _, dep := range podDeps {
pod, err := NewPod(dep.Labels, dep.Namespace)
pod, err := NewPod(dep.Labels, dep.Namespace, dep.RequireSameNode)
if err != nil {
logger.Error.Printf("Cannot initialize pod: %v", err)
continue
@ -37,14 +38,15 @@ func init() {
}
}
func NewPod(labels map[string]string, namespace string) (*Pod, error) {
func NewPod(labels map[string]string, namespace string, requireSameNode bool) (*Pod, error) {
if os.Getenv(PodNameEnvVar) == "" {
return nil, fmt.Errorf(PodNameNotSetErrorFormat, namespace)
}
return &Pod{
namespace: namespace,
labels: labels,
podName: os.Getenv(PodNameEnvVar),
labels: labels,
namespace: namespace,
requireSameNode: requireSameNode,
podName: os.Getenv(PodNameEnvVar),
}, nil
}
@ -65,23 +67,27 @@ func (p Pod) IsResolved(entrypoint entry.EntrypointInterface) (bool, error) {
matchingPods := matchingPodList.Items
if len(matchingPods) == 0 {
return false, fmt.Errorf("No pods found matching labels: %v", p.labels)
return false, fmt.Errorf("Found no pods matching labels: %v", p.labels)
}
hostPodCount := 0
podCount := 0
for _, pod := range matchingPods {
if !isPodOnHost(&pod, myHost) {
podCount++
if p.requireSameNode && !isPodOnHost(&pod, myHost) {
continue
}
hostPodCount++
if isPodReady(pod) {
return true, nil
}
}
if hostPodCount == 0 {
return false, fmt.Errorf("Found no pods on host matching labels: %v", p.labels)
onHostClause := ""
if p.requireSameNode {
onHostClause = " on host"
}
if podCount == 0 {
return false, fmt.Errorf("Found no pods%v matching labels: %v", onHostClause, p.labels)
} else {
return false, fmt.Errorf("Found %v pods on host, but none ready, matching labels: %v", hostPodCount, p.labels)
return false, fmt.Errorf("Found %v pods%v, but none ready, matching labels: %v", podCount, onHostClause, p.labels)
}
}

32
dependencies/pod/pod_test.go vendored
View File

@ -14,6 +14,7 @@ import (
const (
podEnvVariableValue = "podlist"
podNamespace = "test"
requireSameNode = true
)
var testEntrypoint entrypoint.EntrypointInterface
@ -30,14 +31,14 @@ var _ = Describe("Pod", func() {
It(fmt.Sprintf("checks failure of new pod creation without %s set", PodNameEnvVar), func() {
os.Unsetenv(PodNameEnvVar)
pod, err := NewPod(testLabels, podNamespace)
pod, err := NewPod(testLabels, podNamespace, requireSameNode)
Expect(pod).To(BeNil())
Expect(err.Error()).To(Equal(fmt.Sprintf(PodNameNotSetErrorFormat, podNamespace)))
})
It(fmt.Sprintf("creates new pod with %s set and checks its name", PodNameEnvVar), func() {
pod, err := NewPod(testLabels, podNamespace)
pod, err := NewPod(testLabels, podNamespace, requireSameNode)
Expect(pod).NotTo(BeNil())
Expect(err).NotTo(HaveOccurred())
@ -45,7 +46,7 @@ var _ = Describe("Pod", func() {
})
It("is resolved via all pods matching labels ready on same host", func() {
pod, _ := NewPod(map[string]string{"name": mocks.SameHostReadyMatchLabel}, podNamespace)
pod, _ := NewPod(map[string]string{"name": mocks.SameHostReadyMatchLabel}, podNamespace, requireSameNode)
isResolved, err := pod.IsResolved(testEntrypoint)
@ -54,7 +55,7 @@ var _ = Describe("Pod", func() {
})
It("is resolved via some pods matching labels ready on same host", func() {
pod, _ := NewPod(map[string]string{"name": mocks.SameHostSomeReadyMatchLabel}, podNamespace)
pod, _ := NewPod(map[string]string{"name": mocks.SameHostSomeReadyMatchLabel}, podNamespace, requireSameNode)
isResolved, err := pod.IsResolved(testEntrypoint)
@ -63,7 +64,7 @@ var _ = Describe("Pod", func() {
})
It("is not resolved via a pod matching labels not ready on same host", func() {
pod, _ := NewPod(map[string]string{"name": mocks.SameHostNotReadyMatchLabel}, podNamespace)
pod, _ := NewPod(map[string]string{"name": mocks.SameHostNotReadyMatchLabel}, podNamespace, requireSameNode)
isResolved, err := pod.IsResolved(testEntrypoint)
@ -72,7 +73,7 @@ var _ = Describe("Pod", func() {
})
It("is not resolved via pod matching labels ready on different host", func() {
pod, _ := NewPod(map[string]string{"name": mocks.DifferentHostReadyMatchLabel}, podNamespace)
pod, _ := NewPod(map[string]string{"name": mocks.DifferentHostReadyMatchLabel}, podNamespace, requireSameNode)
isResolved, err := pod.IsResolved(testEntrypoint)
@ -80,8 +81,17 @@ var _ = Describe("Pod", func() {
Expect(err).To(HaveOccurred())
})
It("is not resolved via pod matching labels not ready on different host", func() {
pod, _ := NewPod(map[string]string{"name": mocks.DifferentHostNotReadyMatchLabel}, podNamespace)
It("is resolved via pod matching labels ready on different host when requireSameNode=false", func() {
pod, _ := NewPod(map[string]string{"name": mocks.DifferentHostReadyMatchLabel}, podNamespace, false)
isResolved, err := pod.IsResolved(testEntrypoint)
Expect(isResolved).To(BeTrue())
Expect(err).NotTo(HaveOccurred())
})
It("is not resolved via pod matching labels not ready on different host when requireSameNode=false", func() {
pod, _ := NewPod(map[string]string{"name": mocks.DifferentHostNotReadyMatchLabel}, podNamespace, false)
isResolved, err := pod.IsResolved(testEntrypoint)
@ -90,7 +100,7 @@ var _ = Describe("Pod", func() {
})
It("is not resolved via no pods matching labels", func() {
pod, _ := NewPod(map[string]string{"name": mocks.NoPodsMatchLabel}, podNamespace)
pod, _ := NewPod(map[string]string{"name": mocks.NoPodsMatchLabel}, podNamespace, requireSameNode)
isResolved, err := pod.IsResolved(testEntrypoint)
@ -99,7 +109,7 @@ var _ = Describe("Pod", func() {
})
It("is not resolved when getting pods matching labels from api fails", func() {
pod, _ := NewPod(map[string]string{"name": mocks.FailingMatchLabel}, podNamespace)
pod, _ := NewPod(map[string]string{"name": mocks.FailingMatchLabel}, podNamespace, requireSameNode)
isResolved, err := pod.IsResolved(testEntrypoint)
@ -109,7 +119,7 @@ var _ = Describe("Pod", func() {
It(fmt.Sprintf("is not resolved when getting current pod via %s value fails", PodNameEnvVar), func() {
os.Setenv(PodNameEnvVar, mocks.PodNotPresent)
pod, _ := NewPod(map[string]string{"name": mocks.SameHostReadyMatchLabel}, podNamespace)
pod, _ := NewPod(map[string]string{"name": mocks.SameHostReadyMatchLabel}, podNamespace, requireSameNode)
isResolved, err := pod.IsResolved(testEntrypoint)

BIN
kubernetes-entrypoint
View File

Binary file not shown.

7
util/env/env.go vendored
View File

@ -18,8 +18,9 @@ type Dependency struct {
}
type PodDependency struct {
Labels map[string]string
Namespace string
Labels map[string]string
Namespace string
RequireSameNode bool
}
func SplitCommand() []string {
@ -88,8 +89,8 @@ func SplitPodEnvToDeps(env string) []PodDependency {
for i, dep := range deps {
if dep.Namespace == "" {
dep.Namespace = namespace
deps[i] = dep
}
deps[i] = dep
}
return deps

6
util/env/env_test.go vendored
View File

@ -96,17 +96,17 @@ func TestSplitPodEnvToDepsSuccess(t *testing.T) {
defer os.Unsetenv("NAMESPACE")
os.Setenv("NAMESPACE", `TEST_NAMESPACE`)
defer os.Unsetenv("TEST_LIST")
os.Setenv("TEST_LIST", `[{"namespace": "foo", "labels": {"k1": "v1", "k2": "v2"}}, {"labels": {"k1": "v1", "k2": "v2"}}]`)
os.Setenv("TEST_LIST", `[{"namespace": "foo", "labels": {"k1": "v1", "k2": "v2"}, "requireSameNode": true}, {"labels": {"k1": "v1", "k2": "v2"}}]`)
actual := SplitPodEnvToDeps("TEST_LIST")
expected := []PodDependency{
PodDependency{Namespace: "foo", Labels: map[string]string{
"k1": "v1",
"k2": "v2",
}},
}, RequireSameNode: true},
PodDependency{Namespace: "TEST_NAMESPACE", Labels: map[string]string{
"k1": "v1",
"k2": "v2",
}},
}, RequireSameNode: false},
}
if !reflect.DeepEqual(expected, actual) {