Skip to content
This repository was archived by the owner on Mar 27, 2024. It is now read-only.

Single image analysis #20

Merged
merged 8 commits into from
Aug 18, 2017
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .container-diff-tests.sh
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ success=0
while IFS=$' \n\r' read -r differ actual expected; do
diff=$(jq --argfile a "$actual" --argfile b "$expected" -n 'def walk(f): . as $in | if type == "object" then reduce keys[] as $key ( {}; . + { ($key): ($in[$key] | walk(f)) } ) | f elif type == "array" then map( walk(f) ) | f else f end; ($a | walk(if type == "array" then sort else . end)) as $a | ($b | walk(if type == "array" then sort else . end)) as $b | $a == $b')
if ! "$diff" ; then
echo "container diff" "$differ" "diff output is not as expected"
echo "container-diff" "$differ" "diff output is not as expected"
success=1
fi
done < tests/diff_comparisons.txt
Expand Down
236 changes: 148 additions & 88 deletions cmd/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,16 +18,15 @@ import (

var json bool
var eng bool
var save bool

var apt bool
var node bool
var file bool
var history bool
var pip bool

var save bool

var diffFlagMap = map[string]*bool{
var analyzeFlagMap = map[string]*bool{
"apt": &apt,
"node": &node,
"file": &file,
Expand All @@ -47,104 +46,168 @@ var RootCmd = &cobra.Command{

utils.SetDockerEngine(eng)

img1Arg := args[0]
img2Arg := args[1]
diffArgs := []string{}
allDiffers := getAllDiffers()
for _, name := range allDiffers {
if *diffFlagMap[name] == true {
diffArgs = append(diffArgs, name)
analyzeArgs := []string{}
allAnalyzers := getAllAnalyzers()
for _, name := range allAnalyzers {
if *analyzeFlagMap[name] == true {
analyzeArgs = append(analyzeArgs, name)
}
}
// If no differs are specified, perform all diffs as the default
if len(diffArgs) == 0 {
diffArgs = allDiffers

// If no differs/analyzers are specified, perform them all as the default
if len(analyzeArgs) == 0 {
analyzeArgs = allAnalyzers
}

var wg sync.WaitGroup
wg.Add(2)
if len(args) == 1 {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It makes more sense to me to have analyze as a separate command (such that the logic wouldn't be in root.go) both for clarity and for example in case of future additional commands. If someone adds a command and pattern matches, they seem forced into expanding this if else structure. The primary logic already seems separated out so is there a reason not to pull it into it's own command file?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just thought only one command would be preferable for our tool... @nkubala @aaron-prindle do you have any thoughts as to whether we should split this up into a separate command?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I actually think this structure is fine. We should probably at least put a comment here saying semantically what each case means (analyze = one image, diff = two), and then put something about it in the usage/--help command. I do like the tool choosing which path to take based on how many images you provide it though, seems intuitive to me

analyzeImage(args[0], analyzeArgs)
} else {
diffImages(args[0], args[1], analyzeArgs)
}
},
}

glog.Infof("Starting diff on images %s and %s, using differs: %s", img1Arg, img2Arg, diffArgs)
func diffImages(image1Arg, image2Arg string, diffArgs []string) {
var wg sync.WaitGroup
wg.Add(2)

var image1, image2 utils.Image
var err error
go func() {
defer wg.Done()
image1, err = utils.ImagePrepper{img1Arg}.GetImage()
if err != nil {
glog.Error(err.Error())
os.Exit(1)
}
}()
glog.Infof("Starting diff on images %s and %s, using differs: %s", image1Arg, image2Arg, diffArgs)

go func() {
defer wg.Done()
image2, err = utils.ImagePrepper{img2Arg}.GetImage()
if err != nil {
glog.Error(err.Error())
os.Exit(1)
}
}()
var image1, image2 utils.Image
var err error
go func() {
defer wg.Done()
image1, err = utils.ImagePrepper{image1Arg}.GetImage()
if err != nil {
glog.Error(err.Error())
os.Exit(1)
}
}()

diffTypes, err := differs.GetDiffers(diffArgs)
go func() {
defer wg.Done()
image2, err = utils.ImagePrepper{image2Arg}.GetImage()
if err != nil {
glog.Error(err.Error())
os.Exit(1)
}
wg.Wait()

req := differs.DiffRequest{image1, image2, diffTypes}
if diffs, err := req.GetDiff(); err == nil {
// Outputs diff results in alphabetical order by differ name
diffTypes := []string{}
for name := range diffs {
diffTypes = append(diffTypes, name)
}
sort.Strings(diffTypes)
glog.Info("Retrieving diffs")
diffResults := []utils.DiffResult{}
for _, diffType := range diffTypes {
diff := diffs[diffType]
if json {
diffResults = append(diffResults, diff.GetStruct())
} else {
err = diff.OutputText(diffType)
if err != nil {
glog.Error(err)
}
}
}
}()

diffTypes, err := differs.GetAnalyzers(diffArgs)
if err != nil {
glog.Error(err.Error())
os.Exit(1)
}
wg.Wait()

req := differs.DiffRequest{image1, image2, diffTypes}
if diffs, err := req.GetDiff(); err == nil {
// Outputs diff results in alphabetical order by differ name
sortedTypes := []string{}
for name := range diffs {
sortedTypes = append(sortedTypes, name)
}
sort.Strings(sortedTypes)
glog.Info("Retrieving diffs")
diffResults := []utils.DiffResult{}
for _, diffType := range sortedTypes {
diff := diffs[diffType]
if json {
err = utils.JSONify(diffResults)
diffResults = append(diffResults, diff.GetStruct())
} else {
err = diff.OutputText(diffType)
if err != nil {
glog.Error(err)
}
}
fmt.Println()
}
if json {
err = utils.JSONify(diffResults)
if err != nil {
glog.Error(err)
}
}
fmt.Println()
if !save {
glog.Info("Removing image file system directories from system")
if !save {
errMsg := remove(image1.FSPath, true)
errMsg += remove(image2.FSPath, true)
if errMsg != "" {
glog.Error(errMsg)
}
errMsg := remove(image1.FSPath, true)
errMsg += remove(image2.FSPath, true)
if errMsg != "" {
glog.Error(errMsg)
}
} else {
dir, _ := os.Getwd()
glog.Infof("Images were saved at %s as %s and %s", dir, image1.FSPath, image2.FSPath)
}
} else {
glog.Error(err.Error())
os.Exit(1)
}
}

func analyzeImage(imageArg string, analyzerArgs []string) {
image, err := utils.ImagePrepper{imageArg}.GetImage()
if err != nil {
glog.Error(err.Error())
os.Exit(1)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure if this is new to this PR, but GetImage() extracts the image filesystem and saves it on disk right? if so, calling os.Exit(1) here will skip the cleanup of that image if the user didn't want it around. probably want to defer an image cleanup in the root command, and then return an error here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

}
analyzeTypes, err := differs.GetAnalyzers(analyzerArgs)
if err != nil {
glog.Error(err.Error())
os.Exit(1)
}

req := differs.SingleRequest{image, analyzeTypes}
if analyses, err := req.GetAnalysis(); err == nil {
// Outputs analysis results in alphabetical order by differ name
sortedTypes := []string{}
for name := range analyses {
sortedTypes = append(sortedTypes, name)
}
sort.Strings(sortedTypes)
glog.Info("Retrieving diffs")
analyzeResults := []utils.AnalyzeResult{}
for _, analyzeType := range sortedTypes {
analysis := analyses[analyzeType]
if json {
analyzeResults = append(analyzeResults, analysis.GetStruct())
} else {
dir, _ := os.Getwd()
glog.Infof("Images were saved at %s as %s and %s", dir, image1.FSPath, image2.FSPath)
err = analysis.OutputText(analyzeType)
if err != nil {
glog.Error(err)
}
}
}
if json {
err = utils.JSONify(analyzeResults)
if err != nil {
glog.Error(err)
}
}
fmt.Println()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: use glog/the output buffer here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh I actually don't even think this is necessary because there's a newline in the template, so I'll just get rid of it...

if !save {
glog.Info("Removing image file system directory from system")
errMsg := remove(image.FSPath, true)
if errMsg != "" {
glog.Error(errMsg)
}
} else {
glog.Error(err.Error())
os.Exit(1)
dir, _ := os.Getwd()
glog.Infof("Image was saved at %s as %s", dir, image.FSPath)
}
},
} else {
glog.Error(err.Error())
os.Exit(1)
}

}

func getAllDiffers() []string {
allDiffers := []string{}
for name := range diffFlagMap {
allDiffers = append(allDiffers, name)
func getAllAnalyzers() []string {
allAnalyzers := []string{}
for name := range analyzeFlagMap {
allAnalyzers = append(allAnalyzers, name)
}
return allDiffers
return allAnalyzers
}

func validateArgs(args []string) (bool, error) {
Expand All @@ -165,11 +228,11 @@ func validateArgs(args []string) (bool, error) {

func checkArgNum(args []string) (bool, error) {
var errMessage string
if len(args) < 2 {
errMessage = "Too few arguments. Should have two images as arguments: [IMAGE1] [IMAGE2]."
if len(args) < 1 {
errMessage = "Too few arguments. Should have one or two images as arguments."
return false, errors.New(errMessage)
} else if len(args) > 2 {
errMessage = "Too many arguments. Should have two images as arguments: [IMAGE1] [IMAGE2]."
errMessage = "Too many arguments. Should have at most two images as arguments."
return false, errors.New(errMessage)
} else {
return true, nil
Expand All @@ -186,15 +249,12 @@ func checkImage(arg string) bool {
func checkArgType(args []string) (bool, error) {
var buffer bytes.Buffer
valid := true
if !checkImage(args[0]) {
valid = false
errMessage := fmt.Sprintf("Argument %s is not an image ID, URL, or tar\n", args[0])
buffer.WriteString(errMessage)
}
if !checkImage(args[1]) {
valid = false
errMessage := fmt.Sprintf("Argument %s is not an image ID, URL, or tar\n", args[1])
buffer.WriteString(errMessage)
for _, arg := range args {
if !checkImage(arg) {
valid = false
errMessage := fmt.Sprintf("Argument %s is not an image ID, URL, or tar\n", args[0])
buffer.WriteString(errMessage)
}
}
if !valid {
return false, errors.New(buffer.String())
Expand Down
2 changes: 1 addition & 1 deletion cmd/root_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ type testpair struct {

var argNumTests = []testpair{
{[]string{}, false},
{[]string{"one"}, false},
{[]string{"one"}, true},
{[]string{"one", "two"}, true},
{[]string{"one", "two", "three"}, false},
}
Expand Down
13 changes: 9 additions & 4 deletions differs/aptDiff.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,16 +10,21 @@ import (
"github.com/golang/glog"
)

type AptDiffer struct {
type AptAnalyzer struct {
}

// AptDiff compares the packages installed by apt-get.
func (d AptDiffer) Diff(image1, image2 utils.Image) (utils.DiffResult, error) {
diff, err := singleVersionDiff(image1, image2, d)
func (a AptAnalyzer) Diff(image1, image2 utils.Image) (utils.DiffResult, error) {
diff, err := singleVersionDiff(image1, image2, a)
return diff, err
}

func (d AptDiffer) getPackages(image utils.Image) (map[string]utils.PackageInfo, error) {
func (a AptAnalyzer) Analyze(image utils.Image) (utils.AnalyzeResult, error) {
analysis, err := singleVersionAnalysis(image, a)
return analysis, err
}

func (a AptAnalyzer) getPackages(image utils.Image) (map[string]utils.PackageInfo, error) {
path := image.FSPath
packages := make(map[string]utils.PackageInfo)
if _, err := os.Stat(path); err != nil {
Expand Down
2 changes: 1 addition & 1 deletion differs/aptDiff_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ func TestGetAptPackages(t *testing.T) {
},
}
for _, test := range testCases {
d := AptDiffer{}
d := AptAnalyzer{}
image := utils.Image{FSPath: test.path}
packages, err := d.getPackages(image)
if err != nil && !test.err {
Expand Down
Loading