Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New-app/new-build support for pipeline buildconfigs #11897

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions docs/man/man1/oc-new-app.1
Original file line number Diff line number Diff line change
Expand Up @@ -115,8 +115,8 @@ If you provide source code, a new build will be automatically triggered. You can
Search all templates, image streams, and Docker images that match the arguments provided.

.PP
\fB\-\-strategy\fP=""
Specify the build strategy to use if you don't want to detect (docker|source).
\fB\-\-strategy\fP=
Specify the build strategy to use if you don't want to detect (docker|pipeline|source).

.PP
\fB\-\-template\fP=[]
Expand Down
4 changes: 2 additions & 2 deletions docs/man/man1/oc-new-build.1
Original file line number Diff line number Diff line change
Expand Up @@ -103,8 +103,8 @@ Once the build configuration is created a new build will be automatically trigge
Specify the file or directory to copy from the source image and its destination in the build directory. Format: [source]:[destination\-dir].

.PP
\fB\-\-strategy\fP=""
Specify the build strategy to use if you don't want to detect (docker|source).
\fB\-\-strategy\fP=
Specify the build strategy to use if you don't want to detect (docker|pipeline|source).

.PP
\fB\-\-to\fP=""
Expand Down
4 changes: 2 additions & 2 deletions docs/man/man1/openshift-cli-new-app.1
Original file line number Diff line number Diff line change
Expand Up @@ -115,8 +115,8 @@ If you provide source code, a new build will be automatically triggered. You can
Search all templates, image streams, and Docker images that match the arguments provided.

.PP
\fB\-\-strategy\fP=""
Specify the build strategy to use if you don't want to detect (docker|source).
\fB\-\-strategy\fP=
Specify the build strategy to use if you don't want to detect (docker|pipeline|source).

.PP
\fB\-\-template\fP=[]
Expand Down
4 changes: 2 additions & 2 deletions docs/man/man1/openshift-cli-new-build.1
Original file line number Diff line number Diff line change
Expand Up @@ -103,8 +103,8 @@ Once the build configuration is created a new build will be automatically trigge
Specify the file or directory to copy from the source image and its destination in the build directory. Format: [source]:[destination\-dir].

.PP
\fB\-\-strategy\fP=""
Specify the build strategy to use if you don't want to detect (docker|source).
\fB\-\-strategy\fP=
Specify the build strategy to use if you don't want to detect (docker|pipeline|source).

.PP
\fB\-\-to\fP=""
Expand Down
8 changes: 7 additions & 1 deletion pkg/cmd/cli/cmd/newapp.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ import (
"github.com/openshift/origin/pkg/cmd/util/clientcmd"
dockerutil "github.com/openshift/origin/pkg/cmd/util/docker"
configcmd "github.com/openshift/origin/pkg/config/cmd"
"github.com/openshift/origin/pkg/generate"
newapp "github.com/openshift/origin/pkg/generate/app"
newcmd "github.com/openshift/origin/pkg/generate/app/cmd"
"github.com/openshift/origin/pkg/generate/git"
Expand Down Expand Up @@ -170,7 +171,7 @@ func NewCmdNewApplication(name, baseName string, f *clientcmd.Factory, out, erro
cmd.Flags().StringSliceVar(&config.Groups, "group", config.Groups, "Indicate components that should be grouped together as <comp1>+<comp2>.")
cmd.Flags().StringArrayVarP(&config.Environment, "env", "e", config.Environment, "Specify a key-value pair for an environment variable to set into each container. This doesn't apply to objects created from a template, use parameters instead.")
cmd.Flags().StringVar(&config.Name, "name", "", "Set name to use for generated application artifacts")
cmd.Flags().StringVar(&config.Strategy, "strategy", "", "Specify the build strategy to use if you don't want to detect (docker|source).")
cmd.Flags().Var(&config.Strategy, "strategy", "Specify the build strategy to use if you don't want to detect (docker|pipeline|source).")
cmd.Flags().StringP("labels", "l", "", "Label to set in all resources for this application.")
cmd.Flags().BoolVar(&config.InsecureRegistry, "insecure-registry", false, "If true, indicates that the referenced Docker images are on insecure registries and should bypass certificate checking")
cmd.Flags().BoolVarP(&config.AsList, "list", "L", false, "List all local templates and image streams that can be used to create.")
Expand Down Expand Up @@ -505,6 +506,11 @@ func CompleteAppConfig(config *newcmd.AppConfig, f *clientcmd.Factory, c *cobra.
if len(config.SourceImage) == 0 && len(config.SourceImagePath) != 0 {
return kcmdutil.UsageError(c, "--source-image must be specified when --source-image-path is specified.")
}

if config.BinaryBuild && config.Strategy == generate.StrategyPipeline {
return kcmdutil.UsageError(c, "specifying binary builds and the pipeline strategy at the same time is not allowed.")
}

return nil
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/cmd/cli/cmd/newapp_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ func TestNewAppDefaultFlags(t *testing.T) {
},
"strategy": {
flagName: "strategy",
defaultVal: config.Strategy,
defaultVal: "",
},
"labels": {
flagName: "labels",
Expand Down
2 changes: 1 addition & 1 deletion pkg/cmd/cli/cmd/newbuild.go
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ func NewCmdNewBuild(name, baseName string, f *clientcmd.Factory, in io.Reader, o
cmd.Flags().StringVar(&config.To, "to", "", "Push built images to this image stream tag (or Docker image repository if --to-docker is set).")
cmd.Flags().BoolVar(&config.OutputDocker, "to-docker", false, "Have the build output push to a Docker repository.")
cmd.Flags().StringArrayVarP(&config.Environment, "env", "e", config.Environment, "Specify a key-value pair for an environment variable to set into resulting image.")
cmd.Flags().StringVar(&config.Strategy, "strategy", "", "Specify the build strategy to use if you don't want to detect (docker|source).")
cmd.Flags().Var(&config.Strategy, "strategy", "Specify the build strategy to use if you don't want to detect (docker|pipeline|source).")
cmd.Flags().StringVarP(&config.Dockerfile, "dockerfile", "D", "", "Specify the contents of a Dockerfile to build directly, implies --strategy=docker. Pass '-' to read from STDIN.")
cmd.Flags().BoolVar(&config.BinaryBuild, "binary", false, "Instead of expecting a source URL, set the build to expect binary contents. Will disable triggers.")
cmd.Flags().StringP("labels", "l", "", "Label to set in all generated resources.")
Expand Down
30 changes: 21 additions & 9 deletions pkg/generate/app/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"strconv"
"strings"

"github.com/golang/glog"
"github.com/pborman/uuid"
kapi "k8s.io/kubernetes/pkg/api"
"k8s.io/kubernetes/pkg/api/unversioned"
Expand All @@ -18,6 +19,7 @@ import (

buildapi "github.com/openshift/origin/pkg/build/api"
deployapi "github.com/openshift/origin/pkg/deploy/api"
"github.com/openshift/origin/pkg/generate"
"github.com/openshift/origin/pkg/generate/git"
imageapi "github.com/openshift/origin/pkg/image/api"
"github.com/openshift/origin/pkg/util"
Expand Down Expand Up @@ -193,13 +195,19 @@ func (r *SourceRef) BuildSource() (*buildapi.BuildSource, []buildapi.BuildTrigge

// BuildStrategyRef is a reference to a build strategy
type BuildStrategyRef struct {
IsDockerBuild bool
Base *ImageRef
Strategy generate.Strategy
Base *ImageRef
}

// BuildStrategy builds an OpenShift BuildStrategy from a BuildStrategyRef
func (s *BuildStrategyRef) BuildStrategy(env Environment) (*buildapi.BuildStrategy, []buildapi.BuildTriggerPolicy) {
if s.IsDockerBuild {
switch s.Strategy {
case generate.StrategyPipeline:
return &buildapi.BuildStrategy{
JenkinsPipelineStrategy: &buildapi.JenkinsPipelineBuildStrategy{},
}, s.Base.BuildTriggers()

case generate.StrategyDocker:
var triggers []buildapi.BuildTriggerPolicy
strategy := &buildapi.DockerBuildStrategy{
Env: env.List(),
Expand All @@ -212,14 +220,18 @@ func (s *BuildStrategyRef) BuildStrategy(env Environment) (*buildapi.BuildStrate
return &buildapi.BuildStrategy{
DockerStrategy: strategy,
}, triggers

case generate.StrategySource:
return &buildapi.BuildStrategy{
SourceStrategy: &buildapi.SourceBuildStrategy{
From: s.Base.ObjectReference(),
Env: env.List(),
},
}, s.Base.BuildTriggers()
}

return &buildapi.BuildStrategy{
SourceStrategy: &buildapi.SourceBuildStrategy{
From: s.Base.ObjectReference(),
Env: env.List(),
},
}, s.Base.BuildTriggers()
glog.Error("BuildStrategy called with unknown strategy")
return nil, nil
}

// BuildRef is a reference to a build configuration
Expand Down
11 changes: 3 additions & 8 deletions pkg/generate/app/cmd/describe.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"k8s.io/kubernetes/pkg/util/sets"

"github.com/openshift/origin/pkg/cmd/cli/describe"
"github.com/openshift/origin/pkg/generate"
"github.com/openshift/origin/pkg/generate/app"
imageapi "github.com/openshift/origin/pkg/image/api"
)
Expand Down Expand Up @@ -148,16 +149,10 @@ func describeBuildPipelineWithImage(out io.Writer, ref app.ComponentReference, p
}
matches = append(matches, t.Platform)
}
if len(matches) > 0 && !pipeline.Build.Strategy.IsDockerBuild {
if len(matches) > 0 && pipeline.Build.Strategy.Strategy == generate.StrategySource {
fmt.Fprintf(out, " * The source repository appears to match: %s\n", strings.Join(matches, ", "))
}
}
var strategy string
if pipeline.Build.Strategy.IsDockerBuild {
strategy = "Docker"
} else {
strategy = "source"
}
noSource := false
var source string
switch s := pipeline.Build.Source; {
Expand All @@ -175,7 +170,7 @@ func describeBuildPipelineWithImage(out io.Writer, ref app.ComponentReference, p
source = "<unknown>"
}

fmt.Fprintf(out, " * A %s build using %s will be created\n", strategy, source)
fmt.Fprintf(out, " * A %s build using %s will be created\n", pipeline.Build.Strategy.Strategy, source)
if buildOut, err := pipeline.Build.Output.BuildOutput(); err == nil && buildOut != nil && buildOut.To != nil {
switch to := buildOut.To; {
case to.Kind == "ImageStreamTag":
Expand Down
46 changes: 19 additions & 27 deletions pkg/generate/app/cmd/newapp.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,10 @@ import (
"github.com/openshift/origin/pkg/client"
cmdutil "github.com/openshift/origin/pkg/cmd/util"
"github.com/openshift/origin/pkg/dockerregistry"
"github.com/openshift/origin/pkg/generate"
"github.com/openshift/origin/pkg/generate/app"
"github.com/openshift/origin/pkg/generate/dockerfile"
"github.com/openshift/origin/pkg/generate/jenkinsfile"
"github.com/openshift/origin/pkg/generate/source"
imageapi "github.com/openshift/origin/pkg/image/api"
outil "github.com/openshift/origin/pkg/util"
Expand All @@ -44,10 +46,6 @@ const (
GeneratedByNewBuild = "OpenShiftNewBuild"
)

// ErrNoDockerfileDetected is the error returned when the requested build strategy is Docker
// and no Dockerfile is detected in the repository.
var ErrNoDockerfileDetected = errors.New("No Dockerfile was found in the repository and the requested build strategy is 'docker'")

// GenerationInputs control how new-app creates output
// TODO: split these into finer grained structs
type GenerationInputs struct {
Expand All @@ -59,7 +57,7 @@ type GenerationInputs struct {

InsecureRegistry bool

Strategy string
Strategy generate.Strategy

Name string
To string
Expand Down Expand Up @@ -161,8 +159,9 @@ func NewAppConfig() *AppConfig {
return &AppConfig{
Resolvers: Resolvers{
Detector: app.SourceRepositoryEnumerator{
Detectors: source.DefaultDetectors,
Tester: dockerfile.NewTester(),
Detectors: source.DefaultDetectors,
DockerfileTester: dockerfile.NewTester(),
JenkinsfileTester: jenkinsfile.NewTester(),
},
},
}
Expand Down Expand Up @@ -249,18 +248,18 @@ func (c *AppConfig) AddArguments(args []string) []string {
}

// validateBuilders confirms that all images associated with components that are to be built,
// are builders (or we're using a docker strategy).
// are builders (or we're using a non-source strategy).
func (c *AppConfig) validateBuilders(components app.ComponentReferences) error {
if len(c.Strategy) != 0 {
if c.Strategy != generate.StrategyUnspecified {
return nil
}
errs := []error{}
for _, ref := range components {
input := ref.Input()
// if we're supposed to build this thing, and the image/imagestream we've matched it to did not come from an explicit CLI argument,
// and the image/imagestream we matched to is not explicitly an s2i builder, and we're not doing a docker-type build, warn the user
// and the image/imagestream we matched to is not explicitly an s2i builder, and we're doing a source-type build, warn the user
// that this probably won't work and force them to declare their intention explicitly.
if input.ExpectToBuild && input.ResolvedMatch != nil && !app.IsBuilderMatch(input.ResolvedMatch) && input.Uses != nil && !input.Uses.IsDockerBuild() {
if input.ExpectToBuild && input.ResolvedMatch != nil && !app.IsBuilderMatch(input.ResolvedMatch) && input.Uses != nil && input.Uses.GetStrategy() == generate.StrategySource {
errs = append(errs, fmt.Errorf("the image match %q for source repository %q does not appear to be a source-to-image builder.\n\n- to attempt to use this image as a source builder, pass \"--strategy=source\"\n- to use it as a base image for a Docker build, pass \"--strategy=docker\"", input.ResolvedMatch.Name, input.Uses))
continue
}
Expand All @@ -275,13 +274,6 @@ func validateEnforcedName(name string) error {
return nil
}

func validateStrategyName(name string) error {
if name != "docker" && name != "source" {
return fmt.Errorf("invalid strategy: %s. Must be 'docker' or 'source'.", name)
}
return nil
}

func validateOutputImageReference(ref string) error {
if _, err := imageapi.ParseDockerImageReference(ref); err != nil {
return fmt.Errorf("invalid output image reference: %s", ref)
Expand All @@ -304,7 +296,7 @@ func (c *AppConfig) buildPipelines(components app.ComponentReferences, environme
switch {
case refInput.ExpectToBuild:
glog.V(4).Infof("will add %q secrets into a build for a source build of %q", strings.Join(c.Secrets, ","), refInput.Uses)
if err := refInput.Uses.AddBuildSecrets(c.Secrets, refInput.Uses.IsDockerBuild()); err != nil {
if err := refInput.Uses.AddBuildSecrets(c.Secrets); err != nil {
return nil, fmt.Errorf("unable to add build secrets %q: %v", strings.Join(c.Secrets, ","), err)
}

Expand All @@ -317,7 +309,7 @@ func (c *AppConfig) buildPipelines(components app.ComponentReferences, environme
if err != nil {
return nil, fmt.Errorf("can't build %q: %v", from, err)
}
if !inputImage.AsImageStream && from != "scratch" {
if !inputImage.AsImageStream && from != "scratch" && (refInput.Uses == nil || refInput.Uses.GetStrategy() != generate.StrategyPipeline) {
msg := "Could not find an image stream match for %q. Make sure that a Docker image with that tag is available on the node for the build to succeed."
glog.Warningf(msg, from)
}
Expand Down Expand Up @@ -351,6 +343,12 @@ func (c *AppConfig) buildPipelines(components app.ComponentReferences, environme
if c.NoOutput {
pipeline.Build.Output = nil
}
if refInput.Uses != nil && refInput.Uses.GetStrategy() == generate.StrategyPipeline {
pipeline.Build.Output = nil
pipeline.Deployment = nil
pipeline.Image = nil
pipeline.InputImage = nil
}
common = append(common, pipeline)
if err := common.Reduce(); err != nil {
return nil, fmt.Errorf("can't create a pipeline from %s: %v", common, err)
Expand Down Expand Up @@ -613,12 +611,6 @@ func (c *AppConfig) Run() (*AppResult, error) {
}
}

if len(c.Strategy) > 0 {
if err := validateStrategyName(c.Strategy); err != nil {
return nil, err
}
}

if err := optionallyValidateExposedPorts(c, repositories); err != nil {
return nil, err
}
Expand Down Expand Up @@ -901,7 +893,7 @@ func optionallyValidateExposedPorts(config *AppConfig, repositories app.SourceRe
return nil
}

if len(config.Strategy) > 0 && config.Strategy != "docker" {
if config.Strategy != generate.StrategyUnspecified && config.Strategy != generate.StrategyDocker {
return nil
}

Expand Down
18 changes: 9 additions & 9 deletions pkg/generate/app/cmd/newapp_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (

buildapi "github.com/openshift/origin/pkg/build/api"
client "github.com/openshift/origin/pkg/client/testclient"
"github.com/openshift/origin/pkg/generate"
"github.com/openshift/origin/pkg/generate/app"
image "github.com/openshift/origin/pkg/image/api"
templateapi "github.com/openshift/origin/pkg/template/api"
Expand Down Expand Up @@ -339,7 +340,7 @@ func mockSourceRepositories(t *testing.T, file string) []*app.SourceRepository {
"https://github.com/openshift/ruby-hello-world.git",
file,
} {
s, err := app.NewSourceRepository(location)
s, err := app.NewSourceRepository(location, generate.StrategySource)
if err != nil {
t.Fatal(err)
}
Expand All @@ -356,11 +357,10 @@ func TestBuildPipelinesWithUnresolvedImage(t *testing.T) {
t.Fatal(err)
}

sourceRepo, err := app.NewSourceRepository("https://github.com/foo/bar.git")
sourceRepo, err := app.NewSourceRepository("https://github.com/foo/bar.git", generate.StrategyDocker)
if err != nil {
t.Fatal(err)
}
sourceRepo.BuildWithDocker()
sourceRepo.SetInfo(&app.SourceRepositoryInfo{
Dockerfile: dockerFile,
})
Expand Down Expand Up @@ -509,15 +509,15 @@ func TestBuildOutputCycleWithFollowingTag(t *testing.T) {

func TestAllowedNonNumericExposedPorts(t *testing.T) {
tests := []struct {
strategy string
strategy generate.Strategy
allowNonNumericPorts bool
}{
{
strategy: "",
strategy: generate.StrategyUnspecified,
allowNonNumericPorts: true,
},
{
strategy: "source",
strategy: generate.StrategySource,
allowNonNumericPorts: false,
},
}
Expand All @@ -543,15 +543,15 @@ func TestAllowedNonNumericExposedPorts(t *testing.T) {

func TestDisallowedNonNumericExposedPorts(t *testing.T) {
tests := []struct {
strategy string
strategy generate.Strategy
allowNonNumericPorts bool
}{
{
strategy: "",
strategy: generate.StrategyUnspecified,
allowNonNumericPorts: false,
},
{
strategy: "docker",
strategy: generate.StrategyDocker,
allowNonNumericPorts: false,
},
}
Expand Down
Loading