Reamer commented on a change in pull request #4174:
URL: https://github.com/apache/zeppelin/pull/4174#discussion_r685757530



##########
File path: k8s/zeppelin-server.yaml
##########
@@ -213,14 +213,15 @@ rules:
   resources: ["roles", "rolebindings"]
   verbs: ["bind", "create", "get", "update", "patch", "list", "delete", 
"watch"]
 ---
-kind: RoleBinding
+kind: ClusterRoleBinding
 apiVersion: rbac.authorization.k8s.io/v1
 metadata:
   name: zeppelin-server-role-binding
 subjects:
 - kind: ServiceAccount
   name: zeppelin-server
+  namespace: default

Review comment:
       Instead of creating a `ClusterRole` and a `ClusterRoleBinding`, I would 
prefer an additional blueprint with a `Role` and a `RoleBinding`. Inside the 
`RoleBinding` the user has to replace the `Namespace`.

##########
File path: 
zeppelin-plugins/launcher/k8s-standard/src/main/java/org/apache/zeppelin/interpreter/launcher/K8sUtils.java
##########
@@ -74,4 +82,61 @@ private static long convertToBytes(String memory) {
     }
     return memoryAmountBytes;
   }
+
+  /**
+   * return the current namespace
+   * @return the namespace in Config.KUBERNETES_NAMESPACE_PATH if it is 
running inside k8s, otherwise return null
+   */
+  public static String getCurrentK8sNamespace() {
+    try {
+      if (isRunningOnKubernetes()) {
+        return readFile(Config.KUBERNETES_NAMESPACE_PATH, 
Charset.defaultCharset()).trim();
+      } else {
+        return null;
+      }
+    }
+    catch (IOException e){
+      return null;
+    }
+  }
+
+  /**
+   * Get the namespace of the interpreter.
+   * Check Order: zeppelin.k8s.interpreter.namespace -> 
getCurrentK8sNamespace() -> zConf.getK8sNamepsace()
+   * @param properties
+   * @param zConf
+   * @return the interpreter namespace
+   * @throws IOException
+   */
+  public static String getInterpreterNamespace(Properties properties, 
ZeppelinConfiguration zConf) throws IOException {
+    if(properties.containsKey("zeppelin.k8s.interpreter.namespace")){
+      return properties.getProperty("zeppelin.k8s.interpreter.namespace");
+    }
+
+    if (isRunningOnKubernetes()) {
+      return getCurrentK8sNamespace();
+    } else {
+      return zConf.getK8sNamepsace();
+    }
+  }
+
+  /**
+   * Check if i'm running inside of kubernetes or not.
+   * It should return truth regardless of ZeppelinConfiguration.getRunMode().
+   *
+   * Normally, unless Zeppelin is running on Kubernetes, 
K8sStandardInterpreterLauncher shouldn't even have initialized.
+   * However, when ZeppelinConfiguration.getRunMode() is force 'k8s', 
InterpreterSetting.getLauncherPlugin() will try
+   * to use K8sStandardInterpreterLauncher. This is useful for development. It 
allows Zeppelin server running on your
+   * IDE and creates your interpreters in Kubernetes. So any code changes on 
Zeppelin server or kubernetes yaml spec
+   * can be applied without re-building docker image.
+   * @return true, if running on K8s
+   */
+  public static boolean isRunningOnKubernetes() {
+    return new File(Config.KUBERNETES_NAMESPACE_PATH).exists();
+  }
+
+  public static String readFile(String path, Charset encoding) throws 
IOException {

Review comment:
       Please change this to `private`.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscr...@zeppelin.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


Reply via email to