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

Rename k8s dependency #25

Conversation

tadeboro
Copy link
Contributor

@tadeboro tadeboro commented Sep 1, 2021

The community.kubernetes collection has been renamed to kubernetes.core and this commit just makes sure the collection will not get stuck with the old name.

Do note that we had to introduce an upper bound in the requirements since the kubernetes.core broke backward compatibility with version 2.0.0.

Depends on #26

ISSUE TYPE
  • Bugfix Pull Request

@codecov
Copy link

codecov bot commented Sep 1, 2021

Codecov Report

Merging #25 (e5d078e) into main (2aa18fa) will decrease coverage by 1.33%.
The diff coverage is n/a.

❗ Current head e5d078e differs from pull request most recent head e9b159e. Consider uploading reports for the commit e9b159e to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##             main      #25      +/-   ##
==========================================
- Coverage   62.31%   60.98%   -1.34%     
==========================================
  Files          11       10       -1     
  Lines         820      733      -87     
  Branches      146      131      -15     
==========================================
- Hits          511      447      -64     
+ Misses        253      238      -15     
+ Partials       56       48       -8     
Impacted Files Coverage Δ
...s/community/kubevirt/plugins/inventory/kubevirt.py

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 2aa18fa...e9b159e. Read the comment docs.

@tadeboro tadeboro changed the title Rename kubevirt dependency Rename k8s dependency Sep 1, 2021
@Andersson007
Copy link
Contributor

@tadeboro thank you much!

CC @Akasurde

@Andersson007
Copy link
Contributor

Copy link
Contributor

@Andersson007 Andersson007 left a comment

Choose a reason for hiding this comment

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

LGTM

@Andersson007
Copy link
Contributor

<•andersson007_> Andrew Klychkov 
does anyone have any ideas why the doc fragment can't be found https://github.com/ansible-collections/community.kubevirt/pull/23 ? It exists:)
10:33 AM <github-linkbot> 
https://github.com/ansible-collections/community.kubevirt/pull/23 | open, created 2021-09-01T08:09:14Z by Andersson007: Change dependency from community.kubernetes to kubernetes.core  
10:36 AM 
→ Nukien2 joined (~Nukien2@ool-ad022598.dyn.optonline.net)
10:40 AM <•gwmngilfen-work> @gwmngilfen:ansible.im 
Morning
10:41 AM 
⇐ eramirez quit (~eramirez@49.149.131.61) Read error: Connection reset by peer
10:42 AM <felixfontein> Felix Fontein 
andersson007_: zuul's third party check doesn't care about collection dependencies, so if that doc fragment comes from a dependency, it will fail
10:42 AM 
→ eramirez joined (~eramirez@49.149.131.61)
10:42 AM <felixfontein> Felix Fontein 
andersson007_: that's why the third party check always fails in c.g for stable-1 (when we still had dependencies)
10:42 AM 
⇐ eramirez quit (~eramirez@49.149.131.61) Read error: Connection reset by peer
10:43 AM <felixfontein> Felix Fontein 
I would really add proper CI to that collection (at least sanity tests), and not just zuul third party, since that one is really basic (and very slow for what it does)

Copy link
Member

@Akasurde Akasurde left a comment

Choose a reason for hiding this comment

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

how about we make following changes -

  • community.kubevirt 1.* depends upon community.kubernetes 1.*
  • a major release like 2.0 and making it dependent on kubernetes.core 2.0

@tadeboro
Copy link
Contributor Author

tadeboro commented Sep 2, 2021

From IRC:

08:56 <tadeboro> I will wait with 25 a bit more so that people can decide if we want to keep k.c constrained or fix the community.kubevirt to work with 2.0.0.

I just wanted to make sure communty.kubevirt does work with properly restricted kubernetes.core, but as state above, if someone has time to actually fix the collection and make it work with k.c 2.x.y, even better.

@Andersson007
Copy link
Contributor

@tadeboro folks will discuss which way to follow and will put something in here soon, thank you!

The community.kubernetes collection has been renamed to kubernetes.core
and this commit just makes sure the collection will not get stuck with
the old name.
@jseguillon
Copy link

Hi there, I read there were plans to move away from community.k8s to core.k8s.

This sounds important to me since with community.k8s we're stuck on this issue and forced to keep openshift<0.11.

Could someone give me a status on this ? Thanks.

@Andersson007
Copy link
Contributor

@jseguillon hi, thanks for asking!
The collection definitely needs hands, so if you know anyone interested in becoming a maintainer, we'd be happy to discuss this.

@Andersson007
Copy link
Contributor

@jseguillon moreover, as the collection is unmaintained and is broken in Ansible 5, it's going to be removed from the package (we are developing the process now ansible-collections/overview#201).
Though it will still be available on Galaxy for manual installation.
We are looking for new maintainers who can fix and continue to update the code to keep the compatibility with k8s.core.
So if you know folks who use the collection and want to maintain it, please ask them to respond to the issue.
If there are new maintainers, the collection will not be removed from the package.

@tadeboro
Copy link
Contributor Author

tadeboro commented Sep 7, 2022

Closing since collection is dying.

@tadeboro tadeboro closed this Sep 7, 2022
@Andersson007
Copy link
Contributor

@tadeboro thanks!

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.

None yet

4 participants