Skip to content

Commit

Permalink
kubernetesapply: add support for discovery_strategy='selectors-only' (#…
Browse files Browse the repository at this point in the history
…4881)

You can specify this in the tiltfile as

k8s_resource(..., discovery_strategy='selectors-only')

and tilt will create kdisco objects with namespace-only watches (no UIDs).

Fixes #4867
  • Loading branch information
nicks committed Aug 24, 2021
1 parent 4ceda94 commit c574cd8
Show file tree
Hide file tree
Showing 12 changed files with 149 additions and 9 deletions.
2 changes: 1 addition & 1 deletion internal/controllers/core/kubernetesapply/disco.go
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ func (r *Reconciler) toDesiredKubernetesDiscovery(ka *v1alpha1.KubernetesApply)
func (r *Reconciler) toWatchRefs(ka *v1alpha1.KubernetesApply) ([]v1alpha1.KubernetesWatchRef, error) {
seenNamespaces := make(map[k8s.Namespace]bool)
var result []v1alpha1.KubernetesWatchRef
if ka.Status.ResultYAML != "" {
if ka.Status.ResultYAML != "" && ka.Spec.DiscoveryStrategy != v1alpha1.KubernetesDiscoveryStrategySelectorsOnly {
deployed, err := k8s.ParseYAMLFromString(ka.Status.ResultYAML)
if err != nil {
return nil, err
Expand Down
33 changes: 33 additions & 0 deletions internal/controllers/core/kubernetesapply/disco_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,39 @@ func TestCreateAndUpdateDisco(t *testing.T) {
assert.Contains(f.T(), ka.Status.ResultYAML, fmt.Sprintf("uid: %s", uid2))
}

func TestDiscoveryStrategySelectorsOnly(t *testing.T) {
f := newFixture(t)
ka := v1alpha1.KubernetesApply{
ObjectMeta: metav1.ObjectMeta{
Name: "a",
},
Spec: v1alpha1.KubernetesApplySpec{
YAML: testyaml.SanchoYAML,
DiscoveryStrategy: v1alpha1.KubernetesDiscoveryStrategySelectorsOnly,
KubernetesDiscoveryTemplateSpec: &v1alpha1.KubernetesDiscoveryTemplateSpec{
ExtraSelectors: []metav1.LabelSelector{
metav1.LabelSelector{
MatchLabels: map[string]string{"app": "tilt-site"},
},
},
},
},
}
f.Create(&ka)

f.MustReconcile(types.NamespacedName{Name: "a"})
f.MustGet(types.NamespacedName{Name: "a"}, &ka)

var kd v1alpha1.KubernetesDiscovery
f.MustGet(types.NamespacedName{Name: "a"}, &kd)
assert.Equal(f.T(), 1, len(kd.Spec.Watches))

// Make sure we don't contain UID watches
assert.Equal(t, "", kd.Spec.Watches[0].UID)
assert.Equal(t, "default", kd.Spec.Watches[0].Namespace)
assert.Equal(t, map[string]string{"app": "tilt-site"}, kd.Spec.ExtraSelectors[0].MatchLabels)
}

func TestCreateAndDeleteDisco(t *testing.T) {
f := newFixture(t)
ka := v1alpha1.KubernetesApply{
Expand Down
15 changes: 13 additions & 2 deletions internal/k8s/target.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package k8s

import (
"context"
"fmt"
"strings"
"time"
Expand All @@ -25,7 +26,8 @@ func MustTarget(name model.TargetName, yaml string) model.K8sTarget {
panic(fmt.Errorf("MustTarget: %v", err))
}
target, err := NewTarget(name, entities, nil, nil, nil, nil,
nil, model.PodReadinessIgnore, nil, nil, model.UpdateSettings{})
nil, model.PodReadinessIgnore, v1alpha1.KubernetesDiscoveryStrategyDefault,
nil, nil, model.UpdateSettings{})
if err != nil {
panic(fmt.Errorf("MustTarget: %v", err))
}
Expand All @@ -41,6 +43,7 @@ func NewTarget(
imageTargets []model.ImageTarget,
refInjectCounts map[string]int,
podReadinessMode model.PodReadinessMode,
discoveryStrategy v1alpha1.KubernetesDiscoveryStrategy,
allLocators []ImageLocator,
links []model.Link,
updateSettings model.UpdateSettings) (model.K8sTarget, error) {
Expand Down Expand Up @@ -82,6 +85,13 @@ func NewTarget(
},
},
PortForwardTemplateSpec: toPortForwardTemplateSpec(portForwards),
DiscoveryStrategy: discoveryStrategy,
}

kapp := &v1alpha1.KubernetesApply{Spec: apply}
errors := kapp.Validate(context.TODO())
if errors != nil {
return model.K8sTarget{}, errors.ToAggregate()
}

return model.K8sTarget{
Expand All @@ -98,7 +108,8 @@ func NewTarget(

func NewK8sOnlyManifest(name model.ManifestName, entities []K8sEntity, allLocators []ImageLocator) (model.Manifest, error) {
kTarget, err := NewTarget(name.TargetName(), entities, nil, nil, nil, nil,
nil, model.PodReadinessIgnore, allLocators, nil, model.UpdateSettings{})
nil, model.PodReadinessIgnore, v1alpha1.KubernetesDiscoveryStrategyDefault,
allLocators, nil, model.UpdateSettings{})
if err != nil {
return model.Manifest{}, err
}
Expand Down
3 changes: 2 additions & 1 deletion internal/k8s/target_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,14 @@ import (
"github.com/stretchr/testify/require"

"github.com/tilt-dev/tilt/internal/k8s/testyaml"
"github.com/tilt-dev/tilt/pkg/apis/core/v1alpha1"
"github.com/tilt-dev/tilt/pkg/model"
)

func TestNewTargetSortsK8sEntities(t *testing.T) {
entities := MustParseYAMLFromString(t, testyaml.OutOfOrderYaml)
targ, err := NewTarget("foo", entities, nil, nil, nil, nil,
nil, model.PodReadinessWait, nil, nil, model.UpdateSettings{})
nil, model.PodReadinessWait, v1alpha1.KubernetesDiscoveryStrategyDefault, nil, nil, model.UpdateSettings{})
require.NoError(t, err)

expectedKindOrder := []string{"PersistentVolume", "PersistentVolumeClaim", "ConfigMap", "Service", "StatefulSet", "Job", "Pod"}
Expand Down
7 changes: 7 additions & 0 deletions internal/tiltfile/k8s.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"github.com/tilt-dev/tilt/internal/tiltfile/io"
tiltfile_k8s "github.com/tilt-dev/tilt/internal/tiltfile/k8s"
"github.com/tilt-dev/tilt/internal/tiltfile/value"
"github.com/tilt-dev/tilt/pkg/apis/core/v1alpha1"
"github.com/tilt-dev/tilt/pkg/model"
)

Expand Down Expand Up @@ -51,6 +52,8 @@ type k8sResource struct {

podReadinessMode model.PodReadinessMode

discoveryStrategy v1alpha1.KubernetesDiscoveryStrategy

dependencyIDs []model.TargetID

triggerMode triggerMode
Expand Down Expand Up @@ -79,6 +82,7 @@ type k8sResourceOptions struct {
objects []string
manuallyGrouped bool
podReadinessMode model.PodReadinessMode
discoveryStrategy v1alpha1.KubernetesDiscoveryStrategy
links []model.Link
labels map[string]string
}
Expand Down Expand Up @@ -269,6 +273,7 @@ func (s *tiltfileState) k8sResource(thread *starlark.Thread, fn *starlark.Builti
var links links.LinkList
var autoInit = value.BoolOrNone{Value: true}
var labels value.LabelSet
var discoveryStrategy tiltfile_k8s.DiscoveryStrategy

if err := s.unpackArgs(fn.Name(), args, kwargs,
"workload?", &workload,
Expand All @@ -282,6 +287,7 @@ func (s *tiltfileState) k8sResource(thread *starlark.Thread, fn *starlark.Builti
"pod_readiness?", &podReadinessMode,
"links?", &links,
"labels?", &labels,
"discovery_strategy?", &discoveryStrategy,
); err != nil {
return nil, err
}
Expand Down Expand Up @@ -341,6 +347,7 @@ func (s *tiltfileState) k8sResource(thread *starlark.Thread, fn *starlark.Builti
podReadinessMode: podReadinessMode.Value,
links: links.Links,
labels: labelMap,
discoveryStrategy: v1alpha1.KubernetesDiscoveryStrategy(discoveryStrategy),
})

return starlark.None, nil
Expand Down
32 changes: 32 additions & 0 deletions internal/tiltfile/k8s/types.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
package k8s

import (
"fmt"

"go.starlark.net/starlark"

"github.com/tilt-dev/tilt/internal/tiltfile/value"
"github.com/tilt-dev/tilt/pkg/apis/core/v1alpha1"
)

// Deserializing discovery strategy from starlark values.
type DiscoveryStrategy v1alpha1.KubernetesDiscoveryStrategy

func (ds *DiscoveryStrategy) Unpack(v starlark.Value) error {
s, ok := value.AsString(v)
if !ok {
return fmt.Errorf("Must be a string. Got: %s", v.Type())
}

kdStrategy := v1alpha1.KubernetesDiscoveryStrategy(s)
if !(kdStrategy == "" ||
kdStrategy == v1alpha1.KubernetesDiscoveryStrategyDefault ||
kdStrategy == v1alpha1.KubernetesDiscoveryStrategySelectorsOnly) {
return fmt.Errorf("Invalid. Must be one of: %q, %q",
v1alpha1.KubernetesDiscoveryStrategyDefault,
v1alpha1.KubernetesDiscoveryStrategySelectorsOnly)
}

*ds = DiscoveryStrategy(kdStrategy)
return nil
}
4 changes: 4 additions & 0 deletions internal/tiltfile/tiltfile_state.go
Original file line number Diff line number Diff line change
Expand Up @@ -716,6 +716,9 @@ func (s *tiltfileState) assembleK8s() error {
if opts.podReadinessMode != model.PodReadinessNone {
r.podReadinessMode = opts.podReadinessMode
}
if opts.discoveryStrategy != "" {
r.discoveryStrategy = opts.discoveryStrategy
}
r.portForwards = append(r.portForwards, opts.portForwards...)
if opts.triggerMode != TriggerModeUnset {
r.triggerMode = opts.triggerMode
Expand Down Expand Up @@ -1071,6 +1074,7 @@ func (s *tiltfileState) translateK8s(resources []*k8sResource, updateSettings mo
k8sTarget, err := k8s.NewTarget(mn.TargetName(), r.entities,
s.defaultedPortForwards(r.portForwards), r.extraPodSelectors,
r.dependencyIDs, iTargets, r.imageRefMap, s.inferPodReadinessMode(r),
r.discoveryStrategy,
locators, r.links, updateSettings)
if err != nil {
return nil, err
Expand Down
32 changes: 32 additions & 0 deletions internal/tiltfile/tiltfile_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1763,6 +1763,36 @@ k8s_resource(new_name='config', objects=['config'])
)
}

func TestK8sDiscoveryStrategy(t *testing.T) {
f := newFixture(t)
defer f.TearDown()

f.yaml("foo.yaml", deployment("foo", image("gcr.io/foo:stable")))
f.file("Tiltfile", `
k8s_yaml('foo.yaml')
k8s_resource('foo', discovery_strategy='selectors-only')
`)

f.load("foo")
f.assertNextManifest("foo",
deployment("foo"),
v1alpha1.KubernetesDiscoveryStrategySelectorsOnly,
)
}

func TestK8sDiscoveryStrategyInvalid(t *testing.T) {
f := newFixture(t)
defer f.TearDown()

f.yaml("foo.yaml", deployment("foo", image("gcr.io/foo:stable")))
f.file("Tiltfile", `
k8s_yaml('foo.yaml')
k8s_resource('foo', discovery_strategy='typo')
`)

f.loadErrString("Invalid. Must be one of: \"default\", \"selectors-only\"")
}

func TestPodReadinessOverrideDeployment(t *testing.T) {
f := newFixture(t)
defer f.TearDown()
Expand Down Expand Up @@ -6205,6 +6235,8 @@ func (f *fixture) assertNextManifest(name model.ManifestName, opts ...interface{
if !found {
f.t.Fatalf("deployment %v not found in yaml %q", opt.name, yaml)
}
case v1alpha1.KubernetesDiscoveryStrategy:
assert.Equal(f.t, opt, m.K8sTarget().DiscoveryStrategy)
case podReadinessHelper:
assert.Equal(f.t, opt.podReadiness, m.K8sTarget().PodReadinessMode)
case namespaceHelper:
Expand Down
4 changes: 4 additions & 0 deletions pkg/apis/core/v1alpha1/generated.proto

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

15 changes: 13 additions & 2 deletions pkg/apis/core/v1alpha1/kubernetesapply_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package v1alpha1

import (
"context"
fmt "fmt"
"time"

metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand Down Expand Up @@ -187,10 +188,20 @@ func (in *KubernetesApply) IsStorageVersion() bool {
func (in *KubernetesApply) Validate(ctx context.Context) field.ErrorList {
var fieldErrors field.ErrorList
if in.Spec.YAML == "" {
fieldErrors = append(fieldErrors, field.Required(field.NewPath("yaml"), "cannot be empty"))
fieldErrors = append(fieldErrors, field.Required(field.NewPath("spec.yaml"), "cannot be empty"))
}

// TODO(nick): Validate the image locators as well.
kdStrategy := in.Spec.DiscoveryStrategy
if !(kdStrategy == "" ||
kdStrategy == KubernetesDiscoveryStrategyDefault ||
kdStrategy == KubernetesDiscoveryStrategySelectorsOnly) {
fieldErrors = append(fieldErrors, field.Invalid(
field.NewPath("spec.discoveryStrategy"),
kdStrategy,
fmt.Sprintf("Must be one of: %s, %s",
KubernetesDiscoveryStrategyDefault,
KubernetesDiscoveryStrategySelectorsOnly)))
}

return fieldErrors
}
Expand Down
8 changes: 7 additions & 1 deletion pkg/apis/core/v1alpha1/kubernetesdiscovery_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,12 +97,18 @@ type KubernetesWatchRef struct {
// UID is a Kubernetes object UID.
//
// It should either be the exact object UID or the transitive owner.
UID string `json:"uid" protobuf:"bytes,1,opt,name=uid"`
//
// +optional
UID string `json:"uid,omitempty" protobuf:"bytes,1,opt,name=uid"`

// Namespace is the Kubernetes namespace for discovery. Required.
Namespace string `json:"namespace" protobuf:"bytes,2,opt,name=namespace"`

// Name is the Kubernetes object name.
//
// This is not directly used in discovery; it is extra metadata.
//
// +optional
Name string `json:"name,omitempty" protobuf:"bytes,3,opt,name=name"`
}

Expand Down
3 changes: 1 addition & 2 deletions pkg/openapi/zz_generated.openapi.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit c574cd8

Please sign in to comment.