jaytaylor / shipbuilder

The Open-source self-hosted Platform-as-a-Service written in Go
https://shipbuilder.gigawatt.io
Other
409 stars 50 forks source link

Config unset/remove doesn't work #6

Open brockhaywood opened 10 years ago

brockhaywood commented 10 years ago

Calling shipbuilder get:remove VARIABLE -aAPP is not properly removing the variable from the environment. For ex:

$ shipbuilder config:get POSTGRES_ENGINE -aAPP Client connecting via 'ubuntu@APP.. django_postgrespool

$ shipbuilder config:remove POSTGRES_ENGINE -aAPP Client connecting via 'ubuntu@APP.. 0:01 === Removing environment variables.. 0:01 Finished removing environment variables. 0:01 NOTICE: Redeploy deferred, changes will not be active until next deploy is triggered

Notice that no environment variables were removed. Additionally, I don't believe that this should have been deferred.

jaytaylor commented 10 years ago

I just ran a quick test and was not able to reproduce the variable removal failure, but I was able to reproduce the unrequested defer:

./shipbuilder config -aAPP | grep TZ
0:01 TZ:                                     UTC

./shipbuilder config:remove TZ -aAPP
0:01 === Removing environment variables..
0:01
0:01     Removing 'TZ'
Finished removing environment variables.
0:01 NOTICE: Redeploy deferred, changes will not be active until next deploy is triggered

./shipbuilder config -aAPP | grep TZ

I'm happy to look into the defer failure when I have time, but I can't do anything about the more urgent config removal problem unless I have a way to reproduce it.

brockhaywood commented 10 years ago

Ok, I don't have any further information except that multiple individuals have had this issue on our installation. I'll update this ticket with any other findings.

rdpfeffer commented 10 years ago

It seems like this is a downstream effect of an incorrect check against deferred. Digging into the config.json file, I'm noticing that the ENV var is in fact removed. Looking at the code, the check against the deferred flag (line 78 of cmd_config.go) seems to be checking against the empty string, which may be incorrect. This is just my best guess though given the behavior we're noticing. Now, as to why the new config is not getting picked up after redploying, that I am unsure.