github/kubernetes
1
0
Fork 0
mirror of https://github.com/k3s-io/kubernetes.git synced 2025-07-22 19:31:44 +00:00
Code Issues Projects Releases Wiki Activity

Merge pull request #49388 from HotelsDotCom/feature/Dynamic-env-in-subpath

Browse Source 
Automatic merge from submit-queue (batch tested with PRs 58920, 58327, 60577, 49388, 62306). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

Dynamic env in subpath - Fixes Issue 48677

**What this PR does / why we need it**:

**Which issue this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close that issue when PR gets merged)*: fixes #48677

**Special notes for your reviewer**:

**Release note**:

```release-note
Adds the VolumeSubpathEnvExpansion alpha feature to support environment variable expansion
Sub-paths cannot be mounted with a dynamic volume mount name.
This fix provides environment variable expansion to sub paths
This reduces the need to manage symbolic linking within sidecar init containers to achieve the same goal  
```
This commit is contained in:
Kubernetes Submit Queue 2018-05-30 16:09:31 -07:00 committed by GitHub
commit 6b2fc7cb75
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 328 additions and 12 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
pkg/features/kube_features.go
View File

@ -286,6 +286,13 @@ const (
//
// Extend the default scheduler to be aware of volume topology and handle PV provisioning
DynamicProvisioningScheduling utilfeature.Feature = "DynamicProvisioningScheduling"
// owner: @kevtaylor
// alpha: v1.11
//
// Allow subpath environment variable substitution
// Only applicable if the VolumeSubpath feature is also enabled
VolumeSubpathEnvExpansion utilfeature.Feature = "VolumeSubpathEnvExpansion"
)
func init() {
@ -335,6 +342,7 @@ var defaultKubernetesFeatureGates = map[utilfeature.Feature]utilfeature.FeatureS
VolumeSubpath: {Default: true, PreRelease: utilfeature.GA},
BalanceAttachedNodeVolumes: {Default: false, PreRelease: utilfeature.Alpha},
DynamicProvisioningScheduling: {Default: false, PreRelease: utilfeature.Alpha},
VolumeSubpathEnvExpansion: {Default: false, PreRelease: utilfeature.Alpha},
// inherited features from generic apiserver, relisted here to get a conflict if it is changed
// unintentionally on either side:

5
pkg/kubelet/container/helpers.go
View File

@ -133,6 +133,11 @@ func ExpandContainerCommandOnlyStatic(containerCommand []string, envs []v1.EnvVa
return command
}
func ExpandContainerVolumeMounts(mount v1.VolumeMount, envs []EnvVar) (expandedSubpath string) {
mapping := expansion.MappingFuncFor(EnvVarsToMap(envs))
return expansion.Expand(mount.SubPath, mapping)
}
func ExpandContainerCommandAndArgs(container *v1.Container, envs []EnvVar) (command []string, args []string) {
mapping := expansion.MappingFuncFor(EnvVarsToMap(envs))

108
pkg/kubelet/container/helpers_test.go
View File

@ -138,6 +138,114 @@ func TestExpandCommandAndArgs(t *testing.T) {
}
}
func TestExpandVolumeMountsWithSubpath(t *testing.T) {
cases := []struct {
name string
container *v1.Container
envs []EnvVar
expectedSubPath string
expectedMountPath string
}{
{
name: "subpath with no expansion",
container: &v1.Container{
VolumeMounts: []v1.VolumeMount{{SubPath: "foo"}},
},
expectedSubPath: "foo",
expectedMountPath: "",
},
{
name: "volumes with expanded subpath",
container: &v1.Container{
VolumeMounts: []v1.VolumeMount{{SubPath: "foo/$(POD_NAME)"}},
},
envs: []EnvVar{
{
Name: "POD_NAME",
Value: "bar",
},
},
expectedSubPath: "foo/bar",
expectedMountPath: "",
},
{
name: "volumes expanded with empty subpath",
container: &v1.Container{
VolumeMounts: []v1.VolumeMount{{SubPath: ""}},
},
envs: []EnvVar{
{
Name: "POD_NAME",
Value: "bar",
},
},
expectedSubPath: "",
expectedMountPath: "",
},
{
name: "volumes expanded with no envs subpath",
container: &v1.Container{
VolumeMounts: []v1.VolumeMount{{SubPath: "/foo/$(POD_NAME)"}},
},
expectedSubPath: "/foo/$(POD_NAME)",
expectedMountPath: "",
},
{
name: "volumes expanded with leading environment variable",
container: &v1.Container{
VolumeMounts: []v1.VolumeMount{{SubPath: "$(POD_NAME)/bar"}},
},
envs: []EnvVar{
{
Name: "POD_NAME",
Value: "foo",
},
},
expectedSubPath: "foo/bar",
expectedMountPath: "",
},
{
name: "volumes with volume and subpath",
container: &v1.Container{
VolumeMounts: []v1.VolumeMount{{MountPath: "/foo", SubPath: "$(POD_NAME)/bar"}},
},
envs: []EnvVar{
{
Name: "POD_NAME",
Value: "foo",
},
},
expectedSubPath: "foo/bar",
expectedMountPath: "/foo",
},
{
name: "volumes with volume and no subpath",
container: &v1.Container{
VolumeMounts: []v1.VolumeMount{{MountPath: "/foo"}},
},
envs: []EnvVar{
{
Name: "POD_NAME",
Value: "foo",
},
},
expectedSubPath: "",
expectedMountPath: "/foo",
},
}
for _, tc := range cases {
actualSubPath := ExpandContainerVolumeMounts(tc.container.VolumeMounts[0], tc.envs)
if e, a := tc.expectedSubPath, actualSubPath; !reflect.DeepEqual(e, a) {
t.Errorf("%v: unexpected subpath; expected %v, got %v", tc.name, e, a)
}
if e, a := tc.expectedMountPath, tc.container.VolumeMounts[0].MountPath; !reflect.DeepEqual(e, a) {
t.Errorf("%v: unexpected mountpath; expected %v, got %v", tc.name, e, a)
}
}
}
func TestShouldContainerBeRestarted(t *testing.T) {
pod := &v1.Pod{
ObjectMeta: metav1.ObjectMeta{

22
pkg/kubelet/kubelet_pods.go
View File

@ -129,7 +129,8 @@ func (kl *Kubelet) makeBlockVolumes(pod *v1.Pod, container *v1.Container, podVol
}
// makeMounts determines the mount points for the given container.
func makeMounts(pod *v1.Pod, podDir string, container *v1.Container, hostName, hostDomain, podIP string, podVolumes kubecontainer.VolumeMap, mounter mountutil.Interface) ([]kubecontainer.Mount, func(), error) {
func makeMounts(pod *v1.Pod, podDir string, container *v1.Container, hostName, hostDomain, podIP string, podVolumes kubecontainer.VolumeMap, mounter mountutil.Interface, expandEnvs []kubecontainer.EnvVar) ([]kubecontainer.Mount, func(), error) {
// Kubernetes only mounts on /etc/hosts if:
// - container is not an infrastructure (pause) container
// - container is not already mounting on /etc/hosts
@ -166,6 +167,11 @@ func makeMounts(pod *v1.Pod, podDir string, container *v1.Container, hostName, h
return nil, cleanupAction, fmt.Errorf("volume subpaths are disabled")
}
// Expand subpath variables
if utilfeature.DefaultFeatureGate.Enabled(features.VolumeSubpathEnvExpansion) {
mount.SubPath = kubecontainer.ExpandContainerVolumeMounts(mount, expandEnvs)
}
if filepath.IsAbs(mount.SubPath) {
return nil, cleanupAction, fmt.Errorf("error SubPath `%s` must not be an absolute path", mount.SubPath)
}
@ -454,18 +460,18 @@ func (kl *Kubelet) GenerateRunContainerOptions(pod *v1.Pod, container *v1.Contai
opts.Devices = append(opts.Devices, blkVolumes...)
}
mounts, cleanupAction, err := makeMounts(pod, kl.getPodDir(pod.UID), container, hostname, hostDomainName, podIP, volumes, kl.mounter)
envs, err := kl.makeEnvironmentVariables(pod, container, podIP)
if err != nil {
return nil, nil, err
}
opts.Envs = append(opts.Envs, envs...)
mounts, cleanupAction, err := makeMounts(pod, kl.getPodDir(pod.UID), container, hostname, hostDomainName, podIP, volumes, kl.mounter, opts.Envs)
if err != nil {
return nil, cleanupAction, err
}
opts.Mounts = append(opts.Mounts, mounts...)
envs, err := kl.makeEnvironmentVariables(pod, container, podIP)
if err != nil {
return nil, cleanupAction, err
}
opts.Envs = append(opts.Envs, envs...)
// Disabling adding TerminationMessagePath on Windows as these files would be mounted as docker volume and
// Docker for Windows has a bug where only directories can be mounted
if len(container.TerminationMessagePath) != 0 && runtime.GOOS != "windows" {

6
pkg/kubelet/kubelet_pods_test.go
View File

@ -273,7 +273,7 @@ func TestMakeMounts(t *testing.T) {
return
}
mounts, _, err := makeMounts(&pod, "/pod", &tc.container, "fakepodname", "", "", tc.podVolumes, fm)
mounts, _, err := makeMounts(&pod, "/pod", &tc.container, "fakepodname", "", "", tc.podVolumes, fm, nil)
// validate only the error if we expect an error
if tc.expectErr {
@ -296,7 +296,7 @@ func TestMakeMounts(t *testing.T) {
t.Errorf("Failed to enable feature gate for MountPropagation: %v", err)
return
}
mounts, _, err = makeMounts(&pod, "/pod", &tc.container, "fakepodname", "", "", tc.podVolumes, fm)
mounts, _, err = makeMounts(&pod, "/pod", &tc.container, "fakepodname", "", "", tc.podVolumes, fm, nil)
if !tc.expectErr {
expectedPrivateMounts := []kubecontainer.Mount{}
for _, mount := range tc.expectedMounts {
@ -357,7 +357,7 @@ func TestDisabledSubpath(t *testing.T) {
defer utilfeature.DefaultFeatureGate.Set("VolumeSubpath=true")
for name, test := range cases {
_, _, err := makeMounts(&pod, "/pod", &test.container, "fakepodname", "", "", podVolumes, fm)
_, _, err := makeMounts(&pod, "/pod", &test.container, "fakepodname", "", "", podVolumes, fm, nil)
if err != nil && !test.expectError {
t.Errorf("test %v failed: %v", name, err)
}

2
pkg/kubelet/kubelet_pods_windows_test.go
View File

@ -66,7 +66,7 @@ func TestMakeMountsWindows(t *testing.T) {
}
fm := &mount.FakeMounter{}
mounts, _, _ := makeMounts(&pod, "/pod", &container, "fakepodname", "", "", podVolumes, fm)
mounts, _, _ := makeMounts(&pod, "/pod", &container, "fakepodname", "", "", podVolumes, fm, nil)
expectedMounts := []kubecontainer.Mount{
{

189
test/e2e/common/expansion.go
View File

@ -19,8 +19,13 @@ package common
import (
"k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/fields"
"k8s.io/apimachinery/pkg/util/uuid"
"k8s.io/kubernetes/test/e2e/framework"
. "github.com/onsi/ginkgo"
. "github.com/onsi/gomega"
"time"
)
// These tests exercise the Kubernetes expansion syntax $(VAR).
@ -144,4 +149,188 @@ var _ = framework.KubeDescribe("Variable Expansion", func() {
"test-value",
})
})
/*
Testname: var-expansion-subpath
Description: Make sure a container's subpath can be set using an
expansion of environment variables.
*/
It("should allow substituting values in a volume subpath [Feature:VolumeSubpathEnvExpansion][NodeAlphaFeature:VolumeSubpathEnvExpansion]", func() {
podName := "var-expansion-" + string(uuid.NewUUID())
pod := &v1.Pod{
ObjectMeta: metav1.ObjectMeta{
Name: podName,
Labels: map[string]string{"name": podName},
},
Spec: v1.PodSpec{
Containers: []v1.Container{
{
Name: "dapi-container",
Image: busyboxImage,
Command: []string{"sh", "-c", "test -d /testcontainer/" + podName + ";echo $?"},
Env: []v1.EnvVar{
{
Name: "POD_NAME",
Value: podName,
},
},
VolumeMounts: []v1.VolumeMount{
{
Name: "workdir1",
MountPath: "/logscontainer",
SubPath: "$(POD_NAME)",
},
{
Name: "workdir2",
MountPath: "/testcontainer",
},
},
},
},
RestartPolicy: v1.RestartPolicyNever,
Volumes: []v1.Volume{
{
Name: "workdir1",
VolumeSource: v1.VolumeSource{
HostPath: &v1.HostPathVolumeSource{Path: "/tmp"},
},
},
{
Name: "workdir2",
VolumeSource: v1.VolumeSource{
HostPath: &v1.HostPathVolumeSource{Path: "/tmp"},
},
},
},
},
}
f.TestContainerOutput("substitution in volume subpath", pod, 0, []string{
"0",
})
})
/*
Testname: var-expansion-subpath-with-backticks
Description: Make sure a container's subpath can not be set using an
expansion of environment variables when backticks are supplied.
*/
It("should fail substituting values in a volume subpath with backticks [Feature:VolumeSubpathEnvExpansion][NodeAlphaFeature:VolumeSubpathEnvExpansion][Slow]", func() {
podName := "var-expansion-" + string(uuid.NewUUID())
pod := &v1.Pod{
ObjectMeta: metav1.ObjectMeta{
Name: podName,
Labels: map[string]string{"name": podName},
},
Spec: v1.PodSpec{
Containers: []v1.Container{
{
Name: "dapi-container",
Image: busyboxImage,
Env: []v1.EnvVar{
{
Name: "POD_NAME",
Value: "..",
},
},
VolumeMounts: []v1.VolumeMount{
{
Name: "workdir1",
MountPath: "/logscontainer",
SubPath: "$(POD_NAME)",
},
},
},
},
RestartPolicy: v1.RestartPolicyNever,
Volumes: []v1.Volume{
{
Name: "workdir1",
VolumeSource: v1.VolumeSource{
EmptyDir: &v1.EmptyDirVolumeSource{},
},
},
},
},
}
// Pod should fail
testPodFailSubpath(f, pod, "SubPath `..`: must not contain '..'")
})
/*
Testname: var-expansion-subpath-with-absolute-path
Description: Make sure a container's subpath can not be set using an
expansion of environment variables when absoluete path is supplied.
*/
It("should fail substituting values in a volume subpath with absolute path [Feature:VolumeSubpathEnvExpansion][NodeAlphaFeature:VolumeSubpathEnvExpansion][Slow]", func() {
podName := "var-expansion-" + string(uuid.NewUUID())
pod := &v1.Pod{
ObjectMeta: metav1.ObjectMeta{
Name: podName,
Labels: map[string]string{"name": podName},
},
Spec: v1.PodSpec{
Containers: []v1.Container{
{
Name: "dapi-container",
Image: busyboxImage,
Env: []v1.EnvVar{
{
Name: "POD_NAME",
Value: "/tmp",
},
},
VolumeMounts: []v1.VolumeMount{
{
Name: "workdir1",
MountPath: "/logscontainer",
SubPath: "$(POD_NAME)",
},
},
},
},
RestartPolicy: v1.RestartPolicyNever,
Volumes: []v1.Volume{
{
Name: "workdir1",
VolumeSource: v1.VolumeSource{
EmptyDir: &v1.EmptyDirVolumeSource{},
},
},
},
},
}
// Pod should fail
testPodFailSubpath(f, pod, "SubPath `/tmp` must not be an absolute path")
})
})
func testPodFailSubpath(f *framework.Framework, pod *v1.Pod, errorText string) {
pod, err := f.ClientSet.CoreV1().Pods(f.Namespace.Name).Create(pod)
Expect(err).ToNot(HaveOccurred(), "while creating pod")
defer func() {
framework.DeletePodWithWait(f, f.ClientSet, pod)
}()
err = framework.WaitTimeoutForPodRunningInNamespace(f.ClientSet, pod.Name, pod.Namespace, 30*time.Second)
Expect(err).To(HaveOccurred(), "while waiting for pod to be running")
selector := fields.Set{
"involvedObject.kind": "Pod",
"involvedObject.name": pod.Name,
"involvedObject.namespace": f.Namespace.Name,
"reason": "Failed",
}.AsSelector().String()
options := metav1.ListOptions{FieldSelector: selector}
events, err := f.ClientSet.CoreV1().Events(f.Namespace.Name).List(options)
Expect(err).NotTo(HaveOccurred(), "while getting pod events")
Expect(len(events.Items)).NotTo(Equal(0), "no events found")
Expect(events.Items[0].Message).To(ContainSubstring(errorText), "subpath error not found")
}