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

feat: Add Minikube extension #2646

Closed
wants to merge 1 commit into from

Conversation

afbjorklund
Copy link
Contributor

@afbjorklund afbjorklund commented May 25, 2023

What does this PR do?

This PR adds a "minikube" extension, for creating a Kubernetes cluster - similar to the existing "kind" extension

Screenshot/screencast of this PR

podman-desktop-minikube-create

What issues does this PR fix or reference?

How to test this PR?

Create a new minikube cluster container, using the Resources and the "Create new..." button.

There should be an option to install the minikube binary, if it is not already installed on the host.

The docs can go in a separate PR, it would be similar to https://podman-desktop.io/docs/kubernetes/kind

Note for reviewer: this "minikube" extension should be compared with the existing "kind" extension

@deboer-tim
Copy link
Collaborator

I love this - but I am wondering where the bar should be for built-in extensions, and even if this is small whether it is better to start it as an optional extension in its own repo. Now that we have featured extensions/catalog it's easy enough to find and install.

@afbjorklund
Copy link
Contributor Author

Not sure if there is any major difference between kind and minikube in that sense, especially considering the code.

@deboer-tim
Copy link
Collaborator

Totally agreed; I had another sentence questioning if we should move 'out' anything else and removed it since I didn't want to overload this issue. :)

Signed-off-by: Anders F Björklund <anders.f.bjorklund@gmail.com>
@afbjorklund afbjorklund marked this pull request as ready for review May 25, 2023 19:30
@afbjorklund afbjorklund requested review from a team and benoitf as code owners May 25, 2023 19:30
@afbjorklund afbjorklund requested review from jeffmaury and cdrage and removed request for a team May 25, 2023 19:30
@benoitf
Copy link
Collaborator

benoitf commented May 26, 2023

@afbjorklund thanks for this huge PR. It looks very nice

Do you think we could find a home for the extension in a separate repository and add it to the Podman Desktop registry as a 3rd party extension ?
It means that we can have more frequent releases of the extension (which is often a huge benefit when we start a new extension)

Of course, we can support you in setting up the addition to the registry, enhancing the Podman Desktop API for missing pieces, etc.

@afbjorklund
Copy link
Contributor Author

afbjorklund commented May 26, 2023

You mean you can't support both kind and minikube ? That is unfortunate, since it will miss those updates.

i.e. every time you update the kubernetes support, someone will have to carry it forward to the external
(as Sonar so helpfully points out below, there is a lot of common code shared between kind and minikube)

I think the only real difference was if configuration was in yaml or in flags, the rest are just some variables.

 
   // now execute the command to create the cluster
   try {
-    await runCliCommand(kindCli, ['create', 'cluster', '--config', tmpFilePath], { env, logger }, token);
-    if (ingressController) {
-      logger.log('Creating ingress controller resources');
-      await setupIngressController(clusterName);
-    }
-    telemetryLogger.logUsage('createCluster', { provider, httpHostPort, httpsHostPort, ingressController });
+    await runCliCommand(
+      minikubeCli,
+      ['start', '--profile', clusterName, '--driver', driver, '--container-runtime', runtime],
+      { env, logger },
+      token,
+    );
+    telemetryLogger.logUsage('createCluster', { driver, runtime });
   } catch (error) {
     const errorMessage = error instanceof Error ? error.message : error;
     telemetryLogger.logError('createCluster', {

const API_KIND_INTERNAL_API_PORT = 6443;
const API_MINIKUBE_INTERNAL_API_PORT = 8443;

const clusterName = container.Labels['io.x-k8s.kind.cluster'];
const clusterName = container.Labels['name.minikube.sigs.k8s.io'];

@benoitf
Copy link
Collaborator

benoitf commented May 26, 2023

I think we will always have to find a trade-off between having everything included in the assembly and 3rd party extensions.

And each person will come saying: 'hey I've a k3s extension, why it's not part of the default", or "please add microK8s", "add k3d", etc. This is a fully legitimate question but this is why we've recently introduced the registry to allow 3rd party tools to be integrated easily.

We've started the process to move some built-in extensions externally and not include them by default (for example the CRC extension was part of the product but now is moved). So Kind could also be moved in the future.

About your remark about the code and duplicated code.
Each extension is independent for now (so here it comes with duplicated code), else it means we need to provide a library (to share code between 2 extensions) or we need to provide more stuff through the Podman Desktop API (like a more high level API), so shared code is in the core. A 3rd way could be to expose a shared extension (extension will call another extension to do the job)

I'm pretty sure minikube extension will add specific items related to minikube with its own set of properties so it'll have its own and people should have the choice in the Kubernetes flavor they want to run locally.

I also think it's better if extension can live closer to the supported product and follow different lifecycle.

@benoitf
Copy link
Collaborator

benoitf commented May 26, 2023

You mean you can't support both kind and minikube ? That is unfortunate, since it will miss those updates.
i.e. every time you update the kubernetes support, someone will have to carry it forward to the external

I think that Kubernetes support is independent of the extension.
We connect through the k8s API to list pods, etc. Extension is here to mostly start or connect to a local/remote k8s cluster.

So adding updates to the kubernetes support should benefit to all extensions.

@benoitf
Copy link
Collaborator

benoitf commented May 26, 2023

Also I would like to emphasis that it's not because an extension is not in this repository that for example I would not
push changes/enhancement to it as well.

@benoitf
Copy link
Collaborator

benoitf commented May 26, 2023

let's find a home for the extension, and move forward.

I don't know if we can add it to containers organization or if it's possible under the kubernetes umbrella as well

https://github.com/containers/minikube-podman-desktop-ext
https://github.com/kubernetes/minikube-podman-desktop-ext

another suggestion

@afbjorklund

This comment was marked as off-topic.

@benoitf
Copy link
Collaborator

benoitf commented May 26, 2023

yes it would be more natural if extension is on kubernetes-sig GitHub organization 👍

@afbjorklund
Copy link
Contributor Author

afbjorklund commented May 26, 2023

Since the SIG all share the "kubernetes-sigs" (or even "kubernetes") organization, sub-repos are a little problematic.

https://github.com/kubernetes/minikube

https://github.com/kubernetes-sigs/kind

It would also have to set up the CI jobs and infrastructure, and those other things, in order to build and release it.

@rhatdan
Copy link
Member

rhatdan commented May 26, 2023

I have no problem adding the repos to github.com/containers, although I think they should have a podman-desktop prefix.

gitub.com/containers/podman-desktop-minikube
gitub.com/containers/podman-desktop-kind
gitub.com/containers/podman-desktop-docker
...

@afbjorklund
Copy link
Contributor Author

I don't think this will move to kubernetes. If you don't want the extensions "in-tree", I guess new repos like suggested:

./extensions/kind -> github.com/containers/podman-desktop-extension-kind
./extensions/minikube -> github.com/containers/podman-desktop-extension-minikube

Not sure if the documentation should go to the same repository, or if it can still be a part of the "website" directory?

./website/docs/kubernetes/kind
./website/docs/kubernetes/minikube

@benoitf
Copy link
Collaborator

benoitf commented May 31, 2023

@benoitf
Copy link
Collaborator

benoitf commented May 31, 2023

@afbjorklund
Copy link
Contributor Author

My goal was to bring the minikube support of to the level of the kind support, as a part of the ongoing confusion in the project SIGs of which tool to use for learning Kubernetes...

If you already decided in only really supporting Kind and CRC, then I'm not sure if adding a third-party external extension is something anyone will use?

@benoitf
Copy link
Collaborator

benoitf commented Jun 1, 2023

@afbjorklund I still do believe it's a very important contribution and I think people deserve the choice of their favorite tool so I don't see any reason to not bring minikube.
Also as I said, it's not because an extension is not in he main repository that it has no visibility. (CRC is no longer in the main repo and the development of the extension is more efficient as it's done by people of CRC. So on minikube extension you could be the main contributor/committer as well)

@deboer-tim
Copy link
Collaborator

+1. We started development in a single repo/install just b/c that was easiest. We've passed the point where new extensions have been started in their own repo (e.g. sandbox), moved one extension out (e.g. CRC), and have tried to make it as easy as possible to install extensions from other repos. There is no intention that Kind has any special status compared to others and it could be moved too (but that should be separate issue).

@cdrage
Copy link
Contributor

cdrage commented Jun 1, 2023

Also keep in mind that we also have the "featured" list on the front of Podman Desktop and the future task of implementing / showcasing all supported extensions and links to them from within Podman Deskop. So there's still a lot of views / interest that will go towards those tools.

@afbjorklund
Copy link
Contributor Author

Once the project scaffolding is up for the new separate github repository, I will move this code (extensions/minikube) from this PR (#2646) over to it... Expecting that it might need some "link" from the main repository, once it is available.

The documentation PR (#2694) will have to stay for now, but maybe it can be "merged" with the Kind documentation by focusing on the Kubernetes parts and mentioning the small differences where they exist (ingress, image loading)

@benoitf
Copy link
Collaborator

benoitf commented Jun 2, 2023

awesome, I'll scaffold the repository soon

@benoitf
Copy link
Collaborator

benoitf commented Jun 5, 2023

@afbjorklund Hello, I think scaffold is now OK on https://github.com/containers/podman-desktop-extension-minikube

@afbjorklund
Copy link
Contributor Author

afbjorklund commented Jun 5, 2023

Needs a merge between podman-desktop/extensions/minikube/ and podman-desktop-extension-minikube/

In order to get things like .gitignore and package.json working again, just copying over the files did not work

@benoitf
Copy link
Collaborator

benoitf commented Jun 5, 2023

you can open a PR, I could amend your PR to fix conflicts/issues

@benoitf
Copy link
Collaborator

benoitf commented Jun 5, 2023

closing the PR here as code has been contributed to https://github.com/containers/podman-desktop-extension-minikube

OCI image of the extension being published on .io https://ghcr.io/containers/podman-desktop-extension-minikube

thanks @afbjorklund 🎉

image

@benoitf benoitf closed this Jun 5, 2023
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

5 participants