Skip to content

Card devexp_426 - Renaming Clean to Incremental to better match current behaviour, where Incremental needs to be forced #142

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.

Sign up for GitHub

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

Merged
openshift-bot merged 1 commit into from
Feb 23, 2015
Merged

Card devexp_426 - Renaming Clean to Incremental to better match current behaviour, where Incremental needs to be forced #142

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion cmd/sti/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ func newCmdBuild(req *api.Request) *cobra.Command {
},
}

buildCmd.Flags().BoolVar(&(req.Clean), "clean", true, "Perform a clean build")
buildCmd.Flags().BoolVar(&(req.Incremental), "incremental", false, "Perform an incremental build")
buildCmd.Flags().BoolVar(&(req.RemovePreviousImage), "rm", false, "Remove the previous image during incremental builds")
buildCmd.Flags().StringP("env", "e", "", "Specify an environment var NAME=VALUE,NAME2=VALUE2,...")
buildCmd.Flags().StringVarP(&(req.Ref), "ref", "r", "", "Specify a ref to check-out")
Expand Down
2 changes: 1 addition & 1 deletion docs/cli.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ that image and add them to the tar streamed to the container into `/artifacts`.
| Name | Description |
|:-------------------------- |:--------------------------------------------------------|
| `--callbackURL` | URL to be invoked after successful build (see [Callback URL](#callback-url)) |
| `--clean` | Always perform clean build, even if build is eligible for incremental one (default: `true`) |
| `--incremental` | Try performing an incremental build |
| `-e (--env)` | Environment variables passed to the builder eg. `NAME=VALUE,NAME2=VALUE2,...` |
| `--forcePull` | Always pull the builder image, even if it is present locally |
| `-l (--location)` | Location where the scripts and sources will be placed prior doing build (see [STI Scripts](#sti-scripts))|
Expand Down
7 changes: 2 additions & 5 deletions pkg/api/types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@ type Request struct {
// Tag is a result image tag name.
Tag string

// Clean describes whether to perform full build even if the build is eligible for incremental build.
Clean bool
// Incremental describes whether to try to perform incremental build.
Incremental bool

// RemovePreviousImage describes if previous image should be removed after successful build.
// This applies only to incremental builds.
Expand All @@ -43,9 +43,6 @@ type Request struct {
// ForcePull describes if the builder should pull the images from registry prior to building.
ForcePull bool

// Incremental describes incremental status of current build
Incremental bool

// WorkingDir describes temporary directory used for downloading sources, scripts and tar operations.
WorkingDir string

Expand Down
11 changes: 6 additions & 5 deletions pkg/build/strategies/sti/sti.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ type STI struct {
optionalScripts []string
externalScripts map[string]bool
installedScripts map[string]bool
incremental bool

// Interfaces
preparer build.Preparer
Expand Down Expand Up @@ -112,14 +113,14 @@ func (b *STI) Build(request *api.Request) (*api.Result, error) {
return nil, err
}

if b.request.Incremental = b.artifacts.Exists(request); b.request.Incremental {
if b.incremental = b.artifacts.Exists(request); b.incremental {
glog.V(1).Infof("Existing image for tag %s detected for incremental build", request.Tag)
} else {
glog.V(1).Infof("Clean build will be performed")
}

glog.V(2).Infof("Performing source build from %s", request.Source)
if request.Incremental {
if b.incremental {
if err := b.artifacts.Save(request); err != nil {
glog.Warningf("Error saving previous build artifacts: %v", err)
glog.Warning("Clean build will be performed!")
Expand Down Expand Up @@ -202,7 +203,7 @@ func (b *STI) PostExecute(containerID string, location string) error {
previousImageID string
)

if b.request.Incremental && b.request.RemovePreviousImage {
if b.incremental && b.request.RemovePreviousImage {
if previousImageID, err = b.docker.GetImageID(b.request.Tag); err != nil {
glog.Errorf("Error retrieving previous image's metadata: %v", err)
}
Expand All @@ -225,7 +226,7 @@ func (b *STI) PostExecute(containerID string, location string) error {
b.result.ImageID = imageID
glog.V(1).Infof("Tagged %s as %s", imageID, b.request.Tag)

if b.request.Incremental && b.request.RemovePreviousImage && previousImageID != "" {
if b.incremental && b.request.RemovePreviousImage && previousImageID != "" {
glog.V(1).Infof("Removing previously-tagged image %s", previousImageID)
if err = b.docker.RemoveImage(previousImageID); err != nil {
glog.Errorf("Unable to remove previous image: %v", err)
Expand All @@ -244,7 +245,7 @@ func (b *STI) PostExecute(containerID string, location string) error {
// It checks if the previous image exists in the system and if so, then it
// verifies that the save-artifacts script is present.
func (b *STI) Exists(request *api.Request) bool {
if request.Clean {
if !request.Incremental {
return false
}

Expand Down
34 changes: 17 additions & 17 deletions pkg/build/strategies/sti/sti_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ func (f *FakeSTI) Prepare(*api.Request) error {

func (f *FakeSTI) Exists(*api.Request) bool {
f.ExistsCalled = true
return false
return true
}

func (f *FakeSTI) Request() *api.Request {
Expand Down Expand Up @@ -132,7 +132,6 @@ func (f *FakeDockerBuild) Build(*api.Request) (*api.Result, error) {
func TestBuild(t *testing.T) {
incrementalTest := []bool{false, true}
for _, incremental := range incrementalTest {

fh := &FakeSTI{
BuildRequest: &api.Request{Incremental: incremental},
BuildResult: &api.Result{},
Expand Down Expand Up @@ -261,6 +260,7 @@ func TestPostExecute(t *testing.T) {
bh.request.Incremental = incremental
if previousImageID != "" {
bh.request.RemovePreviousImage = true
bh.incremental = incremental
bh.docker.(*test.FakeDocker).GetImageIDResult = previousImageID
}
err := bh.PostExecute("test-container-id", "cmd1")
Expand Down Expand Up @@ -291,8 +291,8 @@ func TestPostExecute(t *testing.T) {

func TestExists(t *testing.T) {
type incrementalTest struct {
// clean flag was passed
clean bool
// incremental flag was passed
incremental bool
// previous image existence
previousImage bool
// script installed
Expand All @@ -302,26 +302,26 @@ func TestExists(t *testing.T) {
}

tests := []incrementalTest{
// 0-1: no clean, no image, no matter what with scripts
{false, false, false, false},
{false, false, true, false},

// 2: no clean, previous image, no scripts
{false, true, false, false},
// 3: no clean, previous image, scripts installed
{false, true, true, true},

// 4-7: clean build - should always return false no matter what other flags are
// 0-1: incremental, no image, no matter what with scripts
{true, false, false, false},
{true, false, true, false},

// 2: incremental, previous image, no scripts
{true, true, false, false},
{true, true, true, false},
// 3: incremental, previous image, scripts installed
{true, true, true, true},

// 4-7: no incremental build - should always return false no matter what other flags are
{false, false, false, false},
{false, false, true, false},
{false, true, false, false},
{false, true, true, false},
}

for i, ti := range tests {
bh := testBuildHandler()
bh.request.WorkingDir = "/working-dir"
bh.request.Clean = ti.clean
bh.request.Incremental = ti.incremental
bh.installedScripts = map[string]bool{api.SaveArtifacts: ti.scriptInstalled}
bh.docker.(*test.FakeDocker).PullResult = ti.previousImage

Expand All @@ -330,7 +330,7 @@ func TestExists(t *testing.T) {
t.Errorf("(%d) Unexpected incremental result: %v. Expected: %v",
i, incremental, ti.expected)
}
if !ti.clean && ti.previousImage && ti.scriptInstalled {
if ti.incremental && ti.previousImage && ti.scriptInstalled {
if len(bh.fs.(*test.FakeFileSystem).ExistsFile) == 0 {
continue
}
Expand Down
6 changes: 3 additions & 3 deletions test/integration/integration_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ func (i *integrationTest) exerciseCleanBuild(tag string, verifyCallback bool, im
BaseImage: imageName,
Source: TestSource,
Tag: tag,
Clean: true,
Incremental: false,
CallbackURL: callbackURL,
ScriptsURL: scriptsURL}

Expand Down Expand Up @@ -230,7 +230,7 @@ func (i *integrationTest) exerciseIncrementalBuild(tag, imageName string, remove
BaseImage: imageName,
Source: TestSource,
Tag: tag,
Clean: true,
Incremental: false,
RemovePreviousImage: removePreviousImage,
}

Expand All @@ -252,7 +252,7 @@ func (i *integrationTest) exerciseIncrementalBuild(tag, imageName string, remove
BaseImage: imageName,
Source: TestSource,
Tag: tag,
Clean: false,
Incremental: true,
RemovePreviousImage: removePreviousImage,
}

Expand Down