Skip to content

Commit

Permalink
tiltfile: resources with extra pod selectors or images should be cons…
Browse files Browse the repository at this point in the history
…idered workloads (#3561)
  • Loading branch information
nicks committed Jul 7, 2020
1 parent 63e1d0c commit d4ba52b
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 14 deletions.
16 changes: 8 additions & 8 deletions internal/tiltfile/k8s.go
Expand Up @@ -55,7 +55,7 @@ type k8sResource struct {

resourceDeps []string

nonWorkload bool
manuallyGrouped bool
}

const deprecatedResourceAssemblyV1Warning = "This Tiltfile is using k8s resource assembly version 1, which has been " +
Expand All @@ -74,7 +74,7 @@ type k8sResourceOptions struct {
tiltfilePosition syntax.Position
resourceDeps []string
objects []string
nonWorkload bool
manuallyGrouped bool
}

func (r *k8sResource) addRefSelector(selector container.RefSelector) {
Expand Down Expand Up @@ -417,11 +417,11 @@ func (s *tiltfileState) k8sResourceV2(thread *starlark.Thread, fn *starlark.Buil
}

resourceName := workload
needsToHaveObjects := false
manuallyGrouped := false
if workload == "" {
resourceName = newName
// If a resource doesn't specify a workload then it needs to have objects to be valid
needsToHaveObjects = true
// If a resource doesn't specify an existing workload then it needs to have objects to be valid
manuallyGrouped = true
}

portForwards, err := convertPortForwards(portForwardsVal)
Expand All @@ -448,11 +448,11 @@ func (s *tiltfileState) k8sResourceV2(thread *starlark.Thread, fn *starlark.Buil
return nil, errors.Wrapf(err, "%s: resource_deps", fn.Name())
}

if needsToHaveObjects && len(objects) == 0 {
if manuallyGrouped && len(objects) == 0 {
return nil, fmt.Errorf("k8s_resource doesn't specify a workload or any objects. All non-workload resources must specify 1 or more objects")
}

if needsToHaveObjects && len(objects) > 0 && newName == "" {
if manuallyGrouped && len(objects) > 0 && newName == "" {
return nil, fmt.Errorf("k8s_resource has only non-workload objects but doesn't provide a new_name")
}

Expand All @@ -465,7 +465,7 @@ func (s *tiltfileState) k8sResourceV2(thread *starlark.Thread, fn *starlark.Buil
autoInit: autoInit,
resourceDeps: resourceDeps,
objects: objects,
nonWorkload: needsToHaveObjects,
manuallyGrouped: manuallyGrouped,
}

return starlark.None, nil
Expand Down
11 changes: 8 additions & 3 deletions internal/tiltfile/tiltfile_state.go
Expand Up @@ -675,12 +675,12 @@ func (s *tiltfileState) assembleK8sV2() error {
}

for workload, opts := range s.k8sResourceOptions {
if opts.nonWorkload {
if opts.manuallyGrouped {
r, err := s.makeK8sResource(opts.newName)
if err != nil {
return err
}
r.nonWorkload = true
r.manuallyGrouped = true
s.k8sByName[opts.newName] = r
}
if r, ok := s.k8sByName[workload]; ok {
Expand Down Expand Up @@ -1098,8 +1098,13 @@ func (s *tiltfileState) translateK8s(resources []*k8sResource) ([]model.Manifest
ResourceDependencies: mds,
}

nonWorkload := false
if r.manuallyGrouped {
nonWorkload = len(r.extraPodSelectors) == 0 && len(r.dependencyIDs) == 0
}

k8sTarget, err := k8s.NewTarget(mn.TargetName(), r.entities, s.defaultedPortForwards(r.portForwards),
r.extraPodSelectors, r.dependencyIDs, r.imageRefMap, r.nonWorkload, locators)
r.extraPodSelectors, r.dependencyIDs, r.imageRefMap, nonWorkload, locators)
if err != nil {
return nil, err
}
Expand Down
9 changes: 6 additions & 3 deletions internal/tiltfile/tiltfile_test.go
Expand Up @@ -1447,7 +1447,8 @@ func TestExtraPodSelectors(t *testing.T) {
f.load()

f.assertNextManifest("foo",
extraPodSelectors(labels.Set{"foo": "bar", "baz": "qux"}, labels.Set{"quux": "corge"}))
extraPodSelectors(labels.Set{"foo": "bar", "baz": "qux"}, labels.Set{"quux": "corge"}),
nonWorkload(false))
}

func TestExtraPodSelectorsNotList(t *testing.T) {
Expand All @@ -1465,7 +1466,8 @@ func TestExtraPodSelectorsDict(t *testing.T) {
f.setupExtraPodSelectors("{'foo': 'bar'}")
f.load()
f.assertNextManifest("foo",
extraPodSelectors(labels.Set{"foo": "bar"}))
extraPodSelectors(labels.Set{"foo": "bar"}),
nonWorkload(false))
}

func TestExtraPodSelectorsElementNotDict(t *testing.T) {
Expand Down Expand Up @@ -2166,7 +2168,8 @@ docker_build('tilt.dev/frontend', '.')
`)

f.load()
m := f.assertNextManifest("um")
m := f.assertNextManifest("um",
nonWorkload(false))
assert.Equal(t, "tilt.dev/frontend",
m.ImageTargets[0].Refs.LocalRef().String())
}
Expand Down

0 comments on commit d4ba52b

Please sign in to comment.