hanahmily commented on code in PR #45:
URL:
https://github.com/apache/skywalking-banyandb-helm/pull/45#discussion_r2684717554
##########
chart/values.yaml:
##########
@@ -734,6 +737,206 @@ cluster:
##
failureThreshold: 60
+ ## @section Configuration for FODC (First Occurrence Data Collection) Proxy
component
+ ##
+ fodcProxy:
Review Comment:
```
fodc:
enabled: true
agent:
....
proxy:
....
```
I prefer the above structure because the agent and proxy should be deployed
simultaneously.
##########
chart/templates/cluster_data_statefulset.yaml:
##########
@@ -289,6 +308,108 @@ spec:
{{- end }}
{{- end }}
{{- end }}
+ {{- if and $.Values.cluster.enabled $.Values.cluster.fodcAgent.enabled
}}
+ - name: fodc-agent
+ image: {{ $.Values.cluster.fodcAgent.image.repository }}:{{ default
$.Values.image.tag $.Values.cluster.fodcAgent.image.tag }}
+ imagePullPolicy: {{ $.Values.cluster.fodcAgent.image.pullPolicy }}
+ {{- with $.Values.cluster.fodcAgent.containerSecurityContext }}
+ securityContext:
+ {{- toYaml . | nindent 12 }}
+ {{- end }}
+ command:
+ - sh
+ - -c
+ - |
+ set -euo pipefail
+ echo "Waiting for BanyanDB observability port (2121) to be
ready..."
+ MAX_ATTEMPTS=60
+ ATTEMPT=0
+ while [ ${ATTEMPT} -lt ${MAX_ATTEMPTS} ]; do
Review Comment:
Perform a connection check in the agent. If I remember correctly, the
agent's connection manager has the capacity to handle reconnections in case of
failure.
##########
chart/values.yaml:
##########
@@ -734,6 +737,206 @@ cluster:
##
failureThreshold: 60
+ ## @section Configuration for FODC (First Occurrence Data Collection) Proxy
component
+ ##
+ fodcProxy:
+ ## @param cluster.fodcProxy.enabled Enable FODC Proxy deployment (boolean)
+ enabled: true
+ ## @param cluster.fodcProxy.podAnnotations Pod annotations for Proxy
+ podAnnotations: {}
+ ## @param cluster.fodcProxy.securityContext Security context for Proxy pods
+ securityContext: {}
+ ## @param cluster.fodcProxy.containerSecurityContext Container-level
security context for Proxy
+ containerSecurityContext: {}
+ ## @param cluster.fodcProxy.env Environment variables for Proxy pods
+ env: []
+ ## @param cluster.fodcProxy.priorityClassName Priority class name for
Proxy pods
+ priorityClassName: ""
+ ## Update strategy for Proxy pods
+ updateStrategy:
+ ## @param cluster.fodcProxy.updateStrategy.type Update strategy type for
Proxy pods
+ type: RollingUpdate
+ rollingUpdate:
+ ## @param
cluster.fodcProxy.updateStrategy.rollingUpdate.maxUnavailable Maximum
unavailable pods during update
+ maxUnavailable: 1
+ ## @param cluster.fodcProxy.updateStrategy.rollingUpdate.maxSurge
Maximum surge pods during update
+ maxSurge: 1
+ ## @param cluster.fodcProxy.podDisruptionBudget Pod disruption budget for
Proxy
+ podDisruptionBudget: {}
+ ## @param cluster.fodcProxy.tolerations Tolerations for Proxy pods
+ tolerations: []
+ ## @param cluster.fodcProxy.nodeSelector Node selector for Proxy pods
+ nodeSelector: []
+ ## @param cluster.fodcProxy.affinity Affinity rules for Proxy pods
+ affinity: {}
+ ## @param cluster.fodcProxy.podAffinityPreset Pod affinity preset for Proxy
+ podAffinityPreset: ""
+ ## @param cluster.fodcProxy.podAntiAffinityPreset Pod anti-affinity preset
for Proxy
+ podAntiAffinityPreset: soft
Review Comment:
They could take place when there are more than 1 replica.
##########
chart/values.yaml:
##########
@@ -734,6 +737,206 @@ cluster:
##
failureThreshold: 60
+ ## @section Configuration for FODC (First Occurrence Data Collection) Proxy
component
+ ##
+ fodcProxy:
+ ## @param cluster.fodcProxy.enabled Enable FODC Proxy deployment (boolean)
+ enabled: true
+ ## @param cluster.fodcProxy.podAnnotations Pod annotations for Proxy
+ podAnnotations: {}
+ ## @param cluster.fodcProxy.securityContext Security context for Proxy pods
+ securityContext: {}
+ ## @param cluster.fodcProxy.containerSecurityContext Container-level
security context for Proxy
+ containerSecurityContext: {}
+ ## @param cluster.fodcProxy.env Environment variables for Proxy pods
+ env: []
+ ## @param cluster.fodcProxy.priorityClassName Priority class name for
Proxy pods
+ priorityClassName: ""
+ ## Update strategy for Proxy pods
+ updateStrategy:
+ ## @param cluster.fodcProxy.updateStrategy.type Update strategy type for
Proxy pods
+ type: RollingUpdate
+ rollingUpdate:
+ ## @param
cluster.fodcProxy.updateStrategy.rollingUpdate.maxUnavailable Maximum
unavailable pods during update
+ maxUnavailable: 1
+ ## @param cluster.fodcProxy.updateStrategy.rollingUpdate.maxSurge
Maximum surge pods during update
+ maxSurge: 1
+ ## @param cluster.fodcProxy.podDisruptionBudget Pod disruption budget for
Proxy
+ podDisruptionBudget: {}
+ ## @param cluster.fodcProxy.tolerations Tolerations for Proxy pods
+ tolerations: []
+ ## @param cluster.fodcProxy.nodeSelector Node selector for Proxy pods
+ nodeSelector: []
+ ## @param cluster.fodcProxy.affinity Affinity rules for Proxy pods
+ affinity: {}
+ ## @param cluster.fodcProxy.podAffinityPreset Pod affinity preset for Proxy
+ podAffinityPreset: ""
+ ## @param cluster.fodcProxy.podAntiAffinityPreset Pod anti-affinity preset
for Proxy
+ podAntiAffinityPreset: soft
+ ## Resource requests/limits for Proxy
+ resources:
+ ## @param cluster.fodcProxy.resources.requests Resource requests for
Proxy pods
+ requests: {}
+ ## @param cluster.fodcProxy.resources.limits Resource limits for Proxy
pods
+ limits: {}
+ image:
+ ## @param cluster.fodcProxy.image.repository Docker repository for FODC
Proxy
+ repository: ghcr.io/apache/skywalking-banyandb-fodc-proxy
+ ## @param cluster.fodcProxy.image.tag Image tag/version for FODC Proxy
(empty for latest)
+ tag: ""
+ ## @param cluster.fodcProxy.image.pullPolicy Image pull policy for FODC
Proxy
+ pullPolicy: IfNotPresent
+ grpcSvc:
+ ## @param cluster.fodcProxy.grpcSvc.labels Labels for Proxy gRPC service
+ labels: {}
+ ## @param cluster.fodcProxy.grpcSvc.annotations Annotations for Proxy
gRPC service
+ annotations: {}
+ ## @param cluster.fodcProxy.grpcSvc.port Port number for Proxy gRPC
service (Agent connections)
+ port: 17912
+ httpSvc:
+ ## @param cluster.fodcProxy.httpSvc.labels Labels for Proxy HTTP service
+ labels: {}
+ ## @param cluster.fodcProxy.httpSvc.annotations Annotations for Proxy
HTTP service
+ annotations: {}
+ ## @param cluster.fodcProxy.httpSvc.port Port number for Proxy HTTP
service
+ port: 17913
+ ## @param cluster.fodcProxy.httpSvc.type Service type for Proxy HTTP
service (ClusterIP, LoadBalancer, NodePort)
+ type: LoadBalancer
+ ## @param cluster.fodcProxy.httpSvc.externalIPs External IP addresses
for Proxy HTTP service
+ externalIPs: []
+ ## @param cluster.fodcProxy.httpSvc.loadBalancerIP Load balancer IP for
Proxy HTTP service
+ loadBalancerIP: null
+ ## @param cluster.fodcProxy.httpSvc.loadBalancerSourceRanges Allowed
source ranges for Proxy HTTP service
+ loadBalancerSourceRanges: []
+ ingress:
Review Comment:
Would you like to make the proxy accessible outside the cluster? If not,
ingress is obsolete. @Fine0830 @wu-sheng
##########
chart/templates/cluster_data_statefulset.yaml:
##########
@@ -134,6 +134,25 @@ spec:
{{- toYaml . | nindent 12 }}
{{- end }}
{{- end }}
+ {{- if and $.Values.cluster.enabled $.Values.cluster.fodcAgent.enabled
$.Values.cluster.fodcProxy.enabled }}
Review Comment:
I do not recommend blocking the node boot until the proxy is available. FODC
is a monitoring system that should not affect the functionality of production
components.
##########
chart/templates/cluster_data_statefulset.yaml:
##########
@@ -289,6 +308,108 @@ spec:
{{- end }}
{{- end }}
{{- end }}
+ {{- if and $.Values.cluster.enabled $.Values.cluster.fodcAgent.enabled
}}
+ - name: fodc-agent
+ image: {{ $.Values.cluster.fodcAgent.image.repository }}:{{ default
$.Values.image.tag $.Values.cluster.fodcAgent.image.tag }}
+ imagePullPolicy: {{ $.Values.cluster.fodcAgent.image.pullPolicy }}
+ {{- with $.Values.cluster.fodcAgent.containerSecurityContext }}
+ securityContext:
+ {{- toYaml . | nindent 12 }}
+ {{- end }}
+ command:
+ - sh
+ - -c
+ - |
+ set -euo pipefail
+ echo "Waiting for BanyanDB observability port (2121) to be
ready..."
+ MAX_ATTEMPTS=60
+ ATTEMPT=0
+ while [ ${ATTEMPT} -lt ${MAX_ATTEMPTS} ]; do
+ if wget -q -T 1 -O- "http://127.0.0.1:2121/metrics" >
/dev/null 2>&1; then
+ echo "BanyanDB observability port is ready, starting FODC
agent..."
+ break
+ fi
+ ATTEMPT=$((ATTEMPT + 1))
+ if [ $((ATTEMPT % 5)) -eq 0 ]; then
+ echo "Waiting for BanyanDB observability port (attempt
${ATTEMPT}/${MAX_ATTEMPTS})..."
+ fi
+ sleep 2
+ done
+ if [ ${ATTEMPT} -ge ${MAX_ATTEMPTS} ]; then
+ echo "Warning: BanyanDB observability port not ready after
${MAX_ATTEMPTS} attempts, starting agent anyway..."
+ fi
+ exec /fodc-agent \
+ --proxy-addr={{ template "banyandb.fullname" $
}}-fodc-proxy-grpc:{{ $.Values.cluster.fodcProxy.grpcSvc.port }} \
+ --node-ip=${POD_NAME}.{{ template "banyandb.fullname" $
}}-data-{{ $roleName }}-headless.${POD_NAMESPACE} \
Review Comment:
Isn't the node IP the localhost/127.0.0.1? Why do you use the headless host
name?
##########
chart/templates/cluster_data_statefulset.yaml:
##########
@@ -289,6 +308,108 @@ spec:
{{- end }}
{{- end }}
{{- end }}
+ {{- if and $.Values.cluster.enabled $.Values.cluster.fodcAgent.enabled
}}
+ - name: fodc-agent
+ image: {{ $.Values.cluster.fodcAgent.image.repository }}:{{ default
$.Values.image.tag $.Values.cluster.fodcAgent.image.tag }}
+ imagePullPolicy: {{ $.Values.cluster.fodcAgent.image.pullPolicy }}
+ {{- with $.Values.cluster.fodcAgent.containerSecurityContext }}
+ securityContext:
+ {{- toYaml . | nindent 12 }}
+ {{- end }}
+ command:
+ - sh
+ - -c
+ - |
+ set -euo pipefail
+ echo "Waiting for BanyanDB observability port (2121) to be
ready..."
+ MAX_ATTEMPTS=60
+ ATTEMPT=0
+ while [ ${ATTEMPT} -lt ${MAX_ATTEMPTS} ]; do
+ if wget -q -T 1 -O- "http://127.0.0.1:2121/metrics" >
/dev/null 2>&1; then
+ echo "BanyanDB observability port is ready, starting FODC
agent..."
+ break
+ fi
+ ATTEMPT=$((ATTEMPT + 1))
+ if [ $((ATTEMPT % 5)) -eq 0 ]; then
+ echo "Waiting for BanyanDB observability port (attempt
${ATTEMPT}/${MAX_ATTEMPTS})..."
+ fi
+ sleep 2
+ done
+ if [ ${ATTEMPT} -ge ${MAX_ATTEMPTS} ]; then
+ echo "Warning: BanyanDB observability port not ready after
${MAX_ATTEMPTS} attempts, starting agent anyway..."
+ fi
+ exec /fodc-agent \
+ --proxy-addr={{ template "banyandb.fullname" $
}}-fodc-proxy-grpc:{{ $.Values.cluster.fodcProxy.grpcSvc.port }} \
+ --node-ip=${POD_NAME}.{{ template "banyandb.fullname" $
}}-data-{{ $roleName }}-headless.${POD_NAMESPACE} \
+ --node-port={{ $.Values.cluster.data.nodeTemplate.grpcSvc.port
}} \
+ --node-role=datanode-{{ $roleName }} \
+ --metrics-endpoint=http://127.0.0.1:2121/metrics \
+ --poll-metrics-interval={{
$.Values.cluster.fodcAgent.config.scrapeInterval }} \
+ --prometheus-listen-addr=:{{
$.Values.cluster.fodcAgent.httpPort }} \
+ --max-metrics-memory-usage-percentage={{
$.Values.cluster.fodcAgent.config.ktmEnabled | ternary 10 5 }} \
+ --heartbeat-interval={{
$.Values.cluster.fodcAgent.config.heartbeatInterval }} \
+ --reconnect-interval=10s
Review Comment:
Could you please parameterize it?
##########
chart/values.yaml:
##########
@@ -734,6 +737,206 @@ cluster:
##
failureThreshold: 60
+ ## @section Configuration for FODC (First Occurrence Data Collection) Proxy
component
+ ##
+ fodcProxy:
+ ## @param cluster.fodcProxy.enabled Enable FODC Proxy deployment (boolean)
+ enabled: true
+ ## @param cluster.fodcProxy.podAnnotations Pod annotations for Proxy
+ podAnnotations: {}
+ ## @param cluster.fodcProxy.securityContext Security context for Proxy pods
+ securityContext: {}
+ ## @param cluster.fodcProxy.containerSecurityContext Container-level
security context for Proxy
+ containerSecurityContext: {}
+ ## @param cluster.fodcProxy.env Environment variables for Proxy pods
+ env: []
+ ## @param cluster.fodcProxy.priorityClassName Priority class name for
Proxy pods
+ priorityClassName: ""
+ ## Update strategy for Proxy pods
+ updateStrategy:
Review Comment:
updateStrategy is useless since the replica is always 1.
--
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]