dokku-redis icon indicating copy to clipboard operation
dokku-redis copied to clipboard

redis:link does not complete correctly if REDIS_URL already set

Open crutch opened this issue 7 years ago • 11 comments

While I was trying to setup our dokku powered application with redis, I set up manually REDIS_URL config variable for our app (I was creating staging env, replicating setup from our production machine and created REDIS_URL with correct staging redis DSN).

Then I attempted to create a dokku/docker link between redis and our app: dokku redis:link <redis> <app>

It finished by saying something like that REDIS_URL is already present in the config. However it was not presented in a way, which would clearly express that the whole command failed (i.e., the docker link was not created).

Since I was aware of the fact that I have entered a correct REDIS_URL manually, I did not consider the message as suspicious and did not check the link manually. Was very surprised to see connection failures in the app logs afterwards...

I suggest either a better explanation of redis:link result (the link was NOT created) or successful creation of link if already present REDIS_URL is correct.

crutch avatar Feb 25 '17 08:02 crutch

Do you know what the exact output was, as well as what the output of dokku config APP | grep REDIS was?

josegonzalez avatar Feb 25 '17 09:02 josegonzalez

$ dokku config foo
=====> foo config vars
REDIS_URL: redis://bar:36f25a834f8303b3e82445e9649860fd7b1a81d20585079d4c46a38e25a74bb2@dokku-redis-bar:6379
$ dokku redis:link bar foo
Already linked as REDIS_URL
$ dokku redis:list
NAME      VERSION      STATUS   EXPOSED PORTS  LINKS
bar       redis:3.2.3  running  -              -

crutch avatar Feb 25 '17 14:02 crutch

Thats not the output i asked for. Can you run the exact dokku config command I ran?

josegonzalez avatar Feb 25 '17 22:02 josegonzalez

@josegonzalez as you wish, exact dokku config command:

$ dokku config APP | grep REDIS
App name must begin with lowercase alphanumeric character

perhaps you would allow me to to change APP for an actual name of my application

dokku config foo | grep REDIS
REDIS_URL: redis://bar:36f25a834f8303b3e82445e9649860fd7b1a81d20585079d4c46a38e25a74bb2@dokku-redis-bar:6379

Sorry, what was the point of your comment? I do not find that grep that useful in this particular scenario.

crutch avatar Feb 25 '17 22:02 crutch

ha yes alright. I was hoping that adding the link would have created something like:

DOKKU_BLUE_REDIS_URL: dsn

in the output of my command. If you link a service and already have a value there, then it'll use a different url by default, similar to what happens on heroku.

josegonzalez avatar Feb 25 '17 22:02 josegonzalez

Well, that did not happen. I created an empty app foo and empty redis service bar for this to try it out. All the outputs I posted are unfiltered.

crutch avatar Feb 25 '17 22:02 crutch

Can you run the following:

dokku --trace redis:link bar foo

and gist the output?

josegonzalez avatar Feb 25 '17 22:02 josegonzalez

Sure, here it is.

crutch avatar Feb 25 '17 22:02 crutch

Ah I see. So you created the service, manually set the config variable, and then tried to link. The link fails because we detect that there is an env var that has the service url already set, so we don't actually write anything else on our end.

What you'd like is for us to fail because it's already set but we don't have the proper setup on our end or ensure all the link stuff is set on the service's side (plus the extra docker options).

I think the way forward here is to just ensure the docker options/links are set properly if we detect the link is there.

For now, I would unset the variable and let the plugin handle the linking. I'll leave this issue open until I/someone else gets around to implementing this functionality.

josegonzalez avatar Feb 25 '17 23:02 josegonzalez

@josegonzalez yes, once I realized what was going on, I unset the config var and repeated the linking.

I see that you got my point - if the linking fails, it should fail more loudly. Or it should ensure that the docker link was created as well, apart from the config variable.

crutch avatar Feb 26 '17 00:02 crutch

Yeah. I'm marking this as an enhancement as dokku-redis is working as advertised - we say use redis:link in our docs - but it can definitely "clean up" after bad setup.

josegonzalez avatar Feb 26 '17 00:02 josegonzalez