kubernetes-client icon indicating copy to clipboard operation
kubernetes-client copied to clipboard

exec and other operations should support a default container

Open shawkins opened this issue 3 years ago • 6 comments

Is your enhancement related to a problem? Please describe

kubectl if you don't specify the container argument will assume the first container for operations like exec. This may be leading to confusion / errors such as #4353 and #4319

Describe the solution you'd like

Similar to kubectl the fabric8 client should default to the first container when working with a multi-container pod.

Describe alternatives you've considered

No response

Additional context

No response

shawkins avatar Aug 26 '22 13:08 shawkins

Is the idea to change all calls to io.fabric8.kubernetes.client.dsl.internal.PodOperationContext#withContainerId(String containerId) to use the first containerId from an internal collection of container IDs? I don't know any of the internals and likely wouldn't be helpful in creating a pr but still am curious. Thanks for opening the issue!

scottmarlow avatar Aug 26 '22 18:08 scottmarlow

Is the idea to change all calls to io.fabric8.kubernetes.client.dsl.internal.PodOperationContext#withContainerId(String containerId) to use the first containerId from an internal collection of container IDs?

The logic would roughly be - if a containerId is not set, get the deployment and use the first container name as the id.

If there's a concern about backwards compatibility with how we're currently doing things, then we'll only use the first container when there are multiple - probably just need to check the kubectl logic for consistency.

shawkins avatar Aug 26 '22 18:08 shawkins

The logic would roughly be - if a containerId is not set, get the deployment and use the first container name as the id.

:+1:

If there's a concern about backwards compatibility with how we're currently doing things, then we'll only use the first container when there are multiple - probably just need to check the kubectl logic for consistency.

What are the possible user cases to be handled? Perhaps something like:

  1. multiple containers in a single pod.
  2. single container in a single pod.
  3. Something else?

scottmarlow avatar Aug 26 '22 19:08 scottmarlow

What are the possible user cases to be handled?

Just the first one is what is currently problematic wrt user expectations. kubectl defaults to the first container, the fabric8 client does not.

However a narrow fix just for exec may not be the most appropriate - there are a lot of locations where a container may be specified so we'll need to determine all the places were defaulting is applicable.

shawkins avatar Aug 26 '22 21:08 shawkins

Similar to kubectl the fabric8 client should default to the first container when working with a multi-container pod.

I thought this was already implemented (we do have something like this on JKube for Watch and Log operations).

What are the possible user cases to be handled? Perhaps something like:

Summary of exec and other Pod operations proposed behaviors with regards to containers:

  1. Pod has no Containers, should <blank>
  2. Pod has a single Container, the single Container should be used by default (no need to call inContainer)
  3. Pod has a single Container and inContainer is used with a different id/name, should _<blank_throw_exception?or_use_single_and_log_warning?>
  4. Pod has >1 Container, the first container should be used by default (no need to call inContainer) <and_log_warning?>
  5. Pod has >1 Container and inContainer is used with a non-existent id/name, should _<blank_throw_exception?or_use_first_and_log_warning?>

manusa avatar Aug 29 '22 05:08 manusa

What is the impact of this issue if I have two (Jenkins pipeline defined) containers, the first is called jakartaeetck-ci and the second is called james-mail. Please assume that my first container jakartaeetck-ci is considered the main (test) application and the second container james-mail is considered a sidecar that runs an email server in the background.

With the fix applied, will the first container always be considered the main application and other containers always be considered sidecars? Is this the right terminology?

scottmarlow avatar Sep 01 '22 13:09 scottmarlow