deis / logger

In-memory log buffer used by Deis Workflow.
https://deis.com
MIT License
18 stars 15 forks source link

v2.9.0 logger still does not work with off-cluster redis #127

Closed rimusz closed 7 years ago

rimusz commented 7 years ago

$ kubectl -n deis logs deis-logger-3305119006-ixc1p 2016/12/06 11:44:46 config error: envconfig.Process: assigning LOGGER_DEIS_LOGGER_REDIS_SERVICE_PORT to DEIS_LOGGER_REDIS_SERVICE_PORT: converting 'redis port' to type int:

dhilipsiva commented 7 years ago

More Context:

I am running on Kubernetes 1.4.6 on GKE with deis version 2.9.0 with helm

Client: &version.Version{SemVer:"v2.0.1", GitCommit:"dc87ea8dd0111164c720fd13b4eb8a905f97ac62", GitTreeState:"clean"}
Server: &version.Version{SemVer:"v2.0.1", GitCommit:"dc87ea8dd0111164c720fd13b4eb8a905f97ac62", GitTreeState:"clean"}

I am trying to deply an off-cluster redis with configuration:

logger:
  redis:
    # Configure the following ONLY if using an off-cluster Redis instance for logger
    db: "logdb"
    host: "1.2.3.4"
    port: 6379
    password: "" # "" == no password

But my deis logger fails with error:

2016-12-06T11:04:28.958212521Z 2016/12/06 11:04:28 config error: envconfig.Process: assigning LOGGER_DEIS_LOGGER_REDIS_SERVICE_PORT to DEIS_LOGGER_REDIS_SERVICE_PORT: converting 'redis port' to type int: 
bacongobbler commented 7 years ago

@dhilipsiva try encasing port in quotes: port: "6379". The kubernetes API requires envvar values as strings, but if left out the yaml parsing library in k8s/client-go will parse it as an int, hence the error.

rimusz commented 7 years ago

@bacongobbler port: "6379" still gives an error

bacongobbler commented 7 years ago

I found this to be the case as well using an off-cluster redis instance. #129 should provide a better error message which then I can identify what the issue is from there.

bacongobbler commented 7 years ago

Here's a better representation of the error:

><> kd logs deis-logger-484450232-7q1gy
2016/12/06 17:49:01 config error: envconfig.Process: assigning LOGGER_DEIS_LOGGER_REDIS_SERVICE_PORT to Port: converting 'redis port' to type int. details: strconv.ParseInt: parsing "redis port": invalid syntax:

So it looks like even though we are supplying the value in values.yaml, "redis port" is still needing to be updated somewhere.

bacongobbler commented 7 years ago

This looks to be a chart templating issue. After installing, here's what I see:

><> kd get secret logger-redis-creds -o yaml
apiVersion: v1
data:
  db: MA==
  host: cmVkaXMgaG9zdA==
  password: cmVkaXMgcGFzc3dvcmQ=
  port: cmVkaXMgcG9ydA==
kind: Secret
metadata:
  annotations:
    helm.sh/hook: pre-install
  creationTimestamp: 2016-12-06T17:25:44Z
  labels:
    app: deis-logger-redis
    heritage: deis
  name: logger-redis-creds
  namespace: deis
  resourceVersion: "5691"
  selfLink: /api/v1/namespaces/deis/secrets/logger-redis-creds
  uid: 04a71f56-bbd9-11e6-96d5-080027242396
type: Opaque

And if I take in that password:

><> echo "cmVkaXMgcG9ydA==" | base64 --decode
redis port

@rimusz @dhilipsiva try editing the logger-redis-creds secret with the expected values base64 encoded, then delete the logger pod to restart it.

bacongobbler commented 7 years ago

For now, the fix is to do the following with your values.yaml until v2.10 (or v2.9.1, ping @mboersma @vdice @kmala):

global:
  logger_redis_location: "off-cluster"
db: "0"
host: "192.168.0.17"
port: "6379"
password: "" # "" == no password

Essentially the redis secret template expects the values at the root rather than being namespaced at redis, which is what the chart expects you to do. deis/redis#10 fixes this but you can use the above as a workaround until we release a patch or release v2.10 in January.

dhilipsiva commented 7 years ago

Thanks for you input @bacongobbler ! You are awesome! 😄