This is an automated email from the ASF dual-hosted git repository.

feiwang pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/kyuubi.git


The following commit(s) were added to refs/heads/master by this push:
     new b537e8a25 [KYUUBI #6183] [K8S] KyuubiConf.getKubernetesConf should set 
namespace even if the Kubernetes context is empty
b537e8a25 is described below

commit b537e8a252de995ab0e81b4e47a4db3b954ef042
Author: Lennon Chin <[email protected]>
AuthorDate: Thu Mar 14 23:04:34 2024 -0700

    [KYUUBI #6183] [K8S] KyuubiConf.getKubernetesConf should set namespace even 
if the Kubernetes context is empty
    
    # :mag: Description
    ## Issue References ๐Ÿ”—
    
    When submit Spark engine to Kubernetes environment, 
`org.apache.kyuubi.engine.KubernetesApplicationOperation#getApplicationInfoByTag`
 will use Kubernetes client to get driver pod info, if we do not set 
`spark.kubernetes.context`configuration in Spark environment, 
`org.apache.kyuubi.config.KyuubiConf#getKubernetesConf` will construct a client 
with **default** namespace, this will cause privilege problem if Kyuubi running 
with a partial rolebinding service account:
    
    ```log
    2024-03-13 18:13:47.409 ERROR KyuubiSessionManager-exec-pool: Thread-56 
org.apache.kyuubi.engine.KubernetesApplicationOperation: Failed to get 
application by label: kyuubi-unique-tag=b61924ef-a93e-46a3-94be-0de70be5fb5e, 
due to Failure executing: GET at: 
https://******:6443/api/v1/namespaces/default/pods?labelSelector=kyuubi-unique-tag&resourceVersion=0.
 Message: pods is forbidden: User "system:serviceaccount:kyuubi:kyuubi" cannot 
list resource "pods" in API group "" in the namespace  [...]
    ```
    
    This will appear as engine startup failure, but in fact the engine is 
started successfully.
    
    ## Describe Your Solution ๐Ÿ”ง
    
    `KyuubiConf.getKubernetesConf` method should set namespace even if the 
Kubernetes context is empty, please see the commit.
    
    ## Types of changes :bookmark:
    
    - [x] Bugfix (non-breaking change which fixes an issue)
    - [ ] New feature (non-breaking change which adds functionality)
    - [ ] Breaking change (fix or feature that would cause existing 
functionality to change)
    
    ## Test Plan ๐Ÿงช
    
    #### Behavior Without This Pull Request :coffin:
    
    #### Behavior With This Pull Request :tada:
    
    #### Related Unit Tests
    
    ---
    
    # Checklist ๐Ÿ“
    
    - [x] This patch was not authored or co-authored using [Generative 
Tooling](https://www.apache.org/legal/generative-tooling.html)
    
    **Be nice. Be informative.**
    
    Closes #6183 from LennonChin/kubernetes-conf-patch.
    
    Closes #6183
    
    69e653011 [Lennon Chin] KubernetesConf should set namespace even if the 
kubernetes context is empty
    
    Authored-by: Lennon Chin <[email protected]>
    Signed-off-by: Wang, Fei <[email protected]>
---
 kyuubi-common/src/main/scala/org/apache/kyuubi/config/KyuubiConf.scala | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git 
a/kyuubi-common/src/main/scala/org/apache/kyuubi/config/KyuubiConf.scala 
b/kyuubi-common/src/main/scala/org/apache/kyuubi/config/KyuubiConf.scala
index 6baca63e5..2aa5030ca 100644
--- a/kyuubi-common/src/main/scala/org/apache/kyuubi/config/KyuubiConf.scala
+++ b/kyuubi-common/src/main/scala/org/apache/kyuubi/config/KyuubiConf.scala
@@ -154,9 +154,9 @@ case class KyuubiConf(loadSysDefault: Boolean = true) 
extends Logging {
         conf.set(key, value)
       }
       conf.set(KUBERNETES_CONTEXT, c)
-      namespace.foreach(ns => conf.set(KUBERNETES_NAMESPACE, ns))
       conf
     }
+    namespace.foreach(ns => conf.set(KUBERNETES_NAMESPACE, ns))
     conf
   }
 

Reply via email to