search

kubernetes-sigs / kubespray

Deploy a Production Ready Kubernetes Cluster
Apache License 2.0
16.1k stars 6.46k forks source link

Provide Kubespray as an Ansible collection #9048

Closed maxgio92 closed 1 year ago

maxgio92 commented 2 years ago

What would you like to be added:

I think it could be very useful to have Kubespray as an Ansible collection. The first step could be to make it usable as a collection pullable from git, ilke so:

collections:
- name: https://github.com/maxgio92/kubespray.git
  src: https://github.com/maxgio92/kubespray.git
  type: git
  version: 2.19.1-collection

This fork tag is actually present and you can test it.

assuming a galaxy.yml file is present at the repository root, containing something like this:

namespace: kubernetes-sigs
name: kubespray
version: 2.19.1-collection
readme: README.md
authors:
- kubespray-approvers
description: Ansible Collection to deploy a Kubernetes cluster with Kubespray
license:
- GPL-2.0-or-later
license_file: ''
tags: []
dependencies: {}
repository: https://github.com/maxgio92/kubespray
documentation: https://kubespray.io/#/docs/getting-started
homepage: https://kubespray.io
issues: https://github.com/kubernetes-sigs/kubespray/issues
build_ignore: []

Why is this needed:

To manage Kubespray included as a dependencies in parent Ansible playbooks/projects.

Next:

Then the collection could also be continuosly built and published to the galaxy.

alegrey91 commented 2 years ago

Agree with the idea. This would be useful in case you want to use kubespray as a dependency of a bigger project (also in terms of management).

ptx96 commented 2 years ago

Guys, any updates on this?

theasp commented 2 years ago

I tried building a collection with @maxgio92's galaxy.yml, using my own namespace. You also need to touch roles/README.md and roles/*/README.md , as it won't import otherwise. Once that is done, I am stuck here:

Starting import: task_id=21490, artifact_id=21492 
Importing with galaxy-importer 0.4.0 
Getting doc strings via ansible-doc 
Finding content inside collection 
Loading role adduser 
Could not get role description, no role metadata found 
Linting role adduser via ansible-lint... 
Import Task "21490" failed: role name invalid format: bastion-ssh-config

So for this to work, the roles would have to be cleaned up with ansible-lint. I guess the roles need to be renamed to have _ instead of - at a minimum.

maxgio92 commented 2 years ago

Hi @theasp, thank you! 🙏🏻

Could you write down the exact commands you run, so that we can reproduce and fix step by step all the issues?

luksi1 commented 2 years ago

We're doing this in our organization and I would be willing to send in a pull request, but there would be considerable changes to the repository structure, so I'd like one of Kubespray's core developers to give the thumbs up on this one.

Currently, I can see that playbooks would need to be moved in to a playbooks folder and the kube library would need to be moved to plugins.

Let me know and I'll get hacking :)

maxgio92 commented 2 years ago

Great @luksi1! Thank you :)

I think it would be great if you'd open a PR, so that we can have also maintiners have a look

luksi1 commented 2 years ago

I've sent a pull request draft, but see already that some refactoring would be required (and I'm not sure Kubespray wants to go in this direction). I'll certainly do the necessary work, but before I get our product owner to invest some hours into this, we'll need some feedback from Kubespray's maintainers.

A simple example would be this code:

https://gitlab.com/kargo-ci/kubernetes-sigs-kubespray/-/jobs/3111396672

The roles directory, in this PR, is now at the root level, requiring installation of the Ansible collection locally, before running the tests. This means refactoring tests to run a setup with ansible-galaxy collection install ... which may, or may not be, a dependency the Kubespray maintainers want.

Personally, I think this is the "right" way to do this and this type of PR would provide a very nice feature, allowing a simpler installation and easier ability to handle Kubespray downstream in other Ansible playbooks.

Let's see what the mainainers have to say.

cristicalin commented 2 years ago

We already have support for installing ansible collections dependencies so adding the actual dependency just requires matching the proper PR commit in the requirements-<ansible_version>.yml, see the example in https://github.com/kubernetes-sigs/kubespray/blob/24632ae81beafbab9710718705d266536b1bf1ec/docs/ansible.md?plain=1#L18, we had to use this for ansible 2.9 which was recently dropped but we have no issue in using it. The CI tests already have this logic (see https://github.com/kubernetes-sigs/kubespray/blob/f184725c5f7b6ea0876463c966f384ccf4fb8de1/tests/scripts/testcases_prepare.sh#L16).

With that being said I would like to actually break kubespray apart into a few reusable independent collections instead of the giant one the proposed PR is suggesting at the moment. This would facilitate future maintenance of the project instead of just adding extra complexity.

Second observation here is that the current change breaks the upgrade path which we exercise in CI, the change would have to be introduced more gradually and some sort of wrappers should be put in place to maintain compatibility with older kubespray releases and avoid breaking current user's environments.

luksi1 commented 2 years ago

@cristicalin This PR will only allow downstream Ansible code to consume Kubespray as a collection. This, I do not believe, exists in the repository's current form.

After this PR, a downstream user would be able to do the following:

  1. Add Kubespray's repository to their requirements.yml file:
---
collections:
- name: https://github.com/kubernetes-sigs/kubespray.git
  type: git
  version: v2.20.0
  1. Install a Kubespray cluster in their environment:
---
- name: Install Kubernetes
  ansible.builtin.import_playbook: kubernetes_sigs.kubespray.cluster

This functionality does not exist in the repository's current form:

  1. The code you are referring to only installs dependencies for this repository. As far as I can see, no tests install this Kubespray repository as an Ansible collection.

  2. No galaxy.yml file exists even if you wanted to add this to repository to your requirements file. For example, if you wanted to add the latest tag to your own requirements.yml and install it - ansible-galaxy install -r requirements.yml - it would bail with:

    ERROR! Neither the collection requirement entry key 'name', nor 'source' point to a concrete resolvable collection artifact. Also 'name' is not an FQCN. A valid collection name must be in the format <namespace>.<collection>. Please make sure that the namespace and the collection name  contain characters from [a-zA-Z0-9_] only.

  3. The structure of the repository would cause havoc in downstream projects, since it's structure does not follow Ansible collection requirements. See https://docs.ansible.com/ansible/latest/dev_guide/developing_collections_structure.html

Regarding, breaking up the repository into independent collections, I do not know if breaking up the repository into independent collections has any significant user value, but you are absolutely right, that a lot of different use-cases require more abstraction than is currently provided. Different use-cases would certainly see value in being able to pick and choose functionality to meet their needs. As I see it, these types of requirements are exactly what Ansible provides with Collections. Going forward, with a few more iterations, you could be able to install different components on different servers in your own way without requiring Kubespray's inventory naming conventions and such. For instance, we could break out runtime components into roles and then allow the user to piece together their own Kubernetes deploy however they see fit. But I think we should have this conversion in a different issue if that's OK?

Regarding, your last observation, this PR is breaking the current upgrade path tests, since it installs the cluster from a different repository. This shouldn't be a problem to fix with a few conditional statements. From a user's perspective, the only change would be that instead of doing this:

ansible-playbook -i my.ini cluster.yml

A user would be required to do this to install or upgrade:

ansible-playbook -i my.ini playbooks/cluster.yml

Personally, I don't see a change in the path to a playbook as a major breaking change, but maybe there are some edge cases I haven't considered. If that's the case, let me know. As a side-note, in this PR, I have duplicated the plugin module in the directories library and modules/plugins simply because the collection will not work when kube.py is in library and the collection will not work when the library is in library. This, I chose, to be able to preserve the same upgrade path as mentioned above, as well as not breaking the current use-case of installing Kubespray directly as a playbook.

But can we, right now, come to an agreement that the end goal of this PR is needed? I chatted with @floryut over on Slack and understood that there is value in this feature, but now, I'm getting mixed signals :) Personally, I think this is very much needed. This PR would 1) follow Ansible's own movement to Collections, 2) open up the possibility to pick and choose features as roles or playbooks from the collection 3) allow users to integrate Kubespray as an upstream dependency, and 4) give a cleaner interface to Kubespray moving forward without breaking the current one.

If we can agree that the use-case that @maxgio92 gives in the desription of the issue, and even here at the top of this comment, is valuable, can we first get a working draft that passes all of the tests and then have a conversation about whether we are breaking any current functionality and if so, how problematic it is?

cristicalin commented 2 years ago

In my opinion, the current approach will add more maintenance effort on the maintainers, and there are not that many of us, to ensure both deployment methods continue working, both the new one with ansible-galaxy and the old one with running the playbook directly.

I think it makes more sense to shift wholesale to the new approach, if it turns out to be the better way to bundle the release, and design a proper way to upgrade from the old git clone and run cluster.yml approach. This means updating the documentation and providing entrypoints that consume the collection instead of calling an embedded playbook from the collection as you are suggesting above. This would cause us to change focus on how we test kubespray but at least would allow us to focus on only one proper way to test it instead of two potentially diverging ways.

k8s-triage-robot commented 1 year ago

The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

You can:

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

nlvw commented 1 year ago

Bump as this issue is important and shouldn't be closed by a bot...

/remove-lifecycle stale

luksi1 commented 1 year ago

A small update on this. I chatted with @cristicalin over a month ago, and although he was concerned that providing Kubespray as a collection would add two ways to deploy Kubespray and add more overhead and administration for the maintainers, he said he would approve this change if:

  1. No changes are made to the existing consumer contract. In other words, everyone could use Kubespray in the same way as they use it now.
  2. It was well documented

I've provided a PR, #9582, but I'm still waiting for a review.

Furthermore, personally, providing Kubespray as a collection is getting more and more acute, as using sub-modules (or forks) is less than ideal and very difficult to lifecycle manage. Additionally, helping onboard other teams and companies is becoming more and more difficult as there is no way to abstract the code base from the product: companies and teams have to download the code base and manipulate it to fulfill their needs rather than simply injecting Kubespray into their existing Ansible setup.

Feel free @nlvw to provide even more use-cases as, from personal experience, maintainers work from their personal needs and rely on users to define their own needs in issues to shape the direction of the project.

k8s-triage-robot commented 1 year ago

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:

You can:

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle rotten

michel-zimmer commented 1 year ago

Thanks for the work you are putting into #9582, @luksi. I'm following closely because this would allow us to use Kubespray with our customers, because maintaining forks or git submodules is usually not an option.

chris-mccoy commented 1 year ago

Same here @luksi1 ! I've been using kubespray as a git submodule, and to be honest, it's a total hack and doesn't integrate very well. I can't wait to see kubepray supported as a collection! Very excited.