Hi @anencore94. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Before coding some other feature such as 'enable -r option to copy directory', 'enable progress bar option' and the others, I want to get some reviews from you guys to check whether I am going the right way.

I also wonder how to implement the test code for this. Is it ok make only functional test for scp cmd at minikube/test/integration/functional_test.go ? or somewhere else? Please let me know.

Thanks a lot in advance!

/assign @tstromberg

Can one of the admins verify this patch?

@anencore94 sorry for the late reply on this PR, this sounds like a good PR, lets remove the TODO notes and yes lets add an integration test to our Funcitonal Test

it could be a subtest simmilar to other tests

	// Parallelized tests
	t.Run("parallel", func(t *testing.T) {
		tests := []struct {
			name      string
			validator validateFunc
		}{
			{"ConfigCmd", validateConfigCmd},
			{"DashboardCmd", validateDashboardCmd},
			{"DryRun", validateDryRun},
// ...add your test here
			{"ScpCmd", validateScp},
..

and u can make a dummy hello world file and try to copy it using scp and validate the file was copied by

minikube ssh cat /tmp/hello_scp.txt

Thanks for your review @medyagh !

I added the functional test for scp cmd as your comment. Could you re-check my PR?

• the functional test for "PR / functional_docker_windows (pull_request)" and "PR / functional_hyperv_windows (pull_request)" failed since the scp command does not support on windows i guess. (or because filepath format is different)
  So I made the functional test to skip when the os is windows.

Instead of shelling out to the scp flag directly, let's instead copy files into minikube the way we do in other parts of the code (for things like preload, or kubernets binaries). Instead of scp:

1. Create a FileAsset with the NewFileAsset function
2. Using a runner, run the built in Copy() function to copy the file asset into minikube

This way anyone can copy files in, regardless of if they have scp installed. This should also allow this command to run across all platforms, not just Linux. So, you shouldn't have to skip the functional test anymore.

Example of this in the code:

minikube/pkg/minikube/cruntime/containerd.go

Lines 350 to 357 in 6afefe6

	fa, err := assets.NewFileAsset(tarballPath, targetDir, targetName, "0644")
	if err != nil {
	return errors.Wrap(err, "getting file asset")
	}
	t := time.Now()
	if err := r.Runner.Copy(fa); err != nil {
	return errors.Wrap(err, "copying file")
	}

If you need any more examples, searching for any call to asset.NewFileAsset should get you there.

@anencore94 I agree with @priyawadhwa lets do that

Since we're no longer using scp I would probably also rename this command to minikube cp (matches docker so is more intuitive)

@priyawadhwa I changed the implementation of scp by copy with your review comments, Thanks!
By the way, it seems the function Copy of Runner doesn't support directory copy on its own. So If we want to copy directory, I guess we have to make directory and copy every files in for loop or some other way with Copy function.
Thus, for now, I just implement "minikube cp" for only file.

I want to implement the directory copying feature with another PR. (or someone else could do this)

/ok-to-test

kvm2 Driver
error collecting results for kvm2 driver: timing run 0 with Minikube (PR 10198): timing cmd: [/home/performance-monitor/.minikube/minikube-binaries/10198/minikube start --driver=kvm2]: starting cmd: fork/exec /home/performance-monitor/.minikube/minikube-binaries/10198/minikube: exec format error
docker Driver
error collecting results for docker driver: timing run 0 with Minikube (PR 10198): timing cmd: [/home/performance-monitor/.minikube/minikube-binaries/10198/minikube start --driver=docker]: starting cmd: fork/exec /home/performance-monitor/.minikube/minikube-binaries/10198/minikube: exec format error

@anencore94 can u plz rebase the PR (pull upstream) ?

Thanks, I've done @medyagh

thank you @anencore94

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: anencore94, medyagh

The full list of commands accepted by this bot can be found here.

The pull request process is described here