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

Workflow updates #171

Merged
merged 8 commits into from
Aug 26, 2023
Merged

Workflow updates #171

merged 8 commits into from
Aug 26, 2023

Conversation

kwizart
Copy link
Contributor

@kwizart kwizart commented Jul 21, 2023

This is a workflow update needed to fix the failing distros and pre-commit checks.

Description

  • Remove debian9
  • Add debian12
  • Update ansible-lint (and others) in pre-commit to fix

Related Issue

NA

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Chore (non-breaking change that does not add functionality or fix an issue)

Checklist:

  • I have read the CONTRIBUTING document.
  • I have run the pre-merge tests locally and they pass.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

Some error where discovered by fixing this ansible-lint:

ignore-errors: Use failed_when and specify error conditions instead of using ignore_errors.
tasks/configure_root_access.yml:2 Task/Handler: configure_root_access | updating root passwords

warning[outdated-tag]: Replaced outdated tag '503' with 'no-handler', replace it to avoid future errors (warning)
tasks/mysql_databases.yml:15

no-changed-when: Commands should not change things if nothing needs doing.
tasks/setup_cluster.yml:124 Task/Handler: setup_cluster | custer bootstrap - killing lingering mysql processes to ensure mysql is stopped

no-changed-when: Commands should not change things if nothing needs doing.
tasks/setup_cluster.yml:141 Task/Handler: setup_cluster | cluster bootstrap - bootstrapping first node

Read documentation for instructions on how to ignore specific rule violations.

@kwizart kwizart mentioned this pull request Jul 21, 2023
9 tasks
@kwizart
Copy link
Contributor Author

kwizart commented Jul 24, 2023

To note: I'm not sure why the debian 12 job is failing, maybe I haven't done anything to tweak the debian base image as appropriate as I'm using debian:12 whereas previous was using mrlesmithjr/debian:11 ...

@eRadical
Copy link
Collaborator

eRadical commented Jul 24, 2023

Yes @kwizart - we need another image in https://github.com/mrlesmithjr/dockerfiles/tree/main

@kwizart
Copy link
Contributor Author

kwizart commented Jul 24, 2023

OK, so I have forced-push to use mrlesmithjr/debian:12 (and defer the fix for a later PR)

@eRadical eRadical mentioned this pull request Jul 24, 2023
9 tasks
@kwizart kwizart mentioned this pull request Jul 24, 2023
9 tasks
@eRadical
Copy link
Collaborator

Now we will wait for @mrlesmithjr to merge debian 12 in dockerfiles and then will see this complete also.

@eRadical
Copy link
Collaborator

Puzzled about the last Debian 12 fail 🤕

@eRadical
Copy link
Collaborator

I think I found the issue.
It may be because of https://peps.python.org/pep-0668/ - I'll try a fix in debian:12 image.

@eRadical
Copy link
Collaborator

Latest error for Debian 12 is:

  TASK [ansible-mariadb-galera-cluster : debian | installing pre-reqs] ***********
  fatal: [node2]: FAILED! => {"changed": false, "msg": "No package matching 'python-pymysql' is available"}
  fatal: [node1]: FAILED! => {"changed": false, "msg": "No package matching 'python-pymysql' is available"}

If @kwizart doesn't have time I'll try to have a go at it when I'm home!

@eRadical
Copy link
Collaborator

For Debian 12 we need: python3-pymysql

@eRadical
Copy link
Collaborator

@kwizart - you just need to copy vars/debian-11.yml as vars/debian-12.yml (because of the first found).

@eRadical
Copy link
Collaborator

eRadical commented Jul 26, 2023

Bummer... we're in "OS contribution rabbit whole" :)

MariaDB does not build yet for Debian 12.
We could use repo for Debian 11 until they do so, but my Debian/Ubuntu experience ends here... so I must summon @elcomtik || @mrlesmithjr ! :|

@eRadical
Copy link
Collaborator

...or we could wait a few days - https://jira.mariadb.org/projects/MDEV/versions/29013 - MariaDB 10.6.15 will be released on 2023-07-27 and most probably will also contain a build for Debian 12 (see https://jira.mariadb.org/browse/MDEV-31476).

@elcomtik
Copy link
Collaborator

I would wait until they build it for Debian 12, doing otherwise may only bring other issues.

@eRadical
Copy link
Collaborator

Bummer, the MDEV-31476 was updated: we will not have MariaDB 10.6.x for Debian 12, only MariaDB 10.11.y.

@elcomtik && @mrlesmithjr - do we upgrade to MariaDB 10.11 for all or only for Debian 12? What are your thoughts?

@elcomtik
Copy link
Collaborator

I would upgrade mariadb version for all distros. Role should be updated continuosly and we just need to inform users pin mariadb version in their deployments.

@kwizart
Copy link
Contributor Author

kwizart commented Aug 21, 2023

I've tried to set it in the molecule config for Debian 12 to avoid to bump the default (either defaults/main.yaml or vars/debian-12) as it's probably better for end-users to select the appropriate mariadb_version themselves.
(so debian-12 users will break if they don't set a higher mariadb_version).

Then it should be possible to deprecate having a default mariadb_version set at all. So it will fully defer this choice/responsibility to users.

An alternative/possible solution would be to only bump vars/debian-12.yml to a sane default.
But this move the problem, as picking 10.11 over 11.1 is hidden to users.

dependabot bot and others added 6 commits August 21, 2023 17:01
Bumps [cryptography](https://github.com/pyca/cryptography) from 39.0.1 to 41.0.0.
- [Changelog](https://github.com/pyca/cryptography/blob/main/CHANGELOG.rst)
- [Commits](pyca/cryptography@39.0.1...41.0.0)

---
updated-dependencies:
- dependency-name: cryptography
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <[email protected]>
Centos Stream 8 is another distribution
This fixes the following error from ansible-lint:

...
  File "/home/runner/.cache/pre-commit/repory1hpslb/py_env-python3.9/lib/python3.9/site-packages/ansible_compat/prerun.py", line 13, in get_cache_dir
    basename = project_dir.resolve().name.encode(encoding="utf-8")
AttributeError: 'str' object has no attribute 'resolve'

This is because ansible-lint doesn't support the new api from ansible-compat v4
see also ansible/ansible-compat#258 (comment)
@eRadical eRadical merged commit 610551c into mrlesmithjr:master Aug 26, 2023
9 checks passed
@eRadical
Copy link
Collaborator

I must say I merged even though I don't agree w/ disabling centos8.
I'll reenable it later.

@kwizart
Copy link
Contributor Author

kwizart commented Aug 27, 2023

I must say I merged even though I don't agree w/ disabling centos8. I'll reenable it later.

Sorry, but this was never raised. Why do you disagree ? I'm a fedora/EPEL contributor so I'm very interested in centos eco-system itslef. But centos8 is just EOL distro. specially over rocky8 and later. As I said centos stream is a a different thing tought.

@kwizart
Copy link
Contributor Author

kwizart commented Aug 27, 2023

I would upgrade mariadb version for all distros. Role should be updated continuosly and we just need to inform users pin mariadb version in their deployments.

I think bumping should be done at some point, but then there is a need to have a safe guard.
That way, users will not suddenly upgrade their mariadb version unexpectedly while upgrading the role.

I would go with: if early in the role the already installed mariadb version doesn't "start with" the major version set in the ansible variable and ansible_allow_upgrade_major_mariadb is False, then the role should exit early asking to set the mariadb_version.

That should prevent any surprise... (I might fill a dedicated ticket/PR for this issue)

With that said, I would probably drop the dedicated mariadb_version set in debian-12. I feel uncomfortable with this on a second tough (as I create another place where the version is set instead in the role of global location).

Btw, thanks for having merged this PR!

@eRadical
Copy link
Collaborator

I have upgraded all CentOS 8 server to CentOS 8 Stream and I haven't felt any difference - of course this is anecdotal evidence but it's the simplest path for those that were and are still on CentOS 8.

The image was already moved by @mrlesmithjr to centos8stream which basically ensures continuous compatibility for those that upgraded from CentOS 8 to CentOS Stream.

Also we should introduce CentOS 9 Stream.

@kwizart
Copy link
Contributor Author

kwizart commented Aug 28, 2023

So if there is a dedicated centos8stream and centos8stream tests, I'm fine. But having centos8 is miss-leading.

@eRadical
Copy link
Collaborator

Not necessarily since everyone knows centos8 was changed into centos8stream.

@kwizart
Copy link
Contributor Author

kwizart commented Aug 28, 2023

Unfortunately no: centos8 (Linux, aka EOL) usage on mirrors are still dramatically high.
So using the same name as previous Centos Linux is miss-leading and using centos8stream will highlight different pace compared to rocky8.

Properly naming things is not is not a side thing.

@eRadical
Copy link
Collaborator

I agree to just disagree on this :)

@kwizart kwizart mentioned this pull request Aug 30, 2023
9 tasks
cmayer89 pushed a commit to cmayer89/ansible-mariadb-galera-cluster that referenced this pull request Sep 7, 2023
* Bump cryptography from 39.0.1 to 41.0.0
* Disable debian9 && centos8 superseeded by rocky8, Centos Stream 8 is another distribution
* Bump pre-commit version to current
* [molecule] Add debian12 scenario
* Add debian-12 vars
* [ansible-lint] remove outdated tag
* [ansible-lint] Add .ansible-lint-ignore

To be fixed at a later point
* Add default mariadb version for debian-12

---------

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
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 this pull request may close these issues.

4 participants