Feature to allow selection of container when connecting to kubernetes pod

Feature to allow selection of container when connecting to kubernetes pod

[Aaron Gonzales]

[-- Attachment #1.1: Type: text/plain, Size: 1465 bytes --]

Hi all,

This is my first time doing this so sorry if there are some mistakes in the
process. I am also not entirely accustomed to elisp so please forgive my
code and provide any feedback you may have.

These new changes allow the user to select a container when connecting to a
kubernetes pod. This is essential for kubernetes Tramp functionality since
the first container may not always be the desired pod. Many pods contain
multiple side-car containers besides the main application container.

Now considering the above, the below changes are not the most desirable
from my perspective. From a user perspective, there are no visual cues to
display what container you selected. I wanted to avoid using ":" separator
between a pod and container since that might overload the meaning of that
separator.

The other issue is the change to tramp-expand-args which now can expect
either a lambda or string. The lambda takes in the entire tramp-file-name
struct but that can be reduced to just passing the pod name for the sake of
kubernetes connections. At the very least this should warrant a function
rename, but I could also see a possible break off to more specific handling
for kubernetes. Currently, Emacs is only accounting for connecting to pods,
but jobs are still relevant when it comes to kubernetes which would require
some selection.

Changes can also be found at
https://github.com/Aaronzinhoo/emacs/tree/feat-tramp-kubernetes-select-container
if needed.

[-- Attachment #1.2: Type: text/html, Size: 1740 bytes --]

[-- Attachment #2: 0001-Add-ability-to-access-specific-kubernetes-pod-s-cont.patch --]
[-- Type: application/octet-stream, Size: 4616 bytes --]

From ae1269815941c4319eaf69c3c3d2730ef4e3d8dc Mon Sep 17 00:00:00 2001
From: Aaron Gonzales <aaronzinho@ucla.edu>
Date: Fri, 30 Dec 2022 04:11:40 -0800
Subject: [PATCH] Add ability to access specific kubernetes pod's container

* lisp/net/tramp-container.el (tramp-kubernetes--pod-containers)
(tramp-kubernetes--pod-container): New functions.

* lisp/net/tramp.el (tramp-expand-args): Call function or expand string
---
 doc/misc/tramp.texi         | 10 ++++++++++
 etc/NEWS                    |  6 ++++++
 lisp/net/tramp-container.el | 21 +++++++++++++++++++++
 lisp/net/tramp.el           |  4 +++-
 4 files changed, 40 insertions(+), 1 deletion(-)

diff --git a/doc/misc/tramp.texi b/doc/misc/tramp.texi
index 80874049fa..bb9cd488e6 100644
--- a/doc/misc/tramp.texi
+++ b/doc/misc/tramp.texi
@@ -927,6 +927,16 @@ Inline methods
 
 This method does not support user names.
 
+@item @option{kubernetes}
+@cindex method @option{kubernetes}
+@cindex @option{kubernetes} method
+
+Integration for selecting a container in a Kubernetes pods.  The
+containers are returned by
+@samp{"kubectl get po pod -o jsonpath='{.spec.containers[*].name}'"}.
+If there is more than one pod and enable-recursive-minibuffers is
+enabled then the user is prompted to select a container.
+
 @end table
 
 
diff --git a/etc/NEWS b/etc/NEWS
index 50937f5e96..cf4c96958e 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -117,6 +117,12 @@ point is not in a comment or a string.  It is by default bound to
 \f
 * Lisp Changes in Emacs 30.1
 
+** Tramp
+
+---
+*** Allow user to select kubernetes container when connecting to pod.
+Allow selection of a container when connecting to kubernetes pod via Tramp.
+
 ** New or changed byte-compilation warnings
 
 ---
diff --git a/lisp/net/tramp-container.el b/lisp/net/tramp-container.el
index 7b94253226..f4eca27a1d 100644
--- a/lisp/net/tramp-container.el
+++ b/lisp/net/tramp-container.el
@@ -116,6 +116,25 @@ tramp-docker--completion-function
                      lines)))
     (mapcar (lambda (m) (list nil m)) (delq nil names))))
 
+;;;###tramp-autoload
+(defun tramp-kubernetes--pod-containers (pod)
+  "Return list of containers in Kubernetes Pod POD."
+  (split-string (shell-command-to-string
+		 (concat tramp-kubernetes-program
+                         " get po "
+                         pod
+                         " -o jsonpath='{.spec.containers[*].name}' "))))
+
+;;;###tramp-autoload
+(defun tramp-kubernetes--pod-container (pod)
+  "Get container in Kubernetes Pod POD.
+
+If `enable-recusrive-minibuffers' set to nil or a pod has a single container then return first container in list by default.  Otherwise allow the user to select from a list of containers"
+  (let ((containers (tramp-kubernetes--pod-containers pod)))
+    (cond ((not enable-recursive-minibuffers) (nth 0 containers))
+          ((length= containers 1) (nth 0 containers))
+          (t (completing-read "Select container: " containers  nil t)))))
+
 ;;;###tramp-autoload
 (defun tramp-kubernetes--completion-function (&rest _args)
   "List Kubernetes pods available for connection.
@@ -185,6 +204,8 @@ tramp-default-remote-shell
                 (tramp-login-args (("exec")
                                    ("%h")
                                    ("-it")
+                                   ("-c")
+                                   (lambda (vec) (tramp-kubernetes--pod-container (tramp-file-name-host vec)))
                                    ("--")
 			           ("%l")))
 		(tramp-config-check tramp-kubernetes--current-context-data)
diff --git a/lisp/net/tramp.el b/lisp/net/tramp.el
index acbd50dc0f..92a5961221 100644
--- a/lisp/net/tramp.el
+++ b/lisp/net/tramp.el
@@ -4729,7 +4729,7 @@ tramp-compute-multi-hops
 (defun tramp-expand-args (vec parameter &rest spec-list)
   "Expand login arguments as given by PARAMETER in `tramp-methods'.
 PARAMETER is a symbol like `tramp-login-args', denoting a list of
-list of strings from `tramp-methods', containing %-sequences for
+list of strings or lambdas that expect `tramp-file-name' type parameter from `tramp-methods', containing %-sequences for
 substitution.  SPEC-LIST is a list of char/value pairs used for
 `format-spec-make'."
   (let ((args (tramp-get-method-parameter vec parameter))
@@ -4738,6 +4738,8 @@ tramp-expand-args
     (tramp-compat-flatten-tree
      (mapcar
       (lambda (x)
+        (cond ((functionp x) (setq x (list (funcall x vec))))
+              (t (setq x (mapcar (lambda (y) (format-spec y spec)) x))))
 	(setq x (mapcar (lambda (y) (format-spec y spec)) x))
 	(unless (member "" x) x))
       args))))
-- 
2.39.0

Re: Feature to allow selection of container when connecting to kubernetes pod

[Aaron Gonzales]

[-- Attachment #1: Type: text/plain, Size: 4662 bytes --]

Including the patch since I sent it as an attachment before.

From ae1269815941c4319eaf69c3c3d2730ef4e3d8dc Mon Sep 17 00:00:00 2001
From: Aaron Gonzales <aaronzinho@ucla.edu>
Date: Fri, 30 Dec 2022 04:11:40 -0800
Subject: [PATCH] Add ability to access specific kubernetes pod's container

* lisp/net/tramp-container.el (tramp-kubernetes--pod-containers)
(tramp-kubernetes--pod-container): New functions.

* lisp/net/tramp.el (tramp-expand-args): Call function or expand string
---
 doc/misc/tramp.texi         | 10 ++++++++++
 etc/NEWS                    |  6 ++++++
 lisp/net/tramp-container.el | 21 +++++++++++++++++++++
 lisp/net/tramp.el           |  4 +++-
 4 files changed, 40 insertions(+), 1 deletion(-)

diff --git a/doc/misc/tramp.texi b/doc/misc/tramp.texi
index 80874049fa..bb9cd488e6 100644
--- a/doc/misc/tramp.texi
+++ b/doc/misc/tramp.texi
@@ -927,6 +927,16 @@ Inline methods

 This method does not support user names.

+@item @option{kubernetes}
+@cindex method @option{kubernetes}
+@cindex @option{kubernetes} method
+
+Integration for selecting a container in a Kubernetes pods.  The
+containers are returned by
+@samp{"kubectl get po pod -o jsonpath='{.spec.containers[*].name}'"}.
+If there is more than one pod and enable-recursive-minibuffers is
+enabled then the user is prompted to select a container.
+
 @end table


diff --git a/etc/NEWS b/etc/NEWS
index 50937f5e96..cf4c96958e 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -117,6 +117,12 @@ point is not in a comment or a string.  It is by
default bound to

 * Lisp Changes in Emacs 30.1

+** Tramp
+
+---
+*** Allow user to select kubernetes container when connecting to pod.
+Allow selection of a container when connecting to kubernetes pod via Tramp.
+
 ** New or changed byte-compilation warnings

 ---
diff --git a/lisp/net/tramp-container.el b/lisp/net/tramp-container.el
index 7b94253226..f4eca27a1d 100644
--- a/lisp/net/tramp-container.el
+++ b/lisp/net/tramp-container.el
@@ -116,6 +116,25 @@ tramp-docker--completion-function
                      lines)))
     (mapcar (lambda (m) (list nil m)) (delq nil names))))

+;;;###tramp-autoload
+(defun tramp-kubernetes--pod-containers (pod)
+  "Return list of containers in Kubernetes Pod POD."
+  (split-string (shell-command-to-string
+ (concat tramp-kubernetes-program
+                         " get po "
+                         pod
+                         " -o jsonpath='{.spec.containers[*].name}' "))))
+
+;;;###tramp-autoload
+(defun tramp-kubernetes--pod-container (pod)
+  "Get container in Kubernetes Pod POD.
+
+If `enable-recusrive-minibuffers' set to nil or a pod has a single
container then return first container in list by default.  Otherwise allow
the user to select from a list of containers"
+  (let ((containers (tramp-kubernetes--pod-containers pod)))
+    (cond ((not enable-recursive-minibuffers) (nth 0 containers))
+          ((length= containers 1) (nth 0 containers))
+          (t (completing-read "Select container: " containers  nil t)))))
+
 ;;;###tramp-autoload
 (defun tramp-kubernetes--completion-function (&rest _args)
   "List Kubernetes pods available for connection.
@@ -185,6 +204,8 @@ tramp-default-remote-shell
                 (tramp-login-args (("exec")
                                    ("%h")
                                    ("-it")
+                                   ("-c")
+                                   (lambda (vec)
(tramp-kubernetes--pod-container (tramp-file-name-host vec)))
                                    ("--")
            ("%l")))
  (tramp-config-check tramp-kubernetes--current-context-data)
diff --git a/lisp/net/tramp.el b/lisp/net/tramp.el
index acbd50dc0f..92a5961221 100644
--- a/lisp/net/tramp.el
+++ b/lisp/net/tramp.el
@@ -4729,7 +4729,7 @@ tramp-compute-multi-hops
 (defun tramp-expand-args (vec parameter &rest spec-list)
   "Expand login arguments as given by PARAMETER in `tramp-methods'.
 PARAMETER is a symbol like `tramp-login-args', denoting a list of
-list of strings from `tramp-methods', containing %-sequences for
+list of strings or lambdas that expect `tramp-file-name' type parameter
from `tramp-methods', containing %-sequences for
 substitution.  SPEC-LIST is a list of char/value pairs used for
 `format-spec-make'."
   (let ((args (tramp-get-method-parameter vec parameter))
@@ -4738,6 +4738,8 @@ tramp-expand-args
     (tramp-compat-flatten-tree
      (mapcar
       (lambda (x)
+        (cond ((functionp x) (setq x (list (funcall x vec))))
+              (t (setq x (mapcar (lambda (y) (format-spec y spec)) x))))
  (setq x (mapcar (lambda (y) (format-spec y spec)) x))
  (unless (member "" x) x))
       args))))
-- 
2.39.0

[-- Attachment #2: Type: text/html, Size: 5595 bytes --]

Re: Feature to allow selection of container when connecting to kubernetes pod

[Michael Albinus]

Aaron Gonzales <aarongonzales1@gmail.com> writes:

> Hi all,

Hi Aaron,

> This is my first time doing this so sorry if there are some mistakes
> in the process. I am also not entirely accustomed to elisp so please
> forgive my code and provide any feedback you may have.

Thanks for your contribution. If you propose a change in Emacs which
would require more discussion, the recommended way is to write a bug report
via 'M-x report-emacs-bug'. Further discussion will happen then there,
which is also good for the archives. Feature proposals are also
discussed this way. Please write such a report.

Since I have only rudimentary knowledge about kubernetes (to say the
best), I Cc Filipp Gunbin who has brought kubernetes to Tramp.

> These new changes allow the user to select a container when connecting
> to a kubernetes pod. This is essential for kubernetes Tramp
> functionality since the first container may not always be the desired
> pod. Many pods contain multiple side-car containers besides the main
> application container.

Sounds useful to me.

> Now considering the above, the below changes are not the most
> desirable from my perspective. From a user perspective, there are no
> visual cues to display what container you selected. I wanted to avoid
> using ":" separator between a pod and container since that might
> overload the meaning of that separator.

Hmm, yes. And your patch has also other disadvantages. After a rough
reading, it looks to me like you can specify the container only
interactively, when reading a file name. But this is not the only way to
invoke Tramp, you could always say in your Lisp code

  (copy-file "/kubernetes:pod:/path/to/file" "/another/local/path/")

No user action expected.

Therefore, I recommend to extend the syntax. The selected container shall
be part of the pod name, like "/kubernetes:pod%container:/path/to/file"
or "/kubernetes:pod[container]:/path/to/file". The container part is
optional, of course. And I'm also open for another syntax.

> The other issue is the change to tramp-expand-args which now can
> expect either a lambda or string. The lambda takes in the entire
> tramp-file-name struct but that can be reduced to just passing the pod
> name for the sake of kubernetes connections. At the very least this
> should warrant a function rename, but I could also see a possible
> break off to more specific handling for kubernetes.

With the extended pod name syntax, we don't need this anymore. The
container name selection could be handled in
tramp-kubernetes--completion-function.

> Currently, Emacs is only accounting for connecting to pods, but jobs
> are still relevant when it comes to kubernetes which would require
> some selection.

Yes, but pls let discuss this in another thread (bug report). Note that
there is also the proposal to support different namespaces, see bug#59797.

Best regards, Michael.

Re: Feature to allow selection of container when connecting to kubernetes pod

[Filipp Gunbin]

Hi Michael and Aaron,

On 31/12/2022 11:22 +0100, Michael Albinus wrote:

>> These new changes allow the user to select a container when connecting
>> to a kubernetes pod. This is essential for kubernetes Tramp
>> functionality since the first container may not always be the desired
>> pod. Many pods contain multiple side-car containers besides the main
>> application container.
>
> Sounds useful to me.

To me too, however I think we should agree on a filename syntax.  I
don't have vast experience with Kubernetes, and thus chose the simplest
thing to do first - just accessing pods by name.  When we add other
specifiers, we should add them in a consistent way.  I haven't looked at
the docs in detail yet, and am mostly offline while it's holiday time
here, but will be able to do that in January.  Of course, if Michael
arrives at some opinion in the meantime, I fully trust his decision.

Thanks,
Filipp