adutra commented on code in PR #135:
URL: https://github.com/apache/polaris/pull/135#discussion_r1734169081
##########
helm/polaris/templates/hpa.yaml:
##########
@@ -0,0 +1,65 @@
+{{/*
+ 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.
+*/}}
+
+{{- if .Values.autoscaling.enabled }}
+{{- if .Capabilities.APIVersions.Has "autoscaling/v2" -}}
+apiVersion: autoscaling/v2
+{{- else if .Capabilities.APIVersions.Has "autoscaling/v2beta2" -}}
Review Comment:
Note: autoscaling v2beta2 has been removed in K8s v1.26, published in
December 2022. As for autoscaling v2beta1, it is really antique at this point.
Shouldn't we just remove support for these versions?
##########
helm/polaris/values.yaml:
##########
@@ -0,0 +1,391 @@
+#
+# 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.
+#
+
+# -- The number of replicas to deploy (horizontal scaling).
+# Beware that replicas are stateless; don't set this number > 1 when using
in-memory meta store manager.
+replicaCount: 1
+
+image:
+ # -- The image repository to pull from.
+ repository: localhost:5001/polaris
+ # -- The image pull policy.
+ pullPolicy: IfNotPresent
+ # -- The image tag.
+ tag: "latest"
+
+toolsImage:
+ # -- The image repository to pull from (must have jar binary included).
+ repository: registry.access.redhat.com/ubi9/openjdk-21
+ # -- The image pull policy.
+ pullPolicy: IfNotPresent
+ # -- The image tag.
+ tag: "latest"
+
+# -- References to secrets in the same namespace to use for pulling any of the
images used by this
+# chart. Each entry is a LocalObjectReference to an existing secret in the
namespace. The secret
+# must contain a .dockerconfigjson key with a base64-encoded Docker
configuration file. See
+#
https://kubernetes.io/docs/tasks/configure-pod-container/pull-image-private-registry/
for more
+# information.
+imagePullSecrets: []
+# - name: registry-creds
Review Comment:
Why not just an array of strings? I know the `imagePullSecrets` field in a
`Deployment` is an array of `LocalObjectReference` objects, but I don't think
we need to expose that complexity to users.
##########
helm/polaris/templates/ingress.yaml:
##########
@@ -0,0 +1,74 @@
+{{/*
+ 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.
+*/}}
+
+{{- $kubeVersion := .Capabilities.KubeVersion.Version -}}
+{{- if .Values.ingress.enabled -}}
+{{- $fullName := include "polaris.fullname" . -}}
+{{- $svcPort := coalesce .Values.service.port (index .Values.service.ports
"polaris-service") -}}
+{{- if semverCompare ">=1.22-0" $kubeVersion -}}
Review Comment:
Same here: these legacy apis were removed in v1.22. I think at this point it
doesn't make sense to keep them in this chart.
##########
helm/polaris/values.yaml:
##########
@@ -0,0 +1,391 @@
+#
+# 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.
+#
+
+# -- The number of replicas to deploy (horizontal scaling).
+# Beware that replicas are stateless; don't set this number > 1 when using
in-memory meta store manager.
+replicaCount: 1
+
+image:
+ # -- The image repository to pull from.
+ repository: localhost:5001/polaris
Review Comment:
Isn't there a more official repository that could be a better default?
##########
helm/polaris/templates/service.yaml:
##########
@@ -0,0 +1,42 @@
+{{/*
+ 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.
+*/}}
+
+apiVersion: v1
+kind: Service
+metadata:
+ name: {{ include "polaris.fullname" . }}
+ labels:
+ {{- include "polaris.labels" . | nindent 4 }}
+ annotations:
+ {{- if .Values.service.annotations }}
+ {{- tpl (toYaml .Values.service.annotations) . | nindent 4 }}
+ {{- end }}
+spec:
+ type: {{ .Values.service.type }}
+ selector:
+ {{- include "polaris.selectorLabels" . | nindent 4 }}
+ ports:
+ {{- range $portName, $portNumber := .Values.service.ports }}
Review Comment:
Could be good to separate the metrics port from the main API port into 2
distinct services. But that can be done later.
##########
helm/polaris/values.yaml:
##########
@@ -0,0 +1,391 @@
+#
+# 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.
+#
+
+# -- The number of replicas to deploy (horizontal scaling).
+# Beware that replicas are stateless; don't set this number > 1 when using
in-memory meta store manager.
+replicaCount: 1
+
+image:
+ # -- The image repository to pull from.
+ repository: localhost:5001/polaris
+ # -- The image pull policy.
+ pullPolicy: IfNotPresent
+ # -- The image tag.
+ tag: "latest"
+
+toolsImage:
+ # -- The image repository to pull from (must have jar binary included).
+ repository: registry.access.redhat.com/ubi9/openjdk-21
+ # -- The image pull policy.
+ pullPolicy: IfNotPresent
+ # -- The image tag.
+ tag: "latest"
+
+# -- References to secrets in the same namespace to use for pulling any of the
images used by this
+# chart. Each entry is a LocalObjectReference to an existing secret in the
namespace. The secret
+# must contain a .dockerconfigjson key with a base64-encoded Docker
configuration file. See
+#
https://kubernetes.io/docs/tasks/configure-pod-container/pull-image-private-registry/
for more
+# information.
+imagePullSecrets: []
+# - name: registry-creds
+
+
+serviceAccount:
+ # -- Specifies whether a service account should be created.
+ create: true
+ # -- Annotations to add to the service account.
+ annotations: {}
+ # -- The name of the service account to use.
+ # If not set and create is true, a name is generated using the fullname
template.
+ name: ""
+
+
+# -- Annotations to apply to polaris pods.
+podAnnotations: {}
+
+# -- Additional Labels to apply to polaris pods.
+podLabels: {}
+
+# -- Additional Labels to apply to polaris configmap.
+configMapLabels: {}
+
+# -- Security context for the polaris pod. See
https://kubernetes.io/docs/tasks/configure-pod-container/security-context/.
+podSecurityContext:
+ {}
+ # fsGroup: 2000
+
+# -- Security context for the polaris container. See
https://kubernetes.io/docs/tasks/configure-pod-container/security-context/.
+securityContext:
+ {}
+ # capabilities:
+ # drop:
+ # - ALL
+ # readOnlyRootFilesystem: true
+ # runAsNonRoot: true
+ # runAsUser: 1000
+
+# Polaris service settings.
+service:
+ # -- The type of service to create.
+ type: ClusterIP
+ # -- The ports the service will listen on. Two ports are required: one for
the Polaris service and
+ # one for the metrics API.
+ # Note: port names must be unique and no more than 15 characters long.
+ ports:
+ # polaris-server: The port the Polaris server listens on for API requests.
+ polaris-service: 8181
+ # polaris-mgmt: The port the Polaris server listens on for metrics API
requests (health checks, metrics, etc.).
+ polaris-metrics: 8182
Review Comment:
Should this be `polaris-mgmt` as implied by the comment above?
--
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]
