Drupal 7 redis.cluster.enabled is now mandatory?
nathanpw opened this issue · 12 comments
This is more of a question, maybe an issue... We recently tried to update our D7 site and saw the following error.:
Error: template: drupal7/templates/job/post-upgrade-reconfigure.yaml:78:32: executing "drupal7/templates/job/post-upgrade-reconfigure.yaml" at <.Values.redis.cluster.enabled>: nil pointer evaluating interface {}.enabled
We had to specify redis.cluster.enabled to false, which we didn't have to before in our yaml.
I noticed the d7 and d9 charts seem to diverge in this.:
I didn't see any mention in the changelog about redis updates or having to specify this value now in d7... I guess I am wondering if you are aware of this, and this is correct or an issue. I appologize if this doesn't make sense, helm isn't my native language. 📦
P.S. Appreciate you reading this and all the hard work that goes into this chart.
We seem to also be running into issues with the REDIS_PASSWORD environment variable not being the same in the Drupal install container/job and in redis. Not sure if this is related... On the drush status, we get the following error. We don't see this in our d9 projects (with a database restore).
Drush command terminated abnormally due to an unrecoverable error. [error]
<h1>Additional uncaught exception thrown while handling exception.</h1><h2>Original</h2><p>RedisException: WRONGPASS invalid username-password pair or user is disabled. in Redis->auth() (line 18 of /var/www/html/sites/all/modules/contrib/redis/lib/Redis/Client/PhpRedis.php).</p><h2>Additional</h2><p>RedisException: WRONGPASS invalid username-password pair or user is disabled. in Redis->auth() (line 18 of /var/www/html/sites/all/modules/contrib/redis/lib/Redis/Client/PhpRedis.php).</p><hr />
+OK
+OK
It appears that the password set in the containers as an environment variable and what is set in the settings.php is not the same. We aren't sure how this is set, any help would be appreciated.
Hi @nathanpw is this dev for WCMS I can take a look right now? Can u MS Teams me link?
The password set in the containers as an environment variable should be the exact same as the settings.php you are correct
The logic also should be virtually identical btw the drupal and drupal 7 one so its looking like rushing the d7 backport a mistake happened :(
I'm looking into it now.
Yup your right @nathanpw something was missed in the backport to Drupal 7 related to the redis changes. All of the D9 sites won't be affected but the D7 site needs the remaining fixes. I am going through it now, sorry for the inconvenience. I forced @zachomedia to rush the merge in to help you but we should have checked it closer.
This commit came in last week from a few collaborators but was done only for D9:
So somethings were missed in the backport and once fixed all your problems should be resolved.
I also only tested the D9 version so should have spun up a d7 one.
With this done though hopefully redis will no longer both you the few sites i run with it haven't had any problems yet.
Okay so I think this is fixed i'm just going to give it a further once over but I just committed these fixes.
This would 100% explain the issues you were having and everything should be set properly.
a) You don't need to add this cluster.enabled
b) Your redis password should be inherited correctly now and not autogenerated
c) None of the D8 / D9 sites are affected this is a backport bug missing some logic
I'm pretty sure if you run the latest helm chart it should just work but if there is any issues due to the unintended bug let me know and i will resolve it for you ^_^
Ok it looks good!
Thanks @sylus, for the quick response and fix! We will look at this, this morning!
We still seem to be having issues. When I shell into the containers the $REDIS_PASSWORD environment variable now seems to match, however in the settings.php $conf['redis_client_password'] doesn't seem to be updated or match what is set in the environment variable. Not sure if this is a config on our part or still an issue in the chart...
In our d9 projects settings.php I see: $settings['redis.connection']['password'] = getenv('REDIS_PASSWORD') ?: '';
Where in our d7 project it seems hard coded.: $conf['redis_client_password'] = "aLongSecurePassword";
Should the d7 use the same getenv assignment?
helm-drupal$ grep -nr redis_client_password
drupal7/conf/settings.d7.php:747:$conf['redis_client_password'] = "{{ .Values.redis.password }}";
The hardcoded password seems to be coming from the default in .: wcms.yaml:172: password: aLongSecurePassword
Yup your right pushing the fix now. (Need to wait 5 mins before u can grab)
Zach is going to write some test coverage for this so won't happen in future, and I have a D7 environment i'm can use now to test this so doesn't happen again.
Thanks for your debugging and quickly pointing me where to look ^_^
This also makes sense since the password moved under the global field so it will be empty in yours and fallback to the default. But now getting from the environment should be ok :D
Ok @nathanpw its out now can you try and report back?
I'm committed to fixing the remaining errors for this and apologize the backport should have been tested more which we will do more in future.
The D9 always get tested since have production deployments with it.
Yeap, thanks! Will test shortly and report back.
That worked for both our d7 sites, thanks again!