[SPARK-27258][K8S]Add the prefix 'spark-' for resourceNamePrefix that starts with number

[hehuiyuan]

What changes were proposed in this pull request?

For SPARK-27258

The value of "spark.app.name" or "--name" starts with number , which causes resourceName does not match regular expression [a-z]([-a-z0-9]*[a-z0-9])?).

For example : the appName = "1min-machinereg-yf"
Exception :
Exception in thread "main" io.fabric8.kubernetes.client.KubernetesClientException: Failure executing: POST at: https://xxx:xxx/api/v1/namespaces/xxx/services. Message: Service "1min-machinereg-yf-1544604108931-driver-svc" is invalid: metadata.name: Invalid value: "1min-machinereg-yf-1544604108931-driver-svc": a DNS-1035 label must consist of lower case alphanumeric characters or '-', start with an alphabetic character, and end with an alphanumeric character (e.g. 'my-name', or 'abc-123', regex used for validation is '[a-z]([-a-z0-9]*[a-z0-9])?'). Received status: Status(apiVersion=v1, code=422, details=StatusDetails(causes=[StatusCause(field=metadata.name, message=Invalid value: "1min-machinereg-yf-1544604108931-driver-svc": a DNS-1035 label must consist of lower case alphanumeric characters or '-', start with an alphabetic character, and end with an alphanumeric character (e.g. 'my-name', or 'abc-123', regex used for validation is '[a-z]([-a-z0-9]*[a-z0-9])?'), reason=FieldValueInvalid, additionalProperties={})], group=null, kind=Service, name=1min-machinereg-yf-1544604108931-driver-svc, retryAfterSeconds=null, uid=null, additionalProperties={}).

[dongjoon-hyun]

ok to test

[dongjoon-hyun]

This variable name is confusing since this is in override val resourceNamePrefix: String = {}.

[dongjoon-hyun]

BTW, did you run the test locally?

[hehuiyuan]

1. For example: resourceNamePrefix = 1min-xxx

After execute this code:

val prefix = "spark-" + resourceNamePrefix.charAt(0) resourceNamePrefix.replaceAll("^[0-9]", prefix)

resourceNamePrefix = spark-1min-xxx,which satisfies the regular expression [a-z]([-a-z0-9]*[a-z0-9])?

2. For example : resourceNamePrefix = min-xxx

After execute this code:

val prefix = "spark-" + resourceNamePrefix.charAt(0) resourceNamePrefix.replaceAll("^[0-9]", prefix)

resourceNamePrefix = min-xxx,which does not change.

Local testing was done for thisoverride val resourceNamePrefix only.

[SparkQA]

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/testing-k8s-prb-make-spark-distribution-unified/9244/

[SparkQA]

Test build #103880 has finished for PR 24188 at commit 6bf2172.

• This patch passes all tests.
• This patch merges cleanly.
• This patch adds no public classes.

[SparkQA]

Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/testing-k8s-prb-make-spark-distribution-unified/9244/

[hehuiyuan]

I think we can only update the resourceNamePrefix of service, which may be better.

For the class of DriverServiceFeatureStep :

  private val preferredServiceName = s"${kubernetesConf.resourceNamePrefix}$DRIVER_SVC_POSTFIX"
  private val resolvedServiceName = if (preferredServiceName.length <= MAX_SERVICE_NAME_LENGTH) {
    preferredServiceName
  } else {
    val randomServiceId = KubernetesUtils.uniqueID(clock = clock)
    val shorterServiceName = s"spark-$randomServiceId$DRIVER_SVC_POSTFIX"
    logWarning(s"Driver's hostname would preferably be $preferredServiceName, but this is " +
      s"too long (must be <= $MAX_SERVICE_NAME_LENGTH characters). Falling back to use " +
      s"$shorterServiceName as the driver service's name.")
    shorterServiceName
  }

Update:

  def isNumber(char: Char) : Boolean = {
    char >= '0' && char <= '9'
  }

  private val preferredServiceName = s"${kubernetesConf.resourceNamePrefix}$DRIVER_SVC_POSTFIX"
  private val resolvedServiceName = if (preferredServiceName.length <= MAX_SERVICE_NAME_LENGTH
    && !isNumber(preferredServiceName.charAt(0))) {
    preferredServiceName
  } else {
    val randomServiceId = KubernetesUtils.uniqueID(clock = clock)
    val shorterServiceName = s"spark-$randomServiceId$DRIVER_SVC_POSTFIX"
    logWarning(s"Driver's hostname would preferably be $preferredServiceName, but this is " +
      s"too long (must be <= $MAX_SERVICE_NAME_LENGTH characters) " +
      s"or the first character of $preferredServiceName is number which is not support." +
      s" Falling back to use $shorterServiceName as the driver service's name.")
    shorterServiceName
  }

[srowen]

Why not just prepend "spark-" if the first character is a digit? if (Character.isDigit(resourceNamePrefix(0))) But don't you actually need to check if it starts with a-z?

[hehuiyuan]

Why not just prepend "spark-" if the first character is a digit? if (Character.isDigit(resourceNamePrefix(0))) But don't you actually need to check if it starts with a-z?

---

Hi, this code does not change the resourceNamePrefix that the first character is not a digit.

1: For example: resourceNamePrefix = 1min-xxx
After execute this code:

val prefix = "spark-" + resourceNamePrefix.charAt(0) resourceNamePrefix.replaceAll("^[0-9]", prefix)

resourceNamePrefix = spark-1min-xxx,
which satisfies the regular expression [a-z]([-a-z0-9]*[a-z0-9])?

2 : For example : resourceNamePrefix = min-xxx
After execute this code:

val prefix = "spark-" + resourceNamePrefix.charAt(0) resourceNamePrefix.replaceAll("^[0-9]", prefix)

resourceNamePrefix = min-xxx,which does not change.

[srowen]

I know, it's just more complex than it needs to be, and doesn't catch all the cases that would fail the pattern you show above: [a-z]([-a-z0-9]*[a-z0-9])?)

[hehuiyuan]

I think we can only update the resourceNamePrefix of service, which may be better.

For the class of DriverServiceFeatureStep :

  private val preferredServiceName = s"${kubernetesConf.resourceNamePrefix}$DRIVER_SVC_POSTFIX"
  private val resolvedServiceName = if (preferredServiceName.length <= MAX_SERVICE_NAME_LENGTH) {
    preferredServiceName
  } else {
    val randomServiceId = KubernetesUtils.uniqueID(clock = clock)
    val shorterServiceName = s"spark-$randomServiceId$DRIVER_SVC_POSTFIX"
    logWarning(s"Driver's hostname would preferably be $preferredServiceName, but this is " +
      s"too long (must be <= $MAX_SERVICE_NAME_LENGTH characters). Falling back to use " +
      s"$shorterServiceName as the driver service's name.")
    shorterServiceName
  }

Update:

  def isNumber(char: Char) : Boolean = {
    char >= '0' && char <= '9'
  }

  private val preferredServiceName = s"${kubernetesConf.resourceNamePrefix}$DRIVER_SVC_POSTFIX"
  private val resolvedServiceName = if (preferredServiceName.length <= MAX_SERVICE_NAME_LENGTH
    && !isNumber(preferredServiceName.charAt(0))) {
    preferredServiceName
  } else {
    val randomServiceId = KubernetesUtils.uniqueID(clock = clock)
    val shorterServiceName = s"spark-$randomServiceId$DRIVER_SVC_POSTFIX"
    logWarning(s"Driver's hostname would preferably be $preferredServiceName, but this is " +
      s"too long (must be <= $MAX_SERVICE_NAME_LENGTH characters) " +
      s"or the first character of $preferredServiceName is number which is not support." +
      s" Falling back to use $shorterServiceName as the driver service's name.")
    shorterServiceName
  }

@dongjoon-hyun @srowen
if the first character of resourceNamePrefix is digit, we do not change the value of resourceNamePrefix.
We can use the variable of shorterServiceName provided by the source code.

[srowen]

I'm not sure what your last comment means. This looks even more complex and the code is still incorrect. Example: what about an ID that starts with "Foo"? it isn't allowed by that regex but isn't detected by this code. This should be a 2 line change.

[hehuiyuan]

I'm not sure what your last comment means. This looks even more complex and the code is still incorrect. Example: what about an ID that starts with "Foo"? it isn't allowed by that regex but isn't detected by this code. This should be a 2 line change.

---

The last comment means that is the another way used to resolve the PR . In this PR, if the first character of resourceNamePrefix is digit, it will change the value of resourceNamePrefix to add a extra prefix "spark-" for Driver Pod and Service.

However, the last comment means :
if the first character of resourceNamePrefix is digit, we do not change the value of resourceNamePrefix.
We can use the variable of shorterServiceName provided by the source code as the name of Service ,but the name of Driver Pod is not changed.

private val preferredServiceName = s"${kubernetesConf.resourceNamePrefix}$DRIVER_SVC_POSTFIX"

If preferredServiceName satisfies preferredServiceName.length <= MAX_SERVICE_NAME_LENGTH && !isNumber(preferredServiceName.charAt(0),we use preferredServiceName.

If these two conditions are not met，we use shorterServiceName.

The code in the last comment hasn't been submitted yet. I would like to ask you some advice.Is this better than the way in the PR?

[hehuiyuan]

Another solution PR

The useless PR can be closed, I just want to make it convenient for you to have a look.