defer manifest generation to build initializer

integration/init_test.go

@@ -75,12 +75,12 @@ func TestInitManifestGeneration(t *testing.T) {
 			expectedManifestPaths: []string{"deployment.yaml"},
 		},
 		// TODO(nkubala): add this back when the --force flag is fixed
-		// {
-		// 	name:                  "microservices",
-		// 	dir:                   "testdata/init/microservices",
-		// 	args:                  []string{"--XXenableManifestGeneration"},
-		// 	expectedManifestPaths: []string{"leeroy-web/deployment.yaml", "leeroy-app/deployment.yaml"},
-		// },
+		{
+			name:                  "microservices",
+			dir:                   "testdata/init/microservices",
+			args:                  []string{"--XXenableManifestGeneration"},
+			expectedManifestPaths: []string{"leeroy-web/deployment.yaml", "leeroy-app/deployment.yaml"},
+		},
 	}
 
 	for _, test := range tests {

pkg/skaffold/initializer/build/builders.go

@@ -72,13 +72,8 @@ type Initializer interface {
 	BuilderImagePairs() []BuilderImagePair
 	// PrintAnalysis writes the project analysis to the provided out stream
 	PrintAnalysis(io.Writer) error
-	// GeneratedPairs returns all builder/image pairs with images generated by skaffold.
-	// Each of these pairs contains a path at which a k8s manifest should be generated.
-	GeneratedPairs() []GeneratedBuilderImagePair
-	// Resolve should be called by the init controller once all generated pairs have
-	// been handled, to signal to the build initializer that all
-	// builder/image pairs have been resolved.
-	Resolve()
+	// GenerateManifests generates image names and manifests for all unresolved pairs
+	GenerateManifests() (map[GeneratedBuilderImagePair][]byte, error)
 }
 
 type emptyBuildInitializer struct {
@@ -100,12 +95,10 @@ func (e *emptyBuildInitializer) PrintAnalysis(io.Writer) error {
 	return nil
 }
 
-func (e *emptyBuildInitializer) GeneratedPairs() []GeneratedBuilderImagePair {
-	return nil
+func (e *emptyBuildInitializer) GenerateManifests() (map[GeneratedBuilderImagePair][]byte, error) {
+	return nil, nil
 }
 
-func (e *emptyBuildInitializer) Resolve() {}
-
 func NewInitializer(builders []InitBuilder, c config.Config) Initializer {
 	switch {
 	case c.SkipBuild:

pkg/skaffold/initializer/build/builders_test.go

@@ -19,12 +19,13 @@ package build
 import (
 	"testing"
 
-	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/initializer/prompt"
-	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/warnings"
+	"github.com/google/go-cmp/cmp"
 
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/build/buildpacks"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/build/jib"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/docker"
+	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/initializer/prompt"
+	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/warnings"
 	"github.com/GoogleContainerTools/skaffold/testutil"
 )
 
@@ -79,7 +80,7 @@ func TestResolveBuilderImages(t *testing.T) {
 						Builder:   jib.ArtifactConfig{BuilderName: "Jib Maven Plugin", File: "pom.xml", Project: "project"},
 						ImageName: "pom.xml-image",
 					},
-					ManifestPath: ".",
+					ManifestPath: "deployment.yaml",
 				},
 			},
 		},
@@ -114,7 +115,7 @@ func TestResolveBuilderImages(t *testing.T) {
 						Builder:   docker.ArtifactConfig{File: "foo"},
 						ImageName: "foo-image",
 					},
-					ManifestPath: ".",
+					ManifestPath: "deployment.yaml",
 				},
 			},
 			shouldMakeChoice: false,
@@ -139,7 +140,7 @@ func TestResolveBuilderImages(t *testing.T) {
 			}
 			err := initializer.resolveBuilderImages()
 			t.CheckErrorAndDeepEqual(test.shouldErr, err, test.expectedPairs, initializer.BuilderImagePairs())
-			t.CheckDeepEqual(test.expectedGeneratedPairs, initializer.GeneratedPairs())
+			t.CheckDeepEqual(test.expectedGeneratedPairs, initializer.generatedBuilderImagePairs, cmp.AllowUnexported())
 		})
 	}
 }

pkg/skaffold/initializer/build/cli.go

@@ -62,12 +62,10 @@ func (c *cliBuildInitializer) PrintAnalysis(out io.Writer) error {
 	return printAnalysis(out, c.enableNewFormat, c.skipBuild, c.builderImagePairs, c.builders, nil)
 }
 
-func (c *cliBuildInitializer) GeneratedPairs() []GeneratedBuilderImagePair {
-	return nil
+func (c *cliBuildInitializer) GenerateManifests() (map[GeneratedBuilderImagePair][]byte, error) {
+	return nil, nil
 }
 
-func (c *cliBuildInitializer) Resolve() {}
-
 func (c *cliBuildInitializer) processCliArtifacts() error {
 	pairs, err := processCliArtifacts(c.cliArtifacts)
 	if err != nil {

pkg/skaffold/initializer/build/init.go

@@ -19,6 +19,9 @@ package build
 import (
 	"io"
 
+	"github.com/pkg/errors"
+
+	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/kubernetes/generator"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest"
 )
 
@@ -63,14 +66,17 @@ func (d *defaultBuildInitializer) PrintAnalysis(out io.Writer) error {
 	return printAnalysis(out, d.enableNewFormat, d.skipBuild, d.builderImagePairs, d.builders, d.unresolvedImages)
 }
 
-func (d *defaultBuildInitializer) GeneratedPairs() []GeneratedBuilderImagePair {
-	return d.generatedBuilderImagePairs
-}
-
-func (d *defaultBuildInitializer) Resolve() {
+func (d *defaultBuildInitializer) GenerateManifests() (map[GeneratedBuilderImagePair][]byte, error) {
+	generatedManifests := map[GeneratedBuilderImagePair][]byte{}
 	for _, pair := range d.generatedBuilderImagePairs {
-		d.builderImagePairs = append(d.builderImagePairs, pair.BuilderImagePair)
+		manifest, err := generator.Generate(pair.ImageName)
+		if err != nil {
+			return nil, errors.Wrap(err, "generating kubernetes manifest")
+		}
+		generatedManifests[pair] = manifest
 	}
+	d.generatedBuilderImagePairs = nil
+	return generatedManifests, nil
 }
 
 // matchBuildersToImages takes a list of builders and images, checks if any of the builders' configured target

pkg/skaffold/initializer/build/resolve.go

@@ -114,6 +114,6 @@ func getGeneratedBuilderPair(b InitBuilder) GeneratedBuilderImagePair {
 			Builder:   b,
 			ImageName: imageName,
 		},
-		ManifestPath: path,
+		ManifestPath: filepath.Join(path, "deployment.yaml"),
 	}
 }

pkg/skaffold/initializer/config_test.go

@@ -45,7 +45,7 @@ func (s stubDeploymentInitializer) Validate() error {
 	panic("no thanks")
 }
 
-func (s stubDeploymentInitializer) GenerateManifests([]build.GeneratedBuilderImagePair) (map[string][]byte, error) {
+func (s stubDeploymentInitializer) AddManifestForImage(string, string) {
 	panic("don't call me")
 }
 
@@ -71,12 +71,8 @@ func (s stubBuildInitializer) BuildConfig() latest.BuildConfig {
 	}
 }
 
-func (s stubBuildInitializer) GeneratedPairs() []build.GeneratedBuilderImagePair {
-	panic("do not call me")
-}
-
-func (s stubBuildInitializer) Resolve() {
-	panic("please don't")
+func (s stubBuildInitializer) GenerateManifests() (map[build.GeneratedBuilderImagePair][]byte, error) {
+	panic("no thank you")
 }
 
 func TestGenerateSkaffoldConfig(t *testing.T) {

pkg/skaffold/initializer/deploy/init.go

@@ -17,7 +17,6 @@ limitations under the License.
 package deploy
 
 import (
-	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/initializer/build"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/initializer/config"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest"
 )
@@ -34,10 +33,10 @@ type Initializer interface {
 	DeployConfig() latest.DeployConfig
 	// GetImages fetches all the images defined in the manifest files.
 	GetImages() []string
-	// GenerateManifests generates k8s manifests for each unresolved builder
-	GenerateManifests([]build.GeneratedBuilderImagePair) (map[string][]byte, error)
 	// Validate ensures preconditions are met before generating a skaffold config
 	Validate() error
+	// AddManifestForImage adds a provided manifest for a given image to the initializer
+	AddManifestForImage(string, string)
 }
 
 type cliDeployInit struct {
@@ -64,28 +63,24 @@ func (c *cliDeployInit) Validate() error {
 	return nil
 }
 
-func (c *cliDeployInit) GenerateManifests([]build.GeneratedBuilderImagePair) (map[string][]byte, error) {
-	return nil, nil
-}
+func (c *cliDeployInit) AddManifestForImage(string, string) {}
 
 type emptyDeployInit struct {
 }
 
-func (c *emptyDeployInit) DeployConfig() latest.DeployConfig {
+func (e *emptyDeployInit) DeployConfig() latest.DeployConfig {
 	return latest.DeployConfig{}
 }
 
-func (c *emptyDeployInit) GetImages() []string {
+func (e *emptyDeployInit) GetImages() []string {
 	return nil
 }
 
-func (c *emptyDeployInit) Validate() error {
+func (e *emptyDeployInit) Validate() error {
 	return nil
 }
 
-func (c *emptyDeployInit) GenerateManifests([]build.GeneratedBuilderImagePair) (map[string][]byte, error) {
-	return nil, nil
-}
+func (e *emptyDeployInit) AddManifestForImage(string, string) {}
 
 func NewInitializer(manifests []string, c config.Config) Initializer {
 	switch {

pkg/skaffold/initializer/deploy/kubectl.go

@@ -17,13 +17,7 @@ limitations under the License.
 package deploy
 
 import (
-	"path/filepath"
-
-	"github.com/pkg/errors"
-
-	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/initializer/build"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/kubernetes"
-	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/kubernetes/generator"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest"
 )
 
@@ -76,23 +70,7 @@ func (k *kubectl) Validate() error {
 	return nil
 }
 
-// GenerateManifests implements the Initializer interface and
-// generates manifests for each unresolved image
-func (k *kubectl) GenerateManifests(unresolved []build.GeneratedBuilderImagePair) (map[string][]byte, error) {
-	generatedManifests := map[string][]byte{}
-	for _, pair := range unresolved {
-		manifest, err := generator.Generate(pair.ImageName)
-		if err != nil {
-			return nil, errors.Wrap(err, "generating kubernetes manifest")
-		}
-		path := filepath.Join(pair.ManifestPath, "deployment.yaml")
-		generatedManifests[path] = manifest
-		k.addManifestForImage(path, pair.ImageName)
-	}
-	return generatedManifests, nil
-}
-
-func (k *kubectl) addManifestForImage(path, image string) {
+func (k *kubectl) AddManifestForImage(path, image string) {
 	k.configs = append(k.configs, path)
 	k.images = append(k.images, image)
 }

pkg/skaffold/initializer/init.go

@@ -61,13 +61,18 @@ func DoInit(ctx context.Context, out io.Writer, c config.Config) error {
 		return buildInitializer.PrintAnalysis(out)
 	}
 
+	// var generatedManifestPairs map[build.GeneratedBuilderImagePair][]byte
 	var generatedManifests map[string][]byte
 	if c.EnableManifestGeneration {
-		generatedManifests, err = deployInitializer.GenerateManifests(buildInitializer.GeneratedPairs())
+		generatedManifestPairs, err := buildInitializer.GenerateManifests()
 		if err != nil {
 			return err
 		}
-		buildInitializer.Resolve()
+		generatedManifests := make(map[string][]byte, len(generatedManifestPairs))
+		for pair, manifest := range generatedManifestPairs {
+			deployInitializer.AddManifestForImage(pair.ImageName, pair.ManifestPath)
+			generatedManifests[pair.ManifestPath] = manifest
+		}
 	}
 
 	if err = deployInitializer.Validate(); err != nil {

pkg/skaffold/initializer/prompt/prompt.go

@@ -50,8 +50,8 @@ func buildConfig(image string, choices []string) (string, error) {
 func WriteSkaffoldConfig(out io.Writer, pipeline []byte, generatedManifests map[string][]byte, filePath string) (bool, error) {
 	fmt.Fprintln(out, string(pipeline))
 
-	for f, m := range generatedManifests {
-		fmt.Fprintln(out, f, "-", string(m))
+	for path, manifest := range generatedManifests {
+		fmt.Fprintln(out, path, "-", string(manifest))
 	}
 
 	manifestString := ""