Merge pull request #11334 from daehyeok/close_asset

Add `Close() error` function into assets.CopyableFile

cmd/minikube/cmd/cp.go

@@ -20,10 +20,12 @@ import (
 	"github.com/pkg/errors"
 	"github.com/spf13/cobra"
 
+	"fmt"
 	"os"
 	pt "path"
 	"strings"
 
+	"k8s.io/klog/v2"
 	"k8s.io/minikube/pkg/minikube/assets"
 	"k8s.io/minikube/pkg/minikube/command"
 	"k8s.io/minikube/pkg/minikube/exit"
@@ -72,6 +74,11 @@ var cpCmd = &cobra.Command{
 			out.ErrLn("%v", errors.Wrap(err, "getting file asset"))
 			os.Exit(1)
 		}
+		defer func() {
+			if err := fa.Close(); err != nil {
+				klog.Warningf("error closing the file %s: %v", fa.GetSourcePath(), err)
+			}
+		}()
 
 		co := mustload.Running(ClusterFlagValue())
 		var runner command.Runner
@@ -85,20 +92,17 @@ var cpCmd = &cobra.Command{
 
 			h, err := machine.GetHost(co.API, *co.Config, *n)
 			if err != nil {
-				out.ErrLn("%v", errors.Wrap(err, "getting host"))
-				os.Exit(1)
+				exit.Error(reason.GuestLoadHost, "Error getting host", err)
 			}
 
 			runner, err = machine.CommandRunner(h)
 			if err != nil {
-				out.ErrLn("%v", errors.Wrap(err, "getting command runner"))
-				os.Exit(1)
+				exit.Error(reason.InternalCommandRunner, "Failed to get command runner", err)
 			}
 		}
 
 		if err = runner.Copy(fa); err != nil {
-			out.ErrLn("%v", errors.Wrap(err, "copying file"))
-			os.Exit(1)
+			exit.Error(reason.InternalCommandRunner, fmt.Sprintf("Fail to copy file %s", fa.GetSourcePath()), err)
 		}
 	},
 }

pkg/drivers/kic/kic.go

@@ -217,6 +217,12 @@ func (d *Driver) prepareSSH() error {
 	if err != nil {
 		return errors.Wrap(err, "create pubkey assetfile ")
 	}
+	defer func() {
+		if err := f.Close(); err != nil {
+			klog.Warningf("error closing the file %s: %v", f.GetSourcePath(), err)
+		}
+	}()
+
 	if err := cmder.Copy(f); err != nil {
 		return errors.Wrap(err, "copying pub key")
 	}

pkg/minikube/assets/vm_assets.go

@@ -44,6 +44,7 @@ type CopyableFile interface {
 	GetPermissions() string
 	GetModTime() (time.Time, error)
 	Seek(int64, int) (int64, error)
+	Close() error
 }
 
 // BaseAsset is the base asset class
@@ -84,6 +85,7 @@ func (b *BaseAsset) GetModTime() (time.Time, error) {
 type FileAsset struct {
 	BaseAsset
 	reader io.ReadSeeker
+	file   *os.File // Optional pointer to close file through FileAsset.Close()
 }
 
 // NewMemoryAssetTarget creates a new MemoryAsset, with target
@@ -95,11 +97,6 @@ func NewMemoryAssetTarget(d []byte, targetPath, permissions string) *MemoryAsset
 func NewFileAsset(src, targetDir, targetName, permissions string) (*FileAsset, error) {
 	klog.V(4).Infof("NewFileAsset: %s -> %s", src, path.Join(targetDir, targetName))
 
-	f, err := os.Open(src)
-	if err != nil {
-		return nil, errors.Wrap(err, "open")
-	}
-
 	info, err := os.Stat(src)
 	if err != nil {
 		return nil, errors.Wrapf(err, "stat")
@@ -109,6 +106,11 @@ func NewFileAsset(src, targetDir, targetName, permissions string) (*FileAsset, e
 		klog.Warningf("NewFileAsset: %s is an empty file!", src)
 	}
 
+	f, err := os.Open(src)
+	if err != nil {
+		return nil, errors.Wrap(err, "open")
+	}
+
 	return &FileAsset{
 		BaseAsset: BaseAsset{
 			SourcePath:  src,
@@ -117,6 +119,7 @@ func NewFileAsset(src, targetDir, targetName, permissions string) (*FileAsset, e
 			Permissions: permissions,
 		},
 		reader: io.NewSectionReader(f, 0, info.Size()),
+		file:   f,
 	}, nil
 }
 
@@ -153,6 +156,14 @@ func (f *FileAsset) Seek(offset int64, whence int) (int64, error) {
 	return f.reader.Seek(offset, whence)
 }
 
+// Close closes the opend file.
+func (f *FileAsset) Close() error {
+	if f.file == nil {
+		return nil
+	}
+	return f.file.Close()
+}
+
 // MemoryAsset is a memory-based asset
 type MemoryAsset struct {
 	BaseAsset
@@ -175,6 +186,11 @@ func (m *MemoryAsset) Seek(offset int64, whence int) (int64, error) {
 	return m.reader.Seek(offset, whence)
 }
 
+// Close implemented for CopyableFile interface. Always return nil.
+func (m *MemoryAsset) Close() error {
+	return nil
+}
+
 // NewMemoryAsset creates a new MemoryAsset
 func NewMemoryAsset(d []byte, targetDir, targetName, permissions string) *MemoryAsset {
 	return &MemoryAsset{
@@ -291,3 +307,8 @@ func (m *BinAsset) Read(p []byte) (int, error) {
 func (m *BinAsset) Seek(offset int64, whence int) (int64, error) {
 	return m.reader.Seek(offset, whence)
 }
+
+// Close implemented for CopyableFile interface. Always return nil.
+func (m *BinAsset) Close() error {
+	return nil
+}

pkg/minikube/bootstrapper/certs.go

@@ -47,18 +47,18 @@ import (
 )
 
 // SetupCerts gets the generated credentials required to talk to the APIServer.
-func SetupCerts(cmd command.Runner, k8s config.KubernetesConfig, n config.Node) ([]assets.CopyableFile, error) {
+func SetupCerts(cmd command.Runner, k8s config.KubernetesConfig, n config.Node) error {
 	localPath := localpath.Profile(k8s.ClusterName)
 	klog.Infof("Setting up %s for IP: %s\n", localPath, n.IP)
 
 	ccs, err := generateSharedCACerts()
 	if err != nil {
-		return nil, errors.Wrap(err, "shared CA certs")
+		return errors.Wrap(err, "shared CA certs")
 	}
 
 	xfer, err := generateProfileCerts(k8s, n, ccs)
 	if err != nil {
-		return nil, errors.Wrap(err, "profile certs")
+		return errors.Wrap(err, "profile certs")
 	}
 
 	xfer = append(xfer, ccs.caCert)
@@ -67,6 +67,14 @@ func SetupCerts(cmd command.Runner, k8s config.KubernetesConfig, n config.Node)
 	xfer = append(xfer, ccs.proxyKey)
 
 	copyableFiles := []assets.CopyableFile{}
+	defer func() {
+		for _, f := range copyableFiles {
+			if err := f.Close(); err != nil {
+				klog.Warningf("error closing the file %s: %v", f.GetSourcePath(), err)
+			}
+		}
+	}()
+
 	for _, p := range xfer {
 		cert := filepath.Base(p)
 		perms := "0644"
@@ -75,19 +83,19 @@ func SetupCerts(cmd command.Runner, k8s config.KubernetesConfig, n config.Node)
 		}
 		certFile, err := assets.NewFileAsset(p, vmpath.GuestKubernetesCertsDir, cert, perms)
 		if err != nil {
-			return nil, errors.Wrapf(err, "key asset %s", cert)
+			return errors.Wrapf(err, "key asset %s", cert)
 		}
 		copyableFiles = append(copyableFiles, certFile)
 	}
 
 	caCerts, err := collectCACerts()
 	if err != nil {
-		return nil, err
+		return err
 	}
 	for src, dst := range caCerts {
 		certFile, err := assets.NewFileAsset(src, path.Dir(dst), path.Base(dst), "0644")
 		if err != nil {
-			return nil, errors.Wrapf(err, "ca asset %s", src)
+			return errors.Wrapf(err, "ca asset %s", src)
 		}
 
 		copyableFiles = append(copyableFiles, certFile)
@@ -107,11 +115,11 @@ func SetupCerts(cmd command.Runner, k8s config.KubernetesConfig, n config.Node)
 	kubeCfg := api.NewConfig()
 	err = kubeconfig.PopulateFromSettings(kcs, kubeCfg)
 	if err != nil {
-		return nil, errors.Wrap(err, "populating kubeconfig")
+		return errors.Wrap(err, "populating kubeconfig")
 	}
 	data, err := runtime.Encode(latest.Codec, kubeCfg)
 	if err != nil {
-		return nil, errors.Wrap(err, "encoding kubeconfig")
+		return errors.Wrap(err, "encoding kubeconfig")
 	}
 
 	if n.ControlPlane {
@@ -121,14 +129,14 @@ func SetupCerts(cmd command.Runner, k8s config.KubernetesConfig, n config.Node)
 
 	for _, f := range copyableFiles {
 		if err := cmd.Copy(f); err != nil {
-			return nil, errors.Wrapf(err, "Copy %s", f.GetSourcePath())
+			return errors.Wrapf(err, "Copy %s", f.GetSourcePath())
 		}
 	}
 
 	if err := installCertSymlinks(cmd, caCerts); err != nil {
-		return nil, errors.Wrapf(err, "certificate symlinks")
+		return errors.Wrapf(err, "certificate symlinks")
 	}
-	return copyableFiles, nil
+	return nil
 }
 
 // CACerts has cert and key for CA (and Proxy)

pkg/minikube/bootstrapper/certs_test.go

@@ -57,8 +57,7 @@ func TestSetupCerts(t *testing.T) {
 	f := command.NewFakeCommandRunner()
 	f.SetCommandToOutput(expected)
 
-	_, err := SetupCerts(f, k8s, config.Node{})
-	if err != nil {
+	if err := SetupCerts(f, k8s, config.Node{}); err != nil {
 		t.Fatalf("Error starting cluster: %v", err)
 	}
 }

pkg/minikube/bootstrapper/kubeadm/kubeadm.go

@@ -848,8 +848,7 @@ func (k *Bootstrapper) DeleteCluster(k8s config.KubernetesConfig) error {
 
 // SetupCerts sets up certificates within the cluster.
 func (k *Bootstrapper) SetupCerts(k8s config.KubernetesConfig, n config.Node) error {
-	_, err := bootstrapper.SetupCerts(k.c, k8s, n)
-	return err
+	return bootstrapper.SetupCerts(k.c, k8s, n)
 }
 
 // UpdateCluster updates the control plane with cluster-level info.

pkg/minikube/cni/custom.go

@@ -21,6 +21,7 @@ import (
 	"path"
 
 	"github.com/pkg/errors"
+	"k8s.io/klog/v2"
 	"k8s.io/minikube/pkg/minikube/assets"
 	"k8s.io/minikube/pkg/minikube/config"
 )
@@ -55,6 +56,11 @@ func (c Custom) Apply(r Runner) error {
 	if err != nil {
 		return errors.Wrap(err, "manifest")
 	}
+	defer func() {
+		if err := m.Close(); err != nil {
+			klog.Warningf("error closing the file %s: %v", m.GetSourcePath(), err)
+		}
+	}()
 
 	return applyManifest(c.cc, r, m)
 }

pkg/minikube/cruntime/containerd.go

@@ -497,6 +497,12 @@ func (r *Containerd) Preload(cfg config.KubernetesConfig) error {
 	if err != nil {
 		return errors.Wrap(err, "getting file asset")
 	}
+	defer func() {
+		if err := fa.Close(); err != nil {
+			klog.Warningf("error closing the file %s: %v", fa.GetSourcePath(), err)
+		}
+	}()
+
 	t := time.Now()
 	if err := r.Runner.Copy(fa); err != nil {
 		return errors.Wrap(err, "copying file")

pkg/minikube/cruntime/crio.go

@@ -349,6 +349,12 @@ func (r *CRIO) Preload(cfg config.KubernetesConfig) error {
 	if err != nil {
 		return errors.Wrap(err, "getting file asset")
 	}
+	defer func() {
+		if err := fa.Close(); err != nil {
+			klog.Warningf("error closing the file %s: %v", fa.GetSourcePath(), err)
+		}
+	}()
+
 	t := time.Now()
 	if err := r.Runner.Copy(fa); err != nil {
 		return errors.Wrap(err, "copying file")

pkg/minikube/cruntime/docker.go

@@ -486,6 +486,12 @@ func (r *Docker) Preload(cfg config.KubernetesConfig) error {
 	if err != nil {
 		return errors.Wrap(err, "getting file asset")
 	}
+	defer func() {
+		if err := fa.Close(); err != nil {
+			klog.Warningf("error closing the file %s: %v", fa.GetSourcePath(), err)
+		}
+	}()
+
 	t := time.Now()
 	if err := r.Runner.Copy(fa); err != nil {
 		return errors.Wrap(err, "copying file")

pkg/minikube/machine/build_images.go

@@ -153,6 +153,12 @@ func transferAndBuildImage(cr command.Runner, k8s config.KubernetesConfig, src s
 	if err != nil {
 		return errors.Wrapf(err, "creating copyable file asset: %s", filename)
 	}
+	defer func() {
+		if err := f.Close(); err != nil {
+			klog.Warningf("error closing the file %s: %v", f.GetSourcePath(), err)
+		}
+	}()
+
 	if err := cr.Copy(f); err != nil {
 		return errors.Wrap(err, "transferring cached image")
 	}

pkg/minikube/machine/cache_binaries.go

@@ -22,6 +22,7 @@ import (
 
 	"github.com/pkg/errors"
 	"golang.org/x/sync/errgroup"
+	"k8s.io/klog/v2"
 	"k8s.io/minikube/pkg/minikube/assets"
 	"k8s.io/minikube/pkg/minikube/bootstrapper"
 	"k8s.io/minikube/pkg/minikube/command"
@@ -51,6 +52,12 @@ func CopyBinary(cr command.Runner, src string, dest string) error {
 	if err != nil {
 		return errors.Wrap(err, "new file asset")
 	}
+	defer func() {
+		if err := f.Close(); err != nil {
+			klog.Warningf("error closing the file %s: %v", f.GetSourcePath(), err)
+		}
+	}()
+
 	if err := cr.Copy(f); err != nil {
 		return errors.Wrapf(err, "copy")
 	}

pkg/minikube/machine/cache_images.go

@@ -287,6 +287,12 @@ func transferAndLoadImage(cr command.Runner, k8s config.KubernetesConfig, src st
 	if err != nil {
 		return errors.Wrapf(err, "creating copyable file asset: %s", filename)
 	}
+	defer func() {
+		if err := f.Close(); err != nil {
+			klog.Warningf("error closing the file %s: %v", f.GetSourcePath(), err)
+		}
+	}()
+
 	if err := cr.Copy(f); err != nil {
 		return errors.Wrap(err, "transferring cached image")
 	}

pkg/minikube/machine/filesync.go

@@ -43,6 +43,14 @@ var guaranteed = map[string]bool{
 // syncLocalAssets syncs files from MINIKUBE_HOME into the cluster
 func syncLocalAssets(cr command.Runner) error {
 	fs, err := localAssets()
+	defer func() {
+		for _, f := range fs {
+			if err := f.Close(); err != nil {
+				klog.Warningf("error closing the file %s: %v", f.GetSourcePath(), err)
+			}
+		}
+	}()
+
 	if err != nil {
 		return err
 	}

pkg/provision/provision.go

@@ -153,6 +153,12 @@ func copyHostCerts(authOptions auth.Options) error {
 		if err != nil {
 			return errors.Wrapf(err, "open cert file: %s", src)
 		}
+		defer func() {
+			if err := f.Close(); err != nil {
+				klog.Warningf("error closing the file %s: %v", f.GetSourcePath(), err)
+			}
+		}()
+
 		if err := execRunner.Copy(f); err != nil {
 			return errors.Wrapf(err, "transferring file: %+v", f)
 		}
@@ -187,6 +193,12 @@ func copyRemoteCerts(authOptions auth.Options, driver drivers.Driver) error {
 		if err != nil {
 			return errors.Wrapf(err, "error copying %s to %s", src, dst)
 		}
+		defer func() {
+			if err := f.Close(); err != nil {
+				klog.Warningf("error closing the file %s: %v", f.GetSourcePath(), err)
+			}
+		}()
+
 		if err := sshRunner.Copy(f); err != nil {
 			return errors.Wrapf(err, "transferring file to machine %v", f)
 		}

translations/de.json

@@ -225,6 +225,7 @@
 	"Failed to change permissions for {{.minikube_dir_path}}: {{.error}}": "Fehler beim Ändern der Berechtigungen für {{.minikube_dir_path}}: {{.error}}",
 	"Failed to check main repository and mirrors for images": "",
 	"Failed to configure metallb IP {{.profile}}": "",
+	"Failed to copying file": "",
 	"Failed to create file": "",
 	"Failed to delete cluster {{.name}}, proceeding with retry anyway.": "",
 	"Failed to delete cluster {{.name}}.": "",

translations/es.json

@@ -230,6 +230,7 @@
 	"Failed to change permissions for {{.minikube_dir_path}}: {{.error}}": "No se han podido cambiar los permisos de {{.minikube_dir_path}}: {{.error}}",
 	"Failed to check main repository and mirrors for images": "",
 	"Failed to configure metallb IP {{.profile}}": "",
+	"Failed to copying file": "",
 	"Failed to create file": "",
 	"Failed to delete cluster {{.name}}, proceeding with retry anyway.": "",
 	"Failed to delete cluster {{.name}}.": "",

translations/fr.json

@@ -227,6 +227,7 @@
 	"Failed to change permissions for {{.minikube_dir_path}}: {{.error}}": "Échec de la modification des autorisations pour {{.minikube_dir_path}} : {{.error}}",
 	"Failed to check main repository and mirrors for images": "",
 	"Failed to configure metallb IP {{.profile}}": "",
+	"Failed to copying file": "",
 	"Failed to create file": "",
 	"Failed to delete cluster {{.name}}, proceeding with retry anyway.": "",
 	"Failed to delete cluster {{.name}}.": "",

translations/ja.json

@@ -217,6 +217,7 @@
 	"Failed to change permissions for {{.minikube_dir_path}}: {{.error}}": "{{.minikube_dir_path}} に対する権限を変更できませんでした。{{.error}}",
 	"Failed to check main repository and mirrors for images": "",
 	"Failed to configure metallb IP {{.profile}}": "",
+	"Failed to copying file": "",
 	"Failed to create file": "",
 	"Failed to delete cluster {{.name}}, proceeding with retry anyway.": "クラスタを削除できませんでしたが、処理を続行します。",
 	"Failed to delete cluster {{.name}}.": "",

translations/ko.json

@@ -245,6 +245,7 @@
 	"Failed to check if machine exists": "머신이 존재하는지 확인하는 데 실패하였습니다",
 	"Failed to check main repository and mirrors for images": "",
 	"Failed to configure metallb IP {{.profile}}": "",
+	"Failed to copying file": "",
 	"Failed to create file": "",
 	"Failed to delete cluster {{.name}}, proceeding with retry anyway.": "",
 	"Failed to delete cluster {{.name}}.": "",

translations/pl.json

@@ -235,6 +235,7 @@
 	"Failed to change permissions for {{.minikube_dir_path}}: {{.error}}": "Nie udało się zmienić uprawnień pliku {{.minikube_dir_path}}: {{.error}}",
 	"Failed to check main repository and mirrors for images": "",
 	"Failed to configure metallb IP {{.profile}}": "",
+	"Failed to copying file": "",
 	"Failed to create file": "",
 	"Failed to delete cluster {{.name}}, proceeding with retry anyway.": "",
 	"Failed to delete cluster {{.name}}.": "",

translations/strings.txt

@@ -211,6 +211,7 @@
 	"Failed to change permissions for {{.minikube_dir_path}}: {{.error}}": "",
 	"Failed to check main repository and mirrors for images": "",
 	"Failed to configure metallb IP {{.profile}}": "",
+	"Failed to copying file": "",
 	"Failed to create file": "",
 	"Failed to delete cluster {{.name}}, proceeding with retry anyway.": "",
 	"Failed to delete cluster {{.name}}.": "",

translations/zh-CN.json

@@ -293,6 +293,7 @@
 	"Failed to check main repository and mirrors for images": "",
 	"Failed to check main repository and mirrors for images for images": "无法检测主仓库和镜像仓库中的镜像",
 	"Failed to configure metallb IP {{.profile}}": "",
+	"Failed to copying file": "",
 	"Failed to create file": "",
 	"Failed to delete cluster {{.name}}, proceeding with retry anyway.": "",
 	"Failed to delete cluster {{.name}}.": "",