Adding custom cache envar and command line argument

[vsoch]

This PR will close #273, allowing the user to specify a custom cache directory base (different from $HOME) to allow for better use in cluster environments (where the $HOME is very tiny!) Specifically, the order of operations is the following:

1. First preference goes to command line setting
2. Second preference goes to environment variable set as CONTAINER_DIFF_CACHEDIR
3. Third preference goes to default ($HOME) as is now.

Additional notes are included below.

Variable Type

I added the variable as a string variable, with option for a single character (c)
to mirror the same ability for "no-cache"

cmd.Flags().StringVarP(&cacheDir, "cache", "c", "", "cache directory base to create .container-diff (default is $HOME).")

Since all commands can use the cache, I added it to addSharedFlags.

With the above, the variable goes into "cacheDir" to be consistent with the use
of cacheDir in pkg/util/image_utils.g. The previous function was also called "cacheDir", but
I renamed to be "getCacheDir" so it more accurately reflects its purpose. The command
line call (for analyze) now looks like this:

$ ./out/container-diff analyze
Error: 'analyze' requires one image as an argument: container-diff analyze [image]
Usage:
  container-diff analyze [flags]

Flags:
  -c, --cache string      cache directory base to create .container-diff (default is $HOME).
  -h, --help              help for analyze
  -j, --json              JSON Output defines if the diff should be returned in a human readable format (false) or a JSON (true).
  -n, --no-cache          Set this to force retrieval of image filesystem on each run.
  -o, --order             Set this flag to sort any file/package results by descending size. Otherwise, they will be sorted by name.
  -q, --quiet             Suppress output to stderr.
  -s, --save              Set this flag to save rather than remove the final image filesystems on exit.
  -t, --type Diff Types   This flag sets the list of analyzer types to use. Set it repeatedly to use multiple analyzers.

Global Flags:
      --format string      Format to output diff in.
  -v, --verbosity string   This flag controls the verbosity of container-diff. (default "warning")

'analyze' requires one image as an argument: container-diff analyze [image]

Notice that I tell the user that it's a base where we create .container-diff.

Examples

We can test that the default cache stiill goes to $HOME. Here I am not specifying anything in the environment or command line (and clearing any old cache that might have been there).

rm -rf $HOME/.container-diff/cache
$ ./out/container-diff analyze ubuntu
...

He's back!

$ ls /home/vanessa/.container-diff/cache/
ubuntu

And then we can test setting something else via command line:

$ ./out/container-diff analyze ubuntu --cache /tmp

And we see the cache with root as /tmp as we would want.

cache

And then via an environment variable, removing the previous first.

rm -rf /tmp/.container-diff
$ CONTAINER_DIFF_CACHEDIR=/tmp ./out/container-diff analyze ubuntu
$ ls /tmp/.container-diff/cache/
ubuntu

Finally, command line takes preference over environment.

rm -rf /tmp/.container-diff

# This export should be ignored
export CONTAINER_DIFF_CACHEDIR=/tmp

# We will use this one on the command line (preference)
mkdir -p $CONTAINER_DIFF_CACHEDIR/pancakes

# pancakes is set in on command line, should take preference

$ ./out/container-diff analyze ubuntu --cache $CONTAINER_DIFF_CACHEDIR/pancakes
$ ls /tmp/pancakes/.container-diff/
cache

This is my first goLang PR so I'd love some guidance on how to write tests. I should have some time later today. Thanks for your help!

[nkubala]

@vsoch thanks so much for the PR! A++ for the write up, I seriously appreciate you taking the time to explain everything :)

The code looks great! I left a few small comments, but aside from those the logic LGTM. I'd love to see a couple tests like you mentioned. You can write them in root_test.go (in the same package as this file), since they'll apply to both subcommands of root. I'd probably start with something like

func TestCacheDir(t *testing.T) {
  tests := []struct{
    name        string
    cliFlag     string
    envVar      string
    expectedDir string
  }{
    // your tests cases here
  }

  for _, tt := range tests {
    t.Run(tt.name, func(t *testing.T) {
      // actual testing logic. probably set the env var, set the global flag based on the cliFlag, then
      // call getCacheDir() and make sure the return value is equal to the expectedDir
    }
  }
}

This is a classic example of table-driven tests, and generally our preferred way of writing golang unit tests. You can write all your tests cases as structs in the tests variable, and this way you don't have to copy any code between them.

Let me know if you have any other questions! Thanks again for doing this :)

[vsoch]

I'll get started on the tests! I need to get used to using gofmt, it's just embarrassing at this point. My editor is set up to convert tabs to spaces because I've had a terrible allergy to tabs in the past, but I possibly need to rethink this, or use the git hooks I saw in the repo to run tests before pushing :) But we are green now! I'm going to read up on tests a bit first and will then work on this promptly.

Happy Friday, and if you're in the Bay Area, take care of your lungs!

[vsoch]

okay first shot at tests are done, ready for review. I did the four cases that I reviewed previously for the example:

• that the default cache is at $HOME when nothing is set
• setting cache just via the command line argument --cache-dir
• setting the environment variable CONTAINER_DIFF_CACHEDIR
• setting both environment and command flag gives preference to command flag

I formatted my code first this time! 🎊

[nkubala]

thanks for the tests, they look great! just a few small nits and this will be ready to go 👍 hopefully this wasn't too much of a pain, sometimes Go formatting can be a little tricky!

[vsoch]

Alright! Changes are done, please review the notes above to see if any further are needed. My two questions were pertaining to the different between importing errors vs github.com/pkg/errors and then what is considered "best practice" for printing a message. I chose a newline followed bythe message, and wanted to ask if some formatting of the err object was also commonly done. For reference, I did t.Errorf("\nError getting home dir").

[nkubala]

thanks so much for sticking with this one! really appreciate the contribution 🎉