Use v1.Container in Debug Containers API #1269

verb · 2017-10-25T15:24:11Z

This proposes an API for Debug Containers that's idiomatic for Kubernetes and uses the existing Container definition. This makes it easier for admission controllers to gate these updates.

The previous Debug Containers proposal avoided changing the core API as much as possible.

verb · 2017-10-25T15:25:26Z

/sig node auth
/assign @lavalamp @liggitt @thockin

liggitt · 2017-10-25T15:33:17Z

contributors/design-proposals/node/troubleshoot-running-pods.md

+}
+```
+
+Note that Debug Containers are not regular containers and should not be used to


s/debug/ephemeral/g, based on where the review of the last PR ended up?

We've ended up in a situation where this is "Debug Containers" to sig-node and the kubelet and "Ephemeral Containers" In the API. I don't think the matter was resolved in the last PR, but I hope we can settle it here.

liggitt · 2017-10-25T15:36:54Z

contributors/design-proposals/node/troubleshoot-running-pods.md

+will not be allowed for Debug Containers. A request will be rejected if any
+field is set other than the following whitelisted fields: `Name`, `Image`,
+`Command`, `Args`, `WorkingDir`, `Env`, `EnvFrom`, `ImagePullPolicy`,
+`SecurityContext`. `TTY` and `Stdin` are always enabled for Debug Containers and


Running an ephemeral container with non-interactive diagnostics and viewing the log would not require tty/stdin. Are there any containers where enabling tty/stdin would cause problems?

Yeah I don't see why you'd mandate those features on?

iirc this came from a conversation from @thockin and @vishh about how having configurable stdin/tty has caused confusion for users and been more trouble than it was worth. I don't feel strongly.

Some containers don't require tty/stdin, but is there a downside to them being always on? This is a new feature, so there aren't backwards compatibility concerns here.

liggitt · 2017-10-25T15:40:36Z

contributors/design-proposals/node/troubleshoot-running-pods.md

+We can either using an existing subresource like `/exec` and have the call
+stream or not based on `PodExecOptions`, or we can create a new subresource that
+never streams. For the sake of consistency, we will create a new subresource
+named `/debug`. This has the added benefit of being able to entirely hide


A separate subresource makes a lot of sense to me. Adding a newly defined container (new image, new security settings, envvars, etc) is very different from exec, defining the API we want for it seems like a good idea.

/execEphemeralContainer -- anything other than /debug.

A new endpoint seems good for the super-security conscious at the expense of making everything more complicated for everyone else. I don't have a strong opinion.

/execEphemeralContainer -- anything other than /debug.

probably want lowercase resources, but yes

A new endpoint seems good for the super-security conscious at the expense of making everything more complicated for everyone else. I don't have a strong opinion.

it's good for isolating the function for policy purposes, but also lets us support envvars, capabilities, and other useful knobs for shaping the environment of the ephemeral container in a way consistent with existing pods

probably want lowercase resources

er, right

it's good for isolating the function for policy purposes, but also lets us support envvars, capabilities, and other useful knobs for shaping the environment of the ephemeral container in a way consistent with existing pods

I stand by my previous response :)

liggitt · 2017-10-25T15:44:38Z

contributors/design-proposals/node/troubleshoot-running-pods.md

+1.  A `POST` of a `PodDebugContainer` to
+    `/api/v1/namespaces/$NS/pods/$POD_NAME/debug` to create a Debug Container
+    running in pod `$POD_NAME`.
+1.  A `DELETE` of `/api/v1/namespaces/$NS/pods/$POD_NAME/debug/$NAME` will stop


Named subresources are a thing I have wanted, but not something we currently have, which could make this tricky. I'd see this as nice-to-have, but not essential (as noted in the previous PR, things like exec sessions are not currently able to be individually terminated via the API)

I suggest allowing only one ephemeral container at a time for the moment, and then you don't need the additional level? We don't support this at the moment.

alternately, DELETE of /api/v1/namespaces/$NS/pods/$POD_NAME/ephemeralcontainers stops all ephemeral containers

Agree we can live without it for now

SGTM, I will omit it for now.

lavalamp · 2017-10-25T17:36:07Z

contributors/design-proposals/node/troubleshoot-running-pods.md

+`SecurityContext`. `TTY` and `Stdin` are always enabled for Debug Containers and
+will be ignored.
+
+The new `/debug` subresource allows the following:


s//debug/something else/g

lavalamp · 2017-10-25T17:38:18Z

contributors/design-proposals/node/troubleshoot-running-pods.md

+the existing attach endpoint, `/api/v1/namespaces/$NS/pods/$POD_NAME/attach`.
+Note that any output of the new container between its creation and subsequent
+attach will not be replayed and can only be viewed using `kubectl log`.
+


Will the ephemeral container appear in the response when I GET the pod?

only the status of it, not the spec

That's fine.

lavalamp · 2017-10-25T17:39:07Z

contributors/design-proposals/node/troubleshoot-running-pods.md

@@ -441,10 +448,11 @@ The kubelet exposes the new functionality in its existing `/exec/` endpoint.
 `ServeExec()` constructs a `v1.Container` based on `PodExecOptions`, calls
 `RunDebugContainer()`, and performs the attach.

-##### Step 4: Reporting DebugStatus
+##### Step 4: Reporting EphemeralContainerStatus


Ah, looks like the answer is yes.

lavalamp · 2017-10-25T17:39:50Z

This LGTM to me if you can fix the naming (consistency + debug -> ephemeral).

liggitt · 2017-10-25T17:43:07Z

contributors/design-proposals/node/troubleshoot-running-pods.md

-Note that `Command` and `Args` must be tracked in the status object because
-there is no spec for Debug Containers or exec. These must either be made
+Note that `Command` and `Args` would have to be tracked in the status object
+because there is no spec for Debug Containers or exec. These must either be made
 available by the runtime or tracked by the kubelet. For Debug Containers this


doesn't the whole ephemeral container have to be tracked by the kubelet anyway?

why just the command and args? admission uses the definitions of containers to determine whether a user can do things like exec/attach to those containers ("cannot exec into a pod with containers running as root", etc). Container status alone (EphemeralContainerStatuses []v1.ContainerStatus) wouldn't include enough information about the ephemeral containers to make that determination.

what if the submitted ephemeral container and its status were both made available in pod status?

type PodStatus struct { ... EphemeralContainers []v1.EphemeralContainer } type EphemeralContainer struct { Spec v1.Container Status v1.ContainerStatus }

The kubelet currently has no method of keeping state of the v1.Container across restarts. After a restart, it can reconstruct a minimum v1.Container based on runtime labels, though. This could (but currently doesn't) include Command and Args, which can be queried from the run time.

Including the spec in the PodStatus would be pretty great, though, so it might be worth solving. @lavalamp what do you think about this?

Remind me why this got reopened for discussion? I feel like if we keep going down this route we will end up at my original position of adding a type of ephemeral to containers, and allowing users to add or remove such containers from the spec.

I think sig-auth just wants to make this works well with admission controllers and has a strong preference for using v1.Container, for many good reasons.

@liggitt This is an important feature to sig-node, but we've been stuck for about a year. If a v1.Container is available to admission controllers for the creation request, can you live with only ContainerStatus for attach, at least for alpha? We don't need to allow exec.

The kubelet currently has no method of keeping state of the v1.Container across restarts.

I expected the kubelet to:

update pod status with the ephemeral container definition/status as it created the container

update the ephemeral container status by name from observed container state during normal sync

remove the ephemeral container from the pod status when the ephemeral container was removed

None of that requires the kubelet remembering the ephemeral container definition beyond the initial create, right?

If a v1.Container is available to admission controllers for the creation request, can you live with only ContainerStatus for attach, at least for alpha? We don't need to allow exec.

Two thoughts:

attach (which is the intended use case here) requires almost identical protections to exec ("cannot attach to a pod with containers running as root", etc). If the create of the ephemeral container and the attach to it are separate API calls, but the container definition is unavailable at attach time, I'm unsure how that would be implementable.

the structure of an ephemeral container field in the pod status could not be changed in an incompatible way (if the schema of the alpha field must change in an incompatible way, a new field name must be used), so if we started with EphemeralContainerStatuses []v1.ContainerStatus, that couldn't grow to include the container definition as alpha progressed.

liggitt · 2017-10-25T17:50:12Z

contributors/design-proposals/node/troubleshoot-running-pods.md

+build services. They have no guarantees and most of the fields of `v1.Container`
+will not be allowed for Debug Containers. A request will be rejected if any
+field is set other than the following whitelisted fields: `Name`, `Image`,
+`Command`, `Args`, `WorkingDir`, `Env`, `EnvFrom`, `ImagePullPolicy`,


are HostNetwork/HostPID/HostIPC inherited from the pod spec?

should also document the fact that not all v1.Container fields are allowed in a way that doesn't preclude supporting new container fields that make sense in the future, and we should reject requests that set disallowed fields rather than silently drop them.

Yeah, I think it's a possibility that we'll want to support more of these fields as time goes on.

Actually, the namespaces will be inherited from the target container. Originally it was going to be inherited from the pod spec. This is something that changed a few weeks ago because we're now going to support isolated PID namespaces indefinitely, and it's not reflected in this API alternative.

With exec++ the target container was provided in PodExecOptions, but switching the API means we either have to provide the target in the URI, the posted EphemeralContainer or... query params? I haven't looked into this at all yet.

thockin

I'm not really the constituency for this level of review, but I don't hate it (actually kind of prefer it). I still dislike that we reuse the same type spec and just allow some-but not-all fields, but we do that in other places, so meh.

thockin · 2017-10-25T21:49:30Z

contributors/design-proposals/node/troubleshoot-running-pods.md

+1.  A `POST` of a `PodDebugContainer` to
+    `/api/v1/namespaces/$NS/pods/$POD_NAME/debug` to create a Debug Container
+    running in pod `$POD_NAME`.
+1.  A `DELETE` of `/api/v1/namespaces/$NS/pods/$POD_NAME/debug/$NAME` will stop


Agree we can live without it for now

thockin · 2017-10-25T21:50:30Z

contributors/design-proposals/node/troubleshoot-running-pods.md

+```
+// EphemeralContainer describes a container to attach to a running pod for troubleshooting.
+type EphemeralContainer struct {
+        metav1.TypeMeta


I assume the name has to be unique within the pod?

Yes, but the container name would be embedded in the ContainerSpec.

This type probably doesn't need an ObjectMeta--I would try it that way and see if any of the encoding stack gives you a problem.

Yes, it must be unique within the pod because it will be used for attach, logs, etc. Thanks for the tip on ObjectMeta, I will try it.

liggitt · 2017-10-26T05:05:28Z

contributors/design-proposals/node/troubleshoot-running-pods.md

-Note that `Command` and `Args` must be tracked in the status object because
-there is no spec for Debug Containers or exec. These must either be made
+Note that `Command` and `Args` would have to be tracked in the status object
+because there is no spec for Debug Containers or exec. These must either be made
 available by the runtime or tracked by the kubelet. For Debug Containers this
 could be stored as runtime labels, but the kubelet currently has no method of
 storing state across restarts for exec. Solving this problem for exec is out of
 scope for Debug Containers, but we will look for a solution as we implement this
 feature.


@smarterclayton was there something were you mentioning that allowed connecting data to running containers?

k8s-github-robot · 2017-11-26T08:06:50Z

This PR hasn't been active in 30 days. It will be closed in 59 days (Jan 24, 2018).

cc @lavalamp @liggitt @thockin @verb

You can add 'keep-open' label to prevent this from happening, or add a comment to keep it open another 90 days

smarterclayton · 2017-12-31T20:48:08Z

I'd like to get this into the alpha slot for 1.10.

liggitt · 2018-01-02T18:29:17Z

contributors/design-proposals/node/troubleshoot-running-pods.md

+        // If specified, the ephemeral container will be started in the namespaces
+        // of this TargetContainerName. If omitted the container will inherit the
+        // behavior of the pod for namespaces.
+        TargetContainerName string `json:"spec" ...`


copy/pasta, json:"targetContainerName"

liggitt · 2018-01-02T18:32:31Z

contributors/design-proposals/node/troubleshoot-running-pods.md

@@ -437,14 +453,15 @@ Containers unless the sandbox is restarted.

 ##### Step 3: kubelet API changes

-The kubelet exposes the new functionality in its existing `/exec/` endpoint.
+The kubelet exposes the new functionality in a new `/podDebug/` endpoint.


needs updating to indicate it is given the v1.Container and optional target container name, and doesn't perform the attach (the attach is a separate call).

does this call block until the new container is picked up by the sync loop or the ephemeral spec/status is reported back to the pod API object? I assume the kubelet is responsible for rejecting racing attempts to run two debug containers with the same name in a given pod... where does that happen?

liggitt · 2018-01-02T19:28:36Z

contributors/design-proposals/node/troubleshoot-running-pods.md

@@ -387,14 +407,10 @@ basic functionality:
 *   `kubectl describe pod` will list status of debug containers running in a pod

 Functionality will be hidden behind an alpha feature flag and disabled by
-default. The following are explicitly out of scope for the 1.9 alpha release:
+default. The following are explicitly out of scope for the 1.10 alpha release:

 *   Exited Debug Containers will be garbage collected as regular containers and
    may disappear from the list of Debug Container Statuses.


Exited Debug Containers will be garbage collected as regular containers and may disappear from the list of Debug Container Statuses.

harmonize this with the "The list of EphemeralContainerStatuses is never truncated." statement below

liggitt · 2018-01-07T17:50:46Z

I'd like to get this into the alpha slot for 1.10.

Agree. API-wise, this seems to have the info I would expect now (v1.Container in debug request, container spec and status persisted to allow gating attach requests).

More detail is needed for information flow and sequence (which component persists which bits, synchronously or asynchronously, etc) and some edge cases (what happens on kubelet restart? kubelet+CRI restart?), but that can be a further iteration.

verb · 2018-01-10T16:29:01Z

@liggitt awesome, thanks for the quick feedback. Since this looks to be heading in the right direction for the API, I'll double check it's still acceptable to sig-node and then do a more complete rewrite addressing your comments.

lavalamp · 2018-05-31T21:02:56Z

contributors/design-proposals/node/troubleshoot-running-pods.md

+
+An Ephemeral Container is not part of the pod specification as it's not part of
+the declared state of the pod, but we describe it using the same primitives as
+in `PodSpec`. An `EphemeralContainer` contains a Spec, a Status and a Target:


what verbs does the subresource support?

lavalamp · 2018-05-31T21:05:04Z

contributors/design-proposals/node/troubleshoot-running-pods.md

+        ...
+        // List of user-initiated ephemeral containers that have been run in this pod.
+        // +optional
+        EphemeralContainers []EphemeralContainer `json:"commands,omitempty" protobuf:"bytes,11,rep,name=ephemeralContainers"`


I still strongly disagree with listing a container spec anywhere inside pod status.

I also maintain that you cannot use the same type as the subresource for this in any case-- no typemeta/metadata makes sense here.

lavalamp · 2018-05-31T21:08:38Z

Sorry, I think I already said these things on the PR. This came up in my email and I thought it was a recent update but now I think it hasn't been updated recently. Sorry for the noise.

vishh · 2018-05-31T21:19:20Z

contributors/design-proposals/node/troubleshoot-running-pods.md

+container occurring between its creation and attach will not be replayed, but it
+can be viewed using `kubectl log`.
+
+#### Alternative 1: "exec++"


@verb can you add a few lines to describe why these alternatives were not chosen at the end?

After discussing with API reviewers and relevant SIG leads, we've agreed that the configuration for Ephemeral Containers should live in the pod spec.

thockin · 2018-08-24T00:10:44Z

/lgtm
/approve

k8s-ci-robot · 2018-08-24T00:10:50Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: thockin

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~contributors/design-proposals/node/OWNERS~~ [thockin]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

kfox1111 · 2018-08-24T00:23:02Z

BTW, speaking of from scratch images, I hit a case in tiller the other day where using a thin image where the go binary is statically linked, there is no /etc/nsswitch.conf, the go resolver will default to 'dns, files' which means if your dns search path happens to have a 'localhost.foo.whatever' and your search path includes foo.whatever, localhost resolves to localhost.foo.whatever's. This is very unexpected/unfortunate. As this is a common k8s method of deployment, we might want to address this. Should we file an issue in k8s? in go?

thockin · 2018-08-24T00:27:38Z

I think that is a broken resolver and a broken DNS config. You should never have a host named localhost, and DNS clients should not try to resolve it

…

On Thu, Aug 23, 2018 at 5:23 PM kfox1111 ***@***.***> wrote: BTW, speaking of from scratch images, I hit a case in tiller the other day where using a thin image where the go binary is statically linked, there is no /etc/nsswitch.conf, the go resolver will default to 'dns, files' which means if your dns search path happens to have a 'localhost.foo.whatever' and your search path includes foo.whatever, localhost resolves to localhost.foo.whatever's. This is very unexpected/unfortunate. As this is a common k8s method of deployment, we might want to address this. Should we file an issue in k8s? in go? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1269 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AFVgVF_t3LOwXtJoBQD4rACpILW12OP3ks5uT0dugaJpZM4QGMMg> .

kfox1111 · 2018-08-24T00:34:07Z

I agree on both accounts. :)

So, its a bug in golangs resolver then and we need to get golang to fix it.

kfox1111 · 2018-08-24T00:38:15Z

one other possibility, as the golang devs have known about the issue for a while and have ignored it, would be to have k8s create /etc/nsswitch.conf by default in the container with some sane defaults like it does resolv.conf and hosts.

thockin · 2018-08-24T17:44:39Z

The problem is that nsswitch (I assume that is what you meant) is an artifact of glibc's DNS resolver. Are we willing to manage that file in perpetuity for all users? Also, this will brek any image that has a custom nsswitch file built-in.

…

On Thu, Aug 23, 2018 at 5:38 PM kfox1111 ***@***.***> wrote: one other possibility, as the golang devs have known about the issue for a while and have ignored it, would be to have k8s create /etc/nslookup.conf by default in the container with some sane defaults like it does resolv.conf and hosts. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1269 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AFVgVG56Vjf8ClThmlrKZOUCAQlyuF5jks5uT0r9gaJpZM4QGMMg> .

kfox1111 · 2018-08-27T18:01:48Z

yeah, thats what I meant. It would suck to maintain it for ever, but it is unclear if anyone else is willing to fix it. Its been a known issue for a long time apparently.

Is it easy to do it conditionally? check if the file exists, and if not inject it?

Alternately, it should be exceedingly rare I think for that file to be non default. We could make it default to always override it, and add a flag not too.

thockin · 2018-08-28T02:41:54Z

I'm not sure what a good answer here looks like. It's sort of a broekn image that depends on it but doesn't provide it. Can we really be in the business of fixing broken images dynamically? Maybe we can instead raise the heat on the Go devs to get a discussion going?

…

On Mon, Aug 27, 2018 at 11:02 AM kfox1111 ***@***.***> wrote: yeah, thats what I meant. It would suck to maintain it for ever, but it is unclear if anyone else is willing to fix it. Its been a known issue for a long time apparently. Is it easy to do it conditionally? check if the file exists, and if not inject it? Alternately, it should be exceedingly rare I think for that file to be non default. We could make it default to always override it, and add a flag not too. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1269 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AFVgVG76CNnPaBIVQzW07qKUL-slcDavks5uVDQZgaJpZM4QGMMg> .

kfox1111 · 2018-08-28T15:16:55Z

the image is not broken in that is provides a libc that never uses it (not glibc). So it doesn't provide a file it isn't going to use. go is assuming it is using glibc and following glibc convention for how to behave when nsswitch is missing. the binary is statically linked though so doesn't actually use libc. The end user is expecting a statically linked binary to work properly on any linux, and is preferring alpine as its smaller and we're encouraging smaller containers. So, its hard to point at any one thing as the thing to fix, but it is unexpectedly effecting users. We can raise it with the go devs again and see what they say.

thockin · 2018-08-28T16:59:00Z

Please feel free to loop me into a discussion with go devs.

…

On Tue, Aug 28, 2018, 8:18 AM kfox1111 ***@***.***> wrote: the image is not broken in that is provides a libc that never uses it (not glibc). So it doesn't provide a file it isn't going to use. go is assuming it is using glibc and following glibc convention for how to behave when nsswitch is missing. the binary is statically linked though so doesn't actually use libc. The end user is expecting a statically linked binary to work properly on any linux, and is preferring alpine as its smaller and we're encouraging smaller containers. So, its hard to point at any one thing as the thing to fix, but it is unexpectedly effecting users. We can raise it with the go devs again and see what they say. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1269 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AFVgVH90U4yWf-ucpEURspPYEbZfFu8nks5uVV8-gaJpZM4QGMMg> .

krmayankk · 2018-12-27T02:03:43Z

contributors/design-proposals/node/troubleshoot-running-pods.md


 ## Feature Summary

 Any new debugging functionality will require training users. We can ease the
 transition by building on an existing usage pattern. We will create a new
 command, `kubectl debug`, which parallels an existing command, `kubectl exec`.
-Whereas `kubectl exec` runs a *process* in a *container*, `kubectl debug` will
-be similar but run a *container* in a *pod*.
+Whereas `kubectl exec` runs a _process_ in a _container_, `kubectl debug` will


i think this is underdocumented here https://kubernetes.io/docs/tasks/debug-application-cluster/get-shell-running-container/ that kubectl exec runs a process in an existing container

krmayankk · 2018-12-27T02:09:11Z

contributors/design-proposals/node/troubleshoot-running-pods.md

-As is already the case with regular containers, Debug Containers can enter
-arbitrary namespaces of another container via `nsenter` when run with
-`CAP_SYS_ADMIN`.
+the pod. When [process namespace sharing](https://features.k8s.io/495) is


the top line should make it more clear that this creates a new container and runs an interactive shell inside that container . This creates a new container and starts an interactive shell inside that container in a pod called target-pod which can examine and signal other processes in the pod

krmayankk · 2018-12-27T02:15:37Z

contributors/design-proposals/node/troubleshoot-running-pods.md

+  // List of user-initiated ephemeral containers to run in this pod.
+  // This field is alpha-level and is only honored by servers that enable the EphemeralContainers feature.
+  // +optional
+  EphemeralContainers []EphemeralContainer `json:"ephemeralContainers,omitempty" protobuf:"bytes,29,opt,name=ephemeralContainers"`


how is this different from regular container ?

krmayankk · 2018-12-27T02:18:53Z

contributors/design-proposals/node/troubleshoot-running-pods.md

+  // If not set then the ephemeral container is run in whatever namespaces are shared
+  // for the pod.
+  // +optional
+  TargetContainerName string `json:"targetContainerName,omitempty" protobuf:"bytes,2,opt,name=targetContainerName"`


all containers of the pods are in same network namespace except possibly pid namespace. Was this field introduced for the pid namespace case only ?

krmayankk · 2018-12-27T02:20:38Z

contributors/design-proposals/node/troubleshoot-running-pods.md

+Much of the utility of Ephemeral Containers comes from the ability to run a
+container within the PID namespace of another container. `TargetContainerName`
+allows targeting a container that doesn't share its PID namespace with the rest
+of the pod. We must modify the CRI to enable this functionality (see below).


So TargetContainerName is only useful for pid namespace case only . It would be good to clarify that it has no meaning when all containers are in same network and pid namespace as pod

…container Use v1.Container in Debug Containers API

k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Oct 25, 2017

k8s-ci-robot assigned lavalamp, liggitt and thockin Oct 25, 2017

k8s-ci-robot added sig/node Categorizes an issue or PR as relevant to SIG Node. sig/auth Categorizes an issue or PR as relevant to SIG Auth. labels Oct 25, 2017

liggitt reviewed Oct 25, 2017

View reviewed changes

lavalamp reviewed Oct 25, 2017

View reviewed changes

liggitt reviewed Oct 25, 2017

View reviewed changes

thockin reviewed Oct 25, 2017

View reviewed changes

liggitt reviewed Oct 26, 2017

View reviewed changes

verb changed the title ~~Use v1.Container in Debug Containers API~~ WIP: Use v1.Container in Debug Containers API Dec 22, 2017

k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Dec 22, 2017

liggitt reviewed Jan 2, 2018

View reviewed changes

liggitt mentioned this pull request Jan 10, 2018

Support the user flag from docker exec in kubectl exec kubernetes/kubernetes#30656

Closed

lavalamp reviewed May 31, 2018

View reviewed changes

vishh reviewed May 31, 2018

View reviewed changes

k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 15, 2018

verb force-pushed the pod-troubleshooting-use-container branch from dcb6695 to 57f2c71 Compare August 16, 2018 16:13

k8s-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Aug 16, 2018

Move Ephemeral Containers into pod.Spec

502b757

After discussing with API reviewers and relevant SIG leads, we've agreed that the configuration for Ephemeral Containers should live in the pod spec.

verb force-pushed the pod-troubleshooting-use-container branch from 57f2c71 to 502b757 Compare August 21, 2018 15:56

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 24, 2018

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 24, 2018

k8s-ci-robot merged commit f1d6261 into kubernetes:master Aug 24, 2018

krmayankk reviewed Dec 27, 2018

View reviewed changes

MadhavJivrajani pushed a commit to MadhavJivrajani/community that referenced this pull request Nov 30, 2021

Merge pull request kubernetes#1269 from verb/pod-troubleshooting-use-…

f5372bb

…container Use v1.Container in Debug Containers API

Use v1.Container in Debug Containers API #1269

Use v1.Container in Debug Containers API #1269

Conversation

verb commented Oct 25, 2017 • edited Loading

verb commented Oct 25, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

liggitt Oct 25, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lavalamp commented Oct 25, 2017

liggitt Oct 25, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

liggitt Oct 25, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

thockin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

k8s-github-robot commented Nov 26, 2017

smarterclayton commented Dec 31, 2017

Choose a reason for hiding this comment

liggitt Jan 2, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

liggitt commented Jan 7, 2018 • edited Loading

verb commented Jan 10, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lavalamp commented May 31, 2018

Choose a reason for hiding this comment

thockin commented Aug 24, 2018

k8s-ci-robot commented Aug 24, 2018

kfox1111 commented Aug 24, 2018

thockin commented Aug 24, 2018 via email

kfox1111 commented Aug 24, 2018

kfox1111 commented Aug 24, 2018 • edited Loading

thockin commented Aug 24, 2018 via email

kfox1111 commented Aug 27, 2018

thockin commented Aug 28, 2018 via email

kfox1111 commented Aug 28, 2018

thockin commented Aug 28, 2018 via email

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

verb commented Oct 25, 2017 •

edited

Loading

liggitt Oct 25, 2017 •

edited

Loading

liggitt Oct 25, 2017 •

edited

Loading

liggitt Oct 25, 2017 •

edited

Loading

liggitt Jan 2, 2018 •

edited

Loading

liggitt commented Jan 7, 2018 •

edited

Loading

kfox1111 commented Aug 24, 2018 •

edited

Loading