Allow removing fields from kubectl debug copied pods or containers

Sayrus commented 3 months ago

What would you like to be added: A way to remove fields from containers when using kubectl debug --copy-to using --custom patches.

Why is this needed: When using kubectl debug with the --copy-to option, everything from the original pod is copied. In my case, this includes livenessProbe and readinessProbe. In many cases, these probes fail. For instance:

When profiling, livenessProbe or readinessProbe may timeout (Related but for ephemeral containers: https://github.com/kubernetes/kubernetes/issues/57187)
The same happens when debugging with blocking breakpoints
When the image or entrypoint has been modified or if the app is not started with the same options as the original (using --image or simply COMMAND [args...])

Thanks to KEP-4292 implemented behind a flag here, it is now possible to rewrite fields so we can "disable" probes by setting a very high initial delay instead. It also allows overwriting environment variable and other stuffs. Unfortunately, since we can't unset fields, we can't do things such as adding an exec probe to a pod with httpGet probe (as it results in a spec with both httpGet and exec which is invalid)

The specific case of probes can be mitigated by using a profile that automatically removes probes (such as general) but this introduces some other limitations if your debugging job requires other capabilities.

I saw https://github.com/kubernetes/kubernetes/pull/123149 which implements this using --keep-* parameters. For my use-case and I would prefer being able to distribute custom profiles as YAML or JSON as it allows non power-user to use different debugging profiles without changing several kubectl arguments.

For client-side patching, Strategic Merge Patch directives are used (or used to? I know it exists as well in Kustomize.). Following this document, a patch to remove probes and a specific env variable could look like:

{
  "env": [
    {
      "name": "DELETE_ME",
      "$patch": "delete"
    }
  ],
  "livenessProbe": null,
  "readinessProbe": null
}

ardaguclu commented 3 months ago

Thanks for opening this issue. As we discussed async on ~Slack~ Email, I agree with that probes should be manageable via flag because I don't think --custom flag will ever be able to support removal of fields; /triage accepted /priority backlog

ardaguclu commented 1 month ago

This is fixed by https://github.com/kubernetes/kubernetes/pull/123149

ardaguclu commented 5 days ago

The referenced PR seems to be merged. I'm closing this. Feel free to reopen, if I'm missing something. Thank you.

/close

k8s-ci-robot commented 5 days ago

@ardaguclu: Closing this issue.

In response to [this](https://github.com/kubernetes/kubectl/issues/1570#issuecomment-2196839208): >The referenced PR seems to be merged. I'm closing this. Feel free to reopen, if I'm missing something. Thank you. > >/close Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes-sigs/prow](https://github.com/kubernetes-sigs/prow/issues/new?title=Prow%20issue:) repository.

kubernetes / kubectl

Allow removing fields from kubectl debug copied pods or containers #1570