RockteMQ-AI commented on code in PR #51:
URL: https://github.com/apache/rocketmq-operator/pull/51#discussion_r3909386454
##########
pkg/apis/rocketmq/v1alpha1/nameservice_types.go:
##########
@@ -49,6 +49,17 @@ type NameServiceSpec struct {
HostPath string `json:"hostPath"`
// VolumeClaimTemplates defines the StorageClass
VolumeClaimTemplates []corev1.PersistentVolumeClaim
`json:"volumeClaimTemplates"`
+ // rocketmq exporter
+ Exporter RocketmqExporter `json:"exporter,omitempty"`
Review Comment:
Exporter annotations from the example cannot work: RocketmqExporter has no
Annotations field and the controller never copies them to the Pod template.
Prometheus discovery configured in the example will not select these pods.
##########
pkg/controller/nameservice/nameservice_controller.go:
##########
@@ -342,8 +343,42 @@ func (r *ReconcileNameService)
statefulSetForNameService(nameService *rocketmqv1
VolumeClaimTemplates:
getVolumeClaimTemplates(nameService),
},
}
+ if nameService.Spec.Exporter.Enabled {
+ exporter := r.createRocketMQExporterContainer(nameService)
+ dep.Spec.Template.Spec.Containers =
append(dep.Spec.Template.Spec.Containers, exporter)
+ }
// Set Broker instance as the owner and controller
controllerutil.SetControllerReference(nameService, dep, r.scheme)
return dep
}
+
+func (r *ReconcileNameService) createRocketMQExporterContainer(nameService
*rocketmqv1alpha1.NameService) (container corev1.Container) {
+ container = corev1.Container{
+ Name: cons.ExporterContainerName,
+ Image: nameService.Spec.Exporter.Image,
+ Env: nameService.Spec.Exporter.Env,
+ ImagePullPolicy: func(specPolicy corev1.PullPolicy)
corev1.PullPolicy {
+ if specPolicy == "" {
+ return corev1.PullAlways
+ }
+ return specPolicy
+ }(nameService.Spec.Exporter.ImagePullPolicy),
+ Resources: func(requirements corev1.ResourceRequirements)
corev1.ResourceRequirements {
+ if requirements.Limits.Memory().IsZero() &&
requirements.Requests.Memory().IsZero() {
+ return corev1.ResourceRequirements{
+ Limits: corev1.ResourceList{
Review Comment:
Providing CPU-only resource settings causes this branch to replace the
entire ResourceRequirements with defaults, silently discarding the user's CPU
values. Default missing resource keys individually, or only default when both
resource lists are empty.
##########
deploy/crds/rocketmq_v1alpha1_nameservice_crd.yaml:
##########
@@ -63,6 +63,9 @@ spec:
items:
type: object
type: array
+ exporter:
+ description: Exporter defines rocketmq metrics
+ type: object
Review Comment:
The CRD declares exporter as an unstructured object. On clusters that prune
unknown CRD fields, enabled/image/env/resources will be removed, so
reconciliation always sees Enabled=false. Define the nested schema (or
explicitly preserve unknown fields).
##########
images/rocketmq-exporter/alpine/application.yml:
##########
@@ -0,0 +1,38 @@
+server:
+ port: 5557
+spring:
+ application:
+ name: rocketmq-exporter
+ http:
+ encoding:
+ charset: UTF-8
+ enabled: true
+ force: true
+logging:
+ config: classpath:logback.xml
+
+rocketmq:
+ config:
+ webTelemetryPath: ${WEB_TELEMETRY_PATH}
+ rocketmqVersion: V4_3_2
Review Comment:
rocketmqVersion is hard-coded to V4_3_2, so the ROCKETMQ_VERSION environment
variable supplied by the example is ignored. This can make exporter commands
incompatible with the deployed RocketMQ version.
##########
images/rocketmq-exporter/alpine/Dockerfile:
##########
@@ -0,0 +1,51 @@
+#
+# 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.
+#
+FROM openjdk:8-alpine
+
+RUN apk add --no-cache bash gettext nmap-ncat curl git openssl busybox-extras
+
+ARG MAVEN_VERSION=3.6.3
+ARG BASE_URL=https://apache.osuosl.org/maven/maven-3/${MAVEN_VERSION}/binaries
+ARG
SHA=c35a1803a6e70a126e80b2b3ae33eed961f83ed74d18fcd16909b2d44d7dada3203f1ffe726c17ef8dcca2dcaa9fca676987befeadc9b9f759967a8cb77181c0
+
+RUN mkdir -p /usr/share/maven /usr/share/maven/ref \
+ && echo "Downlaoding maven" \
+ && curl -fsSL -o /tmp/apache-maven.tar.gz
${BASE_URL}/apache-maven-${MAVEN_VERSION}-bin.tar.gz \
+ \
+ && echo "Checking download hash" \
+ && echo "${SHA} /tmp/apache-maven.tar.gz" | sha512sum -c - \
+ \
+ && echo "Unziping maven" \
+ && tar -xzf /tmp/apache-maven.tar.gz -C /usr/share/maven
--strip-components=1 \
+ \
+ && echo "Cleaning and setting links" \
+ && rm -f /tmp/apache-maven.tar.gz \
+ && ln -s /usr/share/maven/bin/mvn /usr/bin/mvn
+
+RUN mkdir /exporter \
+ && git clone https://github.com/apache/rocketmq-exporter.git \
Review Comment:
The image build clones the exporter default branch without pinning a
revision; the version build argument is not declared or used. Version-tagged
images are therefore non-reproducible and may contain an incompatible exporter
release.
##########
pkg/apis/rocketmq/v1alpha1/nameservice_types.go:
##########
@@ -49,6 +49,17 @@ type NameServiceSpec struct {
HostPath string `json:"hostPath"`
// VolumeClaimTemplates defines the StorageClass
VolumeClaimTemplates []corev1.PersistentVolumeClaim
`json:"volumeClaimTemplates"`
+ // rocketmq exporter
+ Exporter RocketmqExporter `json:"exporter,omitempty"`
Review Comment:
The new Exporter field contains slices, pointers, and resource maps, but no
regenerated deepcopy implementation is included. Existing generated DeepCopy
methods will shallow-copy these fields, violating Kubernetes API object copy
semantics.
##########
images/rocketmq-exporter/alpine/exporterStart.sh:
##########
@@ -0,0 +1,18 @@
+#!/bin/bash
+
+# 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.
+
+java -jar /rocketmq-exporter/rocketmq-exporter.jar
--spring.config.location=/rocketmq-exporter/application.yml
Review Comment:
java is not exec'd, so PID 1 is bash and SIGTERM may not reach the JVM
during Pod termination. Use exec java so the exporter can shut down gracefully.
--
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]
