Merge pull request #89745 from johnbelamaric/kubeconform-cleanup

Various cleanup for the kubeconform command

test/conformance/kubeconform/README.md

@@ -1,12 +1,19 @@
-# Kubetestgen
+# kubeconform
 
-kubetestgen generates a list of behaviors for a resource based on the OpenAPI schema. The purpose is to bootstrap a list of behaviors, and not to produce the final list of behaviors. We expect that the resulting files will be curated to identify a meaningful set of behaviors for the conformance requirements of the targeted resource. This may include addition, modification, and removal of behaviors from the generated list.
+`kubeconform` is used to manage the creation and coverage analysis of conformance behaviors and tests. Currently it performs two functions:
 
+* `gen`. This command generates a list of behaviors for a resource based on the OpenAPI schema. The purpose is to bootstrap a list of behaviors, and not to produce the final list of behaviors. We expect that the resulting files will be curated to identify a meaningful set of behaviors for the conformance requirements of the targeted resource. This may include addition, modification, and removal of behaviors from the generated list.
+* `link`. This command prints the defined behaviors not covered by any test.
+
+## gen
 **Example usage for PodSpec:**
 
+From the root directory of the k/k repo, will produce `pod.yaml` in
+`test/conformance/behaviors`. The `pwd` is needed because of how bazel handles
+working directories with `run`.
+
 ```
-bazel build //test/conformance/kubetestgen:kubetestgen
+$ bazel run  //test/conformance/kubeconform:kubeconform -- --resource io.k8s.api.core.v1.PodSpec --area pod --schema api/openapi-spec/swagger.json --dir `pwd`/test/conformance/behaviors/ gen
-/bazel-out/k8-fastbuild/bin/test/conformance/kubetestgen/linux_amd64_stripped/kubetestgen --resource io.k8s.api.core.v1.PodSpec --area pod --schema api/openapi-spec/swagger.json --dir test/conformance/behaviors/
 ```
 
 **Flags:**
@@ -17,3 +24,9 @@ bazel build //test/conformance/kubetestgen:kubetestgen
 - `dir` - the path to the behaviors directory (default current directory)
 
 **Note**: The tool automatically generates suites based on the object type for a field. All primitive data types are grouped into a default suite, while object data types are grouped into their own suite, one per object.
+
+## link
+
+```
+$ bazel run  //test/conformance/kubeconform:kubeconform -- -dir `pwd`/test/conformance/behaviors/sig-node  -testdata `pwd`/test/conformance/testdata/conformance.yaml link
+```

test/conformance/kubeconform/gen.go

@@ -19,6 +19,7 @@ package main
 import (
 	"fmt"
 	"os"
+	"path/filepath"
 	"sort"
 	"strings"
 
@@ -31,12 +32,11 @@ import (
 
 var defMap map[string]analysis.SchemaRef
 
-func gen(o *options) {
+func gen(o *options) error {
 	defMap = make(map[string]analysis.SchemaRef)
 	d, err := loads.JSONSpec(o.schemaPath)
 	if err != nil {
-		fmt.Printf("ERROR: %s\n", err.Error())
+		return err
-		os.Exit(1)
 	}
 	defs := d.Analyzer.AllDefinitions()
 	sort.Slice(defs, func(i, j int) bool { return defs[i].Name < defs[j].Name })
@@ -105,27 +105,25 @@ func gen(o *options) {
 
 	var area behaviors.Area = behaviors.Area{Area: o.area, Suites: suites}
 	countFields(suites)
-	printYAML(o.behaviorsDir+o.area, area)
+	return printYAML(filepath.Join(o.behaviorsDir, o.area), area)
 }
 
-func printYAML(fileName string, areaO behaviors.Area) {
+func printYAML(fileName string, areaO behaviors.Area) error {
 	f, err := os.Create(fileName + ".yaml")
 	if err != nil {
-		fmt.Printf("ERROR: %s\n", err.Error())
+		return err
-		os.Exit(1)
 	}
 	defer f.Close()
 	y, err := yaml.Marshal(areaO)
 	if err != nil {
-		fmt.Printf("ERROR: %s\n", err.Error())
+		return err
-		os.Exit(1)
 	}
 
 	_, err = f.WriteString(string(y))
 	if err != nil {
-		fmt.Printf("ERROR: %s\n", err.Error())
+		return err
-		os.Exit(1)
 	}
+	return nil
 }
 
 func countFields(suites []behaviors.Suite) {

test/conformance/kubeconform/kubeconform.go

@@ -19,8 +19,11 @@ package main
 import (
 	"flag"
 	"fmt"
+	"os"
 )
 
+// homegrown command structures now but if this grows we may
+// want to adopt whatever kubectl uses
 type options struct {
 	// Flags only used for generating behaviors
 	schemaPath string
@@ -28,37 +31,89 @@ type options struct {
 	area       string
 
 	// Flags only used for linking behaviors
-	testdata    string
+	testdata string
-	listMissing bool
+	listAll  bool
 
 	// Flags shared between CLI tools
 	behaviorsDir string
 }
 
-func parseFlags() *options {
+type actionFunc func(*options) error
+
+func parseFlags() (actionFunc, *options) {
 	o := &options{}
 
-	flag.StringVar(&o.schemaPath, "schema", "", "Path to the OpenAPI schema")
+	f := flag.NewFlagSet(os.Args[0], flag.ExitOnError)
-	flag.StringVar(&o.resource, "resource", ".*", "Resource name")
+	f.StringVar(&o.schemaPath, "schema", "", "Path to the OpenAPI schema")
-	flag.StringVar(&o.area, "area", "default", "Area name to use")
+	f.StringVar(&o.resource, "resource", "", "Resource name")
+	f.StringVar(&o.area, "area", "", "Area name to use")
 
-	flag.StringVar(&o.testdata, "testdata", "../testdata/conformance.yaml", "YAML file containing test linkage data")
+	f.StringVar(&o.testdata, "testdata", "test/conformance/testdata/conformance.yaml", "YAML file containing test linkage data")
-	flag.BoolVar(&o.listMissing, "missing", true, "Only list behaviors missing tests")
+	f.BoolVar(&o.listAll, "all", false, "List all behaviors, not just those missing tests")
 
-	flag.StringVar(&o.behaviorsDir, "dir", "../behaviors", "Path to the behaviors directory")
+	f.StringVar(&o.behaviorsDir, "dir", "test/conformance/behaviors/", "Path to the behaviors directory")
 
+	f.Usage = func() {
+		fmt.Fprintf(os.Stderr,
+			"USAGE\n-----\n%s [ options ] { link | gen }\n",
+			os.Args[0])
+		fmt.Fprintf(os.Stderr, "\nOPTIONS\n-------\n")
+		flag.PrintDefaults()
+		fmt.Fprintf(os.Stderr, "\nACTIONS\n------------")
+		fmt.Fprintf(os.Stderr, `
+  'link' lists behaviors associated with tests
+  'gen' generates behaviors based on the API schema
+`)
+	}
+
+	flag.CommandLine = f
 	flag.Parse()
-	return o
+	if len(flag.Args()) != 1 {
+		flag.CommandLine.Usage()
+		os.Exit(2)
+	}
+
+	var action actionFunc
+	switch flag.Args()[0] {
+	case "gen":
+		action = gen
+		if o.schemaPath == "" {
+			action = nil
+			fmt.Fprintf(os.Stderr, "-schema is required for 'gen'\n")
+		}
+		if o.resource == "" {
+			action = nil
+			fmt.Fprintf(os.Stderr, "-resource is required for 'gen'\n")
+		}
+		if o.area == "" {
+			action = nil
+			fmt.Fprintf(os.Stderr, "-area is required for 'gen'\n")
+		}
+	case "link":
+		action = link
+		if o.testdata == "" {
+			action = nil
+			fmt.Fprintf(os.Stderr, "-testdata is required for 'link'\n")
+		}
+	}
+
+	if o.behaviorsDir == "" {
+		action = nil
+		fmt.Fprintf(os.Stderr, "-dir is required\n")
+	}
+
+	if action == nil {
+		flag.CommandLine.Usage()
+		os.Exit(2)
+	}
+	return action, o
 }
 
 func main() {
-	o := parseFlags()
+	action, o := parseFlags()
-	action := flag.Arg(0)
+	err := action(o)
-	if action == "gen" {
+	if err != nil {
-		gen(o)
+		fmt.Printf("Error: %s\n", err)
-	} else if action == "link" {
+		os.Exit(1)
-		link(o)
-	} else {
-		fmt.Printf("Unknown argument %s\n", action)
 	}
 }

test/conformance/kubeconform/link.go

@@ -28,7 +28,7 @@ import (
 	"k8s.io/kubernetes/test/conformance/behaviors"
 )
 
-func link(o *options) {
+func link(o *options) error {
 	var behaviorFiles []string
 	behaviorsMapping := make(map[string][]string)
 	var conformanceDataList []behaviors.ConformanceData
@@ -36,7 +36,7 @@ func link(o *options) {
 	err := filepath.Walk(o.behaviorsDir,
 		func(path string, info os.FileInfo, err error) error {
 			if err != nil {
-				fmt.Printf("%v", err)
+				return err
 			}
 			r, _ := regexp.Compile(".+.yaml$")
 			if r.MatchString(path) {
@@ -45,23 +45,30 @@ func link(o *options) {
 			return nil
 		})
 	if err != nil {
-		fmt.Printf("%v", err)
+		return err
-		return
+	}
+	fmt.Println()
+	fmt.Printf("Using behaviors from these %d files:\n", len(behaviorFiles))
+	for _, f := range behaviorFiles {
+		fmt.Println("    ", f)
+	}
+	fmt.Println()
+	if o.listAll {
+		fmt.Println("All behaviors:")
+	} else {
+		fmt.Println("Behaviors not covered by any conformance test:")
 	}
-	fmt.Printf("%v", behaviorFiles)
 
 	for _, behaviorFile := range behaviorFiles {
 		var suite behaviors.Suite
 
 		yamlFile, err := ioutil.ReadFile(behaviorFile)
 		if err != nil {
-			fmt.Printf("%v", err)
+			return err
-			return
 		}
 		err = yaml.UnmarshalStrict(yamlFile, &suite)
 		if err != nil {
-			fmt.Printf("%v", err)
+			return err
-			return
 		}
 
 		for _, behavior := range suite.Behaviors {
@@ -71,36 +78,30 @@ func link(o *options) {
 
 	conformanceYaml, err := ioutil.ReadFile(o.testdata)
 	if err != nil {
-		fmt.Printf("%v", err)
+		return err
-		return
 	}
 
 	err = yaml.Unmarshal(conformanceYaml, &conformanceDataList)
 	if err != nil {
-		fmt.Printf("%v", err)
+		return err
-		return
 	}
 
 	for _, data := range conformanceDataList {
 		for _, behaviorID := range data.Behaviors {
 			if _, ok := behaviorsMapping[behaviorID]; !ok {
-				fmt.Printf("Error, cannot find behavior \"%s\"", behaviorID)
+				return fmt.Errorf("cannot find behavior %q", behaviorID)
-				return
 			}
 			behaviorsMapping[behaviorID] = append(behaviorsMapping[behaviorID], data.CodeName)
 		}
 	}
 	printBehaviorsMapping(behaviorsMapping, o)
+	return nil
 }
 
 func printBehaviorsMapping(behaviorsMapping map[string][]string, o *options) {
 	for behaviorID, tests := range behaviorsMapping {
-		if o.listMissing {
+		if o.listAll || tests == nil {
-			if tests == nil {
+			fmt.Println("    ", behaviorID)
-				fmt.Println(behaviorID)
-			} else {
-				fmt.Println(behaviorID)
-			}
 		}
 	}
 }