You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@submarine.apache.org by GitBox <gi...@apache.org> on 2022/01/17 13:56:24 UTC

[GitHub] [submarine] cdmikechen opened a new pull request #870: SUBMARINE-1174. K8sSubmitter refactoring involving Notebook create and delete method

cdmikechen opened a new pull request #870:
URL: https://github.com/apache/submarine/pull/870


   ### What is this PR for?
   The current K8sSubmitter contains a lot of CRD processing logic in addition to the implementation of the interface. 
   The purpose of this PR is to split the CRD processing logic from the class as far as possible and merge it into the resource object for processing.
   
   The current PR refactored part of the code so that we can see the changes of the code after refactoring.
   
   ### What type of PR is it?
   Refactoring
   
   ### Todos
   * [x] - Refactoring `K8sSubmitter`  involving Notebook create and delete method
   
   ### What is the Jira issue?
   https://issues.apache.org/jira/browse/SUBMARINE-1174
   
   ### How should this be tested?
   The refactored codes can use the original test case.
   
   ### Screenshots (if appropriate)
   No
   
   ### Questions:
   * Do the license files need updating? No
   * Are there breaking changes for older versions? No
   * Does this need new documentation? No
   


-- 
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: commits-unsubscribe@submarine.apache.org

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



[GitHub] [submarine] lowc1012 commented on a change in pull request #870: SUBMARINE-1174. K8sSubmitter refactoring involving Notebook create and delete method

Posted by GitBox <gi...@apache.org>.
lowc1012 commented on a change in pull request #870:
URL: https://github.com/apache/submarine/pull/870#discussion_r795138427



##########
File path: submarine-server/server-submitter/submitter-k8s/src/main/java/org/apache/submarine/server/submitter/k8s/K8sClient.java
##########
@@ -0,0 +1,89 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.submarine.server.submitter.k8s;
+
+import io.kubernetes.client.openapi.ApiClient;
+import io.kubernetes.client.openapi.Configuration;
+import io.kubernetes.client.openapi.apis.AppsV1Api;
+import io.kubernetes.client.openapi.apis.CoreV1Api;
+import io.kubernetes.client.openapi.apis.CustomObjectsApi;
+import io.kubernetes.client.util.ClientBuilder;
+import io.kubernetes.client.util.KubeConfig;
+import okhttp3.OkHttpClient;
+import org.apache.submarine.commons.utils.exception.SubmarineRuntimeException;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.FileReader;
+import java.io.IOException;
+
+public class K8sClient {
+
+  public static final String KUBECONFIG_ENV = "KUBECONFIG";
+
+  private static final Logger LOG = LoggerFactory.getLogger(K8sClient.class);
+
+  private ApiClient client = null;
+
+  private final CustomObjectsApi customObjectsApi;
+
+  private final CoreV1Api coreApi;
+
+  private final AppsV1Api appsV1Api;
+
+  public K8sClient() {
+    String path = System.getenv(KUBECONFIG_ENV);
+    try {
+      KubeConfig config = KubeConfig.loadKubeConfig(new FileReader(path));
+      client = ClientBuilder.kubeconfig(config).build();
+    } catch (Exception e) {
+      LOG.warn(String.format("K8s client initialize failed with env path %s. Maybe in cluster mode, " +
+          "try to initialize the client again.", path), e);
+      try {

Review comment:
       Hi @cdmikechen , thanks for your contribution!
   Could we avoid nested try-catch block here?

##########
File path: submarine-server/server-submitter/submitter-k8s/src/main/java/org/apache/submarine/server/submitter/k8s/model/common/Configmap.java
##########
@@ -0,0 +1,136 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.submarine.server.submitter.k8s.model.common;
+
+import io.kubernetes.client.openapi.ApiException;
+import io.kubernetes.client.openapi.models.V1ConfigMap;
+import io.kubernetes.client.openapi.models.V1DeleteOptionsBuilder;
+import io.kubernetes.client.openapi.models.V1ObjectMeta;
+import org.apache.commons.lang3.StringUtils;
+import org.apache.submarine.commons.utils.exception.SubmarineRuntimeException;
+import org.apache.submarine.server.submitter.k8s.K8sClient;
+import org.apache.submarine.server.submitter.k8s.K8sSubmitter;
+import org.apache.submarine.server.submitter.k8s.model.K8sResource;
+import org.apache.submarine.server.submitter.k8s.util.JsonUtils;
+import org.apache.submarine.server.submitter.k8s.util.OwnerReferenceUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.util.LinkedHashMap;
+import java.util.Map;
+
+public class Configmap extends V1ConfigMap implements K8sResource {
+
+  private static final Logger LOG = LoggerFactory.getLogger(Configmap.class);
+
+  public Configmap(String namespace, String name, String... values) {
+    Map<String, String> datas = new LinkedHashMap<>();
+    for (int i = 0, size = values.length; i < size; i += 2) {
+      try {
+        datas.put(values[i], values[i + 1]);
+      } catch (ArrayIndexOutOfBoundsException e) {// Avoid values by odd numbers
+        LOG.warn("Can not find ConfigMap value in index[{}], skip this value", i + 1);
+      }
+    }
+    init(namespace, name, datas);
+  }
+
+  public Configmap(String namespace, String name, Map<String, String> datas) {
+    init(namespace, name, datas);
+  }
+
+  private void init(String namespace, String name, Map<String, String> datas) {
+    /*
+      Required value
+      1. metadata.name
+      2. metadata.namespace
+      3. spec.data
+      4. spec.resources
+      Others are not necessary
+     */
+    V1ObjectMeta metadata = new V1ObjectMeta();
+    metadata.setNamespace(namespace);
+    metadata.setName(name);
+    metadata.setOwnerReferences(OwnerReferenceUtils.getOwnerReference());;
+    this.setMetadata(metadata);
+    this.data(datas);
+  }
+
+  @Override
+  public Configmap read(K8sClient api) {
+    return this;
+  }
+
+  public void resetMeta(K8sClient api) {
+    try {
+      Object object = api.getCoreApi().readNamespacedConfigMap(this.getMetadata().getName(),
+              this.getMetadata().getNamespace(), null, null, null);
+      if (object != null) {
+        String jsonString = JsonUtils.toJson(((Map<String, Object>) object).get("metadata"));
+        V1ObjectMeta meta = JsonUtils.fromJson(jsonString, V1ObjectMeta.class);
+        this.setMetadata(meta);
+      }
+    } catch (ApiException e) {
+      LOG.error("K8s submitter: parse configmap object failed by " + e.getMessage(), e);
+      throw new SubmarineRuntimeException(e.getCode(), "K8s submitter: parse configmap object failed by " +
+              e.getMessage());
+    }
+  }
+
+  @Override
+  public Object create(K8sClient api) {
+    try {
+      return api.getCoreApi().createNamespacedConfigMap(this.getMetadata().getNamespace(), this, "true",

Review comment:
       may produce NPE problem

##########
File path: submarine-server/server-submitter/submitter-k8s/src/main/java/org/apache/submarine/server/submitter/k8s/model/NotebookCR.java
##########
@@ -124,4 +153,75 @@ public NotebookStatus getStatus() {
   public void setStatus(NotebookStatus status) {
     this.status = status;
   }
+
+  @Override
+  public NotebookCR read(K8sClient client) {
+    return this;
+  }
+
+  @Override
+  public Notebook create(K8sClient client) {
+    return create(client, false);
+  }
+
+
+  /**
+   * Create Notebook CRD
+   * @param client K8sClient
+   * @param tolerate Update when create conflicts
+   * @return Notebook
+   */
+  public Notebook create(K8sClient client, boolean tolerate) {
+    Notebook notebook = null;
+    try {
+      Object object = client.getCustomObjectsApi()
+          .createNamespacedCustomObject(this.getGroup(), this.getVersion(),
+          getMetadata().getNamespace(), this.getPlural(), this, "true", null, null);
+      notebook = NotebookUtils.parseObject(object, NotebookUtils.ParseOpt.PARSE_OPT_CREATE);
+    } catch (JsonSyntaxException e) {
+      LOG.error("K8s submitter: parse response object failed by " + e.getMessage(), e);
+      throw new SubmarineRuntimeException(500, "K8s Submitter parse upstream response failed.");
+    } catch (ApiException e) {
+      if (e.getCode() == 409 && tolerate) {// conflict
+        // todo need to replace CRD
+        LOG.warn("K8s submitter: resource already exists, need to replace it.", e);
+        notebook = NotebookUtils.parseObject(this, NotebookUtils.ParseOpt.PARSE_OPT_REPLACE);
+      } else {
+        LOG.error("K8s submitter: parse Notebook object failed by " + e.getMessage(), e);
+        throw new SubmarineRuntimeException(e.getCode(), "K8s submitter: parse Notebook object failed by " +
+                e.getMessage());
+      }
+    }
+    return notebook;
+  }
+
+  @Override
+  public Object replace(K8sClient client) {
+    return null;

Review comment:
       add TODO comment
   https://www.jetbrains.com/help/idea/using-todo.html

##########
File path: submarine-server/server-submitter/submitter-k8s/src/main/java/org/apache/submarine/server/submitter/k8s/model/common/PersistentVolumeClaim.java
##########
@@ -0,0 +1,116 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.submarine.server.submitter.k8s.model.common;
+
+import com.google.gson.JsonSyntaxException;
+import io.kubernetes.client.custom.Quantity;
+import io.kubernetes.client.openapi.ApiException;
+import io.kubernetes.client.openapi.models.V1ObjectMeta;
+import io.kubernetes.client.openapi.models.V1PersistentVolumeClaim;
+import io.kubernetes.client.openapi.models.V1PersistentVolumeClaimSpec;
+import io.kubernetes.client.openapi.models.V1ResourceRequirements;
+import org.apache.submarine.commons.utils.exception.SubmarineRuntimeException;
+import org.apache.submarine.server.submitter.k8s.K8sClient;
+import org.apache.submarine.server.submitter.k8s.K8sSubmitter;
+import org.apache.submarine.server.submitter.k8s.model.K8sResource;
+import org.apache.submarine.server.submitter.k8s.util.NotebookUtils;
+import org.apache.submarine.server.submitter.k8s.util.OwnerReferenceUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.util.Collections;
+
+public class PersistentVolumeClaim extends V1PersistentVolumeClaim implements K8sResource {
+
+  private static final Logger LOG = LoggerFactory.getLogger(PersistentVolumeClaim.class);
+
+  public PersistentVolumeClaim(String namespace, String name, String storage) {
+    /*
+      Required value
+      1. metadata.name
+      2. metadata.namespace
+      3. spec.accessModes
+      4. spec.storageClassName
+      5. spec.resources
+      Others are not necessary
+     */
+    V1ObjectMeta pvcMetadata = new V1ObjectMeta();
+    pvcMetadata.setNamespace(namespace);
+    pvcMetadata.setName(name);
+    pvcMetadata.setOwnerReferences(OwnerReferenceUtils.getOwnerReference());
+    this.setMetadata(pvcMetadata);
+
+    V1PersistentVolumeClaimSpec pvcSpec = new V1PersistentVolumeClaimSpec();
+    pvcSpec.setAccessModes(Collections.singletonList("ReadWriteOnce"));
+    pvcSpec.setStorageClassName(NotebookUtils.SC_NAME);
+    pvcSpec.setResources(new V1ResourceRequirements().putRequestsItem("storage", new Quantity(storage)));
+    this.setSpec(pvcSpec);
+  }
+
+  @Override
+  public PersistentVolumeClaim read(K8sClient api) {
+    return this;
+  }
+
+  @Override
+  public V1PersistentVolumeClaim create(K8sClient api) {
+    try {
+      return api.getCoreApi().createNamespacedPersistentVolumeClaim(
+              this.getMetadata().getNamespace(), this, "true", null, null
+      );
+    } catch (ApiException e) {
+      LOG.error("Exception when creating persistent volume claim " + e.getMessage(), e);
+      throw new SubmarineRuntimeException(e.getCode(), "K8s submitter: Create persistent volume claim for " +
+              "Notebook object failed by " + e.getMessage());
+    }
+  }
+
+  @Override
+  public Object replace(K8sClient api) {
+    return null;
+  }
+
+  @Override
+  public Object delete(K8sClient api) {
+    try {
+      return api.getCoreApi().deleteNamespacedPersistentVolumeClaim(
+              this.getMetadata().getName(), this.getMetadata().getNamespace(), "true",

Review comment:
       May produce NPE problem.

##########
File path: submarine-server/server-submitter/submitter-k8s/src/main/java/org/apache/submarine/server/submitter/k8s/model/common/PersistentVolumeClaim.java
##########
@@ -0,0 +1,116 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.submarine.server.submitter.k8s.model.common;
+
+import com.google.gson.JsonSyntaxException;
+import io.kubernetes.client.custom.Quantity;
+import io.kubernetes.client.openapi.ApiException;
+import io.kubernetes.client.openapi.models.V1ObjectMeta;
+import io.kubernetes.client.openapi.models.V1PersistentVolumeClaim;
+import io.kubernetes.client.openapi.models.V1PersistentVolumeClaimSpec;
+import io.kubernetes.client.openapi.models.V1ResourceRequirements;
+import org.apache.submarine.commons.utils.exception.SubmarineRuntimeException;
+import org.apache.submarine.server.submitter.k8s.K8sClient;
+import org.apache.submarine.server.submitter.k8s.K8sSubmitter;
+import org.apache.submarine.server.submitter.k8s.model.K8sResource;
+import org.apache.submarine.server.submitter.k8s.util.NotebookUtils;
+import org.apache.submarine.server.submitter.k8s.util.OwnerReferenceUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.util.Collections;
+
+public class PersistentVolumeClaim extends V1PersistentVolumeClaim implements K8sResource {
+
+  private static final Logger LOG = LoggerFactory.getLogger(PersistentVolumeClaim.class);
+
+  public PersistentVolumeClaim(String namespace, String name, String storage) {
+    /*
+      Required value
+      1. metadata.name
+      2. metadata.namespace
+      3. spec.accessModes
+      4. spec.storageClassName
+      5. spec.resources
+      Others are not necessary
+     */
+    V1ObjectMeta pvcMetadata = new V1ObjectMeta();
+    pvcMetadata.setNamespace(namespace);
+    pvcMetadata.setName(name);
+    pvcMetadata.setOwnerReferences(OwnerReferenceUtils.getOwnerReference());
+    this.setMetadata(pvcMetadata);
+
+    V1PersistentVolumeClaimSpec pvcSpec = new V1PersistentVolumeClaimSpec();
+    pvcSpec.setAccessModes(Collections.singletonList("ReadWriteOnce"));
+    pvcSpec.setStorageClassName(NotebookUtils.SC_NAME);
+    pvcSpec.setResources(new V1ResourceRequirements().putRequestsItem("storage", new Quantity(storage)));
+    this.setSpec(pvcSpec);
+  }
+
+  @Override
+  public PersistentVolumeClaim read(K8sClient api) {
+    return this;
+  }
+
+  @Override
+  public V1PersistentVolumeClaim create(K8sClient api) {
+    try {
+      return api.getCoreApi().createNamespacedPersistentVolumeClaim(
+              this.getMetadata().getNamespace(), this, "true", null, null

Review comment:
       May produce NPE problem.

##########
File path: submarine-server/server-submitter/submitter-k8s/src/main/java/org/apache/submarine/server/submitter/k8s/model/common/Configmap.java
##########
@@ -0,0 +1,136 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.submarine.server.submitter.k8s.model.common;
+
+import io.kubernetes.client.openapi.ApiException;
+import io.kubernetes.client.openapi.models.V1ConfigMap;
+import io.kubernetes.client.openapi.models.V1DeleteOptionsBuilder;
+import io.kubernetes.client.openapi.models.V1ObjectMeta;
+import org.apache.commons.lang3.StringUtils;
+import org.apache.submarine.commons.utils.exception.SubmarineRuntimeException;
+import org.apache.submarine.server.submitter.k8s.K8sClient;
+import org.apache.submarine.server.submitter.k8s.K8sSubmitter;
+import org.apache.submarine.server.submitter.k8s.model.K8sResource;
+import org.apache.submarine.server.submitter.k8s.util.JsonUtils;
+import org.apache.submarine.server.submitter.k8s.util.OwnerReferenceUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.util.LinkedHashMap;
+import java.util.Map;
+
+public class Configmap extends V1ConfigMap implements K8sResource {
+
+  private static final Logger LOG = LoggerFactory.getLogger(Configmap.class);
+
+  public Configmap(String namespace, String name, String... values) {
+    Map<String, String> datas = new LinkedHashMap<>();
+    for (int i = 0, size = values.length; i < size; i += 2) {
+      try {
+        datas.put(values[i], values[i + 1]);
+      } catch (ArrayIndexOutOfBoundsException e) {// Avoid values by odd numbers
+        LOG.warn("Can not find ConfigMap value in index[{}], skip this value", i + 1);
+      }
+    }
+    init(namespace, name, datas);
+  }
+
+  public Configmap(String namespace, String name, Map<String, String> datas) {
+    init(namespace, name, datas);
+  }
+
+  private void init(String namespace, String name, Map<String, String> datas) {
+    /*
+      Required value
+      1. metadata.name
+      2. metadata.namespace
+      3. spec.data
+      4. spec.resources
+      Others are not necessary
+     */
+    V1ObjectMeta metadata = new V1ObjectMeta();
+    metadata.setNamespace(namespace);
+    metadata.setName(name);
+    metadata.setOwnerReferences(OwnerReferenceUtils.getOwnerReference());;
+    this.setMetadata(metadata);
+    this.data(datas);
+  }
+
+  @Override
+  public Configmap read(K8sClient api) {
+    return this;
+  }
+
+  public void resetMeta(K8sClient api) {
+    try {
+      Object object = api.getCoreApi().readNamespacedConfigMap(this.getMetadata().getName(),
+              this.getMetadata().getNamespace(), null, null, null);
+      if (object != null) {
+        String jsonString = JsonUtils.toJson(((Map<String, Object>) object).get("metadata"));
+        V1ObjectMeta meta = JsonUtils.fromJson(jsonString, V1ObjectMeta.class);
+        this.setMetadata(meta);
+      }
+    } catch (ApiException e) {
+      LOG.error("K8s submitter: parse configmap object failed by " + e.getMessage(), e);
+      throw new SubmarineRuntimeException(e.getCode(), "K8s submitter: parse configmap object failed by " +
+              e.getMessage());
+    }
+  }
+
+  @Override
+  public Object create(K8sClient api) {
+    try {
+      return api.getCoreApi().createNamespacedConfigMap(this.getMetadata().getNamespace(), this, "true",
+              null, null);
+    } catch (ApiException e) {
+      LOG.error("Exception when creating configmap " + e.getMessage(), e);
+      throw new SubmarineRuntimeException(e.getCode(), "K8s submitter: create configmap failed by " +
+              e.getMessage());
+    }
+  }
+
+  @Override
+  public Object replace(K8sClient api) {
+    try {
+      // reset metadata to get resource version so that we can replace configmap
+      if (StringUtils.isBlank(this.getMetadata().getResourceVersion())) {
+        resetMeta(api);
+      }
+      // replace
+      return api.getCoreApi().replaceNamespacedConfigMap(this.getMetadata().getName(),
+              getMetadata().getNamespace(), this, "true", null, null);
+    } catch (ApiException e) {
+      LOG.error("K8s submitter: replace configmap object failed by " + e.getMessage(), e);
+      throw new SubmarineRuntimeException(e.getCode(), "K8s submitter: replace configmap object failed by " +
+              e.getMessage());
+    }
+  }
+
+  @Override
+  public Object delete(K8sClient api) {
+    try {
+      return api.getCoreApi().deleteNamespacedConfigMap(this.getMetadata().getName(),

Review comment:
       may produce NPE problem

##########
File path: submarine-server/server-submitter/submitter-k8s/src/main/java/org/apache/submarine/server/submitter/k8s/model/common/Configmap.java
##########
@@ -0,0 +1,136 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.submarine.server.submitter.k8s.model.common;
+
+import io.kubernetes.client.openapi.ApiException;
+import io.kubernetes.client.openapi.models.V1ConfigMap;
+import io.kubernetes.client.openapi.models.V1DeleteOptionsBuilder;
+import io.kubernetes.client.openapi.models.V1ObjectMeta;
+import org.apache.commons.lang3.StringUtils;
+import org.apache.submarine.commons.utils.exception.SubmarineRuntimeException;
+import org.apache.submarine.server.submitter.k8s.K8sClient;
+import org.apache.submarine.server.submitter.k8s.K8sSubmitter;
+import org.apache.submarine.server.submitter.k8s.model.K8sResource;
+import org.apache.submarine.server.submitter.k8s.util.JsonUtils;
+import org.apache.submarine.server.submitter.k8s.util.OwnerReferenceUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.util.LinkedHashMap;
+import java.util.Map;
+
+public class Configmap extends V1ConfigMap implements K8sResource {
+
+  private static final Logger LOG = LoggerFactory.getLogger(Configmap.class);
+
+  public Configmap(String namespace, String name, String... values) {
+    Map<String, String> datas = new LinkedHashMap<>();
+    for (int i = 0, size = values.length; i < size; i += 2) {
+      try {
+        datas.put(values[i], values[i + 1]);
+      } catch (ArrayIndexOutOfBoundsException e) {// Avoid values by odd numbers
+        LOG.warn("Can not find ConfigMap value in index[{}], skip this value", i + 1);
+      }
+    }
+    init(namespace, name, datas);
+  }
+
+  public Configmap(String namespace, String name, Map<String, String> datas) {
+    init(namespace, name, datas);
+  }
+
+  private void init(String namespace, String name, Map<String, String> datas) {
+    /*
+      Required value
+      1. metadata.name
+      2. metadata.namespace
+      3. spec.data
+      4. spec.resources

Review comment:
       Should be "3. data".
   And I can't find the "spec.resources" in ConfigMap resource.

##########
File path: submarine-server/server-submitter/submitter-k8s/src/main/java/org/apache/submarine/server/submitter/k8s/model/common/Configmap.java
##########
@@ -0,0 +1,136 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.submarine.server.submitter.k8s.model.common;
+
+import io.kubernetes.client.openapi.ApiException;
+import io.kubernetes.client.openapi.models.V1ConfigMap;
+import io.kubernetes.client.openapi.models.V1DeleteOptionsBuilder;
+import io.kubernetes.client.openapi.models.V1ObjectMeta;
+import org.apache.commons.lang3.StringUtils;
+import org.apache.submarine.commons.utils.exception.SubmarineRuntimeException;
+import org.apache.submarine.server.submitter.k8s.K8sClient;
+import org.apache.submarine.server.submitter.k8s.K8sSubmitter;
+import org.apache.submarine.server.submitter.k8s.model.K8sResource;
+import org.apache.submarine.server.submitter.k8s.util.JsonUtils;
+import org.apache.submarine.server.submitter.k8s.util.OwnerReferenceUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.util.LinkedHashMap;
+import java.util.Map;
+
+public class Configmap extends V1ConfigMap implements K8sResource {
+
+  private static final Logger LOG = LoggerFactory.getLogger(Configmap.class);
+
+  public Configmap(String namespace, String name, String... values) {
+    Map<String, String> datas = new LinkedHashMap<>();
+    for (int i = 0, size = values.length; i < size; i += 2) {
+      try {
+        datas.put(values[i], values[i + 1]);
+      } catch (ArrayIndexOutOfBoundsException e) {// Avoid values by odd numbers
+        LOG.warn("Can not find ConfigMap value in index[{}], skip this value", i + 1);
+      }
+    }
+    init(namespace, name, datas);
+  }
+
+  public Configmap(String namespace, String name, Map<String, String> datas) {
+    init(namespace, name, datas);
+  }
+
+  private void init(String namespace, String name, Map<String, String> datas) {
+    /*
+      Required value
+      1. metadata.name
+      2. metadata.namespace
+      3. spec.data
+      4. spec.resources
+      Others are not necessary
+     */
+    V1ObjectMeta metadata = new V1ObjectMeta();
+    metadata.setNamespace(namespace);
+    metadata.setName(name);
+    metadata.setOwnerReferences(OwnerReferenceUtils.getOwnerReference());;
+    this.setMetadata(metadata);
+    this.data(datas);
+  }
+
+  @Override
+  public Configmap read(K8sClient api) {
+    return this;
+  }
+
+  public void resetMeta(K8sClient api) {
+    try {
+      Object object = api.getCoreApi().readNamespacedConfigMap(this.getMetadata().getName(),
+              this.getMetadata().getNamespace(), null, null, null);
+      if (object != null) {
+        String jsonString = JsonUtils.toJson(((Map<String, Object>) object).get("metadata"));
+        V1ObjectMeta meta = JsonUtils.fromJson(jsonString, V1ObjectMeta.class);
+        this.setMetadata(meta);
+      }
+    } catch (ApiException e) {
+      LOG.error("K8s submitter: parse configmap object failed by " + e.getMessage(), e);
+      throw new SubmarineRuntimeException(e.getCode(), "K8s submitter: parse configmap object failed by " +
+              e.getMessage());
+    }
+  }
+
+  @Override
+  public Object create(K8sClient api) {
+    try {
+      return api.getCoreApi().createNamespacedConfigMap(this.getMetadata().getNamespace(), this, "true",
+              null, null);
+    } catch (ApiException e) {
+      LOG.error("Exception when creating configmap " + e.getMessage(), e);
+      throw new SubmarineRuntimeException(e.getCode(), "K8s submitter: create configmap failed by " +
+              e.getMessage());
+    }
+  }
+
+  @Override
+  public Object replace(K8sClient api) {
+    try {
+      // reset metadata to get resource version so that we can replace configmap
+      if (StringUtils.isBlank(this.getMetadata().getResourceVersion())) {

Review comment:
       may produce NPE problem




-- 
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: commits-unsubscribe@submarine.apache.org

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