New module: proxysql_query_rules_fast_routing

[akimrx]

SUMMARY

IF ANYONE WANTS TO FINISH IT BECAUSE OF #26 (comment), FEEL FREE TO DO IT!

New module for fast routing query rules.
ProxySQL documentation

ISSUE TYPE

• New Module Pull Request

COMPONENT NAME

proxysql_query_rules_fast_routing

ADDITIONAL INFORMATION

Closes #8

[akimrx]

I'll leave it like this for now, there is some problem in the logic with delete query rule (check mode) of one of the tests. I'll probably take a look tomorrow or later today.

[Andersson007]

@akimrx thanks for the module!
I didn't review in detail. The tests should be green, you can run them locally using docker before pushing (see integration and sanity guidelines on Ansible doc site).

[akimrx]

@Andersson007 Hi!
Yes, I'll finish the PR and tests. Now there is a lot of work, not always enough time. I think everything will be ready this week.

[akimrx]

Each "apply suggestion" triggers a test that I can't cancel. It's a pain :(
I didn't know it worked that way. This could not be avoided, only if you change it locally and do not use the github features.

[akimrx]

I was worried that the tests were running randomly, although I didn't want to, so I left to test the edits at home, I'll come back later

[codecov]

Codecov Report

Merging #26 (d2d1289) into main (beb3841) will decrease coverage by 0.02%.
The diff coverage is 69.88%.

@@            Coverage Diff             @@
##             main      #26      +/-   ##
==========================================
- Coverage   70.02%   70.00%   -0.03%     
==========================================
  Files           8        9       +1     
  Lines         994     1170     +176     
  Branches      164      195      +31     
==========================================
+ Hits          696      819     +123     
- Misses        222      261      +39     
- Partials       76       90      +14     

Impacted Files	Coverage Δ
...ugins/modules/proxysql_query_rules_fast_routing.py	69.88% <69.88%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update beb3841...d2d1289. Read the comment docs.

[akimrx]

@Andersson007

Integration tests are fixed, there are no more errors in them. As for the Sanity tests, they pass on the current versions, but for the developer version the test falls on the following:

ERROR: Found 2 ignores issue(s) which need to be resolved:
ERROR: tests/sanity/ignore-2.12.txt:1:1: Sanity test 'future-import-boilerplate' does not test path 'roles/proxysql/molecule/default/tests/test_default.py'
ERROR: tests/sanity/ignore-2.12.txt:2:1: Sanity test 'metaclass-boilerplate' does not test path 'roles/proxysql/molecule/default/tests/test_default.py'

This has nothing to do with this PR and is related to the proxysql role, but not with the module.
I have no idea how to fix this.

upd
Okay, I found a way to solve this. Previously, some tests (future-import-boilerplate, metaclass-boilerplate) for a specific file roles/proxysql/molecule/default/tests/test_default.py in the role were ignored, but for some reason this led to its fail, then I replaced the ignoring with a skip for the ansible:devel version - and it worked.

If this option is not suitable, let's think about how to fix it.

[akimrx]

@Andersson007

I have fixed some of the problems, in fact I don't have much time now, I will try to look at all the comments as soon as I get free.

I also want to note that the style of writing the code and many of its features I took from the previous modules of this collection to keep the same style, it seems reasonable when the entire collection is written in the same style :)

I definitely don't have time to refactor existing (old) modules before me. If this was a collection from scratch, I would write in a completely different style :)

P.S. recent minor edits have led to Coverage swearing about uncovered tests, even though only the formatting has changed. It looks strange.

[Andersson007]

The modules now are not idempotent.

"well, there are kinds of modules where idempotence is basically impossible (reboot, shutdown, ...) or requires other information (command, shell, ...) but for modules which have a state parameter, this should be a real exception
or only because of severe limitations (accidental or intentional) of the API that's used" (thanks @felixfontein for wording).

So if it's technically possible and there're no the mentioned limitations (i.e., to fetch the current state attributes and compare them with the desired state and update only when they don't match) , it should be done.
Otherwise, it would violate the Ansible development conventions.

About the inherited style - we should improve things whenever possible.

[Andersson007]

I definitely don't have time to refactor existing (old) modules before me.

It's not required to rewrite the old stuff.
We add new modules if they satisfy the Ansible dev conventions.

And this don't look complicated in general case:

1. Fetch the current state attributes from the system
2. You could create a dictionary
3. Create a dictionary with desired attributes
4. Compare them:

If desired_attrs_dict == current_attrs_dict:
    return changed = False
else:
    update()
    return changed = True

I can't be 100% sure about ProxySQL but speaking about other RDBMS like Postgres or MySQL it's usually not a problem to make modules idempotent.

If any questions, feel free to ask.

P.S. Don't worry about codecov failings

[markuman]

fast routing is only available in proxysql >=1.4.7.
that's long time ago. But it should be at least be mentioned in the docs.

[Andersson007]

@markuman thanks for the note!
@akimrx , is there a way we can help to finish the module?

[akimrx]

@Andersson007

Hi!

At the moment, I do not have time for open source at all, I do not know when I will be able to complete PR, because my son was born and I have a lot of tasks at my main job.

It will be cool if one of the participants completes what I started.

It seems that all that remains here is to add the correct mode for the -C flag.

[Andersson007]

@akimrx hi, thanks for the feedback! And my congratulations!:)
OK, I'll put the help_wanted label and will change the description a bit, thank you! if you once decide to finish it, it would be great!

[markuman]

My test setup

• ubuntu 20.04 docker container that runs proxysql 2.2.0

[testing]
ubuntu ansible_connection=docker

test playbook

---
- hosts: testing
  
  tasks:
    - name: change check mode
      community.proxysql.proxysql_query_rules_fast_routing:
        login_user: admin
        login_password: admin
        username: 'user_ro'
        schemaname: 'default'
        destination_hostgroup: 1
        comment: 'fast route user_ro to default schema'
        state: present
        save_to_disk: yes
        load_to_runtime: yes
      check_mode: yes
    
    - name: change
      community.proxysql.proxysql_query_rules_fast_routing:
        login_user: admin
        login_password: admin
        username: 'user_ro'
        schemaname: 'default'
        destination_hostgroup: 1
        comment: 'fast route user_ro to default schema'
        state: present
        save_to_disk: yes
        load_to_runtime: yes

    - name: no change
      community.proxysql.proxysql_query_rules_fast_routing:
        login_user: admin
        login_password: admin
        username: 'user_ro'
        schemaname: 'default'
        destination_hostgroup: 1
        comment: 'fast route user_ro to default schema'
        state: present
        save_to_disk: yes
        load_to_runtime: yes

    - name: no change check mode
      community.proxysql.proxysql_query_rules_fast_routing:
        login_user: admin
        login_password: admin
        username: 'user_ro'
        schemaname: 'default'
        destination_hostgroup: 1
        comment: 'fast route user_ro to default schema'
        state: present
        save_to_disk: yes
        load_to_runtime: yes
      check_mode: yes

results in

$ ap -i myinv.ini mytest.yml 

PLAY [testing] ******************************************************************************************************************************************************************************************************

TASK [Gathering Facts] **********************************************************************************************************************************************************************************************
[DEPRECATION WARNING]: Distribution ubuntu 20.04 on host ubuntu should use /usr/bin/python3, but is using /usr/bin/python for backward compatibility with prior Ansible releases. A future Ansible release will 
default to using the discovered platform python for this host. See https://docs.ansible.com/ansible/2.11/reference_appendices/interpreter_discovery.html for more information. This feature will be removed in 
version 2.12. Deprecation warnings can be disabled by setting deprecation_warnings=False in ansible.cfg.
ok: [ubuntu]

TASK [change check mode] ********************************************************************************************************************************************************************************************
changed: [ubuntu]

TASK [change] *******************************************************************************************************************************************************************************************************
changed: [ubuntu]

TASK [no change] ****************************************************************************************************************************************************************************************************
ok: [ubuntu]

TASK [no change check mode] *****************************************************************************************************************************************************************************************
ok: [ubuntu]

PLAY RECAP **********************************************************************************************************************************************************************************************************
ubuntu                     : ok=5    changed=2    unreachable=0    failed=0    skipped=0    rescued=0    ignored=0   

check_mode is working correctly for me

[markuman]

LGTM

[markuman]

oh wait

[markuman]

just the documentation is includes copy/paste error

[Andersson007]

@markuman will you have a chance to add more examples after this is merged and add missing_required_lib as I suggested in my review?

[Andersson007]

@akimrx thanks for the new module!
@markuman thanks for reviewing and testing!