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.

Sign up for GitHub

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

Jump to bottom

Kubelet service links error #74529

Merged
merged 2 commits into from
Mar 5, 2019
Merged

Kubelet service links error #74529

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9788d40
Revert "bug: fix segfault when EnableServiceLinks is nil"
liggitt Feb 25, 2019
4ac08be
prevent panic on nil pod.spec.enableServiceLinks
liggitt Feb 25, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions pkg/kubelet/kubelet_pods.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -545,11 +545,11 @@ func (kl *Kubelet) getServiceEnvVarMap(ns string, enableServiceLinks bool) (map[

// Make the environment variables for a pod in the given namespace.
func (kl *Kubelet) makeEnvironmentVariables(pod *v1.Pod, container *v1.Container, podIP string) ([]kubecontainer.EnvVar, error) {
var result []kubecontainer.EnvVar
enableServiceLinks := v1.DefaultEnableServiceLinks
if pod.Spec.EnableServiceLinks != nil {
enableServiceLinks = *pod.Spec.EnableServiceLinks
if pod.Spec.EnableServiceLinks == nil {
return nil, fmt.Errorf("nil pod.spec.enableServiceLinks encountered, cannot construct envvars")
}

var result []kubecontainer.EnvVar
// Note: These are added to the docker Config, but are not included in the checksum computed
// by kubecontainer.HashContainer(...). That way, we can still determine whether an
// v1.Container is already running by its hash. (We don't want to restart a container just
Expand All @@ -559,7 +559,7 @@ func (kl *Kubelet) makeEnvironmentVariables(pod *v1.Pod, container *v1.Container
// To avoid this users can: (1) wait between starting a service and starting; or (2) detect
// missing service env var and exit and be restarted; or (3) use DNS instead of env vars
// and keep trying to resolve the DNS name of the service (recommended).
serviceEnv, err := kl.getServiceEnvVarMap(pod.Namespace, enableServiceLinks)
serviceEnv, err := kl.getServiceEnvVarMap(pod.Namespace, *pod.Spec.EnableServiceLinks)
if err != nil {
return result, err
}
Expand Down
84 changes: 55 additions & 29 deletions pkg/kubelet/kubelet_pods_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -427,10 +427,12 @@ func TestMakeEnvironmentVariables(t *testing.T) {
buildService("not-special", "kubernetes", "", "TCP", 8088),
}

trueValue := true
falseValue := false
testCases := []struct {
name string // the name of the test case
ns string // the namespace to generate environment for
enableServiceLinks bool // enabling service links
enableServiceLinks *bool // enabling service links
container *v1.Container // the container to use
masterServiceNs string // the namespace to read master service info from
nilLister bool // whether the lister should be nil
Expand All @@ -443,7 +445,7 @@ func TestMakeEnvironmentVariables(t *testing.T) {
{
name: "api server = Y, kubelet = Y",
ns: "test1",
enableServiceLinks: false,
enableServiceLinks: &falseValue,
container: &v1.Container{
Env: []v1.EnvVar{
{Name: "FOO", Value: "BAR"},
Expand Down Expand Up @@ -479,7 +481,7 @@ func TestMakeEnvironmentVariables(t *testing.T) {
{
name: "api server = Y, kubelet = N",
ns: "test1",
enableServiceLinks: false,
enableServiceLinks: &falseValue,
container: &v1.Container{
Env: []v1.EnvVar{
{Name: "FOO", Value: "BAR"},
Expand Down Expand Up @@ -508,7 +510,7 @@ func TestMakeEnvironmentVariables(t *testing.T) {
{
name: "api server = N; kubelet = Y",
ns: "test1",
enableServiceLinks: false,
enableServiceLinks: &falseValue,
container: &v1.Container{
Env: []v1.EnvVar{
{Name: "FOO", Value: "BAZ"},
Expand All @@ -530,7 +532,7 @@ func TestMakeEnvironmentVariables(t *testing.T) {
{
name: "api server = N; kubelet = Y; service env vars",
ns: "test1",
enableServiceLinks: true,
enableServiceLinks: &trueValue,
container: &v1.Container{
Env: []v1.EnvVar{
{Name: "FOO", Value: "BAZ"},
Expand Down Expand Up @@ -559,7 +561,7 @@ func TestMakeEnvironmentVariables(t *testing.T) {
{
name: "master service in pod ns",
ns: "test2",
enableServiceLinks: false,
enableServiceLinks: &falseValue,
container: &v1.Container{
Env: []v1.EnvVar{
{Name: "FOO", Value: "ZAP"},
Expand All @@ -581,7 +583,7 @@ func TestMakeEnvironmentVariables(t *testing.T) {
{
name: "master service in pod ns, service env vars",
ns: "test2",
enableServiceLinks: true,
enableServiceLinks: &trueValue,
container: &v1.Container{
Env: []v1.EnvVar{
{Name: "FOO", Value: "ZAP"},
Expand Down Expand Up @@ -610,7 +612,7 @@ func TestMakeEnvironmentVariables(t *testing.T) {
{
name: "pod in master service ns",
ns: "kubernetes",
enableServiceLinks: false,
enableServiceLinks: &falseValue,
container: &v1.Container{},
masterServiceNs: "kubernetes",
nilLister: false,
Expand All @@ -627,7 +629,7 @@ func TestMakeEnvironmentVariables(t *testing.T) {
{
name: "pod in master service ns, service env vars",
ns: "kubernetes",
enableServiceLinks: true,
enableServiceLinks: &trueValue,
container: &v1.Container{},
masterServiceNs: "kubernetes",
nilLister: false,
Expand All @@ -651,7 +653,7 @@ func TestMakeEnvironmentVariables(t *testing.T) {
{
name: "downward api pod",
ns: "downward-api",
enableServiceLinks: false,
enableServiceLinks: &falseValue,
container: &v1.Container{
Env: []v1.EnvVar{
{
Expand Down Expand Up @@ -724,7 +726,7 @@ func TestMakeEnvironmentVariables(t *testing.T) {
{
name: "env expansion",
ns: "test1",
enableServiceLinks: false,
enableServiceLinks: &falseValue,
container: &v1.Container{
Env: []v1.EnvVar{
{
Expand Down Expand Up @@ -820,7 +822,7 @@ func TestMakeEnvironmentVariables(t *testing.T) {
{
name: "env expansion, service env vars",
ns: "test1",
enableServiceLinks: true,
enableServiceLinks: &trueValue,
container: &v1.Container{
Env: []v1.EnvVar{
{
Expand Down Expand Up @@ -952,7 +954,7 @@ func TestMakeEnvironmentVariables(t *testing.T) {
{
name: "configmapkeyref_missing_optional",
ns: "test",
enableServiceLinks: false,
enableServiceLinks: &falseValue,
container: &v1.Container{
Env: []v1.EnvVar{
{
Expand All @@ -973,7 +975,7 @@ func TestMakeEnvironmentVariables(t *testing.T) {
{
name: "configmapkeyref_missing_key_optional",
ns: "test",
enableServiceLinks: false,
enableServiceLinks: &falseValue,
container: &v1.Container{
Env: []v1.EnvVar{
{
Expand Down Expand Up @@ -1004,7 +1006,7 @@ func TestMakeEnvironmentVariables(t *testing.T) {
{
name: "secretkeyref_missing_optional",
ns: "test",
enableServiceLinks: false,
enableServiceLinks: &falseValue,
container: &v1.Container{
Env: []v1.EnvVar{
{
Expand All @@ -1025,7 +1027,7 @@ func TestMakeEnvironmentVariables(t *testing.T) {
{
name: "secretkeyref_missing_key_optional",
ns: "test",
enableServiceLinks: false,
enableServiceLinks: &falseValue,
container: &v1.Container{
Env: []v1.EnvVar{
{
Expand Down Expand Up @@ -1056,7 +1058,7 @@ func TestMakeEnvironmentVariables(t *testing.T) {
{
name: "configmap",
ns: "test1",
enableServiceLinks: false,
enableServiceLinks: &falseValue,
container: &v1.Container{
EnvFrom: []v1.EnvFromSource{
{
Expand Down Expand Up @@ -1124,7 +1126,7 @@ func TestMakeEnvironmentVariables(t *testing.T) {
{
name: "configmap, service env vars",
ns: "test1",
enableServiceLinks: true,
enableServiceLinks: &trueValue,
container: &v1.Container{
EnvFrom: []v1.EnvFromSource{
{
Expand Down Expand Up @@ -1220,7 +1222,7 @@ func TestMakeEnvironmentVariables(t *testing.T) {
{
name: "configmap_missing",
ns: "test1",
enableServiceLinks: false,
enableServiceLinks: &falseValue,
container: &v1.Container{
EnvFrom: []v1.EnvFromSource{
{ConfigMapRef: &v1.ConfigMapEnvSource{LocalObjectReference: v1.LocalObjectReference{Name: "test-config-map"}}},
Expand All @@ -1232,7 +1234,7 @@ func TestMakeEnvironmentVariables(t *testing.T) {
{
name: "configmap_missing_optional",
ns: "test",
enableServiceLinks: false,
enableServiceLinks: &falseValue,
container: &v1.Container{
EnvFrom: []v1.EnvFromSource{
{ConfigMapRef: &v1.ConfigMapEnvSource{
Expand All @@ -1246,7 +1248,7 @@ func TestMakeEnvironmentVariables(t *testing.T) {
{
name: "configmap_invalid_keys",
ns: "test",
enableServiceLinks: false,
enableServiceLinks: &falseValue,
container: &v1.Container{
EnvFrom: []v1.EnvFromSource{
{ConfigMapRef: &v1.ConfigMapEnvSource{LocalObjectReference: v1.LocalObjectReference{Name: "test-config-map"}}},
Expand Down Expand Up @@ -1275,7 +1277,7 @@ func TestMakeEnvironmentVariables(t *testing.T) {
{
name: "configmap_invalid_keys_valid",
ns: "test",
enableServiceLinks: false,
enableServiceLinks: &falseValue,
container: &v1.Container{
EnvFrom: []v1.EnvFromSource{
{
Expand Down Expand Up @@ -1304,7 +1306,7 @@ func TestMakeEnvironmentVariables(t *testing.T) {
{
name: "secret",
ns: "test1",
enableServiceLinks: false,
enableServiceLinks: &falseValue,
container: &v1.Container{
EnvFrom: []v1.EnvFromSource{
{
Expand Down Expand Up @@ -1372,7 +1374,7 @@ func TestMakeEnvironmentVariables(t *testing.T) {
{
name: "secret, service env vars",
ns: "test1",
enableServiceLinks: true,
enableServiceLinks: &trueValue,
container: &v1.Container{
EnvFrom: []v1.EnvFromSource{
{
Expand Down Expand Up @@ -1468,7 +1470,7 @@ func TestMakeEnvironmentVariables(t *testing.T) {
{
name: "secret_missing",
ns: "test1",
enableServiceLinks: false,
enableServiceLinks: &falseValue,
container: &v1.Container{
EnvFrom: []v1.EnvFromSource{
{SecretRef: &v1.SecretEnvSource{LocalObjectReference: v1.LocalObjectReference{Name: "test-secret"}}},
Expand All @@ -1480,7 +1482,7 @@ func TestMakeEnvironmentVariables(t *testing.T) {
{
name: "secret_missing_optional",
ns: "test",
enableServiceLinks: false,
enableServiceLinks: &falseValue,
container: &v1.Container{
EnvFrom: []v1.EnvFromSource{
{SecretRef: &v1.SecretEnvSource{
Expand All @@ -1494,7 +1496,7 @@ func TestMakeEnvironmentVariables(t *testing.T) {
{
name: "secret_invalid_keys",
ns: "test",
enableServiceLinks: false,
enableServiceLinks: &falseValue,
container: &v1.Container{
EnvFrom: []v1.EnvFromSource{
{SecretRef: &v1.SecretEnvSource{LocalObjectReference: v1.LocalObjectReference{Name: "test-secret"}}},
Expand Down Expand Up @@ -1523,7 +1525,7 @@ func TestMakeEnvironmentVariables(t *testing.T) {
{
name: "secret_invalid_keys_valid",
ns: "test",
enableServiceLinks: false,
enableServiceLinks: &falseValue,
container: &v1.Container{
EnvFrom: []v1.EnvFromSource{
{
Expand All @@ -1549,6 +1551,30 @@ func TestMakeEnvironmentVariables(t *testing.T) {
},
},
},
{
name: "nil_enableServiceLinks",
ns: "test",
enableServiceLinks: nil,
container: &v1.Container{
EnvFrom: []v1.EnvFromSource{
{
Prefix: "p_",
SecretRef: &v1.SecretEnvSource{LocalObjectReference: v1.LocalObjectReference{Name: "test-secret"}},
},
},
},
masterServiceNs: "",
secret: &v1.Secret{
ObjectMeta: metav1.ObjectMeta{
Namespace: "test1",
Name: "test-secret",
},
Data: map[string][]byte{
"1234.name": []byte("abc"),
},
},
expectedError: true,
},
}

for _, tc := range testCases {
Expand Down Expand Up @@ -1595,7 +1621,7 @@ func TestMakeEnvironmentVariables(t *testing.T) {
Spec: v1.PodSpec{
ServiceAccountName: "special",
NodeName: "node-name",
EnableServiceLinks: &tc.enableServiceLinks,
EnableServiceLinks: tc.enableServiceLinks,
},
}
podIP := "1.2.3.4"
Expand Down