You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@kvrocks.apache.org by "ColinChamber (via GitHub)" <gi...@apache.org> on 2023/05/02 08:29:06 UTC
[GitHub] [incubator-kvrocks] ColinChamber opened a new pull request, #1415: Add redis cli to nightly image
ColinChamber opened a new pull request, #1415:
URL: https://github.com/apache/incubator-kvrocks/pull/1415
solve #1376
Hi @PragmaTwice
We have been working on improving kvrocks-operator recently and have found that many redis-operators use redis-cli for Kubernetes liveness probes. Although we can use commands like 'echo "PING" | nc localhost 6666', it can be a bit tricky. Additionally, redis-cli is more user-friendly for operations and makes it convenient to query service information within the container. In complex cloud network environments with audit policies, it is not possible for redis-cli from any node to connect to the redis-server
**I pushed the image to my own repository and ran some tests.**
<img width="828" alt="image" src="https://user-images.githubusercontent.com/95634254/235614504-b1ef81e9-1033-443e-befd-54625edac63f.png">
<img width="1272" alt="image" src="https://user-images.githubusercontent.com/95634254/235614840-07886534-96f5-489c-aee2-e6419dd5d11d.png">
--
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: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] PragmaTwice commented on a diff in pull request #1415: Add redis cli to nightly image
Posted by "PragmaTwice (via GitHub)" <gi...@apache.org>.
PragmaTwice commented on code in PR #1415:
URL: https://github.com/apache/incubator-kvrocks/pull/1415#discussion_r1182558899
##########
Dockerfile:
##########
@@ -37,6 +37,10 @@ WORKDIR /kvrocks
COPY --from=build /kvrocks/build/kvrocks ./bin/
+ARG TARGETARCH
+COPY tools/redis-cli-${TARGETARCH} /usr/bin/redis-cli
+RUN chmod a+x /usr/bin/redis-cli
Review Comment:
I think these conditional branches related to arch in the current changes is not necessary. You can just build redis-cli in the dockerfile.
--
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: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] ColinChamber commented on a diff in pull request #1415: Add redis cli to nightly image
Posted by "ColinChamber (via GitHub)" <gi...@apache.org>.
ColinChamber commented on code in PR #1415:
URL: https://github.com/apache/incubator-kvrocks/pull/1415#discussion_r1185916049
##########
Dockerfile:
##########
@@ -37,6 +44,9 @@ WORKDIR /kvrocks
COPY --from=build /kvrocks/build/kvrocks ./bin/
+COPY --from=build /kvrocks/tools/redis-cli ./bin/
+RUN ln -s /kvrocks/bin/redis-cli /usr/bin/redis-cli
Review Comment:
/kvrocks/bin is not in the PATH environment variable, so I created a symlink. Are you suggesting that I should either copy it directly to /usr/bin or add /kvrocks/bin to the PATH?
--
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: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] PragmaTwice commented on a diff in pull request #1415: Add redis cli to nightly image
Posted by "PragmaTwice (via GitHub)" <gi...@apache.org>.
PragmaTwice commented on code in PR #1415:
URL: https://github.com/apache/incubator-kvrocks/pull/1415#discussion_r1185918683
##########
Dockerfile:
##########
@@ -37,6 +44,9 @@ WORKDIR /kvrocks
COPY --from=build /kvrocks/build/kvrocks ./bin/
+COPY --from=build /kvrocks/tools/redis-cli ./bin/
+RUN ln -s /kvrocks/bin/redis-cli /usr/bin/redis-cli
Review Comment:
I think the latter is better.
--
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: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] PragmaTwice commented on a diff in pull request #1415: Add redis cli to nightly image
Posted by "PragmaTwice (via GitHub)" <gi...@apache.org>.
PragmaTwice commented on code in PR #1415:
URL: https://github.com/apache/incubator-kvrocks/pull/1415#discussion_r1182314636
##########
Dockerfile:
##########
@@ -37,6 +37,10 @@ WORKDIR /kvrocks
COPY --from=build /kvrocks/build/kvrocks ./bin/
+ARG TARGETARCH
+COPY tools/redis-cli-${TARGETARCH} /usr/bin/redis-cli
+RUN chmod a+x /usr/bin/redis-cli
Review Comment:
And I want to emphasize my point: the dockerfile cannot have different behavior between built by a user and triggered in CI.
##########
Dockerfile:
##########
@@ -37,6 +37,10 @@ WORKDIR /kvrocks
COPY --from=build /kvrocks/build/kvrocks ./bin/
+ARG TARGETARCH
+COPY tools/redis-cli-${TARGETARCH} /usr/bin/redis-cli
+RUN chmod a+x /usr/bin/redis-cli
Review Comment:
And I want to emphasize my point: the dockerfile cannot have different behavior between built by a user and triggered in the CI.
--
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: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] PragmaTwice commented on pull request #1415: Add redis cli to nightly image
Posted by "PragmaTwice (via GitHub)" <gi...@apache.org>.
PragmaTwice commented on PR #1415:
URL: https://github.com/apache/incubator-kvrocks/pull/1415#issuecomment-1536242972
Thanks @ColinChamber , merging...
--
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: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] PragmaTwice merged pull request #1415: Add redis cli to nightly image
Posted by "PragmaTwice (via GitHub)" <gi...@apache.org>.
PragmaTwice merged PR #1415:
URL: https://github.com/apache/incubator-kvrocks/pull/1415
--
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: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] PragmaTwice commented on a diff in pull request #1415: Add redis cli to nightly image
Posted by "PragmaTwice (via GitHub)" <gi...@apache.org>.
PragmaTwice commented on code in PR #1415:
URL: https://github.com/apache/incubator-kvrocks/pull/1415#discussion_r1185770570
##########
Dockerfile:
##########
@@ -37,6 +44,9 @@ WORKDIR /kvrocks
COPY --from=build /kvrocks/build/kvrocks ./bin/
+COPY --from=build /kvrocks/tools/redis-cli ./bin/
+RUN ln -s /kvrocks/bin/redis-cli /usr/bin/redis-cli
Review Comment:
Is it necessary to add this symlink?
--
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: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] ColinChamber commented on a diff in pull request #1415: Add redis cli to nightly image
Posted by "ColinChamber (via GitHub)" <gi...@apache.org>.
ColinChamber commented on code in PR #1415:
URL: https://github.com/apache/incubator-kvrocks/pull/1415#discussion_r1185762817
##########
Dockerfile:
##########
@@ -37,6 +37,10 @@ WORKDIR /kvrocks
COPY --from=build /kvrocks/build/kvrocks ./bin/
+ARG TARGETARCH
+COPY tools/redis-cli-${TARGETARCH} /usr/bin/redis-cli
+RUN chmod a+x /usr/bin/redis-cli
Review Comment:
done
--
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: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] ColinChamber commented on a diff in pull request #1415: Add redis cli to nightly image
Posted by "ColinChamber (via GitHub)" <gi...@apache.org>.
ColinChamber commented on code in PR #1415:
URL: https://github.com/apache/incubator-kvrocks/pull/1415#discussion_r1182530628
##########
Dockerfile:
##########
@@ -37,6 +37,10 @@ WORKDIR /kvrocks
COPY --from=build /kvrocks/build/kvrocks ./bin/
+ARG TARGETARCH
+COPY tools/redis-cli-${TARGETARCH} /usr/bin/redis-cli
+RUN chmod a+x /usr/bin/redis-cli
Review Comment:
Dockerfile does not support the if-else logical structure, can I handle some logic in x.py?
--
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: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] PragmaTwice commented on a diff in pull request #1415: Add redis cli to nightly image
Posted by "PragmaTwice (via GitHub)" <gi...@apache.org>.
PragmaTwice commented on code in PR #1415:
URL: https://github.com/apache/incubator-kvrocks/pull/1415#discussion_r1182312460
##########
Dockerfile:
##########
@@ -37,6 +37,10 @@ WORKDIR /kvrocks
COPY --from=build /kvrocks/build/kvrocks ./bin/
+ARG TARGETARCH
+COPY tools/redis-cli-${TARGETARCH} /usr/bin/redis-cli
+RUN chmod a+x /usr/bin/redis-cli
Review Comment:
IMHO if a user run `docker build` towards this dockerfile in him/her device instead of CI, a fatal error will appear since there is no `tools` dir.
So I think It is not a reasonable change.
--
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: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] PragmaTwice commented on a diff in pull request #1415: Add redis cli to nightly image
Posted by "PragmaTwice (via GitHub)" <gi...@apache.org>.
PragmaTwice commented on code in PR #1415:
URL: https://github.com/apache/incubator-kvrocks/pull/1415#discussion_r1182314636
##########
Dockerfile:
##########
@@ -37,6 +37,10 @@ WORKDIR /kvrocks
COPY --from=build /kvrocks/build/kvrocks ./bin/
+ARG TARGETARCH
+COPY tools/redis-cli-${TARGETARCH} /usr/bin/redis-cli
+RUN chmod a+x /usr/bin/redis-cli
Review Comment:
And I want to emphasize my point: the dockerfile cannot have different behaviors between built by a user and triggered in the CI.
--
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: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] ColinChamber commented on a diff in pull request #1415: Add redis cli to nightly image
Posted by "ColinChamber (via GitHub)" <gi...@apache.org>.
ColinChamber commented on code in PR #1415:
URL: https://github.com/apache/incubator-kvrocks/pull/1415#discussion_r1182530628
##########
Dockerfile:
##########
@@ -37,6 +37,10 @@ WORKDIR /kvrocks
COPY --from=build /kvrocks/build/kvrocks ./bin/
+ARG TARGETARCH
+COPY tools/redis-cli-${TARGETARCH} /usr/bin/redis-cli
+RUN chmod a+x /usr/bin/redis-cli
Review Comment:
Dockerfile does not support the if-else logical structure, can I handle some logic in x.py?
--
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: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org