parshimers commented on code in PR #8971:
URL: https://github.com/apache/texera/pull/8971#discussion_r4340819549
##########
bin/k8s/values.yaml:
##########
@@ -68,6 +68,17 @@ postgresql:
existingClaim: "postgresql-data-pvc"
initdb:
scriptsConfigMap: "postgresql-init-script"
+ podAnnotations:
Review Comment:
having the pod annotations like this for every service makes it super
annoying to turn this on or off. can it be just one value?
##########
bin/k8s/templates/base/access-control-service/access-control-service-deployment.yaml:
##########
@@ -31,6 +31,10 @@ spec:
metadata:
labels:
app: {{ .Release.Name }}-{{ .Values.accessControlService.name }}
+ {{- with .Values.accessControlService.podAnnotations }}
+ annotations:
+ {{- toYaml . | nindent 8 }}
Review Comment:
hiding the values like this causes bin/k8s/tests/test_helm_values.sh to fail.
doing something like
`{{- toYaml .Values.accessControlService.podAnnotations | nindent 8 }}`
in each instance would probably work
##########
bin/k8s/values.yaml:
##########
@@ -68,6 +68,17 @@ postgresql:
existingClaim: "postgresql-data-pvc"
initdb:
scriptsConfigMap: "postgresql-init-script"
+ podAnnotations:
+ linkerd.io/inject: "enabled"
+ config.linkerd.io/opaque-ports: "5432"
+ config.linkerd.io/proxy-enable-native-sidecar: "true"
Review Comment:
can you please check the /api/computing-unit metrics when there is a
sidecar? claude leads me to beleive the way KubernetesClient.containerUsage
collects the metrics is basically order-dependent and can pick up the sidecar's
metrics instead of the CU's
##########
bin/k8s/values.yaml:
##########
@@ -68,6 +68,17 @@ postgresql:
existingClaim: "postgresql-data-pvc"
initdb:
scriptsConfigMap: "postgresql-init-script"
+ podAnnotations:
+ linkerd.io/inject: "enabled"
+ config.linkerd.io/opaque-ports: "5432"
+ config.linkerd.io/proxy-enable-native-sidecar: "true"
Review Comment:
claude tells me that the default inbound policy in linkerd is
all-unauthenticated. this seems a bit too permissive? probably the only place
we should definitely require that is envoy, right?
##########
bin/k8s/templates/base/gateway/gateway-envoy-proxy.yaml:
##########
Review Comment:
there's something weird going on here with the gateway CRDs. linkerd needs
them, and envoy installs 1.4.1, but then linkerd wants 1.5.1 and won't upgrade
them. can we iron this out somehow?
i also don't quite understand why this is a separate yaml now. we were using
envoy before, right? how is this use different?
##########
bin/k8s/templates/base/workflow-computing-unit-pool/workflow-computing-unit-master-prepull-daemonset.yaml:
##########
@@ -48,7 +52,7 @@ spec:
resources:
limits:
cpu: 1m
- memory: 8Mi
+ memory: 256Mi
Review Comment:
i think the reason for this is that the prepuller is running linkerd. that
doesn't really seem necessary? it's not accepting or making connections to any
other service in the cluster.
--
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]
