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

[Feature] Support command and args #516

Merged

Conversation

yandongxiao
Copy link
Collaborator

@yandongxiao yandongxiao commented Apr 23, 2024

Description

Fixes: #517

Suggestion

Although users can find workarounds, that increases the complexity of their operations.

We can solve the above problems by supporting the command and args field.

  1. Maintained compatibility. If the user does not use this, it will not cause the container to restart.
  2. We have added examples in values.yaml to make it easy to use command and args.
  3. In these examples, after adding the entrypoint in values.yaml, the program structure remains unchanged.
# the original processes
root@kube-starrocks-fe-0:/opt/starrocks# ps -ef
UID          PID    PPID  C STIME TTY          TIME CMD
root           1       0  0 11:01 ?        00:00:00 /bin/bash /opt/starrocks/fe_entrypoint.sh kube-starrocks-fe-service.starrocks
root          16       1 60 11:01 ?        00:00:12 /lib/jvm/default-java/bin/java -Dlog4j2.formatMsgNoLookups=true -Xmx8192m -XX:+UseG1GC -Xlog:gc*:/opt/starrocks/fe/log/fe.gc.log.20240423-110154:time com.starrocks.StarRocksFE -host_type FQDN
root         234       0  0 11:02 pts/0    00:00:00 bash
root         248     234  0 11:02 pts/0    00:00:00 ps -ef

root@kube-starrocks-be-0:/opt/starrocks# ps -ef
UID          PID    PPID  C STIME TTY          TIME CMD
root           1       0  0 11:02 ?        00:00:00 /bin/bash /opt/starrocks/be_entrypoint.sh kube-starrocks-fe-service
root          48       1  2 11:02 ?        00:00:01 /opt/starrocks/be/lib/starrocks_be
root         543       0  1 11:02 pts/0    00:00:00 bash
root         555     543  0 11:03 pts/0    00:00:00 ps -ef

root@kube-starrocks-cn-0:/opt/starrocks# ps -ef
UID          PID    PPID  C STIME TTY          TIME CMD
root           1       0  0 11:02 ?        00:00:00 /bin/bash /opt/starrocks/cn_entrypoint.sh kube-starrocks-fe-service
root          30       1  2 11:02 ?        00:00:01 /opt/starrocks/cn/lib/starrocks_be --cn
root         578       0  0 11:03 pts/0    00:00:00 bash
root         591     578  0 11:03 pts/0    00:00:00 ps -ef


# after entrypoint is specified in values.yaml

root@kube-starrocks-fe-0:/opt/starrocks# ps -ef
UID          PID    PPID  C STIME TTY          TIME CMD
root           1       0  0 10:56 ?        00:00:00 /bin/bash /opt/starrocks/fe_entrypoint.sh kube-starrocks-fe-service.starrocks
root          16       1 30 10:56 ?        00:00:14 /lib/jvm/default-java/bin/java -Dlog4j2.formatMsgNoLookups=true -Xmx8192m -XX:+UseG1GC -Xlog:gc*:/opt/starrocks/fe/log/fe.gc.log.20240423-105617:time com.starrocks.StarRocksFE -host_type FQDN
root         251       0  1 10:57 pts/0    00:00:00 bash
root         263     251  0 10:57 pts/0    00:00:00 ps -ef

root@kube-starrocks-be-0:/opt/starrocks# ps -ef
UID          PID    PPID  C STIME TTY          TIME CMD
root           1       0  0 10:56 ?        00:00:00 /bin/bash /opt/starrocks/be_entrypoint.sh kube-starrocks-fe-service
root          48       1  2 10:56 ?        00:00:01 /opt/starrocks/be/lib/starrocks_be
root         543       0  0 10:57 pts/0    00:00:00 bash
root         556     543  0 10:57 pts/0    00:00:00 ps -ef

root@kube-starrocks-cn-0:/opt/starrocks# ps -ef
UID          PID    PPID  C STIME TTY          TIME CMD
root           1       0  0 10:56 ?        00:00:00 /bin/bash /opt/starrocks/cn_entrypoint.sh kube-starrocks-fe-service
root          30       1  2 10:56 ?        00:00:01 /opt/starrocks/cn/lib/starrocks_be --cn
root         578       0  0 10:57 pts/0    00:00:00 bash
root         590     578  0 10:57 pts/0    00:00:00 ps -ef

Checklist

For operator, please complete the following checklist:

  • run make generate to generate the code.
  • run golangci-lint run to check the code style.
  • run make test to run UT.
  • run make manifests to update the yaml files of CRD.

For helm chart, please complete the following checklist:

  • make sure you have updated the values.yaml
    file of starrocks chart.
  • In scripts directory, run bash create-parent-chart-values.sh to update the values.yaml file of the parent
    chart( kube-starrocks chart).

@yandongxiao yandongxiao marked this pull request as ready for review April 23, 2024 03:12
# If entrypoint is set, the command will be ["bash", "-c"], and the args will be filename of the entrypoint script.
# Pod will be restarted if the entrypoint script is updated.
entrypoint: {}
# script: |
Copy link
Collaborator

Choose a reason for hiding this comment

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

is this comment still valid?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, I have verified it.

@kevincai
Copy link
Collaborator

fix the pr title

@yandongxiao yandongxiao changed the title [Feature] Add preStartScriptLocation field and support in helm chart [Feature] Support command and args Apr 23, 2024
@yandongxiao yandongxiao force-pushed the feature/support-command-and-args branch from bdcbffb to 7f2452b Compare April 23, 2024 03:21
# echo "do something before start fe"
# exec /opt/starrocks/fe_entrypoint.sh $FE_SERVICE_NAME
# # a configmap with name $cluster-fe-entrypoint-script will be created, and the script will be mounted to /etc/init-script/entrypoint.sh
# mountPath: /etc/init-script
Copy link
Collaborator

Choose a reason for hiding this comment

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

will this mountPath critical to end user, or this can be completely transparent to the end user.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

For users, it is indeed unnecessary to specify. We can hide it.

Copy link
Collaborator

Choose a reason for hiding this comment

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

let's remove it for now.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done

@yandongxiao yandongxiao merged commit 83bab97 into StarRocks:main Apr 23, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support command and args
2 participants