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.

Sign up for GitHub

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

Jump to bottom

VM drivers: Fix images getting removed on stop/start #16655

Merged
merged 1 commit into from
Jun 13, 2023
Merged

VM drivers: Fix images getting removed on stop/start #16655

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion pkg/minikube/bootstrapper/kubeadm/kubeadm.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -929,7 +929,12 @@ func (k *Bootstrapper) UpdateCluster(cfg config.ClusterConfig) error {
}

if err := r.Preload(cfg); err != nil {
klog.Infof("preload failed, will try to load cached images: %v", err)
switch err.(type) {
case *cruntime.ErrISOFeature:
out.ErrT(style.Tip, "Existing disk is missing new features ({{.error}}). To upgrade, run 'minikube delete'", out.V{"error": err})
Copy link
Member

Choose a reason for hiding this comment

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

If deleteOnFailure flag is specified minikube should delete and recreate

Copy link
Member Author

Choose a reason for hiding this comment

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

You want that added to the message?

Copy link
Member

Choose a reason for hiding this comment

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

hm...I was thinking maybe if the user allows us to delete minikube we just do it
if --deleteOnFailure...then recreate the cluster instead of telling the user to do it..

Copy link
Member Author

Choose a reason for hiding this comment

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

I tried using --delete-on-failure, but it seems that it only applies to creating the cluster, since the cluster is already created once we get here and this is K8s logic it doesn't retry. If we want this I'd recommend it be in a separate PR since there will have to be a lot of logic added for it.

minikube start --driver qemu --delete-on-failure=true
😄  minikube v1.30.1 on Darwin 13.4 (arm64)
✨  Using the qemu2 driver based on user configuration
🌐  Automatically selected the socket_vmnet network
👍  Starting control plane node minikube in cluster minikube
🔥  Creating qemu2 VM (CPUs=2, Memory=4000MB, Disk=20000MB) ...
🐳  Preparing Kubernetes v1.27.2 on Docker 24.0.2 ...

❌  Exiting due to K8S_INSTALL_FAILED: Failed to update cluster: this error is expected

╭───────────────────────────────────────────────────────────────────────────────────────────╮
│                                                                                           │
│    😿  If the above advice does not help, please let us know:                             │
│    👉  https://github.com/kubernetes/minikube/issues/new/choose                           │
│                                                                                           │
│    Please run `minikube logs --file=logs.txt` and attach logs.txt to the GitHub issue.    │
│                                                                                           │
╰───────────────────────────────────────────────────────────────────────────────────────────╯

default:
klog.Infof("preload failed, will try to load cached images: %v", err)
}
}

if cfg.KubernetesConfig.ShouldLoadCachedImages {
Expand Down
17 changes: 0 additions & 17 deletions pkg/minikube/node/start.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -413,23 +413,6 @@ func configureRuntimes(runner cruntime.CommandRunner, cc config.ClusterConfig, k
klog.Errorf("unable to disable preinstalled bridge CNI(s): %v", err)
}

// Preload is overly invasive for bare metal, and caching is not meaningful.
// KIC handles preload elsewhere.
if driver.IsVM(cc.Driver) {
if err := cr.Preload(cc); err != nil {
switch err.(type) {
case *cruntime.ErrISOFeature:
out.ErrT(style.Tip, "Existing disk is missing new features ({{.error}}). To upgrade, run 'minikube delete'", out.V{"error": err})
default:
klog.Warningf("%s preload failed: %v, falling back to caching images", cr.Name(), err)
}

if err := machine.CacheImagesForBootstrapper(cc.KubernetesConfig.ImageRepository, cc.KubernetesConfig.KubernetesVersion, viper.GetString(cmdcfg.Bootstrapper)); err != nil {
exit.Error(reason.RuntimeCache, "Failed to cache images", err)
}
}
}

inUserNamespace := strings.Contains(cc.KubernetesConfig.FeatureGates, "KubeletInUserNamespace=true")
// for docker container runtime: ensure containerd is properly configured by calling Enable(), as docker could be bound to containerd
// it will also "soft" start containerd, but it will not disable others; docker will disable containerd if not used in the next step
Expand Down