k8s: defer extra pod labels conversion to labels.Selector (#4495)

Extra pod labels come in as one or more maps of label keys and values
(`[]labels.Set`). Once converted into a list of `labels.Selector`,
it can be used for matching, but introspecting it is more difficult,
as it requires conversion, which produces a list of
`labels.Requirement`. The biggest problem that introduces is that
`labels.Requirement` is more expressive, so backwards conversion
means a bunch of error-handling for (what should be) impossible
cases.

This just passes the `labels.Set`s as they're parsed from the
`Tiltfile` through and stores them that way on the target in state.
Once they're actually used, they get converted to the actual
`labels.Selector`s for matching.

This will simplify the new intermediary step where they'll get
converted to API label selector objects, so that conversion can
happen without hiccups or excessive boilerplate.

internal/engine/k8swatch/pod_watch.go

@@ -70,7 +70,8 @@ func (w *PodWatcher) diff(ctx context.Context, st store.RStore) podWatchTaskList
 	var extraSelectors []ExtraSelector
 	if len(taskList.watchableNamespaces) > 0 {
 		for _, mt := range state.Targets() {
-			for _, ls := range mt.Manifest.K8sTarget().ExtraPodSelectors {
+			for _, labelSet := range mt.Manifest.K8sTarget().ExtraPodSelectors {
+				ls := labelSet.AsSelector()
 				if !ls.Empty() {
 					extraSelectors = append(extraSelectors, ExtraSelector{name: mt.Manifest.Name, labels: ls})
 				}

internal/engine/k8swatch/pod_watch_test.go

@@ -105,8 +105,8 @@ func TestPodWatchExtraSelectors(t *testing.T) {
 	f := newPWFixture(t)
 	defer f.TearDown()
 
-	ls1 := labels.Set{"foo": "bar"}.AsSelector()
-	ls2 := labels.Set{"baz": "quu"}.AsSelector()
+	ls1 := labels.Set{"foo": "bar"}
+	ls2 := labels.Set{"baz": "quu"}
 	manifest := f.addManifestWithSelectors("server", ls1, ls2)
 
 	f.pw.OnChange(f.ctx, f.store, store.LegacyChangeSummary())
@@ -125,7 +125,7 @@ func TestPodWatchHandleSelectorChange(t *testing.T) {
 	f := newPWFixture(t)
 	defer f.TearDown()
 
-	ls1 := labels.Set{"foo": "bar"}.AsSelector()
+	ls1 := labels.Set{"foo": "bar"}
 	manifest := f.addManifestWithSelectors("server1", ls1)
 
 	f.pw.OnChange(f.ctx, f.store, store.LegacyChangeSummary())
@@ -139,7 +139,7 @@ func TestPodWatchHandleSelectorChange(t *testing.T) {
 	f.assertObservedPods(p)
 	f.clearPods()
 
-	ls2 := labels.Set{"baz": "quu"}.AsSelector()
+	ls2 := labels.Set{"baz": "quu"}
 	manifest2 := f.addManifestWithSelectors("server2", ls2)
 	f.removeManifest("server1")
 
@@ -251,7 +251,7 @@ func TestPodWatchReadd(t *testing.T) {
 	f.assertObservedPods(p)
 }
 
-func (f *pwFixture) addManifestWithSelectors(manifestName string, ls ...labels.Selector) model.Manifest {
+func (f *pwFixture) addManifestWithSelectors(manifestName string, ls ...labels.Set) model.Manifest {
 	state := f.store.LockMutableStateForTesting()
 	m := manifestbuilder.New(f, model.ManifestName(manifestName)).
 		WithK8sYAML(testyaml.SanchoYAML).

internal/k8s/target.go

@@ -28,7 +28,7 @@ func NewTarget(
 	name model.TargetName,
 	entities []K8sEntity,
 	portForwards []model.PortForward,
-	extraPodSelectors []labels.Selector,
+	extraPodSelectors []labels.Set,
 	dependencyIDs []model.TargetID,
 	refInjectCounts map[string]int,
 	podReadinessMode model.PodReadinessMode,

internal/testutils/manifestbuilder/manifestbuilder.go

@@ -25,7 +25,7 @@ type ManifestBuilder struct {
 
 	k8sPodReadiness    model.PodReadinessMode
 	k8sYAML            string
-	k8sPodSelectors    []labels.Selector
+	k8sPodSelectors    []labels.Set
 	k8sImageLocators   []k8s.ImageLocator
 	dcConfigPaths      []string
 	localCmd           string
@@ -71,7 +71,7 @@ func (b ManifestBuilder) WithK8sYAML(yaml string) ManifestBuilder {
 	return b
 }
 
-func (b ManifestBuilder) WithK8sPodSelectors(podSelectors []labels.Selector) ManifestBuilder {
+func (b ManifestBuilder) WithK8sPodSelectors(podSelectors []labels.Set) ManifestBuilder {
 	b.k8sPodSelectors = podSelectors
 	return b
 }

internal/tiltfile/k8s.go

@@ -47,7 +47,7 @@ type k8sResource struct {
 	portForwards []model.PortForward
 
 	// labels for pods that we should watch and associate with this resource
-	extraPodSelectors []labels.Selector
+	extraPodSelectors []labels.Set
 
 	podReadinessMode model.PodReadinessMode
 
@@ -68,7 +68,7 @@ type k8sResourceOptions struct {
 	// if non-empty, how to rename this resource
 	newName           string
 	portForwards      []model.PortForward
-	extraPodSelectors []labels.Selector
+	extraPodSelectors []labels.Set
 	triggerMode       triggerMode
 	autoInit          bool
 	tiltfilePosition  syntax.Position
@@ -339,7 +339,7 @@ func (s *tiltfileState) k8sResource(thread *starlark.Thread, fn *starlark.Builti
 	return starlark.None, nil
 }
 
-func selectorFromSkylarkDict(d *starlark.Dict) (labels.Selector, error) {
+func labelSetFromStarlarkDict(d *starlark.Dict) (labels.Set, error) {
 	ret := make(labels.Set)
 
 	for _, t := range d.Items() {
@@ -356,29 +356,29 @@ func selectorFromSkylarkDict(d *starlark.Dict) (labels.Selector, error) {
 		ret[string(k)] = string(v)
 	}
 	if len(ret) > 0 {
-		return ret.AsSelector(), nil
+		return ret, nil
 	} else {
 		return nil, nil
 	}
 }
 
-func podLabelsFromStarlarkValue(v starlark.Value) ([]labels.Selector, error) {
+func podLabelsFromStarlarkValue(v starlark.Value) ([]labels.Set, error) {
 	if v == nil {
 		return nil, nil
 	}
 
 	switch x := v.(type) {
 	case *starlark.Dict:
-		s, err := selectorFromSkylarkDict(x)
+		s, err := labelSetFromStarlarkDict(x)
 		if err != nil {
 			return nil, err
 		} else if s == nil {
 			return nil, nil
 		} else {
-			return []labels.Selector{s}, nil
+			return []labels.Set{s}, nil
 		}
 	case *starlark.List:
-		var ret []labels.Selector
+		var ret []labels.Set
 
 		it := x.Iterate()
 		defer it.Done()
@@ -388,7 +388,7 @@ func podLabelsFromStarlarkValue(v starlark.Value) ([]labels.Selector, error) {
 			if !ok {
 				return nil, fmt.Errorf("pod labels elements must be dicts; got %T", i)
 			}
-			s, err := selectorFromSkylarkDict(d)
+			s, err := labelSetFromStarlarkDict(d)
 			if err != nil {
 				return nil, err
 			} else if s != nil {

internal/tiltfile/tiltfile_test.go

@@ -6059,13 +6059,12 @@ func k8sObject(name string, kind string) k8sObjectHelper {
 }
 
 type extraPodSelectorsHelper struct {
-	labels []labels.Selector
+	labels []labels.Set
 }
 
-func extraPodSelectors(labels ...labels.Set) extraPodSelectorsHelper {
-	ret := extraPodSelectorsHelper{}
-	for _, ls := range labels {
-		ret.labels = append(ret.labels, ls.AsSelector())
+func extraPodSelectors(labelSets ...labels.Set) extraPodSelectorsHelper {
+	ret := extraPodSelectorsHelper{
+		labels: append([]labels.Set(nil), labelSets...),
 	}
 	return ret
 }

pkg/model/k8s_target.go

@@ -34,7 +34,7 @@ type K8sTarget struct {
 	YAML         string
 	PortForwards []PortForward
 	// labels for pods that we should watch and associate with this resource
-	ExtraPodSelectors []labels.Selector
+	ExtraPodSelectors []labels.Set
 
 	// Each K8s entity should have a display name for user interfaces
 	// that balances brevity and uniqueness

pkg/model/manifest_test.go

@@ -172,20 +172,20 @@ var equalitytests = []struct {
 	{
 		"k8s.ExtraPodSelectors equal",
 		Manifest{}.WithDeployTarget(K8sTarget{
-			ExtraPodSelectors: []labels.Selector{labels.Set{"foo": "bar"}.AsSelector()},
+			ExtraPodSelectors: []labels.Set{{"foo": "bar"}},
 		}),
 		Manifest{}.WithDeployTarget(K8sTarget{
-			ExtraPodSelectors: []labels.Selector{labels.Set{"foo": "bar"}.AsSelector()},
+			ExtraPodSelectors: []labels.Set{{"foo": "bar"}},
 		}),
 		false,
 	},
 	{
 		"k8s.ExtraPodSelectors unequal",
 		Manifest{}.WithDeployTarget(K8sTarget{
-			ExtraPodSelectors: []labels.Selector{labels.Set{"foo": "bar"}.AsSelector()},
+			ExtraPodSelectors: []labels.Set{{"foo": "bar"}},
 		}),
 		Manifest{}.WithDeployTarget(K8sTarget{
-			ExtraPodSelectors: []labels.Selector{labels.Set{"foo": "baz"}.AsSelector()},
+			ExtraPodSelectors: []labels.Set{{"foo": "baz"}},
 		}),
 		true,
 	},