-
Notifications
You must be signed in to change notification settings - Fork 89
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
[PR #452/8107530 backport][stable-2] Sync GHA workflow w/ the collection template #453
Conversation
Codecov Report
@@ Coverage Diff @@
## stable-2 #453 +/- ##
=============================================
- Coverage 75.88% 56.45% -19.43%
=============================================
Files 26 12 -14
Lines 2297 1750 -547
Branches 540 443 -97
=============================================
- Hits 1743 988 -755
- Misses 381 606 +225
+ Partials 173 156 -17
Flags with carried forward coverage won't be shown. Click here to find out more.
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
mmh, not sure if something changed in GHA, but I can't deploy the full log to see what failed. |
hm, all the integration tests fail. I backported another PR to stable-1 and stable-2 today and everything was green there.. Let's close and re-open it |
it shows in logs now:
@laurent-indermuehle @webknjaz Ideas? I don't think anything changes in CI as my backports today (they also went through these tests) were green, resp there were no errors. |
The backport is not equivalent to the original. The main branch has different matrix factor structure in the workflow. The names are different and so is the value pattern. |
@webknjaz thanks for the explanations! I'll try that asap. |
@webknjaz thanks for helping with this! |
I haven't noticed any other differences in what's being invoked. There's what seems to be failing now: TASK [test_mysql_user : assert user1 TLS requirements] *************************
task path: /root/ansible_collections/community/mysql/tests/output/.tmp/integration/test_mysql_user-35cc58f2-ÅÑŚÌβŁÈ/tests/integration/targets/test_mysql_user/tasks/tls_requirements.yml:179
fatal: [testhost]: FAILED! => {
"msg": "The conditional check ''NONE' in reqs' failed. The error was: error while evaluating conditional ('NONE' in reqs): {{(old_result is skipped | ternary(new_result, old_result)).stdout.split('REQUIRE')[1].split(separator)[0].strip() | default('NONE') }}: list object has no element 1"
} Is there something on this branch that may indirectly influence the integration tests? Could it be just flaky? |
run: pip install https://github.com/ansible/ansible/archive/${{ matrix.ansible }}.tar.gz --disable-pip-version-check | ||
|
||
- name: Set MySQL version (${{ matrix.mysql }}) | ||
run: "sed -i 's/^mysql_version:.*/mysql_version: \"${{ matrix.mysql }}\"/g' ${{ env.mysql_version_file }}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This sed
is different on the main branch. There, it doesn't wrap the version with double quotes and here it does.
-sed -i 's/^mysql_version:.*/mysql_version: "${{ matrix.mysql }}"/g' ${{ env.mysql_version_file }}
+sed -i "s/^mysql_version:.*/mysql_version: $DB_VERSION/g" ${{ env.mysql_version_file }}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For registering the "reqs" vars, the playbook needs "db_version.version.major <= 5". I feel like I have overlooked how "mysql_version" is used. I'll have a look at it today if I can. Thanks @webknjaz for your investigation!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pleas forget about my previous comment. db_version is set by setup_mysql. Nothing to do with GHA.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I need to find a way to see errors in the GHA logs. 48000 lines to parse... My browser dies in the process...
@Andersson007 how did you manage to do that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@laurent-indermuehle click the gear, view logs, i searched by "FAILED:" ignoring stuff with ignoring
or something like that, after you find the first error, it'll give you a clue how to parse the rest
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Andersson007 The issue with "View raw logs" is that all texts are now black on white and color codes are displayed.
But have you tried "View job summary"? It displays only errors. Thought, I can't see if they are ignored or not.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@laurent-indermuehle ah, cool, thanks:) i think they has introduced this since i last time tried to view errors there a long time ago - things usually pop up when testing locally. Good to know, thanks
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I need to find a way to see errors in the GHA logs. 48000 lines to parse... My browser dies in the process...
Using the built-in search on the job page helps. The UI downloads the logs in chunks. And yes, downloading a raw logs and using something like Vim that loads files by chunks would help.
I managed to see one of the many integration errors :
Despite many tests using the same task name, I could find the actual test: But, When I run the same tests (Python 3.9, MySQL 8.0.22, mysqlclient 2.0.1) I can't reproduce the issue. I'm confused. Also, the tests are runned with maximum verbosity, which produce 48k lines for each integration tests. It's a nightmare to scroll. The last errors is displayed on line 44k/48k. Any way to filter out the last 4k lines? |
@laurent-indermuehle i don't know a solution, i use search by |
@laurent-indermuehle you could use https://github.com/marketplace/actions/debugging-with-tmate to dig deeper. |
@webknjaz I need advices please. This is a nightmare to debug. Have you an idea what can differ between your composite action and the run command we used before? For instance, this was working before: - name: attempt connection with newly created user ignoring hostname
mysql_user:
name: "{{ user_name_2 }}"
password: "{{ user_password_2 }}"
host: 127.0.0.1
login_user: '{{ user_name_1 }}'
login_password: '{{ user_password_1 }}'
login_host: 127.0.0.1
login_port: '{{ mysql_primary_port }}'
ca_cert: /tmp/cert.pem
check_hostname: no
register: result
ignore_errors: yes
- assert:
that:
- result is succeeded or 'pymysql >= 0.7.11 is required' in result.msg But now it fails because result.msg contains I've reverted the file .github/workflows/ansible-test-plugins.yml to test the value with the old run comand. There, the value of result.msg is Any Idea? (edited to rename "container" into "composite action" |
There's also a traceback in the log: The full traceback is:
File "/tmp/ansible_mysql_variables_payload_5uqw2qov/ansible_mysql_variables_payload.zip/ansible_collections/community/mysql/plugins/modules/mysql_variables.py", line 210, in main
File "/tmp/ansible_mysql_variables_payload_5uqw2qov/ansible_mysql_variables_payload.zip/ansible_collections/community/mysql/plugins/module_utils/mysql.py", line 106, in mysql_connect
db_connection = mysql_driver.connect(autocommit=autocommit, **config)
File "/usr/local/lib/python3.8/dist-packages/pymysql/__init__.py", line 94, in Connect
return Connection(*args, **kwargs)
File "/usr/local/lib/python3.8/dist-packages/pymysql/connections.py", line 253, in __init__
self.ctx = self._create_ssl_ctx(ssl)
File "/usr/local/lib/python3.8/dist-packages/pymysql/connections.py", line 333, in _create_ssl_ctx
ctx = ssl.create_default_context(cafile=ca, capath=capath)
File "/usr/lib/python3.8/ssl.py", line 745, in create_default_context
context.load_verify_locations(cafile, capath, cadata) So it is coming from OTOH, it's great for the test stability to rely on a config that may be located in places outside the project directory. For reproducibility, the tests should point to a carefully constructed config generated specifically for testing. |
Hey @webknjaz thank you for taking the time to study the problem.
The runner uses ubuntu-latest which as Python 3.10 pre-installed. What do you mean by "in the context of GHA"? And how can you know the context? To me it's far from obvious if you're in a Ubuntu started by GHA or in a Ubuntu started by
I could debug $HOME value and check how
What would be the second env? Docker container launched by ansible-test?
This is exactly why I keep suffering on this issue. I can't figured out yet how everything works and don't like the fact that the tests may act differently when we change unrelated things. |
I meant the working
It'd probably be best to have a config file checked into the repository and make sure that all the tests use that config file via explicit CLI options or env vars. |
Git diff shows nothing. The PR works when tested locally. It's only on GHA that the assertions fails. Thanks to your explanation that helped me understand the GHA setup, I managed to get inside the GHA instance and run a test with --docker-terminate never. Then I entered that container and try to connect to mysql using same parameters as the test. The error message I got was different than the one seen by the tests. It's normal because the tests uses the ansible plugin and I used the mysql command. The latter said "ssl is required but the server doesn't support it" So, to me, it means 2 things:
@Andersson007 do you think I'm right?
This is what we do. Except that for GHA the variables comes from the matrix and when testing locally we have to manually adjust variables. Is there a way to merge GHA matrix with local testing? Maybe with a shell script that reads values from .github/workflow/ansible-test-plugins.yml? |
No, |
* Sync GHA workflow w/ the collection template * Drop the trailing pre-cmd semicolon * Recover missing `-e` flag of `sed` * Use relative paths for version configs * Unquote `env.connector_version_file` * Use string formatting to fix the substitution problem (cherry picked from commit 8107530)
I run that on my Workstation and want to compare variables between GHA and my machine.
6cb3b10
to
de38a96
Compare
I have the final answer! This highlight the fact that our test setup is way too complicated, at least for me. So many files responsible to choose the versions, the engine, the connectors, ... And also they are lying to us, the title says "mysql_5.7.31" but really it is "MariaDB 10.5.4" !!! I'll close this PR because 1) it is messy 2) it uses the guideline from the patchback-bot instead of the ansible guidelines. I created a new PR for this backporting #452 to stable-2 : #463 |
@laurent-indermuehle thanks for investigating and working on this! Very important discoveries! |
I don't think there's anything wrong with those guidelines. They should produce the same result. OTOH, you're right — it's best if I'll teach Patchback to link external docs as suggested in sanitizers/patchback-github-app#19. |
Sync GHA workflow w/ the collection template
Drop the trailing pre-cmd semicolon
Recover missing
-e
flag ofsed
Use relative paths for version configs
Unquote
env.connector_version_file
Use string formatting to fix the substitution problem
(cherry picked from commit 8107530)
SUMMARY
This backport-2 needed a manual merge resolution.
ISSUE TYPE
COMPONENT NAME
ADDITIONAL INFORMATION