github/kubernetes
4
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Switch ephemeralcontainers SR to Pod Kind

Browse Source 
This changes the `/ephemeralcontainers` subresource of `/pods` to use
the `Pod` kind rather than `EphemeralContainers`.

When designing this API initially it seemed preferable to create a new
kind containing only the pod's ephemeral containers, similar to how
binding and scaling work.

It later became clear that this made admission control more difficult
because the controller wouldn't be presented with the entire Pod, so we
updated this to operate on the entire Pod, similar to how `/status`
works.
This commit is contained in:
Lee Verberne
2021-04-09 13:53:13 +02:00
parent dd72c4534c
commit d22dc5cb72
10 changed files with 164 additions and 213 deletions
Download Patch File Download Diff File Expand all files Collapse all files

163
test/integration/pods/pods_test.go
View File

@@ -235,31 +235,27 @@ func TestPodCreateEphemeralContainers(t *testing.T) {
// setUpEphemeralContainers creates a pod that has Ephemeral Containers. This is a two step
// process because Ephemeral Containers are not allowed during pod creation.
func setUpEphemeralContainers(podsClient typedv1.PodInterface, pod *v1.Pod, containers []v1.EphemeralContainer) error {
if _, err := podsClient.Create(context.TODO(), pod, metav1.CreateOptions{}); err != nil {
return fmt.Errorf("failed to create pod: %v", err)
func setUpEphemeralContainers(podsClient typedv1.PodInterface, pod *v1.Pod, containers []v1.EphemeralContainer) (*v1.Pod, error) {
result, err := podsClient.Create(context.TODO(), pod, metav1.CreateOptions{})
if err != nil {
return nil, fmt.Errorf("failed to create pod: %v", err)
}
if len(containers) == 0 {
return nil
return result, nil
}
pod.Spec.EphemeralContainers = containers
if _, err := podsClient.Update(context.TODO(), pod, metav1.UpdateOptions{}); err == nil {
return fmt.Errorf("unexpected allowed direct update of ephemeral containers during set up: %v", err)
return nil, fmt.Errorf("unexpected allowed direct update of ephemeral containers during set up: %v", err)
}
ec, err := podsClient.GetEphemeralContainers(context.TODO(), pod.Name, metav1.GetOptions{})
result, err = podsClient.UpdateEphemeralContainers(context.TODO(), pod.Name, pod, metav1.UpdateOptions{})
if err != nil {
return fmt.Errorf("unable to get ephemeral containers for test case set up: %v", err)
return nil, fmt.Errorf("failed to update ephemeral containers for test case set up: %v", err)
}
ec.EphemeralContainers = containers
if _, err = podsClient.UpdateEphemeralContainers(context.TODO(), pod.Name, ec, metav1.UpdateOptions{}); err != nil {
return fmt.Errorf("failed to update ephemeral containers for test case set up: %v", err)
}
return nil
return result, nil
}
func TestPodPatchEphemeralContainers(t *testing.T) {
@@ -303,12 +299,14 @@ func TestPodPatchEphemeralContainers(t *testing.T) {
original: nil,
patchType: types.StrategicMergePatchType,
patchBody: []byte(`{
"ephemeralContainers": [{
"name": "debugger1",
"image": "debugimage",
"imagePullPolicy": "Always",
"terminationMessagePolicy": "File"
}]
"spec": {
"ephemeralContainers": [{
"name": "debugger1",
"image": "debugimage",
"imagePullPolicy": "Always",
"terminationMessagePolicy": "File"
}]
}
}`),
valid: true,
},
@@ -317,12 +315,14 @@ func TestPodPatchEphemeralContainers(t *testing.T) {
original: nil,
patchType: types.MergePatchType,
patchBody: []byte(`{
"ephemeralContainers":[{
"name": "debugger1",
"image": "debugimage",
"imagePullPolicy": "Always",
"terminationMessagePolicy": "File"
}]
"spec": {
"ephemeralContainers":[{
"name": "debugger1",
"image": "debugimage",
"imagePullPolicy": "Always",
"terminationMessagePolicy": "File"
}]
}
}`),
valid: true,
},
@@ -330,15 +330,17 @@ func TestPodPatchEphemeralContainers(t *testing.T) {
name: "create single container (JSON)",
original: nil,
patchType: types.JSONPatchType,
// Because ephemeralContainers is optional, a JSON patch of an empty ephemeralContainers must add the
// list rather than simply appending to it.
patchBody: []byte(`[{
"op":"add",
"path":"/ephemeralContainers/-",
"value":{
"path":"/spec/ephemeralContainers",
"value":[{
"name":"debugger1",
"image":"debugimage",
"imagePullPolicy": "Always",
"terminationMessagePolicy": "File"
}
}]
}]`),
valid: true,
},
@@ -356,12 +358,14 @@ func TestPodPatchEphemeralContainers(t *testing.T) {
},
patchType: types.StrategicMergePatchType,
patchBody: []byte(`{
"ephemeralContainers":[{
"name": "debugger2",
"image": "debugimage",
"imagePullPolicy": "Always",
"terminationMessagePolicy": "File"
}]
"spec": {
"ephemeralContainers":[{
"name": "debugger2",
"image": "debugimage",
"imagePullPolicy": "Always",
"terminationMessagePolicy": "File"
}]
}
}`),
valid: true,
},
@@ -379,17 +383,19 @@ func TestPodPatchEphemeralContainers(t *testing.T) {
},
patchType: types.MergePatchType,
patchBody: []byte(`{
"ephemeralContainers":[{
"name": "debugger1",
"image": "debugimage",
"imagePullPolicy": "Always",
"terminationMessagePolicy": "File"
},{
"name": "debugger2",
"image": "debugimage",
"imagePullPolicy": "Always",
"terminationMessagePolicy": "File"
}]
"spec": {
"ephemeralContainers":[{
"name": "debugger1",
"image": "debugimage",
"imagePullPolicy": "Always",
"terminationMessagePolicy": "File"
},{
"name": "debugger2",
"image": "debugimage",
"imagePullPolicy": "Always",
"terminationMessagePolicy": "File"
}]
}
}`),
valid: true,
},
@@ -408,7 +414,7 @@ func TestPodPatchEphemeralContainers(t *testing.T) {
patchType: types.JSONPatchType,
patchBody: []byte(`[{
"op":"add",
"path":"/ephemeralContainers/-",
"path":"/spec/ephemeralContainers/-",
"value":{
"name":"debugger2",
"image":"debugimage",
@@ -431,9 +437,25 @@ func TestPodPatchEphemeralContainers(t *testing.T) {
},
},
patchType: types.MergePatchType,
patchBody: []byte(`{"ephemeralContainers":[]}`),
patchBody: []byte(`{"spec": {"ephemeralContainers":[]}}`),
valid: false,
},
{
name: "remove the single container (JSON)",
original: []v1.EphemeralContainer{
{
EphemeralContainerCommon: v1.EphemeralContainerCommon{
Name: "debugger1",
Image: "debugimage",
ImagePullPolicy: "Always",
TerminationMessagePolicy: "File",
},
},
},
patchType: types.JSONPatchType,
patchBody: []byte(`[{"op":"remove","path":"/ephemeralContainers/0"}]`),
valid: false, // disallowed by policy rather than patch semantics
},
{
name: "remove all containers (JSON)",
original: []v1.EphemeralContainer{
@@ -447,14 +469,14 @@ func TestPodPatchEphemeralContainers(t *testing.T) {
},
},
patchType: types.JSONPatchType,
patchBody: []byte(`[{"op":"remove","path":"/ephemeralContainers/0"}]`),
valid: false,
patchBody: []byte(`[{"op":"remove","path":"/ephemeralContainers"}]`),
valid: false, // disallowed by policy rather than patch semantics
},
}
for i, tc := range cases {
pod := testPod(fmt.Sprintf("ephemeral-container-test-%v", i))
if err := setUpEphemeralContainers(client.CoreV1().Pods(ns.Name), pod, tc.original); err != nil {
if _, err := setUpEphemeralContainers(client.CoreV1().Pods(ns.Name), pod, tc.original); err != nil {
t.Errorf("%v: %v", tc.name, err)
}
@@ -643,18 +665,13 @@ func TestPodUpdateEphemeralContainers(t *testing.T) {
}
for i, tc := range cases {
pod := testPod(fmt.Sprintf("ephemeral-container-test-%v", i))
if err := setUpEphemeralContainers(client.CoreV1().Pods(ns.Name), pod, tc.original); err != nil {
pod, err := setUpEphemeralContainers(client.CoreV1().Pods(ns.Name), testPod(fmt.Sprintf("ephemeral-container-test-%v", i)), tc.original)
if err != nil {
t.Errorf("%v: %v", tc.name, err)
}
ec, err := client.CoreV1().Pods(ns.Name).GetEphemeralContainers(context.TODO(), pod.Name, metav1.GetOptions{})
if err != nil {
t.Errorf("%v: unable to get ephemeral containers: %v", tc.name, err)
}
ec.EphemeralContainers = tc.update
if _, err := client.CoreV1().Pods(ns.Name).UpdateEphemeralContainers(context.TODO(), pod.Name, ec, metav1.UpdateOptions{}); tc.valid && err != nil {
pod.Spec.EphemeralContainers = tc.update
if _, err := client.CoreV1().Pods(ns.Name).UpdateEphemeralContainers(context.TODO(), pod.Name, pod, metav1.UpdateOptions{}); tc.valid && err != nil {
t.Errorf("%v: failed to update ephemeral containers: %v", tc.name, err)
} else if !tc.valid && err == nil {
t.Errorf("%v: unexpected allowed update to ephemeral containers", tc.name)
@@ -690,29 +707,21 @@ func TestPodEphemeralContainersDisabled(t *testing.T) {
},
},
}
if err := setUpEphemeralContainers(client.CoreV1().Pods(ns.Name), pod, nil); err != nil {
pod, err := setUpEphemeralContainers(client.CoreV1().Pods(ns.Name), pod, nil)
if err != nil {
t.Error(err)
}
ec, err := client.CoreV1().Pods(ns.Name).GetEphemeralContainers(context.TODO(), pod.Name, metav1.GetOptions{})
if err == nil {
t.Errorf("got nil error when getting ephemeral containers with feature disabled, wanted %q", metav1.StatusReasonNotFound)
} else if se := err.(*errors.StatusError); se.ErrStatus.Reason != metav1.StatusReasonNotFound {
t.Errorf("got error reason %q when getting ephemeral containers with feature disabled, want %q: %#v", se.ErrStatus.Reason, metav1.StatusReasonNotFound, se)
}
ec.EphemeralContainers = []v1.EphemeralContainer{
{
EphemeralContainerCommon: v1.EphemeralContainerCommon{
Name: "debugger",
Image: "debugimage",
ImagePullPolicy: "Always",
TerminationMessagePolicy: "File",
},
pod.Spec.EphemeralContainers = append(pod.Spec.EphemeralContainers, v1.EphemeralContainer{
EphemeralContainerCommon: v1.EphemeralContainerCommon{
Name: "debugger",
Image: "debugimage",
ImagePullPolicy: "Always",
TerminationMessagePolicy: "File",
},
}
})
if _, err := client.CoreV1().Pods(ns.Name).UpdateEphemeralContainers(context.TODO(), pod.Name, ec, metav1.UpdateOptions{}); err == nil {
if _, err := client.CoreV1().Pods(ns.Name).UpdateEphemeralContainers(context.TODO(), pod.Name, pod, metav1.UpdateOptions{}); err == nil {
t.Errorf("got nil error when updating ephemeral containers with feature disabled, wanted %q", metav1.StatusReasonNotFound)
} else if se := err.(*errors.StatusError); se.ErrStatus.Reason != metav1.StatusReasonNotFound {
t.Errorf("got error reason %q when updating ephemeral containers with feature disabled, want %q: %#v", se.ErrStatus.Reason, metav1.StatusReasonNotFound, se)