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(none-driver): support Kubernetes v1.26 #74

Merged
merged 3 commits into from
Mar 21, 2023

Conversation

maxbrunet
Copy link
Contributor

@maxbrunet maxbrunet commented Feb 18, 2023

Hello there! 👋

@medyagh
Copy link
Owner

medyagh commented Mar 3, 2023

@maxbrunet I wonder is this something minikube could do by default ? like if minikube detect it is none driver and k8s 1.26 theninstall those required packages for the users?

@maxbrunet
Copy link
Contributor Author

I feels like it falls out of the scope of Minikube. Minikube with none driver installs Kubernetes itself. If it gets into dependencies, then we might say it should install Docker and so on, and what if another container runtime and/or network plugin is selected, should it be able to install them too?

Minikube depends on pre-built images for any other driver, it never worries about the dependencies. Please correct me if I am wrong, I am not a maintainer.

I have not tested, but this PR should work for older K8s version that are still supported (down to 1.24, and likely down to 1.23 or 1.22).

Copy link

@kakkoyun kakkoyun left a comment

Choose a reason for hiding this comment

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

LGTM

@kakkoyun
Copy link

@medyagh It'd be awesome to get this merge 🤗

src/none-driver.ts Outdated Show resolved Hide resolved
src/none-driver.ts Outdated Show resolved Hide resolved
src/none-driver.ts Outdated Show resolved Hide resolved
@spowelljr
Copy link
Collaborator

@maxbrunet After #106 gets merged tomorrow I'll rebase your PR and take a final look and if all is good I'll merge it.

@spowelljr spowelljr force-pushed the feat/none-dirver/kubernetes-v1.26 branch from ca0920f to 25e2551 Compare March 21, 2023 20:56
Copy link
Collaborator

@spowelljr spowelljr left a comment

Choose a reason for hiding this comment

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

Thank you very much for your patience, looks good to me

@spowelljr spowelljr merged commit 36ea757 into medyagh:master Mar 21, 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.

While running the action get the permission denied error
4 participants