fix: remove duplicate line in known_hosts when minikube deletes

cmd/minikube/cmd/delete.go

@@ -33,6 +33,7 @@
 	"github.com/docker/machine/libmachine"
 	"github.com/spf13/cobra"
 	"github.com/spf13/viper"
+	"k8s.io/client-go/util/homedir"
 	"k8s.io/klog/v2"
 	cmdcfg "k8s.io/minikube/cmd/minikube/cmd/config"
 	"k8s.io/minikube/pkg/drivers/kic"
@@ -53,6 +54,7 @@
 	"k8s.io/minikube/pkg/minikube/reason"
 	"k8s.io/minikube/pkg/minikube/sshagent"
 	"k8s.io/minikube/pkg/minikube/style"
+	"k8s.io/minikube/pkg/util"
 )
 
 var (
@@ -232,7 +234,15 @@
 	delCtx, cancel := context.WithTimeout(context.Background(), 5*time.Minute)
 	defer cancel()
 
+	// use a background goroutine to run this delete
+	// so that this will not block for a long time
+	deleteKnownHostsChannel := make(chan struct{})
+	go func() {
+		deleteKnownHosts()
+		deleteKnownHostsChannel <- struct{}{}
+	}()
 	if deleteAll {
+
 		deleteContainersAndVolumes(delCtx, oci.Docker)
 		deleteContainersAndVolumes(delCtx, oci.Podman)
 
@@ -244,6 +254,7 @@
 		} else {
 			out.Step(style.DeletingHost, "Successfully deleted all profiles")
 		}
+
 	} else {
 		if len(args) > 0 {
 			exit.Message(reason.Usage, "usage: minikube delete")
@@ -270,6 +281,8 @@
 			delete.PossibleLeftOvers(delCtx, cname, driver.Podman)
 		}
 	}
+	// if deleteKnownHosts hasn't finished, then wait
+	<-deleteKnownHostsChannel
 
 	// If the purge flag is set, go ahead and delete the .minikube directory.
 	if purge {
@@ -725,3 +738,51 @@
 
 	return pids, nil
 }
+
+// deleteKnownHosts reads minikube nodes' keys from the minikube home folder
+// and removes them from the known_hosts file. This is the long term solution
+// for issue https://github.com/kubernetes/minikube/issues/16868
+func deleteKnownHosts() {
+	// remove this line from known_hosts file if it exists
+	knownHosts := filepath.Join(homedir.HomeDir(), ".ssh", "known_hosts")
+	machinesDir := filepath.Join(localpath.MiniPath(), "machines")
+	fileInfo, err := os.ReadDir(machinesDir)
+	if err != nil {
+		klog.Warningf("error reading machines in minikube home: %v", err)
+		return
+	}
+	if _, err := os.Stat(knownHosts); err != nil {
+		klog.Warningf("no host files found when cleaning host files: %v", err)
+		return
+	}
+	for _, file := range fileInfo {
+		if file.IsDir() {
+			continue
+		}
+		nodeName := file.Name()
+
+		knownHostPath := filepath.Join(localpath.MiniPath(), "machines", nodeName, "known_host")
+		_, err := os.Stat(knownHostPath)
+		if errors.Is(err, fs.ErrNotExist) {
+			continue
+		}
+		if err != nil {
+			klog.Warningf("error reading %s: %v", knownHostPath, err)
+			continue
+		}
+		// if this file exists, remove this line from known_hosts
+		key, err := os.ReadFile(knownHostPath)
+		if err != nil {
+			klog.Warningf("error reading key from %s: %v", knownHostPath, err)
+			continue
+		}
+		if err := util.RemoveLineFromFile(string(key), knownHosts); err != nil {
+			klog.Warningf("failed to remove key: %v", err)
+		}
+		// and, remove the file which stores this key
+		if err := os.Remove(knownHostPath); err != nil {
+			klog.Warningf("failed to remove %s: %v", knownHostPath, err)
+		}
+	}
+
+}

cmd/minikube/cmd/docker-env.go

@@ -390,7 +390,7 @@ docker-cli install instructions: https://minikube.sigs.k8s.io/docs/tutorials/doc
 			dockerPath = ""
 		}
 
-		if dockerPath != "" {
+		if dockerPath != "" && co.Config.KubernetesConfig.ContainerRuntime == constants.Docker {
 			out, err := tryDockerConnectivity("docker", ec)
 			if err != nil { // docker might be up but been loaded with wrong certs/config
 				// to fix issues like this #8185
@@ -427,7 +427,9 @@ docker-cli install instructions: https://minikube.sigs.k8s.io/docs/tutorials/doc
 			// TODO: refactor to work with docker, temp fix to resolve regression
 			if cr == constants.Containerd {
 				// eventually, run something similar to ssh --append-known
-				appendKnownHelper(nodeName, true)
+				if err := appendKnownHelper(nodeName, true); err != nil {
+					exit.Error(reason.AppendKnownError, "failed to append keys to known_hosts", err)
+				}
 			}
 		}
 	},

cmd/minikube/cmd/ssh-host.go

@@ -27,6 +27,7 @@ import (
 	"k8s.io/minikube/pkg/minikube/config"
 	"k8s.io/minikube/pkg/minikube/driver"
 	"k8s.io/minikube/pkg/minikube/exit"
+	"k8s.io/minikube/pkg/minikube/localpath"
 	"k8s.io/minikube/pkg/minikube/machine"
 	"k8s.io/minikube/pkg/minikube/mustload"
 	"k8s.io/minikube/pkg/minikube/node"
@@ -45,11 +46,13 @@ var sshHostCmd = &cobra.Command{
 	Short: "Retrieve the ssh host key of the specified node",
 	Long:  "Retrieve the ssh host key of the specified node.",
 	Run: func(_ *cobra.Command, _ []string) {
-		appendKnownHelper(nodeName, appendKnown)
+		if err := appendKnownHelper(nodeName, appendKnown); err != nil {
+			exit.Error(reason.AppendKnownError, "failed to append keys to known_hosts", err)
+		}
 	},
 }
 
-func appendKnownHelper(nodeName string, appendKnown bool) {
+func appendKnownHelper(nodeName string, appendKnown bool) error {
 	cname := ClusterFlagValue()
 	co := mustload.Running(cname)
 	if co.CP.Host.DriverName == driver.None {
@@ -99,7 +102,7 @@ func appendKnownHelper(nodeName string, appendKnown bool) {
 		knownHosts := filepath.Join(sshDir, "known_hosts")
 
 		if sshutil.KnownHost(host, knownHosts) {
-			return
+			return nil
 		}
 
 		f, err := os.OpenFile(knownHosts, os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0600)
@@ -122,8 +125,16 @@ func appendKnownHelper(nodeName string, appendKnown bool) {
 
 		fmt.Fprintf(os.Stderr, "Host added: %s (%s)\n", knownHosts, host)
 
-		return
+		// then store it into minikube home folder
+		// so that when we execute `minikube delete`
+		// these keys can be removed properly
+		_, cc := mustload.Partial(ClusterFlagValue())
+		knownHostPath := filepath.Join(localpath.MiniPath(), "machines", config.MachineName(*cc, *n), "known_host")
+		if err := os.WriteFile(knownHostPath, []byte(keys), 0644); err != nil {
+			return fmt.Errorf("WriteFile to %s: %v", knownHostPath, err)
+		}
 	}
+	return nil
 }
 
 func init() {

pkg/minikube/reason/reason.go

@@ -158,6 +158,8 @@ var (
 	InternalSemverParse = Kind{ID: "MK_SEMVER_PARSE", ExitCode: ExProgramError}
 	// minikube was unable to daemonize the minikube process
 	DaemonizeError = Kind{ID: "MK_DAEMONIZE", ExitCode: ExProgramError}
+	// minikube was unable to add keys to known_hosts
+	AppendKnownError = Kind{ID: "MK_APPEND_KNOWN", ExitCode: ExProgramError}
 
 	// insufficient cores available for use by minikube and kubernetes
 	RsrcInsufficientCores = Kind{ID: "RSRC_INSUFFICIENT_CORES", ExitCode: ExInsufficientCores, Style: style.UnmetRequirement}

pkg/util/utils.go

@@ -17,6 +17,7 @@ limitations under the License.
 package util
 
 import (
+	"bufio"
 	"fmt"
 	"os"
 	"os/user"
@@ -163,3 +164,39 @@ func MaskProxyPasswordWithKey(v string) string {
 	}
 	return v
 }
+
+// RemoveLineFromFile removes the specified line from the given file
+func RemoveLineFromFile(knownHostLine string, filePath string) error {
+	fd, err := os.OpenFile(filePath, os.O_CREATE|os.O_RDWR, 0666)
+	if err != nil {
+		return fmt.Errorf("failed to open %s: %v", filePath, err)
+	}
+	defer fd.Close()
+
+	// read each line from known_hosts and find the line we want to delete
+	scanner := bufio.NewScanner(fd)
+	newLines := make([]string, 0)
+	for scanner.Scan() {
+		line := string(scanner.Bytes())
+		if strings.TrimSpace(line) != strings.TrimSpace(knownHostLine) {
+			// remove the line which is identical with the key
+			newLines = append(newLines, line)
+		}
+	}
+	// remove the contents and move to the head of this file
+	if err := fd.Truncate(0); err != nil {
+		return fmt.Errorf("failed to truncate %s: %v", filePath, err)
+	}
+	if _, err := fd.Seek(0, 0); err != nil {
+		return fmt.Errorf("failed to seek %s: %v", filePath, err)
+	}
+
+	// write the new content into the file
+
+	for _, line := range newLines {
+		if _, err := fmt.Fprintln(fd, line); err != nil {
+			return fmt.Errorf("failed to write to %s: %v", filePath, err)
+		}
+	}
+	return nil
+}

pkg/util/utils_test.go

@@ -19,6 +19,7 @@ package util
 import (
 	"os"
 	"os/user"
+	"strings"
 	"syscall"
 	"testing"
 
@@ -310,3 +311,35 @@ func TestMaskProxyPasswordWithKey(t *testing.T) {
 		}
 	}
 }
+
+var testFileContent = `
+127.0.0.1       localhost
+127.0.0.1       mbp13
+255.255.255.255 broadcasthost
+::1             localhost
+35.244.161.222  remove.test
+`
+
+func TestRemoveLineFromFile(t *testing.T) {
+	filePath := "./test_known_hosts"
+	// first we write the content into the test file
+	if err := os.WriteFile(filePath, []byte(testFileContent), 0666); err != nil {
+		t.Fatalf("Failed to write to test file, err=%v", err)
+	}
+	defer os.Remove(filePath)
+
+	line := "35.244.161.222  remove.test"
+	if err := RemoveLineFromFile(line, filePath); err != nil {
+		t.Fatalf("Failed to run RemoveLineFromFile, err=%v", err)
+	}
+	// now read the file again to see whether this line still exists
+
+	newContent, err := os.ReadFile(filePath)
+	if err != nil {
+		t.Fatalf("Failed to read file, err=%v", err)
+	}
+	if strings.Contains(string(newContent), line) {
+		t.Errorf("Failed to remove the line")
+	}
+
+}