From 85724cf2c3c17f5e1cba9f791d9a6f6d77a56bfd Mon Sep 17 00:00:00 2001 From: Aaron Crickenberger Date: Mon, 1 Jun 2020 17:02:48 -0700 Subject: [PATCH] behaviors: fix kubeconform link kubeconform was choking on a typo in the description field, so I fixed the typo while adding friendlier logging to tell me which file was invalid I got curious why tests didn't catch this, and it turns out kubeconform and the behavior tests use different codepaths to load and validate. So I merged them together --- test/conformance/behaviors/BUILD | 12 ++- test/conformance/behaviors/behaviors.go | 74 +++++++++++++++++++ test/conformance/behaviors/behaviors_test.go | 40 +--------- .../behaviors/sig-node/pod-spec.yaml | 4 +- test/conformance/kubeconform/link.go | 31 ++------ 5 files changed, 96 insertions(+), 65 deletions(-) create mode 100644 test/conformance/behaviors/behaviors.go diff --git a/test/conformance/behaviors/BUILD b/test/conformance/behaviors/BUILD index c628970328e..5cb79459a9d 100644 --- a/test/conformance/behaviors/BUILD +++ b/test/conformance/behaviors/BUILD @@ -2,9 +2,16 @@ load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test") go_library( name = "go_default_library", - srcs = ["types.go"], + srcs = [ + "behaviors.go", + "types.go", + ], importpath = "k8s.io/kubernetes/test/conformance/behaviors", visibility = ["//visibility:public"], + deps = [ + "//staging/src/k8s.io/apimachinery/pkg/util/errors:go_default_library", + "//vendor/gopkg.in/yaml.v2:go_default_library", + ], ) go_test( @@ -12,9 +19,6 @@ go_test( srcs = ["behaviors_test.go"], data = glob(["*/*.yaml"]), embed = [":go_default_library"], - deps = [ - "//vendor/gopkg.in/yaml.v2:go_default_library", - ], ) filegroup( diff --git a/test/conformance/behaviors/behaviors.go b/test/conformance/behaviors/behaviors.go new file mode 100644 index 00000000000..95eba28d1b4 --- /dev/null +++ b/test/conformance/behaviors/behaviors.go @@ -0,0 +1,74 @@ +/* +Copyright 2020 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package behaviors + +import ( + "fmt" + "io/ioutil" + utilerrors "k8s.io/apimachinery/pkg/util/errors" + "os" + "path/filepath" + "regexp" + + "gopkg.in/yaml.v2" +) + +// BehaviorFileList returns a list of eligible behavior files in or under dir +func BehaviorFileList(dir string) ([]string, error) { + var behaviorFiles []string + + r, _ := regexp.Compile(".+.yaml$") + err := filepath.Walk(dir, + func(path string, info os.FileInfo, err error) error { + if err != nil { + return err + } + if r.MatchString(path) { + behaviorFiles = append(behaviorFiles, path) + } + return nil + }, + ) + return behaviorFiles, err +} + +// LoadSuite loads a Behavior Suite from .yaml file at path +func LoadSuite(path string) (*Suite, error) { + var suite Suite + bytes, err := ioutil.ReadFile(path) + if err != nil { + return nil, fmt.Errorf("error loading suite %s: %v", path, err) + } + err = yaml.UnmarshalStrict(bytes, &suite) + if err != nil { + return nil, fmt.Errorf("error loading suite %s: %v", path, err) + } + return &suite, nil +} + +// ValidateSuite validates that the given suite has no duplicate behavior IDs +func ValidateSuite(suite *Suite) error { + var errs []error + behaviorsByID := make(map[string]bool) + for _, b := range suite.Behaviors { + if _, ok := behaviorsByID[b.ID]; ok { + errs = append(errs, fmt.Errorf("Duplicate behavior ID: %s", b.ID)) + } + behaviorsByID[b.ID] = true + } + return utilerrors.NewAggregate(errs) +} diff --git a/test/conformance/behaviors/behaviors_test.go b/test/conformance/behaviors/behaviors_test.go index b3f7163314e..ee49755fe16 100644 --- a/test/conformance/behaviors/behaviors_test.go +++ b/test/conformance/behaviors/behaviors_test.go @@ -17,30 +17,11 @@ limitations under the License. package behaviors import ( - "io/ioutil" - "os" - "path/filepath" - "regexp" "testing" - - "gopkg.in/yaml.v2" ) func TestValidate(t *testing.T) { - var behaviorFiles []string - - err := filepath.Walk(".", - func(path string, info os.FileInfo, err error) error { - if err != nil { - t.Errorf("%q", err.Error()) - } - - r, _ := regexp.Compile(".+.yaml$") - if r.MatchString(path) { - behaviorFiles = append(behaviorFiles, path) - } - return nil - }) + behaviorFiles, err := BehaviorFileList(".") if err != nil { t.Errorf("%q", err.Error()) } @@ -51,25 +32,12 @@ func TestValidate(t *testing.T) { } func validateSuite(path string, t *testing.T) { - var suite Suite - yamlFile, err := ioutil.ReadFile(path) + suite, err := LoadSuite(path) if err != nil { t.Errorf("%q", err.Error()) } - err = yaml.Unmarshal(yamlFile, &suite) - + err = ValidateSuite(suite) if err != nil { - t.Errorf("%q", err.Error()) - } - - behaviorIDList := make(map[string]bool) - - for _, behavior := range suite.Behaviors { - - // Ensure no behavior IDs are duplicated - if _, ok := behaviorIDList[behavior.ID]; ok { - t.Errorf("Duplicate behavior ID: %s", behavior.ID) - } - behaviorIDList[behavior.ID] = true + t.Errorf("error validating %s: %q", path, err.Error()) } } diff --git a/test/conformance/behaviors/sig-node/pod-spec.yaml b/test/conformance/behaviors/sig-node/pod-spec.yaml index c2462fbe690..7b25dacd5f5 100644 --- a/test/conformance/behaviors/sig-node/pod-spec.yaml +++ b/test/conformance/behaviors/sig-node/pod-spec.yaml @@ -42,8 +42,8 @@ behaviors: description: When hostIPC is set to false, the Pod MUST NOT use the host's inter-process communication namespace. - id: pod/spec/label/create - decription: Create a Pod with a unique label. Query for the Pod with the label as selector MUST be successful. + description: Create a Pod with a unique label. Query for the Pod with the label as selector MUST be successful. - id: pod/spec/label/patch - decription: A patch request must be able to update the pod to change the value of an existing Label. Query for the Pod with the new value for the label MUST be successful. + description: A patch request must be able to update the pod to change the value of an existing Label. Query for the Pod with the new value for the label MUST be successful. - id: pod/spec/container/resources description: Create a Pod with CPU and Memory request and limits. Pod status MUST have QOSClass set to PodQOSGuaranteed. diff --git a/test/conformance/kubeconform/link.go b/test/conformance/kubeconform/link.go index 1bdfe830830..eadb5e8c5b0 100644 --- a/test/conformance/kubeconform/link.go +++ b/test/conformance/kubeconform/link.go @@ -19,9 +19,6 @@ package main import ( "fmt" "io/ioutil" - "os" - "path/filepath" - "regexp" "gopkg.in/yaml.v2" @@ -29,30 +26,21 @@ import ( ) func link(o *options) error { - var behaviorFiles []string behaviorsMapping := make(map[string][]string) var conformanceDataList []behaviors.ConformanceData - err := filepath.Walk(o.behaviorsDir, - func(path string, info os.FileInfo, err error) error { - if err != nil { - return err - } - r, _ := regexp.Compile(".+.yaml$") - if r.MatchString(path) { - behaviorFiles = append(behaviorFiles, path) - } - return nil - }) + behaviorFiles, err := behaviors.BehaviorFileList(o.behaviorsDir) if err != nil { return err } + 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 { @@ -60,17 +48,14 @@ func link(o *options) error { } for _, behaviorFile := range behaviorFiles { - var suite behaviors.Suite - - yamlFile, err := ioutil.ReadFile(behaviorFile) + suite, err := behaviors.LoadSuite(behaviorFile) if err != nil { return err } - err = yaml.UnmarshalStrict(yamlFile, &suite) + err = behaviors.ValidateSuite(suite) if err != nil { - return err + return fmt.Errorf("error validating %s: %q", behaviorFile, err.Error()) } - for _, behavior := range suite.Behaviors { behaviorsMapping[behavior.ID] = nil } @@ -78,12 +63,12 @@ func link(o *options) error { conformanceYaml, err := ioutil.ReadFile(o.testdata) if err != nil { - return err + return fmt.Errorf("%s: %v", o.testdata, err) } err = yaml.Unmarshal(conformanceYaml, &conformanceDataList) if err != nil { - return err + return fmt.Errorf("%s: %v", o.testdata, err) } for _, data := range conformanceDataList {