AmadeusITGroup / Redis-Operator

Redis Operator creates/configures/manages Redis clusters atop Kubernetes
MIT License
167 stars 62 forks source link

Fix the config typo. #20

Closed tuapuikia closed 6 years ago

tuapuikia commented 6 years ago

The correct syntax is maxmemory and maxmemory-policy. https://raw.githubusercontent.com/antirez/redis/4.0/redis.conf

codecov-io commented 6 years ago

Codecov Report

Merging #20 into master will not change coverage. The diff coverage is 0%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master      #20   +/-   ##
=======================================
  Coverage   37.01%   37.01%           
=======================================
  Files          37       37           
  Lines        3409     3409           
=======================================
  Hits         1262     1262           
  Misses       2030     2030           
  Partials      117      117
Impacted Files Coverage Δ
pkg/redisnode/config.go 0% <0%> (ø) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 1669bf2...04a4a91. Read the comment docs.

clamoriniere1A commented 6 years ago

Hi @tuapuikia , Thank you for opening this PR. The typo that you are referring is in the argument of the redis-node process. But if you look at what is really written in the redis configuration file, is the correct syntax: https://github.com/AmadeusITGroup/Redis-Operator/blob/master/pkg/redisnode/node.go#L60 and https://github.com/AmadeusITGroup/Redis-Operator/blob/master/pkg/redisnode/node.go#L66

I agree that can be misleading, but at this end the configuration file syntax is correct. @tuapuikia is it fine for you with my explanation?

If you want to go further with this PR. I think you should add new args instead of replacing in order to not break other users. And also set the previous args as deprecated.

Last remarks you need also to update the redis-cluster helm chart in order to use the new args: https://github.com/AmadeusITGroup/Redis-Operator/blob/master/chart/redis-cluster/templates/RedisCluster.yml#L29

Let me know if you need help or have some questions.

tuapuikia commented 6 years ago

Hi,

I get this error when try to create new cluster.

kubectl logs -f rediscluster-redis-cluster-sbb74 Program started at: 2018-08-03 02:15:52.177418618 +0000 UTC m=+0.058763779 BUILDTIME=2018-07-03/17:42:00 TAG=latest COMMIT=695c49903eceee361c910c007ca9bc63ca99fbd0 VERSION=0.1.0 unknown flag: --max-memory-policy Usage of /redisnode: --alsologtostderr log to standard error as well as files (default false) --bin string redis server binary file name (default "redis-server") --c string redis config file path (default "/redis-server/redis.conf") --cluster-node-timeout int redis node timeout (ms) (default 2000) --d duration delay before that the redis-server is started (default 10s) --http-addr string the http server listen address (default "0.0.0.0:8080") --kubeconfig string Location of kubecfg file for access to kubernetes master service --log_backtrace_at traceLocation when logging hits line file:N, emit a stack trace (default :0) --log_dir string If non-empty, write log files in this directory --logtostderr log to standard error instead of files (default false) --master string The address of the Kubernetes API server. Overrides any value in kubeconfig. Only required if out-of-cluster. --max-memory uint32 redis max memory --ns string redis-node k8s namespace --port string redis server listen port (default "6379") --rdt int redis dial timeout (ms) (default 2000) --rename-command-file string Name of the file where rename-commands option for redis are available, disabled if empty --rename-command-path string Path to the folder where rename-commands option for redis are available (default "/etc/secret-volume") --rs string redis-node k8s service name --stderrthreshold severity logs at or above this threshold go to stderr (default 2) --t duration Max time waiting for redis to start (default 10s) -v, --v Level log level for V logs (default 0) --vmodule moduleSpec comma-separated list of pattern=N settings for file-filtered logging

tuapuikia commented 6 years ago

ok, rebuild the latest docker image and it working now. Thanks you for checking it.