model: move k8s-specific info to model.deployInfo [ch1136] (#871)

* prelim code reorg

* yaml moved over

* all k8s properties off manifest

* stray code

internal/engine/image_build_and_deployer.go

@@ -22,12 +22,6 @@ import (
 	"k8s.io/api/core/v1"
 )
 
-type deployableImageManifest interface {
-	K8sYAML() string
-	ManifestName() model.ManifestName
-	DockerRef() reference.Named
-}
-
 var _ BuildAndDeployer = &ImageBuildAndDeployer{}
 
 type ImageBuildAndDeployer struct {
@@ -83,7 +77,7 @@ func (ibd *ImageBuildAndDeployer) BuildAndDeploy(ctx context.Context, manifest m
 	ps := build.NewPipelineState(ctx, numStages)
 	defer func() { ps.End(ctx, err) }()
 
-	err = manifest.ValidateK8sManifest()
+	err = manifest.ValidateDockerK8sManifest()
 	if err != nil {
 		return store.BuildResult{}, err
 	}
@@ -182,15 +176,23 @@ func (ibd *ImageBuildAndDeployer) build(ctx context.Context, manifest model.Mani
 }
 
 // Returns: the entities deployed and the namespace of the pod with the given image name/tag.
-func (ibd *ImageBuildAndDeployer) deploy(ctx context.Context, ps *build.PipelineState, manifest deployableImageManifest, ref reference.NamedTagged) ([]k8s.K8sEntity, k8s.Namespace, error) {
+func (ibd *ImageBuildAndDeployer) deploy(ctx context.Context, ps *build.PipelineState, manifest model.Manifest, ref reference.NamedTagged) ([]k8s.K8sEntity, k8s.Namespace, error) {
+	k8sInfo := manifest.K8sInfo()
+	if k8sInfo.Empty() {
+		// If a non-yaml manifest reaches this code, something is wrong.
+		// If we change BaD structure such that that might reasonably happen,
+		// this should be a `RedirectToNextBuilder` error.
+		return nil, "", fmt.Errorf("manifest %s has no k8s deploy info", manifest.Name)
+	}
+
 	ps.StartPipelineStep(ctx, "Deploying")
 	defer ps.EndPipelineStep(ctx)
 
 	ps.StartBuildStep(ctx, "Parsing Kubernetes config YAML")
 
 	// TODO(nick): The parsed YAML should probably be a part of the model?
 	// It doesn't make much sense to re-parse it and inject labels on every deploy.
-	entities, err := k8s.ParseYAMLFromString(manifest.K8sYAML())
+	entities, err := k8s.ParseYAMLFromString(k8sInfo.YAML)
 	if err != nil {
 		return nil, "", err
 	}

internal/engine/image_build_and_deployer_test.go

@@ -67,7 +67,8 @@ func TestDeployTwinImages(t *testing.T) {
 	f := newIBDFixture(t)
 	defer f.TearDown()
 
-	manifest := NewSanchoManifest().AppendK8sYAML(SanchoTwinYAML)
+	sancho := NewSanchoManifest()
+	manifest := sancho.WithDeployInfo(sancho.K8sInfo().AppendYAML(SanchoTwinYAML))
 	result, err := f.ibd.BuildAndDeploy(f.ctx, manifest, store.BuildStateClean)
 	assert.NoError(t, err)
 

internal/engine/parse.go

@@ -6,9 +6,13 @@ import (
 )
 
 func ParseYAMLFromManifests(manifests ...model.Manifest) ([]k8s.K8sEntity, error) {
-	allEntities := []k8s.K8sEntity{}
+	var allEntities []k8s.K8sEntity
 	for _, m := range manifests {
-		entities, err := k8s.ParseYAMLFromString(m.K8sYAML())
+		k8sInfo := m.K8sInfo()
+		if k8sInfo.Empty() {
+			continue
+		}
+		entities, err := k8s.ParseYAMLFromString(k8sInfo.YAML)
 		if err != nil {
 			return nil, err
 		}

internal/engine/portforwardcontroller.go

@@ -124,17 +124,14 @@ type portForwardEntry struct {
 	cancel             func()
 }
 
-type portForwardableManifest interface {
-	PortForwards() []model.PortForward
-}
-
 // Extract the port-forward specs from the manifest. If any of them
 // have ContainerPort = 0, populate them with the default port in the pod spec.
 // Quietly drop forwards that we can't populate.
-func PopulatePortForwards(m portForwardableManifest, pod store.Pod) []model.PortForward {
+func PopulatePortForwards(m model.Manifest, pod store.Pod) []model.PortForward {
 	cPorts := pod.ContainerPorts
-	forwards := make([]model.PortForward, 0, len(m.PortForwards()))
-	for _, forward := range m.PortForwards() {
+	fwds := m.K8sInfo().PortForwards
+	forwards := make([]model.PortForward, 0, len(fwds))
+	for _, forward := range fwds {
 		if forward.ContainerPort == 0 {
 			if len(cPorts) == 0 {
 				continue

internal/engine/portforwardcontroller_test.go

@@ -20,10 +20,12 @@ func TestPortForward(t *testing.T) {
 	m := model.Manifest{
 		Name: "fe",
 	}
-	m = m.WithPortForwards([]model.PortForward{
-		{
-			LocalPort:     8080,
-			ContainerPort: 8081,
+	m = m.WithDeployInfo(model.K8sInfo{
+		PortForwards: []model.PortForward{
+			{
+				LocalPort:     8080,
+				ContainerPort: 8081,
+			},
 		},
 	})
 	state.ManifestStates["fe"] = &store.ManifestState{
@@ -66,9 +68,11 @@ func TestPortForwardAutoDiscovery(t *testing.T) {
 	m := model.Manifest{
 		Name: "fe",
 	}
-	m = m.WithPortForwards([]model.PortForward{
-		{
-			LocalPort: 8080,
+	m = m.WithDeployInfo(model.K8sInfo{
+		PortForwards: []model.PortForward{
+			{
+				LocalPort: 8080,
+			},
 		},
 	})
 	state.ManifestStates["fe"] = &store.ManifestState{

internal/engine/testdata_test.go

@@ -39,7 +39,7 @@ func NewSanchoManifest() model.Manifest {
 		Entrypoint: model.Cmd{Argv: []string{"/go/bin/sancho"}},
 	}
 
-	m = m.WithDockerRef(SanchoRef).WithK8sYAML(SanchoYAML)
+	m = m.WithDockerRef(SanchoRef).WithDeployInfo(model.K8sInfo{YAML: SanchoYAML})
 
 	return m
 }
@@ -51,7 +51,7 @@ func NewSanchoStaticManifest() model.Manifest {
 		StaticBuildPath:  "/path/to/build",
 	}
 
-	m = m.WithDockerRef(SanchoRef).WithK8sYAML(SanchoYAML)
+	m = m.WithDockerRef(SanchoRef).WithDeployInfo(model.K8sInfo{YAML: SanchoYAML})
 	return m
 }
 

internal/engine/upper.go

@@ -501,7 +501,7 @@ func populateContainerStatus(ctx context.Context, ms *store.ManifestState, podIn
 	podInfo.ContainerPorts = ports
 
 	forwards := PopulatePortForwards(ms.Manifest, *podInfo)
-	if len(forwards) < len(ms.Manifest.PortForwards()) {
+	if len(forwards) < len(ms.Manifest.K8sInfo().PortForwards) {
 		logger.Get(ctx).Infof(
 			"WARNING: Resource %s is using port forwards, but no container ports on pod %s",
 			ms.Manifest.Name, podInfo.PodID)

internal/engine/upper_test.go

@@ -446,7 +446,7 @@ func TestRebuildDockerfileViaImageBuild(t *testing.T) {
 	// Second call: new manifest!
 	call = f.nextCall("new manifest")
 	assert.Equal(t, "FROM iron/go:dev", call.manifest.BaseDockerfile)
-	assert.Equal(t, testyaml.SnackYAMLPostConfig, call.manifest.K8sYAML())
+	assert.Equal(t, testyaml.SnackYAMLPostConfig, call.manifest.K8sInfo().YAML)
 
 	// Since the manifest changed, we cleared the previous build state to force an image build
 	assert.False(t, call.state.HasImage())

internal/model/deploy_info.go

@@ -1,6 +1,10 @@
 package model
 
-import "reflect"
+import (
+	"reflect"
+
+	"github.com/windmilleng/tilt/internal/yaml"
+)
 
 type deployInfo interface {
 	deployInfo()
@@ -14,3 +18,20 @@ type DCInfo struct {
 
 func (DCInfo) deployInfo()    {}
 func (dc DCInfo) Empty() bool { return reflect.DeepEqual(dc, DCInfo{}) }
+
+type K8sInfo struct {
+	YAML         string
+	PortForwards []PortForward
+}
+
+func (K8sInfo) deployInfo()     {}
+func (k8s K8sInfo) Empty() bool { return reflect.DeepEqual(k8s, K8sInfo{}) }
+
+func (k8s K8sInfo) AppendYAML(y string) K8sInfo {
+	if k8s.YAML == "" {
+		k8s.YAML = y
+	} else {
+		k8s.YAML = yaml.ConcatYAML(k8s.YAML, y)
+	}
+	return k8s
+}

internal/model/manifest.go

@@ -9,7 +9,6 @@ import (
 
 	"github.com/docker/distribution/reference"
 	"github.com/windmilleng/tilt/internal/sliceutils"
-	"github.com/windmilleng/tilt/internal/yaml"
 )
 
 type ManifestName string
@@ -25,14 +24,11 @@ type Manifest struct {
 	// TODO(maia): buildInfo
 
 	// Info needed to deploy. Can be k8s yaml, docker compose, etc.
-	// TODO(maia): move yaml stuff into here
 	deployInfo deployInfo
 
-	// Properties for all k8s builds
-	k8sYaml      string
-	dockerRef    reference.Named
-	portForwards []PortForward
-	cachePaths   []string
+	// All docker stuff
+	cachePaths []string
+	dockerRef  reference.Named
 
 	// Properties for fast_build (builds that support
 	// iteration based on past artifacts)
@@ -66,6 +62,19 @@ func (m Manifest) IsDC() bool {
 	return !m.DCInfo().Empty()
 }
 
+func (m Manifest) K8sInfo() K8sInfo {
+	switch info := m.deployInfo.(type) {
+	case K8sInfo:
+		return info
+	default:
+		return K8sInfo{}
+	}
+}
+
+func (m Manifest) IsK8s() bool {
+	return !m.K8sInfo().Empty()
+}
+
 func (m Manifest) WithDeployInfo(info deployInfo) Manifest {
 	m.deployInfo = info
 	return m
@@ -125,12 +134,14 @@ func (m Manifest) Validate() error {
 	return nil
 }
 
-func (m Manifest) ValidateK8sManifest() error {
+// ValidateDockerK8sManifest indicates whether this manifest is a valid Docker-buildable &
+// k8s-deployable manifest.
+func (m Manifest) ValidateDockerK8sManifest() error {
 	if m.dockerRef == nil {
 		return fmt.Errorf("[validateK8sManifest] manifest %q missing image ref", m.Name)
 	}
 
-	if m.K8sYAML() == "" {
+	if m.K8sInfo().YAML == "" {
 		return fmt.Errorf("[validateK8sManifest] manifest %q missing k8s YAML", m.Name)
 	}
 
@@ -148,12 +159,11 @@ func (m Manifest) ValidateK8sManifest() error {
 }
 
 func (m1 Manifest) Equal(m2 Manifest) bool {
-	primitivesMatch := m1.Name == m2.Name && m1.k8sYaml == m2.k8sYaml && m1.dockerRef == m2.dockerRef && m1.BaseDockerfile == m2.BaseDockerfile && m1.StaticDockerfile == m2.StaticDockerfile && m1.StaticBuildPath == m2.StaticBuildPath && m1.tiltFilename == m2.tiltFilename
+	primitivesMatch := m1.Name == m2.Name && m1.dockerRef == m2.dockerRef && m1.BaseDockerfile == m2.BaseDockerfile && m1.StaticDockerfile == m2.StaticDockerfile && m1.StaticBuildPath == m2.StaticBuildPath && m1.tiltFilename == m2.tiltFilename
 	entrypointMatch := m1.Entrypoint.Equal(m2.Entrypoint)
 	mountsMatch := reflect.DeepEqual(m1.Mounts, m2.Mounts)
 	reposMatch := reflect.DeepEqual(m1.repos, m2.repos)
 	stepsMatch := m1.stepsEqual(m2.Steps)
-	portForwardsMatch := reflect.DeepEqual(m1.portForwards, m2.portForwards)
 	dockerignoresMatch := reflect.DeepEqual(m1.dockerignores, m2.dockerignores)
 	buildArgsMatch := reflect.DeepEqual(m1.StaticBuildArgs, m2.StaticBuildArgs)
 	cachePathsMatch := stringSlicesEqual(m1.cachePaths, m2.cachePaths)
@@ -162,16 +172,20 @@ func (m1 Manifest) Equal(m2 Manifest) bool {
 	dc2 := m2.DCInfo()
 	dockerComposeEqual := reflect.DeepEqual(dc1, dc2)
 
+	k8s1 := m1.K8sInfo()
+	k8s2 := m2.K8sInfo()
+	k8sEqual := reflect.DeepEqual(k8s1, k8s2)
+
 	return primitivesMatch &&
 		entrypointMatch &&
 		mountsMatch &&
 		reposMatch &&
-		portForwardsMatch &&
 		stepsMatch &&
 		buildArgsMatch &&
 		cachePathsMatch &&
 		dockerignoresMatch &&
-		dockerComposeEqual
+		dockerComposeEqual &&
+		k8sEqual
 }
 
 func stringSlicesEqual(a, b []string) bool {
@@ -230,15 +244,6 @@ func (m Manifest) LocalRepos() []LocalGithubRepo {
 	return m.repos
 }
 
-func (m Manifest) WithPortForwards(pf []PortForward) Manifest {
-	m.portForwards = pf
-	return m
-}
-
-func (m Manifest) PortForwards() []PortForward {
-	return m.portForwards
-}
-
 func (m Manifest) TiltFilename() string {
 	return m.tiltFilename
 }
@@ -248,26 +253,6 @@ func (m Manifest) WithTiltFilename(f string) Manifest {
 	return m
 }
 
-func (m Manifest) K8sYAML() string {
-	return m.k8sYaml
-}
-
-func (m Manifest) AppendK8sYAML(y string) Manifest {
-	if m.k8sYaml == "" {
-		return m.WithK8sYAML(y)
-	}
-	if y == "" {
-		return m
-	}
-
-	return m.WithK8sYAML(yaml.ConcatYAML(m.k8sYaml, y))
-}
-
-func (m Manifest) WithK8sYAML(y string) Manifest {
-	m.k8sYaml = y
-	return m
-}
-
 func (m Manifest) DockerRef() reference.Named {
 	return m.dockerRef
 }