Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Setup from scratch needs two runs #157

Closed
cboltz opened this issue Dec 26, 2016 · 8 comments · Fixed by #163
Closed

Setup from scratch needs two runs #157

cboltz opened this issue Dec 26, 2016 · 8 comments · Fixed by #163

Comments

@cboltz
Copy link
Contributor

cboltz commented Dec 26, 2016

When setting up MySQL from scratch (without a pre-existing /var/lib/mysql/), I get errors like

          ID: mysql_delete_anonymous_user_localhost.localdomain
    Function: mysql_user.absent
        Name:
      Result: False
     Comment: MySQL Error 1045: Access denied for user 'salt'@'localhost' (using password: YES)

This happens because mysql_delete_anonymous_user_* runs before the salt_user gets created.

Additionally, mysql.salt-user isn't included in init.sls, which means by default salt_user doesn't get created at all.

Proof of concept-patch:

--- a/mysql/init.sls
+++ b/mysql/init.sls
@@ -11,6 +11,7 @@
 
 include:
   - mysql.server
+  - mysql.salt-user
   - mysql.database
   - mysql.user
 {% if mysql_dev %}

--- a/mysql/server.sls
+++ b/mysql/server.sls
@@ -52,6 +52,7 @@ mysql_delete_anonymous_user_{{ host }}:
     {% endif %}
     - connection_charset: utf8
     - require:
+      - mysql_root_password
       - service: mysqld
       - pkg: mysql_python
       {%- if (mysql_salt_user == mysql_root_user) and mysql_root_password %}

Note that this patch is not perfect - requiring mysql_root_password delays mysql_delete_anonymous_user_* enough to get it working. However it uses the mysql_salt_user, not root - so the dependency on mysql_root_password is technically wrong ;-)

(CC @ccboltz)

@aboe76
Copy link
Member

aboe76 commented Feb 5, 2017

@cboltz did you test the latest version of this formula?
it works on opensuse Leap. without issues.

@cboltz
Copy link
Contributor Author

cboltz commented Mar 5, 2017

not if you define a salt_user ;-) - see my mail on the openSUSE heroes ML for details.

@aboe76
Copy link
Member

aboe76 commented Mar 8, 2017

@cboltz
isn't it easier to extend the delete anonymouse stuff with
if the salt_user isn't the mysql_root_user require the salt-user sls state first

something like

servers.sls

{% for host in ['localhost', 'localhost.localdomain', salt['grains.get']('fqdn')] %}
mysql_delete_anonymous_user_{{ host }}:
  mysql_user:
    - absent
    - host: {{ host or "''" }}
    - name: ''
    - connection_host: '{{ mysql_host }}'
    - connection_user: '{{ mysql_salt_user }}'
    {% if mysql_salt_password %}
    - connection_pass: '{{ mysql_salt_password }}'
    {% endif %}
    - connection_charset: utf8
    - require:
      - service: mysqld
      - pkg: mysql_python
      {%- if (mysql_salt_user == mysql_root_user) and mysql_root_password %}
      - cmd: mysql_root_password
      {%- endif %}
      {%- if (mysql_salt_user != mysql_root_user)
      - sls: salt-user
      {%- endif %}
{% endfor %}
{% endif %}
{% endif %}

@cboltz
Copy link
Contributor Author

cboltz commented Mar 8, 2017

I'll take everything that works ;-) - and if it's better than my patch/pull request, I won't object.

@aboe76
Copy link
Member

aboe76 commented Mar 8, 2017

I haven't tested it I have tested the default mysql-formula on opensuse tumble and leap without your problems, that's why I was asking if you were running from the latest formula.

@aboe76
Copy link
Member

aboe76 commented Mar 9, 2017

@cboltz could you test it in your environment?

@cboltz
Copy link
Contributor Author

cboltz commented Mar 9, 2017

Just to be sure - your proposal means to add (near the end of the section you quoted, around line 60 in server.sls)

      {%- if (mysql_salt_user != mysql_root_user)
      - sls: salt-user
      {%- endif %}

Right?

If I change this to - sls: mysql.salt-user (note the added mysql.) and also add mysql.salt-user to init.sls (as in my initial patch), it works :-)

@aboe76
Copy link
Member

aboe76 commented Mar 9, 2017

Yes, that's the idea, except I'm not happy to hear that the mysql.salt-user state should be added by default to the init.sls maybe a similar wrap as the mysql.dev state could be achieved here.

{% set mysql_salt_user = salt['pillar.get']('mysql:salt_user:salt_user_name', False) %}
include:
{% if mysql_salt_user %}
  - mysql.salt-user
{% endif %}

aboe76 pushed a commit that referenced this issue Mar 9, 2017
This fixes #157

* Avoid failures on first run with a salt_user defined

Add a dependency on sls: mysql.salt-user to ensure MySQL users
(especially the salt_user) get created early enough - without that,
parts of the first run will fail (but a second run works).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants