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.

Sign up for GitHub

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

Jump to bottom

Incorporate new changes in version module #1177

Closed
wants to merge 2 commits into from
Closed

Incorporate new changes in version module #1177

wants to merge 2 commits into from

Conversation

Akasurde
Copy link
Member

@Akasurde Akasurde commented Jan 6, 2022

SUMMARY

Signed-off-by: Abhijeet Kasurde akasurde@redhat.com

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

changelogs/fragments/dist_version.yml
plugins/module_utils/_version.py
plugins/module_utils/version.py

@Akasurde
Incorporate new changes in version module
25317bc 
Signed-off-by: Abhijeet Kasurde <akasurde@redhat.com>
@Akasurde Akasurde mentioned this pull request Jan 6, 2022
Merged
@softwarefactory-project-zuul
Copy link

Build failed.

felixfontein
felixfontein reviewed Jan 6, 2022
View reviewed changes
plugins/module_utils/version.py Outdated Show resolved Hide resolved
plugins/module_utils/version.py Outdated Show resolved Hide resolved
@Akasurde @felixfontein
Apply suggestions from code review
eb63826 
Co-authored-by: Felix Fontein <felix@fontein.de>
@softwarefactory-project-zuul
Copy link

Build failed.

@mariolenz
Copy link
Collaborator

recheck

@softwarefactory-project-zuul
Copy link

Build failed.

@mariolenz
Copy link
Collaborator

mariolenz commented Jan 6, 2022 •
edited
Loading

For the record:

ansible-test-sanity-docker-stable-2.9 FAILURE in 12m 44s

and

ansible-test-cloud-integration-vcenter7_1esxi-python36-stable211_1_of_2 FAILURE in 51m 51s

@mariolenz
Copy link
Collaborator

recheck

@mariolenz
Copy link
Collaborator

For the record:

ansible-test-cloud-integration-vcenter7_1esxi-python36-stable211_1_of_2 FAILURE in 1h 00m 54s

ansible-test-cloud-integration-vcenter7_1esxi-python36-stable211_2_of_2 FAILURE in 52m 51s

@mariolenz
Copy link
Collaborator

recheck

@softwarefactory-project-zuul
Copy link

Build failed.

@mariolenz
Copy link
Collaborator

recheck

@softwarefactory-project-zuul
Copy link

Build failed.

@mariolenz
Copy link
Collaborator

For the record:

❌ ansible-test-cloud-integration-vcenter7_2esxi-python36-stable211 FAILURE in 21m 05s
❌ ansible-test-cloud-integration-vcenter7_1esxi-python36-stable211_1_of_2 FAILURE in 1h 04m 36s
❌ ansible-test-cloud-integration-vcenter7_1esxi-python36-stable211_2_of_2 FAILURE in 51m 06s

@mariolenz
Copy link
Collaborator

recheck

@softwarefactory-project-zuul
Copy link

Build failed.

@mariolenz
Copy link
Collaborator

mariolenz commented Jan 7, 2022 •
edited
Loading

This time only one failure:

❌ ansible-test-cloud-integration-vcenter7_1esxi-python36-stable211_1_of_2 FAILURE in 43m 25s

Looks like the vmware_guest_network integration test failed in TASK [prepare_vmware_tests : Collect the list of the existing VM] with:

"msg": "Authentication failure. code: 401, json: {\"type\":\"com.vmware.vapi.std.errors.unauthenticated\",\"value\":{\"error_type\":\"UNAUTHENTICATED\",\"messages\":[{\"args\":[],\"default_message\":\"Authentication required.\",\"id\":\"com.vmware.vapi.endpoint.method.authentication.required\"}],\"challenge\":\"Basic realm=\\\"VAPI endpoint\\\",SIGN realm=dafb4e808359ecfe915e81ea47c8e144afbc0690,service=\\\"VAPI endpoint\\\",sts=\\\"https://vcenter.test/sts/STSService/vsphere.local\\\"\"}}"

@mariolenz
Copy link
Collaborator

recheck

@softwarefactory-project-zuul
Copy link

Build failed.

@mariolenz
Copy link
Collaborator

recheck

@mariolenz
Copy link
Collaborator

ansible-test-cloud-integration-vcenter7_1esxi-python36-stable211_1_of_2 FAILURE in vmware_host_config_manager

ansible-test-cloud-integration-vcenter7_1esxi-python36-stable211_2_of_2 FAILURE in vmware_host_package_info

@mariolenz
Copy link
Collaborator

recheck

@softwarefactory-project-zuul
Copy link

Build failed.

@mariolenz
Copy link
Collaborator

ansible-test-cloud-integration-vcenter7_1esxi-python36-stable211_1_of_2 FAILURE in vmware_host_ntp_info

@mariolenz
Copy link
Collaborator

@goneri ansible/check keeps failing, but as far as I can see this is not really consistent. It looks like one time it's this integration test that fails, next time another and then a third.

Do we have a problem in the CI?

@mariolenz
Copy link
Collaborator

recheck

@softwarefactory-project-zuul
Copy link

Build failed.

@goneri
Copy link
Member

goneri commented Jan 7, 2022

Sighs, indeed. I will take a look next week. Thank you @mariolenz.

@goneri
Copy link
Member

goneri commented Jan 8, 2022

I increased the memory of the VCSA instance to 32GB, hopefully, this will improve the situation: ansible-network/windmill-config#929

@goneri
Copy link
Member

goneri commented Jan 8, 2022

recheck

@softwarefactory-project-zuul
Copy link

Build failed.

@goneri
Copy link
Member

goneri commented Jan 8, 2022

@mariolenz, all the tests passed. I will upgrade SoftwareFactory's VCSA instance to 32GB too.

goneri
goneri reviewed Jan 8, 2022
View reviewed changes
changelogs/fragments/dist_version.yml
@@ -0,0 +1,2 @@
bugfixes:
- "Various modules and plugins - use vendored version of ``distutils.version`` instead of the deprecated Python standard library ``distutils``."
Copy link
Member

Choose a reason for hiding this comment

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

Do we really need to embed this? It's pretty easy to compare two Ansible versions with a bit of Python.

mariolenz
mariolenz reviewed Jan 9, 2022
View reviewed changes
Copy link
Collaborator

@mariolenz mariolenz left a comment

Choose a reason for hiding this comment

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

Do we really need this? As far as I can see, this is based on ansible-collections/community.general#3984 which seems to fix an issue with Ansible <2.9.20 and ansible-base <2.10.8.

However, those versions are quite old. Ansible 2.9.27 and ansible-base 2.10.16 are currently available. I should think that people who want to run the latest version of this collection would also run a current version of ansible.

@felixfontein
Copy link
Contributor

If you merge something that breaks these versions which the collection claims to support, it is a breaking change and cannot be released in a new minor or patch release, but has to wait for a new major release. It doesn't matter whether these Ansible/ansible-base versions are seldomly used, or whether users of them can upgrade.

So either you revert the original PR (which broke these versions), or you fix it with this PR by vendoring that file.

@mariolenz
Copy link
Collaborator

If you merge something that breaks these versions which the collection claims to support, it is a breaking change and cannot be released in a new minor or patch release, but has to wait for a new major release. It doesn't matter whether these Ansible/ansible-base versions are seldomly used, or whether users of them can upgrade.

Good point. But #1165 will be part of the next (major) version 2.0.0 which I'm planning to prepare soon.

So either you revert the original PR (which broke these versions), or you fix it with this PR by vendoring that file.

Or, alternatively, we change the required ansible version to >=2.10.8 in version 2.0.0 of this collection.

@mariolenz mariolenz mentioned this pull request Jan 9, 2022
Closed
@felixfontein
Copy link
Contributor

Both are fine as well. (As long as you don't forget to mention it in the changelog as a breaking_change :) )

@mariolenz
Copy link
Collaborator

@Akasurde Instead of this PR, I think it would be better to change the minimal required ansible version to 2.10.8 in release 2 of this collection. Could you please have a look at PR #1182?

This was referenced Jan 11, 2022
Closed
Merged
softwarefactory-project-zuul bot pushed a commit that referenced this pull request Jan 11, 2022
@mariolenz
Change required ansible version to 2.11.0 (#1184)
10c2f11 
Change required ansible version to 2.11.0

I'd like to change the supported ansible version in order to avoid #1177. Anyway, I think it's time to get rid of ansible 2.9 and 2.10.
See #1182 for more information. But since this PR looks somehow broken, I'm trying to open a new one.
@mariolenz
Copy link
Collaborator

@Akasurde #1165 didn't make it in the stable-1.x branch. It will be part of the upcoming major release 2.0.0. But since we'll drop support for Ansible 3 and before there (#1184), we don't need any fixes or workarounds for Ansible <2.9.20 and ansible-base <2.10.8.

cc @goneri @felixfontein

@mariolenz mariolenz closed this Jan 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@goneri goneri goneri left review comments

@mariolenz mariolenz mariolenz left review comments

@felixfontein felixfontein felixfontein approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@Akasurde @mariolenz @goneri @felixfontein