Skip to content

[HWORKS-2117] Make rondb-helm works for RonDB 24.10 #46

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Apr 14, 2025
Merged

[HWORKS-2117] Make rondb-helm works for RonDB 24.10 #46

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
73fc83b
[HWORKS-2117] Make rondb-helm works for RonDB 24.10
KernelMaker Apr 3, 2025
617e2e3
[HWORKS-2117] Make rondb-helm works for RonDB 24.10 [PATCH 2]
KernelMaker Apr 11, 2025
d8b3559
[HWORKS-2117] Make rondb-helm work with RonDB 24.10 [PATCH 3]
KernelMaker Apr 14, 2025
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 Chart.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,11 @@ name: rondb

# This is the chart version. This version number should be incremented each time you make changes
# to the chart and its templates, including the app version.
version: 0.6.8
version: 0.7.0
Copy link
Contributor

Choose a reason for hiding this comment

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

A general comment, are we planning to create a branch for rondb-helm to be based on 22.10 since i assume we might need to do some fixes in the future for 4.2 which uses the rondb-helm chart 0.6.5 . If so, i think we should adapt the version of the rondb-helm chart to reflect that as well @kouzant @KernelMaker

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I discussed with Antonios last week, and the conclusion was that there will be a standalone branch for 22.10 in the future. The default branch will serve as the main development branch for 24.10 going forward.

We haven’t finalized when or where exactly to create the new branch for 22.10 yet, but I assume it should start from the last commit before this patch is merged? @kouzant @maismail

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah exactly. As @KernelMaker said we'll create the 0.6 branch. We waited until Serverless migration in case we had to do some changes so we don't need to backport

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, make sense, thanks!


# This is the version number of the application being deployed. This version number should be
# incremented each time you make changes to the application.
appVersion: 22.10.7-0.7
appVersion: 24.10.0

# kubeVersion: A SemVer range of compatible Kubernetes versions (optional)
kubeVersion: ">= 1.27.0-0"
Expand Down
5 changes: 0 additions & 5 deletions files/configs/rest_api.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,6 @@
"ServerIP": "0.0.0.0",
"ServerPort": 4406
},
"GRPC": {
"Enable": true,
"ServerIP": "0.0.0.0",
"ServerPort": 5406
},
"RonDB": {
"Mgmds": [
{
Expand Down
26 changes: 26 additions & 0 deletions files/scripts/ndbmtds.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,32 @@ do
ln -s ${RONDB_VOLUME}/${dir} ${BASE_DIR}/${dir}
done

LOG_DIR="${BASE_DIR}/log/"
echo "[K8s Entrypoint ndbmtd] check log dir: ${LOG_DIR}"
if [ -d "$LOG_DIR" ]; then
ls -al "$LOG_DIR"

# Double-checked config.ini:
# DataDir = ${BASE_DIR}/log
# So ndb_* will move the generated error and trace log files from this directory.
#
# CAUTION:
# If additional files are configured to be stored in this directory in the future,
# be careful with this move operation — it may affect unrelated files.
files=($(find "$LOG_DIR" -maxdepth 1 -type f -name 'ndb_*'))
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is there a common suffix we can filter tracefiles? I'm thinking of your comment in case we/somebody adds more files under log directory. If not, then that's good enough

Copy link
Collaborator Author

@KernelMaker KernelMaker Apr 14, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Here are the log file names I got from the test env:
ndb_4_error.log, ndb_4_trace.log.1, ndb_4_trace.log.1_t1, ndb_4_trace.log.2, ndb_4_trace.log.2_t1, ndb_4_trace.log.3, ndb_4_trace.log.3_t1, ndb_4_trace.log.4, ndb_4_trace.log.4_t1, ndb_4_trace.log.next

They don’t have a common suffix.. maybe ndb_*log*

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah I think filter just the logs is safer, in case something else accidentally ends in this directory

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

[HWORKS-2117] Make rondb-helm work with RonDB 24.10 [PATCH 3]

Update regex from ndb_* to ndb*_log* for safer archiving of trace and error logs.


if [ "${#files[@]}" -gt 0 ]; then
timestamp=$(date +%Y%m%d_%H%M%S)
target_dir="$LOG_DIR/issue_at_$timestamp"
mkdir -p "$target_dir"

echo "[K8s Entrypoint ndbmtd] $target_dir generated in ${LOG_DIR}"
for file in "${files[@]}"; do
mv "$file" "$target_dir/"
done
fi
fi

INITIAL_START=
# This is the first file that is read by the ndbmtd
# WARNING: This env var needs to be aware of symlinks created here
Expand Down
13 changes: 1 addition & 12 deletions templates/rdrs-aws-TargetGroupBinding.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,16 +11,5 @@ spec:
name: {{ .Values.meta.rdrs.externalLoadBalancer.name }}
port: 4406
targetGroupARN: {{ .Values.global.unmanagedLoadBalancers.rdrs.http.targetGroupId }}
---
apiVersion: elbv2.k8s.aws/v1beta1
kind: TargetGroupBinding
metadata:
name: rdrs-grpc
namespace: {{ .Release.Namespace }}
spec:
serviceRef:
name: {{ .Values.meta.rdrs.externalLoadBalancer.name }}
port: 5406
targetGroupARN: {{ .Values.global.unmanagedLoadBalancers.rdrs.grpc.targetGroupId }}
{{- end -}}
{{- end -}}
{{- end -}}
14 changes: 1 addition & 13 deletions templates/rdrs.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ spec:
# here is just a sanity check if the main container is restarted.
{{ include "rondb.resolveOwnIp" $ | indent 12 }}

rdrs -config={{ include "rondb.dataDir" $ }}/rest_api.json
rdrs2 --config {{ include "rondb.dataDir" $ }}/rest_api.json
ports:
- containerPort: 4406
- containerPort: 5406
Expand Down Expand Up @@ -152,10 +152,6 @@ spec:
protocol: TCP
port: 4406
targetPort: 4406
- name: grpc
protocol: TCP
port: 5406
targetPort: 5406
---
# This is for the Ingress controller to route traffic to the RDRS
apiVersion: v1
Expand All @@ -176,10 +172,6 @@ spec:
protocol: TCP
port: 4406
targetPort: 4406
- name: grpc
protocol: TCP
port: 5406
targetPort: 5406
{{ if
and (gt (.Values.clusterSize.minNumRdrs | int) 0)
(not (eq .Values.clusterSize.minNumRdrs .Values.clusterSize.maxNumRdrs))
Expand Down Expand Up @@ -236,8 +228,4 @@ spec:
protocol: TCP
port: 4406
targetPort: 4406
- name: grpc
protocol: TCP
port: 5406
targetPort: 5406
{{- end }}
4 changes: 2 additions & 2 deletions values.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,12 +90,12 @@ images:
name: rondb
registry: docker.io
repository: hopsworks
tag: 22.10.7-0.7
tag: 24.10.0
toolbox:
name: hwutils
registry: docker.io
repository: hopsworks
tag: '0.7'
tag: '0.8'
isMultiNodeCluster: true
meta:
binlogServers:
Expand Down
Loading