jerryshao commented on code in PR #13724:
URL: https://github.com/apache/gravitino/pull/13724#discussion_r4246116056


##########
plugins/k8s-job-executor/src/main/java/org/apache/gravitino/job/k8s/K8sJobExecutorConfigs.java:
##########
@@ -0,0 +1,510 @@
+/*
+ * 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.gravitino.job.k8s;
+
+import com.google.common.base.Preconditions;
+import com.google.common.collect.ImmutableMap;
+import java.util.Map;
+import java.util.TreeMap;
+import java.util.regex.Pattern;
+import javax.annotation.Nullable;
+import org.apache.commons.lang3.StringUtils;
+
+/**
+ * The configurations of the {@code k8s} job executor, set with the {@code
+ * gravitino.jobExecutor.k8s.} prefix in the Gravitino server configuration.
+ */
+public class K8sJobExecutorConfigs {
+
+  /** The name of the job executor. */
+  public static final String K8S_JOB_EXECUTOR_NAME = "k8s";
+
+  /** The name of the Kubernetes cluster the jobs run in, which is part of the 
job execution id. */
+  public static final String CLUSTER = "cluster";
+
+  /** The default cluster name. */
+  public static final String DEFAULT_CLUSTER = "default";
+
+  /** The kubeconfig file of the job cluster. */
+  public static final String KUBECONFIG = "kubeconfig";
+
+  /** The kubeconfig context to use, the current context if not set. */
+  public static final String CONTEXT = "context";
+
+  /**
+   * The API server URL of the job cluster, an alternative to the kubeconfig. 
It must start with
+   * {@code https://}, or with {@code http://} if no token file is set.
+   */
+  public static final String MASTER_URL = "masterUrl";
+
+  /** The CA certificate file of the API server, used with {@link 
#MASTER_URL}. */
+  public static final String CA_CERT_FILE = "caCertFile";
+
+  /**
+   * The bearer token file to access the API server, used with {@link 
#MASTER_URL}. It is read again
+   * whenever it changes, so the token can be rotated.
+   */
+  public static final String TOKEN_FILE = "tokenFile";
+
+  /** How long to wait to connect to the API server. */
+  public static final String CONNECTION_TIMEOUT_MS = "connectionTimeoutInMs";
+
+  /** The default connection timeout, 5 seconds. */
+  public static final long DEFAULT_CONNECTION_TIMEOUT_MS = 5_000L;
+
+  /** How long to wait for the response of a request to the API server. */
+  public static final String REQUEST_TIMEOUT_MS = "requestTimeoutInMs";
+
+  /** The default request timeout, 10 seconds. */
+  public static final long DEFAULT_REQUEST_TIMEOUT_MS = 10_000L;
+
+  /**
+   * How many times a failed request to the API server is tried again, for 
example if the server
+   * can't be reached or answers with a server error. Together with the 
timeouts it bounds how long
+   * a submission, a status query or a cancellation waits while the job 
cluster is down.
+   */
+  public static final String REQUEST_RETRIES = "requestRetries";
+
+  /** The default number of request retries. */
+  public static final long DEFAULT_REQUEST_RETRIES = 2L;
+
+  /**
+   * The namespace the jobs run in, unless a job sets its own. It must be set: 
the namespace needs
+   * the permissions and the Spark operator set up for the jobs, so none is 
assumed.
+   */
+  public static final String NAMESPACE = "namespace";
+
+  /** The prefix of the names of the Kubernetes resources created for the 
jobs. */
+  public static final String NAME_PREFIX = "namePrefix";
+
+  /** The default name prefix. */
+  public static final String DEFAULT_NAME_PREFIX = "gravitino-job";
+
+  /** How long the resources listed in a namespace are cached for the job 
status queries. */
+  public static final String STATUS_CACHE_TTL_MS = "statusCacheTtlInMs";
+
+  /** The default status cache TTL, 5 seconds. */
+  public static final long DEFAULT_STATUS_CACHE_TTL_MS = 5_000L;
+
+  /** A job whose resource still has no status after this time is considered 
failed. */
+  public static final String NO_STATUS_TIMEOUT_MS = "noStatusTimeoutInMs";
+
+  /** The default no status timeout, 10 minutes. */
+  public static final long DEFAULT_NO_STATUS_TIMEOUT_MS = 10 * 60 * 1000L;
+
+  /** The Spark image of the driver and executors, the default of a job's 
container image. */
+  public static final String SPARK_IMAGE = "spark.image";
+
+  /** The Spark version of the image, set as the SparkApplication runtime 
version. */
+  public static final String SPARK_VERSION = "spark.sparkVersion";
+
+  /** The service account of the Spark driver, the default of a job's driver 
service account. */
+  public static final String SPARK_SERVICE_ACCOUNT = "spark.serviceAccount";
+
+  /** The default Spark driver service account. */
+  public static final String DEFAULT_SPARK_SERVICE_ACCOUNT = "spark";
+
+  /** How long the pods of a finished Spark job are retained, so that its 
output can be read. */
+  public static final String SPARK_RESOURCE_RETAIN_DURATION_MS = 
"spark.resourceRetainDurationInMs";
+
+  /** The default pod retain duration, 1 day. */
+  public static final long DEFAULT_SPARK_RESOURCE_RETAIN_DURATION_MS = 24 * 60 
* 60 * 1000L;
+
+  /** The time to live of a SparkApplication after it stops. */
+  public static final String SPARK_TTL_AFTER_STOP_MS = 
"spark.ttlAfterStopInMs";
+
+  /** The default SparkApplication time to live after it stops, 7 days. */
+  public static final long DEFAULT_SPARK_TTL_AFTER_STOP_MS = 7 * 24 * 60 * 60 
* 1000L;
+
+  /** How long the operator waits for the driver pod to start. */
+  public static final String SPARK_DRIVER_START_TIMEOUT_MS = 
"spark.driverStartTimeoutInMs";
+
+  /** How long the operator waits for the driver to be ready. */
+  public static final String SPARK_DRIVER_READY_TIMEOUT_MS = 
"spark.driverReadyTimeoutInMs";
+
+  /**
+   * The default of the operator's start timeouts, 1 hour, so that a job waits 
for resources instead
+   * of failing after the operator's default of 5 minutes.
+   */
+  public static final long DEFAULT_SPARK_START_TIMEOUT_MS = 60 * 60 * 1000L;
+
+  /**
+   * The prefix of the default Spark configurations of every Spark job. The 
job's own Spark
+   * configurations take precedence.
+   */
+  public static final String SPARK_CONF_PREFIX = "spark.conf.";
+
+  // The resource name is the prefix, a dash and a job id of up to 19 digits. 
Keep it within 44
+  // characters, so that the operator doesn't hash-truncate the Spark app id 
derived from it.
+  private static final int MAX_NAME_PREFIX_LENGTH = 24;
+
+  // A DNS-1123 label, which the resource name must be.
+  private static final Pattern NAME_PREFIX_PATTERN = 
Pattern.compile("[a-z]([-a-z0-9]*[a-z0-9])?");
+
+  private final String cluster;
+  @Nullable private final String kubeconfig;
+  @Nullable private final String context;
+  @Nullable private final String masterUrl;
+  @Nullable private final String caCertFile;
+  @Nullable private final String tokenFile;
+  private final String namespace;
+  private final int connectionTimeoutMs;
+  private final int requestTimeoutMs;
+  private final int requestRetries;
+  private final String namePrefix;
+  private final long statusCacheTtlMs;
+  private final long noStatusTimeoutMs;
+  private final String sparkImage;
+  private final String sparkVersion;
+  private final String sparkServiceAccount;
+  private final long sparkResourceRetainDurationMs;
+  private final long sparkTtlAfterStopMs;
+  private final long sparkDriverStartTimeoutMs;
+  private final long sparkDriverReadyTimeoutMs;
+  private final Map<String, String> sparkConf;
+
+  /**
+   * Parses and validates the job executor configurations.
+   *
+   * @param configs the configurations without the {@code 
gravitino.jobExecutor.k8s.} prefix
+   * @throws IllegalArgumentException if a configuration is invalid
+   */
+  public K8sJobExecutorConfigs(Map<String, String> configs) {
+    this.cluster = stringValue(configs, CLUSTER, DEFAULT_CLUSTER);
+    Preconditions.checkArgument(
+        !cluster.contains("/"), "%s must not contain '/': %s", CLUSTER, 
cluster);
+
+    this.kubeconfig = stringValue(configs, KUBECONFIG, null);
+    this.context = stringValue(configs, CONTEXT, null);
+    this.masterUrl = stringValue(configs, MASTER_URL, null);
+    this.caCertFile = stringValue(configs, CA_CERT_FILE, null);
+    this.tokenFile = stringValue(configs, TOKEN_FILE, null);
+    Preconditions.checkArgument(
+        masterUrl == null || (kubeconfig == null && context == null),
+        "%s can't be set together with %s or %s",
+        MASTER_URL,
+        KUBECONFIG,
+        CONTEXT);
+    Preconditions.checkArgument(
+        masterUrl != null || (caCertFile == null && tokenFile == null),
+        "%s and %s can only be set when %s is set",
+        CA_CERT_FILE,
+        TOKEN_FILE,
+        MASTER_URL);
+    // Without a scheme the client would pick one by probing the server, and 
fall back to plain
+    // HTTP if the probe fails, for example because the server isn't reachable 
at startup.
+    boolean https = StringUtils.startsWithIgnoreCase(masterUrl, "https://";);
+    Preconditions.checkArgument(
+        masterUrl == null || https || 
StringUtils.startsWithIgnoreCase(masterUrl, "http://";),
+        "%s must start with https:// or http://: %s",
+        MASTER_URL,
+        masterUrl);
+    Preconditions.checkArgument(
+        masterUrl == null || https || tokenFile == null,
+        "%s must start with https:// when %s is set, otherwise the token would 
be sent in clear"
+            + " text: %s",
+        MASTER_URL,
+        TOKEN_FILE,
+        masterUrl);

Review Comment:
   [Nit] `caCertFile` with an `http://` master URL is accepted and can never do 
anything.
   
   `http://` is deliberately allowed when no token file is set, which is the 
right call for something like `kubectl proxy`. But `caCertFile` is then dead 
configuration: it only ever feeds the TLS context, so an operator who sets both 
gets an unverified, unencrypted connection while the configuration reads as if 
the API server's certificate were being checked. Rejecting that pair here, next 
to the token rule, costs one more precondition and removes the ambiguity. A 
warning would do as well.
   
   Verified by: reading this constructor on a2db2c7 (`caCertFile` is only 
required to come with `masterUrl`, lines 203-209) and 
`K8sClientUtils.createConnectionConfig`, which passes it to `withCaCertFile` 
regardless of the scheme (K8sClientUtils.java:170-173).



##########
plugins/k8s-job-executor/src/main/java/org/apache/gravitino/job/k8s/K8sClientUtils.java:
##########
@@ -0,0 +1,273 @@
+/*
+ * 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.gravitino.job.k8s;
+
+import com.google.common.base.Preconditions;
+import io.fabric8.kubernetes.api.model.NamedContext;
+import io.fabric8.kubernetes.client.Config;
+import io.fabric8.kubernetes.client.ConfigBuilder;
+import io.fabric8.kubernetes.client.KubernetesClient;
+import io.fabric8.kubernetes.client.KubernetesClientBuilder;
+import io.fabric8.kubernetes.client.OAuthTokenProvider;
+import java.io.File;
+import java.io.IOException;
+import java.io.UncheckedIOException;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.nio.file.attribute.FileTime;
+import java.util.concurrent.TimeUnit;
+import java.util.regex.Pattern;
+import javax.annotation.Nullable;
+import org.apache.commons.lang3.StringUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/** Creates the Kubernetes client of the job cluster from the job executor 
configurations. */
+public final class K8sClientUtils {
+
+  /**
+   * An {@link OAuthTokenProvider} that reads the token from a file, and reads 
it again whenever the
+   * file changes, so that a rotated token is picked up without restarting the 
server.
+   */
+  static final class FileTokenProvider implements OAuthTokenProvider {
+
+    // How often a token file that stays empty or unreadable is reported.
+    private static final long FAILURE_LOG_INTERVAL_NANOS = 
TimeUnit.MINUTES.toNanos(5);
+
+    private final Path tokenFile;
+
+    private FileTime lastModified;
+
+    private String token;
+
+    private boolean failing;
+
+    private long failureLoggedAtNanos;
+
+    FileTokenProvider(Path tokenFile) {
+      this.tokenFile = tokenFile;
+    }
+
+    @Override
+    public synchronized String getToken() {
+      try {
+        FileTime modified = Files.getLastModifiedTime(tokenFile);
+        if (token == null || !modified.equals(lastModified)) {
+          String content = new String(Files.readAllBytes(tokenFile), 
StandardCharsets.UTF_8).trim();
+          if (StringUtils.isBlank(content)) {
+            if (token == null) {
+              throw new IllegalStateException("The token file " + tokenFile + 
" is empty");
+            }
+            // The file is being rewritten. Its modification time isn't 
remembered: the new token
+            // may be written within the same tick of the file system's clock, 
and would then
+            // never be read.
+            return keepToken("is empty", null);
+          }
+          token = content;
+          lastModified = modified;
+        }
+      } catch (IOException e) {
+        if (token == null) {
+          throw new UncheckedIOException("Failed to read the token file " + 
tokenFile, e);
+        }
+        // For example, the file is being replaced.
+        return keepToken("can't be read", e);
+      }
+      failing = false;
+      return token;
+    }
+
+    /**
+     * Keeps the token read before while the file has no token, checking the 
file again on the next
+     * request. The problem is reported when it starts, and then at an 
interval rather than on every
+     * request, so that a file that stays broken remains visible without 
flooding the log.
+     */
+    private String keepToken(String problem, @Nullable IOException cause) {
+      long now = System.nanoTime();
+      if (!failing || now - failureLoggedAtNanos >= 
FAILURE_LOG_INTERVAL_NANOS) {
+        LOG.warn(
+            "The token file {} {}, keeping the token read before. Requests 
will fail once that"
+                + " token expires",
+            tokenFile,
+            problem,
+            cause);
+        failing = true;
+        failureLoggedAtNanos = now;
+      }
+      return token;
+    }
+  }
+
+  private static final Logger LOG = 
LoggerFactory.getLogger(K8sClientUtils.class);
+
+  private static final Pattern SERVER_SCHEME =
+      Pattern.compile("^https?://", Pattern.CASE_INSENSITIVE);
+
+  private K8sClientUtils() {}
+
+  /**
+   * Creates the Kubernetes client of the job cluster.
+   *
+   * @param configs the job executor configurations
+   * @return the Kubernetes client
+   */
+  public static KubernetesClient createClient(K8sJobExecutorConfigs configs) {
+    return new 
KubernetesClientBuilder().withConfig(createClientConfig(configs)).build();
+  }
+
+  /**
+   * Creates the Kubernetes client configuration of the job cluster. It is, by 
precedence:
+   *
+   * <ul>
+   *   <li>the API server URL, with its CA certificate and token file, if the 
master URL is set;
+   *   <li>the given kubeconfig file and context, if the kubeconfig is set;
+   *   <li>fabric8's default lookup otherwise, that is the {@code KUBECONFIG} 
environment variable
+   *       or {@code ~/.kube/config} with the given context, then the 
in-cluster service account.
+   * </ul>
+   *
+   * <p>In every case the default namespace of the client is the namespace of 
the job executor
+   * configurations, rather than the one of the kubeconfig context or of the 
pod Gravitino runs in.
+   * The connection timeout, the request timeout and the number of request 
retries are the ones of
+   * the job executor configurations too. They bound how long a call to the 
client waits while the
+   * job cluster is down. With the defaults that is about 15 seconds if the 
API server can't be
+   * reached, and up to about 30 seconds if it accepts the requests but never 
answers them, as a
+   * request that timed out is tried again too.
+   *
+   * @param configs the job executor configurations
+   * @return the Kubernetes client configuration
+   * @throws IllegalArgumentException if a configured file doesn't exist, the 
token file has no
+   *     token, or the kubeconfig has no context that selects a cluster with a 
server
+   */
+  static Config createClientConfig(K8sJobExecutorConfigs configs) {
+    Config config = createConnectionConfig(configs);
+    config.setNamespace(configs.namespace());
+    // The client would otherwise try a request again 10 times, which holds a 
status query or a
+    // job submission for about two minutes while the job cluster can't be 
reached.
+    config.setConnectionTimeout(configs.connectionTimeoutMs());
+    config.setRequestTimeout(configs.requestTimeoutMs());
+    config.setRequestRetryBackoffLimit(configs.requestRetries());
+    return config;
+  }
+
+  private static Config createConnectionConfig(K8sJobExecutorConfigs configs) {
+    if (configs.masterUrl() != null) {
+      ConfigBuilder builder = new 
ConfigBuilder(Config.empty()).withMasterUrl(configs.masterUrl());
+      if (configs.caCertFile() != null) {
+        checkFileExists(configs.caCertFile(), 
K8sJobExecutorConfigs.CA_CERT_FILE);
+        builder.withCaCertFile(configs.caCertFile());
+      }
+      if (configs.tokenFile() != null) {
+        checkFileExists(configs.tokenFile(), K8sJobExecutorConfigs.TOKEN_FILE);
+        FileTokenProvider tokenProvider = new 
FileTokenProvider(Paths.get(configs.tokenFile()));
+        // Read the token once now, so that a file without a token fails the 
startup rather than
+        // the first request.
+        try {
+          tokenProvider.getToken();
+        } catch (IllegalStateException | UncheckedIOException e) {
+          throw new IllegalArgumentException(
+              String.format(
+                  "No token can be read from the file %s set by %s",
+                  configs.tokenFile(), K8sJobExecutorConfigs.TOKEN_FILE),
+              e);
+        }
+        builder.withOauthTokenProvider(tokenProvider);
+      }
+      return builder.build();
+    }
+
+    Config config;
+    if (configs.kubeconfig() != null) {
+      checkFileExists(configs.kubeconfig(), K8sJobExecutorConfigs.KUBECONFIG);
+      File kubeconfig = new File(configs.kubeconfig());
+      String server = K8sKubeconfigUtils.getSelectedServer(kubeconfig, 
configs.context());
+      config = Config.fromKubeconfig(configs.context(), kubeconfig);
+      // The client reads the file itself, so make sure it ended up with the 
server found above.
+      // This is a safety net with no known input that triggers it: it only 
fails if the client
+      // reads a kubeconfig differently from the check above, or the file 
changes in between.
+      Preconditions.checkArgument(
+          getHostAndPath(server).equals(getHostAndPath(config.getMasterUrl())),
+          "The kubeconfig %s set by %s selects the server %s, but the client 
would use %s",
+          configs.kubeconfig(),
+          K8sJobExecutorConfigs.KUBECONFIG,
+          server,
+          config.getMasterUrl());
+    } else {
+      config = Config.autoConfigure(configs.context());
+      checkDefaultLookup(config, configs);
+    }
+    return config;
+  }
+
+  /**
+   * Checks the client configuration that the default lookup found. The lookup 
may merge several
+   * kubeconfig files, so they aren't checked one by one like a configured 
kubeconfig. Instead, the
+   * result must not be one of those where the client silently stays on its 
default API server, the
+   * one of the cluster Gravitino runs in, although a kubeconfig was found.
+   */
+  private static void checkDefaultLookup(Config config, K8sJobExecutorConfigs 
configs) {
+    NamedContext current = config.getCurrentContext();
+    String currentName = current == null ? null : current.getName();

Review Comment:
   [Question] One shape of this hole is left: nothing configured at all.
   
   With no kubeconfig anywhere and no in-cluster service account, 
`Config.autoConfigure` finds no kubeconfig file, so `tryServiceAccount` runs 
and leaves the master URL at `Config.DEFAULT_MASTER_URL` (it only sets one from 
`KUBERNETES_SERVICE_HOST` and `KUBERNETES_SERVICE_PORT`, Config.java:719-728). 
`getCurrentContext()` is null, so this check passes and the server starts with 
a client aimed at `https://kubernetes.default.svc`, which resolves nowhere 
outside a cluster. The operator learns that from the first job submission 
rather than at startup, which is what the other two checks in this method avoid.
   
   Dropping the `getCurrentContext() == null` arm would cover it, because 
fabric8 moves the master URL off the default in a real pod through those two 
environment variables, which kubelet always injects. If you would rather not 
rely on that, requiring what fabric8 looks for in a pod 
(`findServiceAccountTokenFile`, Config.java:738) in this case gives the same 
startup failure without the assumption. The message would need the in-cluster 
wording added, since the context name would then be null in exactly this case.
   
   Minor, in the same line: the first format argument is 
`config.getCurrentContext() == null ? null : ...`, and the message can only be 
built when the context isn't null, so the `null` arm is unreachable today. It 
becomes load-bearing if you take the first option above.
   
   Verified by: reading `autoConfigure`, `tryServiceAccount` and 
`postAutoConfigure` in the fabric8 7.2.0 sources fetched in this run 
(Config.java:268-308, 717-750), and `TestK8sClientUtils`, which always points 
the `kubeconfig` system property at a file in this branch.



##########
plugins/k8s-job-executor/src/main/java/org/apache/gravitino/job/k8s/K8sJobExecutorConfigs.java:
##########
@@ -0,0 +1,510 @@
+/*
+ * 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.gravitino.job.k8s;
+
+import com.google.common.base.Preconditions;
+import com.google.common.collect.ImmutableMap;
+import java.util.Map;
+import java.util.TreeMap;
+import java.util.regex.Pattern;
+import javax.annotation.Nullable;
+import org.apache.commons.lang3.StringUtils;
+
+/**
+ * The configurations of the {@code k8s} job executor, set with the {@code
+ * gravitino.jobExecutor.k8s.} prefix in the Gravitino server configuration.
+ */
+public class K8sJobExecutorConfigs {
+
+  /** The name of the job executor. */
+  public static final String K8S_JOB_EXECUTOR_NAME = "k8s";
+
+  /** The name of the Kubernetes cluster the jobs run in, which is part of the 
job execution id. */
+  public static final String CLUSTER = "cluster";
+
+  /** The default cluster name. */
+  public static final String DEFAULT_CLUSTER = "default";
+
+  /** The kubeconfig file of the job cluster. */
+  public static final String KUBECONFIG = "kubeconfig";
+
+  /** The kubeconfig context to use, the current context if not set. */
+  public static final String CONTEXT = "context";
+
+  /**
+   * The API server URL of the job cluster, an alternative to the kubeconfig. 
It must start with
+   * {@code https://}, or with {@code http://} if no token file is set.
+   */
+  public static final String MASTER_URL = "masterUrl";
+
+  /** The CA certificate file of the API server, used with {@link 
#MASTER_URL}. */
+  public static final String CA_CERT_FILE = "caCertFile";
+
+  /**
+   * The bearer token file to access the API server, used with {@link 
#MASTER_URL}. It is read again
+   * whenever it changes, so the token can be rotated.
+   */
+  public static final String TOKEN_FILE = "tokenFile";
+
+  /** How long to wait to connect to the API server. */
+  public static final String CONNECTION_TIMEOUT_MS = "connectionTimeoutInMs";
+
+  /** The default connection timeout, 5 seconds. */
+  public static final long DEFAULT_CONNECTION_TIMEOUT_MS = 5_000L;
+
+  /** How long to wait for the response of a request to the API server. */
+  public static final String REQUEST_TIMEOUT_MS = "requestTimeoutInMs";
+
+  /** The default request timeout, 10 seconds. */
+  public static final long DEFAULT_REQUEST_TIMEOUT_MS = 10_000L;
+
+  /**
+   * How many times a failed request to the API server is tried again, for 
example if the server
+   * can't be reached or answers with a server error. Together with the 
timeouts it bounds how long
+   * a submission, a status query or a cancellation waits while the job 
cluster is down.
+   */
+  public static final String REQUEST_RETRIES = "requestRetries";

Review Comment:
   [Nit] `requestRetries` can't lengthen the wait this javadoc describes; 
`requestTimeoutInMs` is what bounds it.
   
   fabric8 gives the whole retry sequence a deadline of `requestTimeout + 
min(requestTimeout, 5 s)` (StandardHttpClient.java:157-160, with 
`MAX_ADDITIONAL_REQUEST_TIMEOUT` at line 49), so with the defaults the sequence 
is cut at 15 s no matter how many retries are allowed - which is also what 
bounds the 16 s you measured for an unreachable API server, rather than the 3 
attempts. Raising `requestRetries` therefore has no effect on the worst case, 
and lowering `requestTimeoutInMs` shortens the retries along with the single 
request.
   
   That makes the keys harder to tune than the javadoc suggests. Saying here 
that the number of retries only decides how often a failure is retried inside 
the window set by `requestTimeoutInMs`, and keeping the "it bounds how long a 
submission waits" sentence on `REQUEST_TIMEOUT_MS` instead, would match the 
behavior. The same sentence is in `K8sClientUtils.createClientConfig`'s javadoc 
(K8sClientUtils.java:147-149), where the 15 s figure is right for the wrong 
reason.
   
   Verified by: reading `retryWithExponentialBackoff` and `shouldRetry` in the 
fabric8 7.2.0 `kubernetes-client-api` sources fetched in this run, plus 
`ExponentialBackoffIntervalCalculator` (its `shouldRetry` confirms the limit is 
a retry count) and `OperationSupport.withRequestTimeout`, which is what puts 
the timeout on the request in the first place.



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to