Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat:Expoter$$sentinel #2925

Open
wants to merge 8 commits into
base: unstable
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .github/workflows/pika.yml
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,8 @@ jobs:

- name: Install Deps
run: |
sudo apt-get install -y autoconf libprotobuf-dev protobuf-compiler
sudo apt-get install -y clang-tidy-12
sudo apt-get update
sudo apt-get install -y autoconf libprotobuf-dev protobuf-compiler clang-tidy

- name: Configure CMake
# Configure CMake in a 'build' subdirectory. `CMAKE_BUILD_TYPE` is only required if you are using a single-configuration generator such as make.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
{{- $files := .Files.Glob "dashboards/*.json" }}
{{- if $files }}
apiVersion: v1
kind: ConfigMapList
items:
{{- range $path, $fileContents := $files }}
{{- $dashboardName := regexReplaceAll "(^.*/)(.*)\\.json$" $path "${2}" }}
- apiVersion: v1
kind: ConfigMap
metadata:
name: {{ printf "%s-grafana-%s" (include "pika.name" $) $dashboardName | trunc 63 | trimSuffix "-" }}
labels:
grafana_dashboard: "1"
app: {{ template "pika.name" $ }}-grafana
{{ include "pika.labels" $ | indent 6 }}
data:
{{ $dashboardName }}.json: {{ $.Files.Get $path | toJson }}
{{- end }}
{{- end }}
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
apiVersion: apps.kubeblocks.io/v1alpha1
kind: ComponentDefinition
metadata:
name: pika-exporter
namespace: {{ .Release.Namespace }}
labels:
{{- include "pika.labels" . | nindent 4 }}
spec:
provider: pika
description: A pika exporter component definition
serviceKind: pika-exporter
serviceVersion: {{ .Chart.AppVersion }}
services:
- name: expoter
spec:
ports:
- name: expoter
port: 9121
targetPort: expoter
updateStrategy: Serial
Comment on lines +8 to +20
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Fix typo in service name

The spec and services section is well-defined, but there's a typo in the service name:

services:
  - name: expoter  # Should be "exporter"

Please correct this typo to ensure consistency and avoid potential issues.

The rest of the section, including the use of Helm templating for serviceVersion and the "Serial" updateStrategy, looks good.

configs:
- name: pika-config
templateRef: pika-conf-template
namespace: {{ .Release.Namespace }}
volumeName: config
vars:
## reference to the pika-codis-dashboard service
- name: DASHBOARD_ADDR
valueFrom:
serviceVarRef:
compDef: pika-codis-dashboard
name: dashboard
optional: true
host: Optional
runtime:
initContainers:
- name: wait-codis-dashboard
env:
- name: DASHBOARD_ADDR
value: "$(KB_CLUSTER_NAME)-codis-dashboard"
image: busybox:1.28
command:
- 'sh'
- '-c'
- "until nc -z ${DASHBOARD_ADDR} 18080; do echo waiting for codis dashboard; sleep 2; done;"
containers:
- name: pika-exporter
image: {{ include "pikaExporter.image" . }}
imagePullPolicy: {{ include "pikaExporter.imagePullPolicy" . }}
ports:
- name: expoter
containerPort: 9121
volumeMounts:
- name: config
mountPath: /etc/pika
env:
- name: DASHBOARD_ADDR
value: "$(KB_CLUSTER_NAME)-codis-dashboard"
command:
- "/pika/bin/pika_exporter"
args:
- "-config"
- "/etc/pika/info.toml"
- "-codis.addr"
- "http://$(DASHBOARD_ADDR):18080/topom"
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
apiVersion: apps.kubeblocks.io/v1alpha1
kind: ComponentDefinition
metadata:
name: redis-sentinel
namespace: {{ .Release.Namespace }}
labels:
{{- include "pika.labels" . | nindent 4 }}
Comment on lines +1 to +7
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Fix YAML syntax error in labels section

There's a YAML syntax error in the labels section. The indentation seems to be correct, but the linter is reporting an issue.

To resolve this, try adjusting the Helm template function call. Replace:

  labels:
    {{- include "pika.labels" . | nindent 4 }}

with:

  labels: {{- include "pika.labels" . | nindent 4 }}

This change puts the template function on the same line as labels:, which might resolve the syntax error while maintaining the correct indentation of the resulting labels.

🧰 Tools
🪛 yamllint

[error] 7-7: syntax error: expected the node content, but found '-'

(syntax)

spec:
clusterSize: 3
podSecurityContext:
runAsUser: 1000
fsGroup: 1000
redisSentinelConfig:
redisReplicationName: redis-replication
kubernetesConfig:
image: 'quay.io/opstree/redis-sentinel:v7.0.7'
imagePullPolicy: IfNotPresent
resources:
requests:
cpu: 101m
memory: 128Mi
limits:
cpu: 101m
memory: 128Mi
updateStrategy: Serial
Comment on lines +15 to +25
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Consider adjusting resource management and update strategy

The Kubernetes configuration is generally well-structured, but there are a few points to consider:

  1. Resource Management: The requests and limits are identical (101m CPU, 128Mi memory). While this prevents over-commitment, it might be overly restrictive and could impact scaling.

  2. Update Strategy: The 'Serial' update strategy ensures controlled updates but might slow down the update process for larger deployments.

Consider the following adjustments:

  1. Differentiate resource requests and limits:
resources:
  requests:
    cpu: 50m
    memory: 64Mi
  limits:
    cpu: 101m
    memory: 128Mi
  1. Evaluate if a more parallel update strategy would be beneficial:
updateStrategy: RollingUpdate

These changes could improve resource utilization and update efficiency while maintaining stability.

configs:
- name: pika-config
templateRef: pika-conf-template
namespace: {{ .Release.Namespace }}
volumeName: config
containers:
- name: redis-sentinel
image: {{ include "pikaExporter.image" . }}
imagePullPolicy: {{ include "pikaExporter.imagePullPolicy" . }}
ports:
- name: expoter
containerPort: 9121
volumeMounts:
- name: config
mountPath: /etc/pika
env:
- name: DASHBOARD_ADDR
value: "$(KB_CLUSTER_NAME)-codis-dashboard"
command:
- "/pika/bin/pika_exporter"
args:
- "-config"
- "/etc/pika/info.toml"
- "-codis.addr"
- "http://$(DASHBOARD_ADDR):18080/topom"
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
apiVersion: apps.kubeblocks.io/v1alpha1
kind: ComponentVersion
metadata:
name: pika-exporter
labels:
{{- include "pika.labels" . | nindent 4 }}
Comment on lines +3 to +6
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Fix YAML syntax error in labels section.

There's a YAML syntax error in the labels section. The indentation for the include statement is incorrect.

Please apply the following fix:

 metadata:
   name: pika-exporter
   labels:
-    {{- include "pika.labels" . | nindent 4 }}
+    {{- include "pika.labels" . | nindent 4 }}

Ensure that the {{- is at the beginning of the line, without any leading spaces.

Committable suggestion was skipped due to low confidence.

🧰 Tools
🪛 yamllint

[error] 6-6: syntax error: expected the node content, but found '-'

(syntax)

spec:
compatibilityRules:
- compDefs:
- pika-exporter
releases:
- {{ .Chart.AppVersion }}
releases:
- name: {{ .Chart.AppVersion }}
changes:
serviceVersion: {{ .Chart.AppVersion }}
images:
codis-dashboard: {{ include "pikaExporter.image" . }}
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
apiVersion: redis.redis.opstreelabs.in/v1beta1
kind: RedisSentinel
metadata:
name: redis-sentinel
spec:
clusterSize: 3
podSecurityContext:
runAsUser: 1000
fsGroup: 1000
redisSentinelConfig:
redisReplicationName: redis-replication
kubernetesConfig:
image: 'quay.io/opstree/redis-sentinel:v7.0.7'
imagePullPolicy: IfNotPresent
resources:
requests:
cpu: 101m
memory: 128Mi
limits:
cpu: 101m
memory: 128Mi
Loading