Add labels to the test framework. (istio#12819)

* Add basic label support to the test framework.

* Refactor test framework surface area to use fluent-style.

* Apply labels to CircleCI tests & stable integration tests.

* Add early exit support to avoid running setup functions when the label
set can never match.

* Add Citadel tests as presubmit tests.

* Remove environments from label usage.

* Fixup some of the label usages, and convert some of the test entry points.

* Fixup label usage.

* Redisable sidecar tests.

* Accommodate PR feedback.

* Accommodate CR feedback.

* Add more CR fixup.

Author: ozevren

Diff for: .circleci/config.yml

@@ -472,7 +472,7 @@ jobs:
               export PATH=$GOPATH/bin:$PATH
               make localTestEnv
               set -o pipefail
-              make test.integration.local T=-v | tee -a /go/out/tests/build-log.txt
+              make test.integration.local.presubmit T=-v | tee -a /go/out/tests/build-log.txt
         - <<: *recordZeroExitCodeIfTestPassed
         - <<: *recordNonzeroExitCodeIfTestFailed
         - <<: *markJobFinishesOnGCS
@@ -508,7 +508,7 @@ jobs:
           no_output_timeout: 20m
           name: make test.integration.kube
           command: |
-            make test.integration.kube T="-v" | tee -a /go/out/tests/build-log.txt
+            make test.integration.kube.presubmit T="-v" | tee -a /go/out/tests/build-log.txt
       - <<: *recordZeroExitCodeIfTestPassed
       - <<: *recordNonzeroExitCodeIfTestFailed
       - <<: *markJobFinishesOnGCS

Diff for: pkg/test/framework/context.go

@@ -18,19 +18,12 @@ import (
 	"testing"
 
 	"istio.io/istio/pkg/test/framework/components/environment"
-
 	"istio.io/istio/pkg/test/framework/resource"
 )
 
 // SuiteContext contains suite-level items used during runtime.
 type SuiteContext interface {
 	resource.Context
-
-	// Skip indicates that all of the tests in this suite should be skipped.
-	Skip(reason string)
-
-	// Skip indicates that all of the tests in this suite should be skipped.
-	Skipf(reasonfmt string, args ...interface{})
 }
 
 // TestContext is a test-level context that can be created as part of test executing tests.

Diff for: pkg/test/framework/core/flags.go

@@ -19,6 +19,8 @@ import (
 	"fmt"
 	"os"
 
+	"istio.io/istio/pkg/test/framework/label"
+
 	"istio.io/istio/pkg/test/framework/components/environment"
 )
 
@@ -28,15 +30,21 @@ var (
 
 // SettingsFromCommandLine returns settings obtained from command-line flags. flag.Parse must be called before
 // calling this function.
-func SettingsFromCommandLine(testID string) *Settings {
+func SettingsFromCommandLine(testID string) (*Settings, error) {
 	if !flag.Parsed() {
 		panic("flag.Parse must be called before this function")
 	}
 
 	s := settingsFromCommandLine.Clone()
 	s.TestID = testID
 
-	return s
+	f, err := label.ParseSelector(s.SelectorString)
+	if err != nil {
+		return nil, err
+	}
+	s.Selector = f
+
+	return s, nil
 }
 
 // init registers the command-line flags that we can exposed for "go test".
@@ -52,4 +60,7 @@ func init() {
 
 	flag.BoolVar(&settingsFromCommandLine.CIMode, "istio.test.ci", settingsFromCommandLine.CIMode,
 		"Enable CI Mode. Additional logging and state dumping will be enabled.")
+
+	flag.StringVar(&settingsFromCommandLine.SelectorString, "istio.test.select", settingsFromCommandLine.SelectorString,
+		"Comma separatated list of labels for selecting tests to run (e.g. 'foo,+bar-baz').")
 }

Diff for: pkg/test/framework/core/settings.go

@@ -19,6 +19,8 @@ import (
 	"path"
 	"strings"
 
+	"istio.io/istio/pkg/test/framework/label"
+
 	"istio.io/istio/pkg/test/framework/components/environment"
 
 	"github.com/google/uuid"
@@ -48,6 +50,12 @@ type Settings struct {
 	// Local working directory root for creating temporary directories / files in. If left empty,
 	// os.TempDir() will be used.
 	BaseDir string
+
+	// The label selector that the user has specified.
+	SelectorString string
+
+	// The label selector, in parsed form.
+	Selector label.Selector
 }
 
 // RunDir is the name of the dir to output, for this particular run.
@@ -82,11 +90,11 @@ func DefaultSettings() *Settings {
 func (s *Settings) String() string {
 	result := ""
 
-	result += fmt.Sprintf("Environment: %v\n", s.Environment)
-	result += fmt.Sprintf("TestID:      %s\n", s.TestID)
-	result += fmt.Sprintf("RunID:       %s\n", s.RunID.String())
-	result += fmt.Sprintf("NoCleanup:   %v\n", s.NoCleanup)
-	result += fmt.Sprintf("BaseDir:     %s\n", s.BaseDir)
-
+	result += fmt.Sprintf("Environment:  %v\n", s.Environment)
+	result += fmt.Sprintf("TestID:       %s\n", s.TestID)
+	result += fmt.Sprintf("RunID:        %s\n", s.RunID.String())
+	result += fmt.Sprintf("NoCleanup:    %v\n", s.NoCleanup)
+	result += fmt.Sprintf("BaseDir:      %s\n", s.BaseDir)
+	result += fmt.Sprintf("Selector:     %v\n", s.Selector)
 	return result
 }

Diff for: pkg/test/framework/label/filter.go

@@ -0,0 +1,115 @@
+// Copyright 2019 Istio 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 label
+
+import (
+	"fmt"
+	"regexp"
+	"strings"
+)
+
+// Selector is a Set of label filter expressions that get applied together to decide whether tests should be selected
+// for execution or not.
+type Selector struct {
+	// The constraints are and'ed together.
+	present Set
+	absent  Set
+}
+
+var _ fmt.Stringer = Selector{}
+
+func NewSelector(present []Instance, absent []Instance) Selector {
+	return Selector{
+		present: NewSet(present...),
+		absent:  NewSet(absent...),
+	}
+}
+
+var userLabelRegex = regexp.MustCompile("^[a-zA-Z][a-zA-Z0-9_]*(\\.[a-zA-Z0-9_]+)*$")
+
+func ParseSelector(s string) (Selector, error) {
+	var present, absent []Instance
+
+	parts := strings.Split(s, ",")
+	for _, p := range parts {
+		if len(p) == 0 {
+			continue
+		}
+
+		var negative bool
+		switch p[0] {
+		case '-':
+			negative = true
+			p = p[1:]
+		case '+':
+			p = p[1:]
+		}
+
+		if !userLabelRegex.Match([]byte(p)) {
+			return Selector{}, fmt.Errorf("invalid label name: %q", p)
+		}
+
+		l := Instance(p)
+		if !all.contains(l) {
+			return Selector{}, fmt.Errorf("unknown label name: %q", p)
+		}
+
+		if negative {
+			absent = append(absent, l)
+		} else {
+			present = append(present, l)
+		}
+	}
+
+	pSet := NewSet(present...)
+	aSet := NewSet(absent...)
+
+	if pSet.containsAny(aSet) || aSet.containsAny(pSet) {
+		return Selector{}, fmt.Errorf("conflicting selector specification: %q", s)
+	}
+
+	return NewSelector(present, absent), nil
+}
+
+// Selects returns true, if the given label set satisfies the Selector.
+func (f *Selector) Selects(inputs Set) bool {
+	return !inputs.containsAny(f.absent) && inputs.containsAll(f.present)
+}
+
+// Excludes returns false, if the given set of labels, even combined with new ones, could end up satisfying the Selector.
+// It returns false, if Matches would never return true, even if new labels are added to the input set.
+func (f *Selector) Excludes(inputs Set) bool {
+	return inputs.containsAny(f.absent)
+}
+
+func (f Selector) String() string {
+	var result string
+
+	for _, p := range f.present.All() {
+		if result != "" {
+			result += ","
+		}
+		result += "+" + string(p)
+	}
+
+	for _, p := range f.absent.All() {
+		if result != "" {
+			result += ","
+		}
+		result += "-" + string(p)
+	}
+
+	return result
+}

Diff for: pkg/test/framework/label/instance.go

@@ -0,0 +1,105 @@
+// Copyright 2019 Istio 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 label
+
+import (
+	"sort"
+	"strings"
+)
+
+// Instance is a label instance.
+type Instance string
+
+// Set is a set of labels
+type Set map[Instance]struct{}
+
+// NewSet returns a new label set.
+func NewSet(labels ...Instance) Set {
+	s := make(map[Instance]struct{})
+	for _, l := range labels {
+		s[l] = struct{}{}
+	}
+
+	return s
+}
+
+// Add adds the given labels and returns a new, combined set
+func (l Set) Add(labels ...Instance) Set {
+	c := l.Clone()
+	for _, label := range labels {
+		c[label] = struct{}{}
+	}
+
+	return c
+}
+
+// Merge returns a set that is merging of l and s.
+func (l Set) Merge(s Set) Set {
+	c := l.Clone()
+	for k, v := range s {
+		c[k] = v
+	}
+
+	return c
+}
+
+// Clone this set of labels
+func (l Set) Clone() Set {
+	s := make(map[Instance]struct{})
+	for k, v := range l {
+		s[k] = v
+	}
+
+	return s
+}
+
+// All returns all labels in this set.
+func (l Set) All() []Instance {
+	r := make([]Instance, 0, len(l))
+	for label := range l {
+		r = append(r, label)
+	}
+
+	sort.Slice(r, func(i, j int) bool {
+		return strings.Compare(string(r[i]), string(r[j])) < 0
+	})
+	return r
+}
+
+func (l Set) contains(label Instance) bool {
+	_, found := l[label]
+	return found
+}
+
+func (l Set) containsAny(other Set) bool {
+	for l2 := range other {
+		if l.contains(l2) {
+			return true
+		}
+	}
+
+	return false
+}
+
+func (l Set) containsAll(other Set) bool {
+	for l2 := range other {
+		if l.contains(l2) {
+			continue
+		}
+		return false
+	}
+
+	return true
+}

Diff for: pkg/test/framework/setup.go renamed to pkg/test/framework/label/labels.go

@@ -12,19 +12,16 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-package framework
+package label
 
-import (
-	"istio.io/istio/pkg/test/framework/components/environment"
+const (
+	// Presubmit indicates that the test should be run as part of a presubmit run.
+	Presubmit Instance = "presubmit"
+
+	// Postsubmit indicates that the test should be run as part of a postsubmit run.
+	Postsubmit Instance = "postsubmit"
 )
 
-// RequireEnvironment will mark the suite as skipped if the expected environment is not found.
-func RequireEnvironment(n environment.Name) SetupFn { // nolint:interfacer
-	return func(ctx SuiteContext) error {
-		if ctx.Environment().EnvironmentName() != n {
-			ctx.Skipf("RequireEnvironment (%s): Test suite does not support environment: %q",
-				n.String(), ctx.Environment().EnvironmentName())
-		}
-		return nil
-	}
-}
+var all = NewSet(
+	Presubmit,
+	Postsubmit)