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

mysql_replication: add basic CI tests with MySQL 5.6 #63124

Merged
merged 4 commits into from
Oct 4, 2019

Conversation

Andersson007
Copy link
Contributor

SUMMARY

mysql_replication: add basic CI tests with MySQL 5.6, master-standby configuration in one container

it's needed for testing hanged PRs, e.g. #56767

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

mysql_replication

@ansibot
Copy link
Contributor

ansibot commented Oct 4, 2019

@ansibot ansibot added affects_2.10 This issue/PR affects Ansible v2.10 community_review In order to be merged, this PR must follow the community review workflow. database Database category feature This issue/PR relates to a feature request. mysql needs_triage Needs a first human triage before being processed. support:community This issue/PR relates to code supported by the Ansible community. test This PR relates to tests. labels Oct 4, 2019
Copy link
Contributor

@bmalynovytch bmalynovytch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good job again @Andersson007 !
LGTM but maybe should you add a test that checks, after the replication is started, that what you did on master is also done on the slave (ie: creating a table in the previously added db)

@ansibot ansibot added shipit This PR is ready to be merged by Core and removed community_review In order to be merged, this PR must follow the community review workflow. needs_triage Needs a first human triage before being processed. labels Oct 4, 2019
@Andersson007
Copy link
Contributor Author

Andersson007 commented Oct 4, 2019

@bmalynovytch , thank you for reviewing and the suggestion!
I think the master log pos on the slave certainly indicates (when we check slave status) that it works correctly. Look at line 79
However i could add this check later, first of all, i'd like to solve related PRs, e.g. #56767, ASAP

@ansibot ansibot added community_review In order to be merged, this PR must follow the community review workflow. and removed shipit This PR is ready to be merged by Core labels Oct 4, 2019
@Andersson007
Copy link
Contributor Author

@bmalynovytch i added the suggested checks :) please look.

@bmalynovytch
Copy link
Contributor

I thought you'd add a SELECT to check the inserted values 🙂

LGTM
shipit

@ansibot ansibot added shipit This PR is ready to be merged by Core and removed community_review In order to be merged, this PR must follow the community review workflow. labels Oct 4, 2019
@ansibot ansibot added community_review In order to be merged, this PR must follow the community review workflow. and removed shipit This PR is ready to be merged by Core labels Oct 4, 2019
@bmalynovytch
Copy link
Contributor

Btw, I'm wondering if we shouldn't add a little timer before checking replication occurred properly, as the test could be run before replication actually happens, as it is async, or add a small retry loop, to be sure people don't get false negative tests, just because of load on the test / CI env.

@Andersson007
Copy link
Contributor Author

yep, just in case, added

@felixfontein felixfontein merged commit 66de3d4 into ansible:devel Oct 4, 2019
@Andersson007
Copy link
Contributor Author

Thank you, guys, for reviewing and merging!

@ansible ansible locked and limited conversation to collaborators Nov 13, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
affects_2.10 This issue/PR affects Ansible v2.10 community_review In order to be merged, this PR must follow the community review workflow. database Database category feature This issue/PR relates to a feature request. mysql support:community This issue/PR relates to code supported by the Ansible community. test This PR relates to tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants