Skip to content

Commit

Permalink
defer manifest generation to build initializer
Browse files Browse the repository at this point in the history
  • Loading branch information
nkubala committed Feb 26, 2020
1 parent 8d4812f commit 286d11d
Show file tree
Hide file tree
Showing 10 changed files with 48 additions and 72 deletions.
15 changes: 4 additions & 11 deletions pkg/skaffold/initializer/build/builders.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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:
Expand Down
11 changes: 6 additions & 5 deletions pkg/skaffold/initializer/build/builders_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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",
},
},
},
Expand Down Expand Up @@ -114,7 +115,7 @@ func TestResolveBuilderImages(t *testing.T) {
Builder: docker.ArtifactConfig{File: "foo"},
ImageName: "foo-image",
},
ManifestPath: ".",
ManifestPath: "deployment.yaml",
},
},
shouldMakeChoice: false,
Expand All @@ -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())
})
}
}
Expand Down
6 changes: 2 additions & 4 deletions pkg/skaffold/initializer/build/cli.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
18 changes: 12 additions & 6 deletions pkg/skaffold/initializer/build/init.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion pkg/skaffold/initializer/build/resolve.go
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,6 @@ func getGeneratedBuilderPair(b InitBuilder) GeneratedBuilderImagePair {
Builder: b,
ImageName: imageName,
},
ManifestPath: path,
ManifestPath: filepath.Join(path, "deployment.yaml"),
}
}
10 changes: 3 additions & 7 deletions pkg/skaffold/initializer/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}

Expand All @@ -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) {
Expand Down
19 changes: 7 additions & 12 deletions pkg/skaffold/initializer/deploy/init.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand All @@ -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 {
Expand All @@ -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 {
Expand Down
26 changes: 4 additions & 22 deletions pkg/skaffold/initializer/deploy/kubectl.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,10 @@ limitations under the License.
package deploy

import (
"path/filepath"
"fmt"
"os"

"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"
)

Expand Down Expand Up @@ -76,23 +73,8 @@ 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) {
fmt.Fprintf(os.Stdout, "adding manifest path %s for image %s\n", path, image)
k.configs = append(k.configs, path)
k.images = append(k.images, image)
}
9 changes: 7 additions & 2 deletions pkg/skaffold/initializer/init.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.ManifestPath, pair.ImageName)
generatedManifests[pair.ManifestPath] = manifest
}
}

if err = deployInitializer.Validate(); err != nil {
Expand Down
4 changes: 2 additions & 2 deletions pkg/skaffold/initializer/prompt/prompt.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 := ""
Expand Down

0 comments on commit 286d11d

Please sign in to comment.