saltstack / salt

Software to automate the management and configuration of any infrastructure or application at scale. Get access to the Salt software package repository here:
https://repo.saltproject.io/
Apache License 2.0
14.13k stars 5.47k forks source link

Postgres cluster is always created with SQL_ASCII encoding, regardless of locale #4543

Closed bkonkle closed 10 years ago

bkonkle commented 11 years ago

In our state, we're making sure to set the locale to UTF-8 before installing Postgres. If the locale is not set, Postgres defaults to SQL_ASCII, causing problems with our applications. Our state looks like this:

en_US.UTF-8:
    locale.system

postgresql:
    pkg.installed:
        - names:
            - postgresql
            - postgresql-contrib
        - require:
            - locale: "en_US.UTF-8"

Unfortunately, regardless of the locale the cluster is always generated at install with the SQL_ASCII encoding. If we drop and recreate the cluster, however, it uses the correct UTF-8 encoding.

We've tried running update-locale manually and restarting the salt-minion before running state.highstate to make sure that the process has the right environment, but it still initializes the cluster with SQL_ASCII. We've also tried adding the locale environment variables to salt-minion's upstart script. The only thing that works is installing Postgres manually outside of Salt.

This happens for us when using Salt with Rackspace, but I'm not entirely certain it's isolated to just Rackspace.

UtahDave commented 11 years ago

I believe this is happening because all salt executions set the locale to C when they're run. @s0undt3ch, do you think this is what's happening here?

johtso commented 11 years ago

@bkonkle, have you verified that the locale has definitely been modified as expected? It seems like the locale module fails silently if there is no LANG= line the the locale file. (https://github.com/saltstack/salt/issues/4551)

An alternative solution is to explicitly specify the encoding when creating the database:

postgres-db:
  postgres_database.present:
    - name: dbname
    - encoding: UTF8
    - lc_ctype: en_US.UTF8
    - lc_collate: en_US.UTF8
    - template: template0
    - owner: dbowner
    - runas: postgres
    - require:
      - postgres_user: psqluser
bkonkle commented 11 years ago

When creating a cluster outside of salt using 'root', the cluster is created successfully with UTF-8. Typing locale in the command prompt confirms that all environment variables are correctly set for UTF-8.

I tried explicitly setting the encoding, but the state failed because the template01 database in the cluster is created with SQL_ASCII encoding.

s0undt3ch commented 11 years ago

@UtahDave, yes, I think the issue is related to we resetting the locale to C. Maybe we should allow that to be optional?

danmur commented 11 years ago

A temporary workaround, if anyone is interested:

install_postgres_localehack:
  cmd.run:
    - name: LC_ALL="en_AU.UTF-8" apt-get install postgresql-9.1
    - unless: "[[ `dpkg -s postgresql-9.1 |grep Status` == *installed* ]]"

# This is here as a placeholder for other states to require, as the above
# hack shouldn't be required forever
postgresql-9.1:
  pkg:
    - installed
    - require:
      - cmd.run: install_postgres_localehack

The pkg.installed for postgresql-9.1 won't do anything as postgres is already installed, but it's a good placeholder. It should be possible to remove install_postgres_localehack once there's another way of doing it.

s0undt3ch commented 11 years ago

7394 should fix this, let me know if it did not.

bkonkle commented 11 years ago

Thank you! I'll test this out soon.

s0undt3ch commented 11 years ago

Thanks!

fivethreeo commented 10 years ago

Wrong fix, allow apt/dpkg/yum to run with a set locale same as psql.

s0undt3ch commented 10 years ago

Which is the wrong fix? The actual fix? The workaround presented here?

Got any more data for us?

fivethreeo commented 10 years ago

The issue mentioned at the top is that the locale is set to C when apt is run causing the cluster to be created as SQL_ASCII.

The referenced commit does not affect this. It only makes psql run using the default locale.

See danmur's comment.

jwmayfield commented 10 years ago

This issue is closed, but I was searching for an answer to the same issue. I finally figured out something that worked for me:

default_locale:
  cmd.run:
      - name: update-locale LANG=en_US.UTF-8

postgresql:
    pkg.installed:
        - name: postgresql-9.3
        - require:
            - cmd: default_locale

Although we are grabbing a version of Postgres from the postgresql.org apt repos, which is a difference with what the OP is doing.

FinweVI commented 10 years ago

Just had to deal with the same problem ... The hack of @danmur works fine with "-y" at the end to say yes to the apt prompt.

jsuchal commented 10 years ago

The problem is quite deep, here is what I found out.

When you run update-locale the locale is updated, but the current environment variables are not updated. If you want a quick fix you just do a slight change to @jwmayfield state.

default_locale:
  cmd.run:
    - name: update-locale LC_ALL=en_US.UTF-8 && export LC_ALL=en_US.UTF-8

If you don't want to run this every time you run state you have to use unless.

default_locale:
  cmd.run:
    - name: update-locale LC_ALL=en_US.UTF-8 && export LC_ALL=en_US.UTF-8
    - unless: test $LC_ALL = "en_US.UTF-8"

However this will not work since cmd.run is reseting system locale by default and the test always fails since LC_ALL is equal to C, see https://github.com/saltstack/salt/blob/develop/salt/modules/cmdmod.py#L521

So you need to override this setting with:

default_locale:
  cmd.run:
    - name: update-locale LC_ALL=en_US.UTF-8 && export LC_ALL=en_US.UTF-8
    - unless: test $LC_ALL = "en_US.UTF-8"
    - kwargs: { reset_system_locale: False }

However this will not work since cmd.run state ignores **kwargs. https://github.com/saltstack/salt/blob/develop/salt/states/cmd.py#L614-L617

@thatch45 do I open a new issue to expose reset_system_locale for cmd.run? It would be probably better to somehow fix http://docs.saltstack.com/en/latest/ref/states/all/salt.states.locale.html#module-salt.states.locale to update current environment.

However what you can do right now to avoid running this state every time is this:

default_locale:
  cmd.run:
    - name: update-locale LC_ALL=en_US.UTF-8 && export LC_ALL=en_US.UTF-8
    - unless: cat /etc/default/locale | grep LC_ALL=en_US.UTF-8
jarus commented 10 years ago

This problem is very annoying. It would be nice to set environment variables in the pkg.installed state.

fivethreeo commented 10 years ago

Actually it seems you can.

postgresql: pkg.installed:

On 10 April 2014 18:03, Christoph Heer notifications@github.com wrote:

This problem is very annoying. It would be nice to set environment variables in the pkg.installed state.

Reply to this email directly or view it on GitHubhttps://github.com/saltstack/salt/issues/4543#issuecomment-40104243 .

Øyvind Saltvik

bdejong commented 10 years ago

I was able to reproduce this using vagrant. I tried a few approaches and destroyed and set up the VM each time.

So... help?

fivethreeo commented 10 years ago

Did you use the latest salt from git?

Sendt fra min iPhone

Den 14. apr. 2014 kl. 11:38 skrev bdejong notifications@github.com:

I was able to reproduce this using vagrant. I tried a few approaches and destroyed and set up the VM each time.

@jwmayfield 's fix does not fix it using locale.system and requiring that does not fix it setting the locale manually for the postgres installation (as per @fivethreeo 's fix) does not fix it @jsuchal 's fix doesn't seem to work either. So... help?

— Reply to this email directly or view it on GitHub.

jsuchal commented 10 years ago

@bdejong I've setup multiple production servers (ubuntu 12.10) with my fix and all of them have en_US.UTF-8 locale, so I don't know where the problem is.

s0undt3ch commented 10 years ago

I need you to test the following, if possible.

For those attempting to do this on ubuntu systems, using latest salt develop, please make the following changes:

diff --git a/salt/modules/aptpkg.py b/salt/modules/aptpkg.py
index 7e2fa5d..658627c 100644
--- a/salt/modules/aptpkg.py
+++ b/salt/modules/aptpkg.py
@@ -443,7 +443,8 @@ def install(name=None,
         cmd.extend(targets)

     __salt__['cmd.run'](cmd, env=kwargs.get('env'), python_shell=False,
-                        output_loglevel='debug')
+                        output_loglevel='debug',
+                        reset_system_locale=kwargs.get('reset_system_locale', True))
     __context__.pop('pkg.list_pkgs', None)
     new = list_pkgs()
     return salt.utils.compare_dicts(old, new)

Update the state file to something similar to:

postgresql:
  pkg.installed:
    - name: postgresql-9.3
    - reset_system_locale: false

Restart your minion, retry, and report what happened...

bdejong commented 10 years ago

@fivethreeo The latest git release (installed via bootstrap) is failing (the requests package isn't installed), but I tried the daily one and no luck. Destroyed my VM, ran from zero, same problem, postgres is installing the templates as SQL_ASCII:

                              List of databases
   Name    |   Owner   | Encoding  | Collate | Ctype |   Access privileges
-----------+-----------+-----------+---------+-------+-----------------------
 postgres  | postgres  | SQL_ASCII | C       | C     |
 template0 | postgres  | SQL_ASCII | C       | C     | =c/postgres          +
           |           |           |         |       | postgres=CTc/postgres
 template1 | postgres  | SQL_ASCII | C       | C     | =c/postgres          +
           |           |           |         |       | postgres=CTc/postgres

@s0undt3ch I will try to see what that gives... But I'll need to figure out how to install from git AND make a change.

FYI, my current trial is running this - because many possible solutions are being proposed:

postgresql:
    pkg.installed:
        - name: postgresql-9.1
        - env:
            - LC_ALL: en_US.UTF8

if I get it correct I need to apply the patch you suggest and run

postgresql:
    pkg.installed:
        - name: postgresql-9.1
        - env:
            - LC_ALL: en_US.UTF8
            - reset_system_locale: false

Or do I need to add more stuff like the dependency on for example

en_US.UTF-8:
  locale.system

Afaic the easiest fix -by far- would be to have this last state working AND change the current locale. As far as I understand @jsuchal 's opinion this state fixes the system locale, but this has no influence on what salt is using to run apt-get.

FYI, the locale of my base system (i.e. precise64 on vagrant) has its locale set to en_US (which is of course the basis of the problem). I suppose that if I fix the locale first and then run salt everything would be fine. But fixing it manually kind of defeats the purpose.

bdejong commented 10 years ago

So, just for sanity sake I did the following:

  1. created a new vagrant box from zero
  2. logged in and changed the locale manually to en_US.utf-8 (previously en_US)
  3. sudo apt-get install postgresql-9.1

The resulting postgres tables:

postgres=# \list
                                  List of databases
   Name    |  Owner   | Encoding |   Collate   |    Ctype    |   Access privileges
-----------+----------+----------+-------------+-------------+-----------------------
 postgres  | postgres | UTF8     | en_US.UTF-8 | en_US.UTF-8 |
 template0 | postgres | UTF8     | en_US.UTF-8 | en_US.UTF-8 | =c/postgres          +
           |          |          |             |             | postgres=CTc/postgres
 template1 | postgres | UTF8     | en_US.UTF-8 | en_US.UTF-8 | =c/postgres          +
           |          |          |             |             | postgres=CTc/postgres

So it is definitely confirmed that (manually) changing the locale before installing postgres fixes the issue.

jsuchal commented 10 years ago

@bdejong The main issue as far I understand it is that updating by the locale you are not updating the locale for the current enviroment/bash. So even if you update locale, all subsequent commands (eg. apt) will use the old locale from environment. You need to source/export them manually or restart your session. Since salt commands are usually ran within one session this is the issue. Could you clarify how did you manually changed the locale in step 2?

The fix by @s0undt3ch fixes the issue that salt commands are resseting locale (that's another issue), but if the locale is not set for current environment, the installation will fail again AFAIK. Disclaimer: I have not tried the patch.

bdejong commented 10 years ago

@jsuchal I was just checking everything by hand (for sanity sake), no by using salt. So I edited the locale in /etc/default/locale, logged out, back in, installed postgres by calling apt-get install manually. I think the only thing this confirms is that you are right in your assumption.

I didn't try out @s0undt3ch 's fix either but as far as I can see it is impossible that this patch fixes the issue at hand...

What I will try now is the following

  1. change the locale by hand
  2. then run salt highstate

If salt takes the locale from the system all will be well. If not then we probably need the fix from @s0undt3ch to fix that particular problem, but this still doesn't solve the root issue we have here and that is that there is no way to set the locale before installing postgres :)

s0undt3ch commented 10 years ago

@bdejong nothing will work without my change. The issue is that salt needs to reset to a know locale so it knows how to parse the output of everything run through it. In some cases, like this one, we need to be able to tell salt not to reset the system locale, the code exists, it's only used right now when running psql. This problem, however, is happening at install time, we're not passing reset_system_locale to cmd when installing(which is the patch I showed you).

I should be able to test the fix this week, but if you happen to confirm the fix sooner, the better.

@bdejong regarding the bootstrap, you need the devel version, I'll release a stable version this week (https://raw.github.com/saltstack/salt-bootstrap/develop/bootstrap-salt.sh).

s0undt3ch commented 10 years ago

@bdejong once the patch is included, all your state needs is:

postgresql:
    pkg.installed:
        - name: postgresql-9.1
        - reset_system_locale: false

This way, the system locale will be used. If you need to change the system locale, please do that before installing PostgreSQL.

s0undt3ch commented 10 years ago

you can apply the patch to the installed salt, on ubuntu systems it should be something like /usr/lib/python2.7/dist-packages/salt/modules/aptpkg.py

bdejong commented 10 years ago

@s0undt3ch OK, understood, this solves part of the issue

Another part of the issue was (most probably) discovered by @jsuchal : there is no way to reset the system locale so that it affects the current locale by using

en_US.UTF-8:
  locale.system

As this (we assume) probably only changes the locale for the next session, but not the current one. Does this make sense? I am testing this assumption right now...

jsuchal commented 10 years ago

@bdejong Exactly my point. Not sure if this behavior changed since last release though.

s0undt3ch commented 10 years ago

@bkonkle it makes sense, though, since we're shelling out, we should get a new session for every command. If it does not fix the issue, totally, let me know.

s0undt3ch commented 10 years ago

@jsuchal how are you testing that the locale has not changed once updated?

bdejong commented 10 years ago

I'm testing this right now... I will apply your patch and then use:

en_US.UTF-8:
    locale.system

postgresql:
    pkg.installed:
        - name: postgresql-9.1
        - require:
            locale: en_US.UTF-8
        - reset_system_locale: false

Make sense?

jsuchal commented 10 years ago

@s0undt3ch I was running dummy commands through salt that echoed current locale...

s0undt3ch commented 10 years ago

@jsuchal remember the part that I said that salt resets the locale when shelling out unless told not to? :smile:

s0undt3ch commented 10 years ago

@bdejong makes sense, but thinking of it, @jsuchal might not be totally off, he's probably right since salt would need to be restarted to pickup the new locale....

bdejong commented 10 years ago

I think @jsuchal is right. Did the test with the patched file (I patched 2014.1.1, not git) and I got this:

postgres=# \list
                             List of databases
   Name    |   Owner   | Encoding | Collate | Ctype |   Access privileges
-----------+-----------+----------+---------+-------+-----------------------
 postgres  | postgres  | LATIN1   | en_US   | en_US |
 template0 | postgres  | LATIN1   | en_US   | en_US | =c/postgres          +
           |           |          |         |       | postgres=CTc/postgres
 template1 | postgres  | LATIN1   | en_US   | en_US | =c/postgres          +
           |           |          |         |       | postgres=CTc/postgres

Which I suppose makes sense as my original locale was en_US and the postgress install is picking up on that. So, um, now salt needs two patches? :D

jsuchal commented 10 years ago

@bdejong i think you are still missing the reset_system_locale: false part there, right?

en_US.UTF-8:
    locale.system

postgresql:
    pkg.installed:
        - name: postgresql-9.1
        - reset_system_locale: false
        - require:
            locale: en_US.UTF-8
jsuchal commented 10 years ago

@s0undt3ch I hardly remember what exactly I've tried two weeks ago, but at some point I got the right locale from a command ran by salt. Sorry, I just can't remember now.

s0undt3ch commented 10 years ago

So, it seems that salt does not need to be restarted....

root@pedro-test-develop ~]# locale
LANG=C
LC_CTYPE="C"
LC_NUMERIC="C"
LC_TIME="C"
LC_COLLATE="C"
LC_MONETARY="C"
LC_MESSAGES="C"
LC_PAPER="C"
LC_NAME="C"
LC_ADDRESS="C"
LC_TELEPHONE="C"
LC_MEASUREMENT="C"
LC_IDENTIFICATION="C"
LC_ALL=
[root@pedro-test-develop ~]# salt-call -l quiet locale.get_locale                                                                                                                                                                              
local:
    pt_PT.UTF-8
[root@pedro-test-develop ~]# salt-call state.single locale.system name=en_US.UTF-8                                                                                                                                                             
[INFO    ] The `dmidecode` binary is not available on the system. GPU grains will not be available.
[INFO    ] The `dmidecode` binary is not available on the system. GPU grains will not be available.
[INFO    ] Loading fresh modules for state activity
[INFO    ] Running state [en_US.UTF-8] at time 14:53:14.741627
[INFO    ] Executing state locale.system for en_US.UTF-8
[INFO    ] Executing command 'localectl' in directory '/root'
[INFO    ] output:    System Locale: LANG=pt_PT.UTF-8
       VC Keymap: n/a
      X11 Layout: n/a
[INFO    ] Executing command 'localectl' in directory '/root'
[INFO    ] output:    System Locale: LANG=pt_PT.UTF-8
       VC Keymap: n/a
      X11 Layout: n/a
[INFO    ] Executing command 'localectl set-locale LANG="en_US.UTF-8"' in directory '/root'
[INFO    ] output: 
[INFO    ] {'locale': 'en_US.UTF-8'}
[INFO    ] Completed state [en_US.UTF-8] at time 14:53:14.764969
local:
----------
          ID: en_US.UTF-8
    Function: locale.system
      Result: True
     Comment: Set system locale en_US.UTF-8
     Changes:   
              ----------
              locale:
                  en_US.UTF-8

Summary
------------
Succeeded: 1
Failed:    0
------------
Total:     1
[root@pedro-test-develop ~]# locale
LANG=C
LC_CTYPE="C"
LC_NUMERIC="C"
LC_TIME="C"
LC_COLLATE="C"
LC_MONETARY="C"
LC_MESSAGES="C"
LC_PAPER="C"
LC_NAME="C"
LC_ADDRESS="C"
LC_TELEPHONE="C"
LC_MEASUREMENT="C"
LC_IDENTIFICATION="C"
LC_ALL=

The last C output is from the current shell which needs a new session. Salt starts a new session for it's commands, so, the patch should work...

bdejong commented 10 years ago

@jsuchal no it was there! @s0undt3ch weird because...

Remember: my system has locale "en_US"

Should I try again with salt from git?! Or would this be because you run two separate single calls and I'm running a salt config file?!

Again I'm running this:

en_US.UTF-8:
    locale.system

postgresql:
    pkg.installed:
        - name: postgresql-9.1
        - reset_system_locale: false
        - require:
            - locale: en_US.UTF-8
s0undt3ch commented 10 years ago

To bootstrap a patched minion, try the following(might not work):

curl -L https://raw.github.com/saltstack/salt-bootstrap/develop/bootstrap-salt.sh -o install_salt.sh
sudo sh install_salt.sh -g https://github.com/s0undt3ch/salt.git git c07d3bd344f0c9b13f038c1118af25faefc34f1a
jsuchal commented 10 years ago

@s0undt3ch Not sure but, does salt also call each command with new session if you run state.highstate?

s0undt3ch commented 10 years ago

Ok, I need to run this myself so I don't send you on wild goose chases.... I'll try it tomorrow.

bdejong commented 10 years ago

@s0undt3ch I tried, with dev, with your patch, same result:

----------
          ID: en_US.UTF-8
    Function: locale.system
      Result: True
     Comment: Set system locale en_US.UTF-8
     Changes:
              ----------
              locale:
                  en_US.UTF-8
----------
          ID: postgresql
    Function: pkg.installed
        Name: postgresql-9.1
      Result: True
     Comment: The following packages were installed/updated: postgresql-9.1.
[snip]

and postgres:

postgres=# \list
                             List of databases
   Name    |  Owner   | Encoding | Collate | Ctype |   Access privileges
-----------+----------+----------+---------+-------+-----------------------
 postgres  | postgres | LATIN1   | en_US   | en_US |
 template0 | postgres | LATIN1   | en_US   | en_US | =c/postgres          +
           |          |          |         |       | postgres=CTc/postgres
 template1 | postgres | LATIN1   | en_US   | en_US | =c/postgres          +
           |          |          |         |       | postgres=CTc/postgres

So I'm almost 100% sure that @jsuchal is correct. I will try with a test-state as well instead of installing postgres to test.

jsuchal commented 10 years ago

@bdejong I confirm this. Running a state.highstate ends up with LATIN1. However passing LC_ALL to env works. @fivethreeo was right, but missed a dash in .UTF-8.

en_US.UTF-8:
  locale.system

postgresql:
  pkg.installed:
    - name: postgresql-9.1
    - reset_system_locale: false
    - env:
      - LC_ALL: en_US.UTF-8
    - require:
      - locale: en_US.UTF-8

This also suggests that locale is not changed for current session and that salt is using stale environment vars.

bdejong commented 10 years ago

@jsuchal so then the minimal path to success would be

  1. install the patched version of salt with @s0undt3ch 's patch
  2. use this state:
postgresql:
  pkg.installed:
    - name: postgresql-9.1
    - reset_system_locale: false
    - env:
      - LC_ALL: en_US.UTF-8

?

jsuchal commented 10 years ago

However you must run it with the patch by @s0undt3ch because you get

postgres=# \l
                             List of databases
   Name    |  Owner   | Encoding  | Collate | Ctype |   Access privileges   
-----------+----------+-----------+---------+-------+-----------------------
 postgres  | postgres | SQL_ASCII | C       | C     | 
 template0 | postgres | SQL_ASCII | C       | C     | =c/postgres          +
           |          |           |         |       | postgres=CTc/postgres
 template1 | postgres | SQL_ASCII | C       | C     | =c/postgres          +
           |          |           |         |       | postgres=CTc/postgres
(3 rows)

So the reset_system_locale fix is good, but does not solve the updating-locale-does-not-update-current-session-env bug/feature.

bdejong commented 10 years ago

@jsuchal OK, I just confirmed on my system! This works, but like you say, with the patch by @s0undt3ch

For people coming for this ticket for a solution how to get postgres installed with utf-8 enabled templates on a non-utf-8-locale system, these are the steps involved:

  1. make sure you are running a version of salt with this patch included
  2. use this state to get postgres installed with utf-8 enabled template databases:
postgresql:
    pkg.installed:
        - name: postgresql-9.1
        - env:
            - LC_ALL: en_US.UTF-8
        - reset_system_locale: false

( maybe it would be good to submit a different bug report regarding the updating-locale-does-not-update-current-session-env bug?)

jsuchal commented 10 years ago

:+1:

fivethreeo commented 10 years ago

But according to this https://github.com/s0undt3ch/salt/blob/develop/salt/modules/cmdmod.py#L355line, LC_ALL should be set to what env says anyway.

On 14 April 2014 18:14, Jano Suchal notifications@github.com wrote:

[image: :+1:]

— Reply to this email directly or view it on GitHubhttps://github.com/saltstack/salt/issues/4543#issuecomment-40385546 .

Øyvind Saltvik