redis-stack-server docker startup script error

[zcw159357]

if set module args with space will cause entrypoint.sh report error msg.
like I set

REDISEARCH_ARGS=FRISOINI /opt/dict/friso.ini

and get

/entrypoint.sh: 21: [: FRISOINI: unexpected operator

I checked the script and I think it may be better to surround ${REDISEARCH_ARGS} with " in the if judgment?

if [ -z ${REDISEARCH_ARGS} ]; then
REDISEARCH_ARGS="MAXSEARCHRESULTS 10000 MAXAGGREGATERESULTS 10000"
fi

if [ -z ${REDISGRAPH_ARGS} ]; then
REDISGRAPH_ARGS="MAX_QUEUED_QUERIES 25 TIMEOUT 1000 RESULTSET_SIZE 10000"
fi

to

if [ -z "${REDISEARCH_ARGS}" ]; then
REDISEARCH_ARGS="MAXSEARCHRESULTS 10000 MAXAGGREGATERESULTS 10000"
fi

if [ -z "${REDISGRAPH_ARGS}" ]; then
REDISGRAPH_ARGS="MAX_QUEUED_QUERIES 25 TIMEOUT 1000 RESULTSET_SIZE 10000"
fi

and I see #158 with same error, but not fixing the script.

[zcw159357]

BTW why set REDISEARCH_ARGS to "MAXSEARCHRESULTS 10000 MAXAGGREGATERESULTS 10000" while these params are already hard coded in line 34 ?

[chayim]

@zcw159357 There's a bunch of cruft there that we should really clean up. Thanks for this - it forced me to do it as part of the PR I'll link shortly.

At one point an argument that is redeclared would win. But I recognize the confusion. Henceforth defaults will be set when none are set.

As validation - here's the output of my docker (local) testing case, after building with a custom tag.

Starting the docker: docker run -p 6379:6379 -e REDISEARCH_ARGS="MAXSEARCHRESULTS 5" -it redis:mytest

Yields:

redis-cli
127.0.0.1:6379> ping
PONG
127.0.0.1:6379> module list
<snip>
3) 1) "name"
   2) "search"
   3) "ver"
   4) (integer) 20411
   5) "path"
   6) "/opt/redis-stack/lib/redisearch.so"
   7) "args"
   8) 1) "MAXSEARCHRESULTS"
      2) "5"

Starting the docker without the search arguments: docker run -p 6379:6379 -e REDISEARCH_ARGS="MAXSEARCHRESULTS 5" -it redis:mytest

Yields:

redis-cli
127.0.0.1:6379> ping
PONG
127.0.0.1:6379> module list
<snip>
2) 1) "name"
   2) "search"
   3) "ver"
   4) (integer) 20411
   5) "path"
   6) "/opt/redis-stack/lib/redisearch.so"
   7) "args"
   8) 1) "MAXSEARCHRESULTS"
      2) "10000"
      3) "MAXAGGREGATERESULTS"
      4) "10000"

[zcw159357]

@chayim Thanks for replying. I get you'd like to set a default value for MAXSEARCHRESULTS and MAXAGGREGATERESULTS, what I didn't get is why bothering set value for REDISEARCH_ARGS in line 21,22,23 when REDISEARCH_ARGS is empty, isn't it already been done in line 34(old version)?

And after #192 ,when starting with docker run -p 6379:6379 -e REDISEARCH_ARGS="MAXSEARCHRESULTS 5" -it redis:mytest MAXAGGREGATERESULTS will fallback to use the redis default value(which is unlimited) not 10000 right?
I see you add -- to REDIS_ARGS wouldn't that be confusing? The startup command could be like docker run -p 6379:6379 -e REDISEARCH_ARGS="MAXSEARCHRESULTS 5" -e REDIS_ARGS="requirepass xxx --some-other-config yyy --some-other-config2 zzz" if there's more than one arg to add.

[zcw159357]

and.... the /entrypoint.sh: 21: [: FRISOINI: unexpected operator msg would still exists?

[chayim]

and.... the /entrypoint.sh: 21: [: FRISOINI: unexpected operator msg would still exists?

If you pass the variables in quoted, then it should be fine. So in the docker run -e REDISERACH_ARGS="FRISOINI ..."

[chayim]

@chayim Thanks for replying. I get you'd like to set a default value for MAXSEARCHRESULTS and MAXAGGREGATERESULTS, what I didn't get is why bothering set value for REDISEARCH_ARGS in line 21,22,23 when REDISEARCH_ARGS is empty, isn't it already been done in line 34(old version)?

The idea is, if you're modifying these variables it's a series of known, desired choices. This was the issue with overrides.

And after #192 ,when starting with docker run -p 6379:6379 -e REDISEARCH_ARGS="MAXSEARCHRESULTS 5" -it redis:mytest MAXAGGREGATERESULTS will fallback to use the redis default value(which is unlimited) not 10000 right? I see you add -- to REDIS_ARGS wouldn't that be confusing? The startup command could be like docker run -p 6379:6379 -e REDISEARCH_ARGS="MAXSEARCHRESULTS 5" -e REDIS_ARGS="requirepass xxx --some-other-config yyy --some-other-config2 zzz" if there's more than one arg to add.

This piece is... painful, admittedly. I prefer the "--thingyoucareabout value" syntax. However, in the case of a single argument from several locations we've had asks against it. I'm frankly, happy to walk that back - I don't love it, and feel that both choices are suboptimal.

[zcw159357]

and.... the /entrypoint.sh: 21: [: FRISOINI: unexpected operator msg would still exists?

If you pass the variables in quoted, then it should be fine. So in the docker run -e REDISERACH_ARGS="FRISOINI ..."

I don't think so..... It'll resulting in --loadmodule /opt/redis-stack/lib/redisearch.so "FRISOINI ..." and will get errors.

[chayim]

Ah, I see - yes the double quoting.. it's wrong. I'm going to reopen this, and the other associated bug; settle something for both.

[chayim]

@zcw159357 Okay - I think we're fine here - with the current setup. Caveat: I am reverting the "--" but only for REDIS_ARGS. The trade here is that I've added documentation in the README.

To validate, I ran the following, the upcoming change to reverse the "--" prefix on REDIS_ARGS.

1. I build the focal deb: invoke package -d focal -p redis-stack-server -s ubuntu20.04 -t deb
2. Next, I copied it into place, so that the docker will behave:

mkdir redis-stack
cp *.deb redis-stack

3. I generated the docker build file: invoke dockergen -d redis-stack-server
4. Then, I built the docker with a custom tag, just to be sure: ```invoke dockerbuild -a x86_64 -d envs/dockers/Dockerfile.redis-stack-server -t myfoo:chayimisfoo -r .`

Finally, I ran a variety of permutations:

1. Noauth: docker run -p 6379:6379 -it docker.io/library/myfoo:chayimisfoo

2. Auth and module defaults docker run -p 6379:6379 -e REDIS_ARGS="--requirepass foo" -it docker.io/library/myfoo:chayimisfoo

3. Auth and a single REDISEARCH argument: docker run -p 6379:6379 -e REDISEARCH_ARGS="MAXSEARCHRESULTS 5" -e REDIS_ARGS="--requirepass foo" -it docker.io/library/myfoo:chayimisfoo

4. Auth and multiple REDISEARCH args: docker run -p 6379:6379 -e REDISEARCH_ARGS="MAXSEARCHRESULTS 5 FRISOINI 111" -e REDIS_ARGS="--requirepass foo" -it docker.io/library/myfoo:chayimisfoo

5. Ensuring broken/invalid arguments break things: docker run -p 6379:6379 -e REDISEARCH_ARGS="YARINVALIDARGUMENT 15" -it docker.io/library/myfoo:chayimisfoo

In each case, the module arguments were validated via MODULE LIST when connected with the redis-cli.

At this point, I think it's fair to call this closed.

[zcw159357]

@chayim Sorry...but I still need to say.... /entrypoint.sh: 21: [: MAXSEARCHRESULTS: unexpected operator this issue hasn't been fixed yet.... Yes, it doesn't affect the functions, it's only a warning msg... But it'll show up every time when there's space in REDISEARCH_ARGS or REDISGRAPH_ARGS

docker run -e REDIS_ARGS="--requirepass foo" -e REDISEARCH_ARGS="MAXSEARCHRESULTS 5" redis/redis-stack:latest will still resulting in this bash error msg, quote the MAXSEARCHRESULTS 5 has no help.

quote the arg would be better right? if [ -z ${REDISEARCH_ARGS} ]; then to if [ -z "${REDISEARCH_ARGS}" ]; then

[chayim]

Interesting; So I changed shells to see all sorts of output swallowed - thank you. I've added quotes to the REDISEARCH_ARGS, REDISGRAPH_ARGS args - as well as REDIS_DATA_DIR. For good measure, I'll hope no one uses this with the latter. I'll see if I can get this into today's release.

[zcw159357]

@chayim Thanks for all the work!