addons: Fixes multiple files behavior in files rootfs

[laozc]

Some files may be left uncopied to the VM when there are multiple
files in the .minikube/files directory as the code misinterprets
the last parent dir as parent dirs for all files, which results
some files with the same name were not copied.

This patch fixes the behavior.

[minikube-bot]

Can one of the admins verify this patch?

[tstromberg]

Nice work. Any chance you could add a unit or integration test to ensure that we don't accidentally reverse this behavior in the future?

[laozc]

Is the patch to add unit test in a large size than expected?

[tstromberg]

Is the patch to add unit test in a large size than expected?

Yeah. I'm going to have to give some thought to the use of afero here. Give me a few days. Thanks!

[laozc]

Sure.
Let me know if there is a better way to handle it.

[tstromberg]

I needed some time to think about this PR, and while I like the concept of afero.Fs, I don't think this current approach of embedding it around the minikube code base works for me. I feel that passing nil to a function is generally a sign of a hastily tacked on API. Then again, so is having a MakeMiniPath function in a constants package (not your fault!)

Feel free to correct me - but I don't feel that using the real filesystem here will negatively impact our overall test run time.

It's less elegant, but what do you think about adding a helper to test_addons.go, which sets up a new filesystem directory for minikube:

func setupTestDir() (path, error) {
  path, err := ioutil.TempDir()
  os.Setenv(constants.MinikubeHome, path)
  return path, err
}

Then you can gather a list of files written to disk via filepath.Walk, and compare it against expectations:

path, err := setupTestDir()
got := filepath.Walk(path)

    if diff := cmp.Diff(got, want); diff != "" {
        t.Errorf("files differ: (-want +got)\n%s", diff)
    }

It's certainly less elegant, but what are your thoughts?

[laozc]

Thanks for the advice.
Changed the test cases to use a temporary dir for Minikube config as suggested.
It should eliminate the code change in non-relevant files by now.

[tstromberg]

/lgtm

@minikube-bot OK to test

[tstromberg]

/approve

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: laozc, tstromberg

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

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [tstromberg]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment