[TASK][EASY] IllegalArgumentException: 'ArrayBuffer(PROMETHEUS, REST, THRIFT_BINARY)' in kyuubi.frontend.protocols is invalid.

[FourSpaces]

Code of Conduct

• I agree to follow this project's Code of Conduct

Search before asking

• I have searched in the issues and found no similar issues.

Describe the bug

I got this problem when deploying Kyuubi with k8s charts of Kyuubi

Affects Version(s)

1.7.1

Kyuubi Server Log Output

2023-08-29 07:00:05.228 INFO org.apache.kyuubi.zookeeper.EmbeddedZookeeper: Service[EmbeddedZookeeper] is stopped.
Exception in thread "main" java.lang.IllegalArgumentException: 'ArrayBuffer(PROMETHEUS, REST, THRIFT_BINARY)' in kyuubi.frontend.protocols is invalid. the frontend protocol should be one or more of THRIFT_BINARY,THRIFT_HTTP,REST,MYSQL,TRINO
	at org.apache.kyuubi.config.TypedConfigBuilder.$anonfun$checkValue$1(ConfigBuilder.scala:173)
	at org.apache.kyuubi.config.TypedConfigBuilder.$anonfun$transform$1(ConfigBuilder.scala:167)
	at scala.Option.map(Option.scala:230)
	at org.apache.kyuubi.config.ConfigEntryWithDefault.readFrom(ConfigEntry.scala:106)
	at org.apache.kyuubi.config.KyuubiConf.get(KyuubiConf.scala:99)
	at org.apache.kyuubi.server.KyuubiServer.frontendServices$lzycompute(KyuubiServer.scala:150)
	at org.apache.kyuubi.server.KyuubiServer.frontendServices(KyuubiServer.scala:149)
	at org.apache.kyuubi.service.Serverable.initialize(Serverable.scala:45)
	at org.apache.kyuubi.server.KyuubiServer.initialize(KyuubiServer.scala:178)
	at org.apache.kyuubi.server.KyuubiServer$.startServer(KyuubiServer.scala:59)
	at org.apache.kyuubi.server.KyuubiServer$.main(KyuubiServer.scala:99)
	at org.apache.kyuubi.server.KyuubiServer.main(KyuubiServer.scala)
2023-08-29 07:00:07.437 INFO org.apache.zookeeper.server.SessionTrackerImpl: SessionTrackerImpl exited loop!
2023-08-29 07:00:07.439 WARN org.apache.kyuubi.zookeeper.EmbeddedZookeeper: Service[EmbeddedZookeeper] is not started(STOPPED) yet.

Kyuubi Engine Log Output

No response

Kyuubi Server Configurations

## Helm chart provided Kyuubi configurations
kyuubi.kubernetes.namespace=kyuubi
kyuubi.frontend.connection.url.use.hostname=false
kyuubi.frontend.thrift.binary.bind.port=10009
kyuubi.frontend.thrift.http.bind.port=10010
kyuubi.frontend.rest.bind.port=10099
kyuubi.frontend.mysql.bind.port=3309
kyuubi.frontend.protocols=PROMETHEUS,REST,THRIFT_BINARY

# Kyuubi Metrics
kyuubi.metrics.enabled=true
kyuubi.metrics.reporters=

## User provided Kyuubi configurations

Kyuubi Engine Configurations

No response

Additional context

It seems that the configuration generated by the k8s charts of Kyuubi is incorrect. I have found the corresponding logic.

kyuubi/charts/kyuubi/templates/_helpers.tpl

Lines 22 to 33 in 19d54ec

	{{- define "kyuubi.frontend.protocols" -}}
	{{- $protocols := list }}
	{{- range $name, $frontend := .Values.server }}
	{{- if $frontend.enabled }}
	{{- $protocols = $name | snakecase | upper | append $protocols }}
	{{- end }}
	{{- end }}
	{{- if not $protocols }}
	{{ fail "At least one frontend protocol must be enabled!" }}
	{{- end }}
	{{- $protocols | join "," }}
	{{- end }}

The correct logic should exclude 'prometheus'.

{{- define "kyuubi.frontend.protocols" -}}
  {{- $protocols := list }}
  {{- range $name, $frontend := .Values.server }}
    {{- if eq $frontend.enabled (ne $name "prometheus") }}
      {{- $protocols = $name | snakecase | upper | append $protocols }}
    {{- end }}
  {{- end }}
  {{- if not $protocols }}
    {{ fail "At least one frontend protocol must be enabled!" }}
  {{- end }}
  {{- $protocols |  join "," }}
{{- end }}

Are you willing to submit PR?

• Yes. I would be willing to submit a PR with guidance from the Kyuubi community to fix.
• No. I cannot submit a PR at this time.

[github-actions]

Hello @FourSpaces,
Thanks for finding the time to report the issue!
We really appreciate the community's efforts to improve Apache Kyuubi.

[yaooqinn]

cc @dnskr @pan3793

[dnskr]

@FourSpaces Thanks for reporting the issue.
The bug was introduced in the PR #5132 and the solution you suggest should fix the issue. The alternative solution is to move prometheus outside server and rename it to e.g. monitoring, because it is not a part of Kyuubi server actually.
What do you think?

I’m on vacation at the moment, so I may respond slowly.

[FourSpaces]

@dnskr Do you mean to remove 'prometheus' node from 'server' node?

[pan3793]

Seems we can make the following change.

  server:
    ...
-   prometheus:
-     enabled: true
-     port: 10019
-     service:
-      type: ClusterIP
-      port: "{{ .Values.server.prometheus.port }}"
-      nodePort: ~
-      annotations: {}
    ...
+ monitoring:
+   prometheus:
+     enabled: true
+     port: 10019

- metricsReporters: ~

• move prometheus node from server to monitoring as @dnskr suggested
• metricsReporters could be inferred from monitoring's keys in runtime thus we can remove it from the values.yaml

[dnskr]

Hi @FourSpaces!
I came back from vacation and I'm ready to assist if needed. Do you want to submit the PR?

[FourSpaces]

OK, I will complete and submit the PR before the end of this Sunday.

[FourSpaces]

@dnskr I have submitted a PR, please check if there are any issues.