Get working with k8s.core 2 and above #34

snecklifter · 2021-09-15T18:49:56Z

SUMMARY

Drop Python 2 support
Kubernetes Core compatibility
Some OpenShift -> Kubernetes renaming

Fixes #33

ISSUE TYPE

Feature Pull Request

codecov · 2021-09-15T18:51:36Z

Codecov Report

Merging #34 (0c28bea) into main (3414a94) will decrease coverage by 24.84%.
The diff coverage is n/a.

@@             Coverage Diff             @@
##             main      #34       +/-   ##
===========================================
- Coverage   62.31%   37.47%   -24.85%     
===========================================
  Files          11        5        -6     
  Lines         820      499      -321     
  Branches      146       85       -61     
===========================================
- Hits          511      187      -324     
- Misses        253      299       +46     
+ Partials       56       13       -43

Impacted Files	Coverage Δ
...irt/tests/unit/plugins/modules/test_kubevirt_vm.py	`7.14% <0.00%> (-92.86%)`	⬇️
...irt/tests/unit/plugins/modules/test_kubevirt_rs.py	`9.52% <0.00%> (-90.48%)`	⬇️
...ommunity/kubevirt/plugins/module_utils/kubevirt.py	`27.69% <0.00%> (-25.69%)`	⬇️
...t/tests/unit/plugins/module_utils/test_kubevirt.py	`100.00% <0.00%> (ø)`
...tions/community/kubevirt/tests/unit/compat/mock.py
.../community/kubevirt/plugins/modules/kubevirt_rs.py
...unity/kubevirt/tests/unit/plugins/modules/utils.py
.../community/kubevirt/plugins/modules/kubevirt_vm.py
...s/community/kubevirt/tests/unit/compat/unittest.py
...rt/tests/unit/plugins/modules/kubevirt_fixtures.py
... and 1 more

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 3414a94...0c28bea. Read the comment docs.

snecklifter · 2021-09-15T18:53:32Z

@Andersson007 @mnecas this is what I've managed so far. The unit tests for 2.9 are failing and some are being skipped for the rest. Would appreciate some eyes as this is obviously a large breaking change and working with python isn't my day job. But its a start... :)

Andersson007 · 2021-09-16T06:16:16Z

@snecklifter thanks for the PR!
~~I see units against 2.9 failed~~

Andersson007 · 2021-09-16T06:20:59Z

also this definitely requires manual testing

Andersson007 · 2021-09-16T06:39:21Z

Several important things:

When the PR is ready for merge, i would announce the changes in a separate dedicated PR (let's call it the announcement PR) that will add only a similar changelog fragment announcing the breaking changes that will be introduced in 2.0.0, etc.
We should merge the announcement PR ASAP and release the fragment with the next minor / patch release.
We should have a porting guide as well.
IMO, we should plan 2.0.0 release right after Ansible 5.0.0 (IIRC in November) because
a) users should be informed some time before, have porting guide ready, etc.
b) If we release 2.0.0 after Ansible 5.0.0, 2.0.0 will be included in Ansible 6.0.0 but will be available for manual installation from Galaxy.
It will allow to mitigate the move and avoid mass production failings and anger (as well as stress for us).
We should merge this PR right before 2.0.0 release.
Would be a great thing releasing an alpha version first and waiting for feedback from users (we can announce this in Bullhorn, IRC, etc.). See community.dns release history as an example. It will prevent inclusion of the release in Ansible and accident installations via ansilbe-galaxy --latest but will allow interested users to install and test the version through specifying that alpha version directly.

changelogs/fragments/k8s-core-2-compatibility.yml

snecklifter · 2021-09-17T13:25:35Z

Several important things:

When the PR is ready for merge, i would announce the changes in a separate dedicated PR (let's call it the announcement PR) that will add only a similar changelog fragment announcing the breaking changes that will be introduced in 2.0.0, etc.

We should merge the announcement PR ASAP and release the fragment with the next minor / patch release.

We should have a porting guide as well.

IMO, we should plan 2.0.0 release right after Ansible 5.0.0 (IIRC in November) because
a) users should be informed some time before, have porting guide ready, etc.
b) If we release 2.0.0 after Ansible 5.0.0, 2.0.0 will be included in Ansible 6.0.0 but will be available for manual installation from Galaxy.
It will allow to mitigate the move and avoid mass production failings and anger (as well as stress for us).

We should merge this PR right before 2.0.0 release.

Would be a great thing releasing an alpha version first and waiting for feedback from users (we can announce this in Bullhorn, IRC, etc.). See community.dns release history as an example. It will prevent inclusion of the release in Ansible and accident installations via ansilbe-galaxy --latest but will allow interested users to install and test the version through specifying that alpha version directly.

This all makes sense to me, would just appreciate some input on the failing tests if anyone has the bandwidth.

Andersson007 · 2021-09-17T13:40:59Z

This all makes sense to me, would just appreciate some input on the failing tests if anyone has the bandwidth.

OK, i'll try to take a look on Monday

tadeboro

I did the initial review, and the code looks OK in general. However, tests still reference the openshift Python library, which is strange since requirements do not list it anymore. So I guess this is something that still needs to be updated, right?

As for the failing tests on Ansible 2.9, I think the ansible-test's import machinery has troubles with the symlinks. I am not 100% sure, but since the kubernetes.dynamic is a symlink to the kubernetes.base.dynamic, this seems to be a reasonable explanation. Fixing this could be fun since Ansible 2.9 is in security-fixes-only maintenance mode.

Do note that the same tests also fail on kubernetes.core. Not sure why its CI did not pick this, though.

All in all, I think this PR is one significant step in the right direction. Thank you, @snecklifter, for your effort. It is greatly appreciated!

plugins/module_utils/kubevirt.py

plugins/modules/kubevirt_cdi_upload.py

plugins/modules/kubevirt_preset.py

plugins/modules/kubevirt_rs.py

plugins/modules/kubevirt_template.py

plugins/modules/kubevirt_vm.py

tadeboro · 2021-09-18T22:05:19Z

tests/unit/plugins/modules/kubevirt_fixtures.py

@@ -2,15 +2,14 @@
 __metaclass__ = type

 import pytest
+import openshift.dynamic


This is a bit strange. I thought collection does not use openshift client anymore?

@tadeboro its a good point, I hit lots of errors when moving to kubernetes.dynamic so thought I'd break down into two changesets but am re-attempting to debug the original errors now.

tadeboro · 2021-09-18T22:06:08Z

tests/unit/plugins/modules/kubevirt_fixtures.py

    openshift.dynamic.Resource.search = MagicMock()
    openshift.dynamic.Resource.watch = MagicMock()


Again, do we still need openshift client?

See above, thanks for the review, much appreciated.

- Drop Python 2 support - Use kubernetes.core - Migrate from OpenShift to Kubernetes for some tests

jseguillon · 2022-03-24T10:30:33Z

Hi everyone,
As explained in #27 I'm especially concerned in keeping this collection alive since its a dependency for molecule KubeVirt driver I'm maintaining (https://github.com/jseguillon/molecule-kubevirt).

@snecklifter are you still motivated in working on this issue ?
@tadeboro same question for review ?

tadeboro · 2022-03-24T10:49:14Z

@tadeboro same question for review?

I more or less stopped using Ansible and following the things happening in the community, so it would be better to find a reviewer with more up-to-date Ansible and community knowledge.

snecklifter requested a review from tadeboro September 15, 2021 18:50

snecklifter assigned mnecas and unassigned mnecas Sep 15, 2021

snecklifter requested review from Andersson007 and mnecas September 15, 2021 18:51

snecklifter mentioned this pull request Sep 15, 2021

Enable Turbo mode #35

Open

Andersson007 reviewed Sep 16, 2021

View reviewed changes

changelogs/fragments/k8s-core-2-compatibility.yml Outdated Show resolved Hide resolved

tadeboro suggested changes Sep 18, 2021

View reviewed changes

snecklifter force-pushed the k8s-core-2 branch 12 times, most recently from 126f396 to 17ece39 Compare September 21, 2021 14:33

Get working with k8s.core 2 and above

0c28bea

- Drop Python 2 support - Use kubernetes.core - Migrate from OpenShift to Kubernetes for some tests

snecklifter force-pushed the k8s-core-2 branch from 17ece39 to 0c28bea Compare September 21, 2021 18:43

felixfontein mentioned this pull request Mar 23, 2022

Community Working Group Meeting Agenda 2022 ansible/community#645

Closed

felixfontein mentioned this pull request Mar 23, 2022

Describe how collections can be removed from the Ansible package ansible-collections/overview#201

Merged

Andersson007 mentioned this pull request Mar 24, 2022

We are looking for new maintainers and contributors #27

Closed

This was referenced Apr 24, 2022

[Vote ended on 2022-05-01] Removal of broken collection community.kubevirt from Ansible 6 ansible-community/community-topics#92

Closed

Vote in community-topics on removing community.kubevirt from Ansible 6 #43

Open

snecklifter closed this Sep 18, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Get working with k8s.core 2 and above #34

Get working with k8s.core 2 and above #34

snecklifter commented Sep 15, 2021

codecov bot commented Sep 15, 2021 •

edited

Loading

snecklifter commented Sep 15, 2021

Andersson007 commented Sep 16, 2021 •

edited

Loading

Andersson007 commented Sep 16, 2021

Andersson007 commented Sep 16, 2021

snecklifter commented Sep 17, 2021

Andersson007 commented Sep 17, 2021

tadeboro left a comment

tadeboro Sep 18, 2021

snecklifter Sep 20, 2021

tadeboro Sep 18, 2021

snecklifter Sep 20, 2021

jseguillon commented Mar 24, 2022

tadeboro commented Mar 24, 2022

		openshift.dynamic.Resource.search = MagicMock()
		openshift.dynamic.Resource.watch = MagicMock()

Get working with k8s.core 2 and above #34

Get working with k8s.core 2 and above #34

Conversation

snecklifter commented Sep 15, 2021

SUMMARY

ISSUE TYPE

codecov bot commented Sep 15, 2021 • edited Loading

Codecov Report

snecklifter commented Sep 15, 2021

Andersson007 commented Sep 16, 2021 • edited Loading

Andersson007 commented Sep 16, 2021

Andersson007 commented Sep 16, 2021

snecklifter commented Sep 17, 2021

Andersson007 commented Sep 17, 2021

tadeboro left a comment

Choose a reason for hiding this comment

tadeboro Sep 18, 2021

Choose a reason for hiding this comment

snecklifter Sep 20, 2021

Choose a reason for hiding this comment

tadeboro Sep 18, 2021

Choose a reason for hiding this comment

snecklifter Sep 20, 2021

Choose a reason for hiding this comment

jseguillon commented Mar 24, 2022

tadeboro commented Mar 24, 2022

codecov bot commented Sep 15, 2021 •

edited

Loading

Andersson007 commented Sep 16, 2021 •

edited

Loading