Merge pull request #570 from gabemontero/bug1366475

Merged by openshift-bot

cmd/s2i/main.go

@@ -216,7 +216,9 @@ func newCmdRebuild(cfg *api.Config) *cobra.Command {
 				cfg.PullAuthentication = docker.LoadAndGetImageRegistryAuth(r, cfg.Tag)
 			}
 
-			err := build.GenerateConfigFromLabels(cfg)
+			pr, err := docker.GetRebuildImage(cfg)
+			checkErr(err)
+			err = build.GenerateConfigFromLabels(cfg, pr)
 			checkErr(err)
 
 			if len(args) >= 2 {

hack/test-stirunimage.sh

@@ -62,15 +62,6 @@ function test_debug() {
     echo
 }
 
-set +e
-img_count=$(docker images | grep -c sti_test/sti-fake)
-set -e
-
-if [ "${img_count}" != "10" ]; then
-    echo "You do not have necessary test images, be sure to run 'hack/build-test-images.sh' beforehand."
-    exit 1
-fi
-
 trap cleanup EXIT SIGINT
 
 echo "working dir:  ${WORK_DIR}"
@@ -114,6 +105,12 @@ check_result $? ""
 grep "Copying sources" "${WORK_DIR}/s2i-non-repo.log"
 check_result $? "${WORK_DIR}/s2i-non-repo.log"
 
+test_debug "s2i rebuild"
+s2i build https://github.com/openshift/sti-php.git --context-dir=5.5/test/test-app registry.access.redhat.com/openshift3/php-55-rhel7 rack-test-app --incremental=true --loglevel=5 &> "${WORK_DIR}/s2i-pre-rebuild.log"
+check_result $? "${WORK_DIR}/s2i-pre-rebuild.log"
+s2i rebuild rack-test-app:latest rack-test-app:v1 --force-pull=false -p never --loglevel=5 &> "${WORK_DIR}/s2i-rebuild.log"
+check_result $? "${WORK_DIR}/s2i-rebuild.log"
+
 test_debug "s2i usage"
 
 s2i usage openshift/ruby-20-centos7 &> "${WORK_DIR}/s2i-usage.log"

pkg/api/describe/describer.go

@@ -10,6 +10,7 @@ import (
 
 	"github.com/openshift/source-to-image/pkg/api"
 	"github.com/openshift/source-to-image/pkg/build"
+	"github.com/openshift/source-to-image/pkg/docker"
 )
 
 // Config returns the Config object in nice readable, tabbed format.
@@ -97,16 +98,21 @@ func describeBuilderImage(config *api.Config, image string, out io.Writer) {
 		Tag:                config.Tag,
 		IncrementalAuthentication: config.IncrementalAuthentication,
 	}
-	build.GenerateConfigFromLabels(c)
-	if len(c.DisplayName) > 0 {
-		fmt.Fprintf(out, "Builder Name:\t%s\n", c.DisplayName)
-	}
-	fmt.Fprintf(out, "Builder Image:\t%s\n", config.BuilderImage)
-	if len(c.BuilderImageVersion) > 0 {
-		fmt.Fprintf(out, "Builder Image Version:\t%s\n", c.BuilderImageVersion)
-	}
-	if len(c.BuilderBaseImageVersion) > 0 {
-		fmt.Fprintf(out, "Builder Base Version:\t%s\n", c.BuilderBaseImageVersion)
+	pr, err := docker.GetBuilderImage(c)
+	if err == nil {
+		build.GenerateConfigFromLabels(c, pr)
+		if len(c.DisplayName) > 0 {
+			fmt.Fprintf(out, "Builder Name:\t%s\n", c.DisplayName)
+		}
+		fmt.Fprintf(out, "Builder Image:\t%s\n", config.BuilderImage)
+		if len(c.BuilderImageVersion) > 0 {
+			fmt.Fprintf(out, "Builder Image Version:\t%s\n", c.BuilderImageVersion)
+		}
+		if len(c.BuilderBaseImageVersion) > 0 {
+			fmt.Fprintf(out, "Builder Base Version:\t%s\n", c.BuilderBaseImageVersion)
+		}
+	} else {
+		fmt.Fprintf(out, "Error describing image:\t%s\n", err.Error())
 	}
 }
 

pkg/build/config.go

@@ -9,12 +9,15 @@ import (
 
 // GenerateConfigFromLabels generates the S2I Config struct from the Docker
 // image labels.
-func GenerateConfigFromLabels(config *api.Config) error {
-	result, err := docker.GetBuilderImage(config)
-	if err != nil {
-		return err
+func GenerateConfigFromLabels(config *api.Config, metadata *docker.PullResult) error {
+	if config == nil {
+		return fmt.Errorf("config must be provided to GenerateConfigFromLabels")
 	}
-	labels := result.Image.Config.Labels
+	if metadata == nil {
+		return fmt.Errorf("image metadata must be provided to GenerateConfigFromLabels")
+	}
+
+	labels := metadata.Image.Config.Labels
 
 	if builderVersion, ok := labels["io.openshift.builder-version"]; ok {
 		config.BuilderImageVersion = builderVersion

pkg/docker/util.go

@@ -263,6 +263,20 @@ func IsReachable(config *api.Config) bool {
 	return d.Ping() == nil
 }
 
+func pullAndCheck(image string, docker Docker, pullPolicy api.PullPolicy, config *api.Config, forcePull bool) (*PullResult, error) {
+	r, err := PullImage(image, docker, pullPolicy, forcePull)
+	if err != nil {
+		return nil, err
+	}
+
+	err = CheckAllowedUser(docker, image, config.AllowedUIDs, r.OnBuild)
+	if err != nil {
+		return nil, err
+	}
+
+	return r, nil
+}
+
 // GetBuilderImage processes the config and performs operations necessary to make
 // the Docker image specified as BuilderImage available locally.
 // It returns information about the base image, containing metadata necessary
@@ -273,17 +287,19 @@ func GetBuilderImage(config *api.Config) (*PullResult, error) {
 		return nil, err
 	}
 
-	r, err := PullImage(config.BuilderImage, d, config.BuilderPullPolicy, config.ForcePull)
+	return pullAndCheck(config.BuilderImage, d, config.BuilderPullPolicy, config, config.ForcePull)
+}
+
+// GetRebuildImage obtains the metadata information for the image
+// specified in a s2i rebuild operation.  Assumptions are made that
+// the build is available locally since it should have been previously built.
+func GetRebuildImage(config *api.Config) (*PullResult, error) {
+	d, err := New(config.DockerConfig, config.PullAuthentication)
 	if err != nil {
 		return nil, err
 	}
 
-	err = CheckAllowedUser(d, config.BuilderImage, config.AllowedUIDs, r.OnBuild)
-	if err != nil {
-		return nil, err
-	}
-
-	return r, nil
+	return pullAndCheck(config.Tag, d, config.BuilderPullPolicy, config, config.ForcePull)
 }
 
 // GetRuntimeImage processes the config and performs operations necessary to make
@@ -294,17 +310,8 @@ func GetRuntimeImage(config *api.Config, docker Docker) error {
 		pullPolicy = api.DefaultRuntimeImagePullPolicy
 	}
 
-	pullResult, err := PullImage(config.RuntimeImage, docker, pullPolicy, false)
-	if err != nil {
-		return err
-	}
-
-	err = CheckAllowedUser(docker, config.RuntimeImage, config.AllowedUIDs, pullResult.OnBuild)
-	if err != nil {
-		return err
-	}
-
-	return nil
+	_, err := pullAndCheck(config.RuntimeImage, docker, pullPolicy, config, false)
+	return err
 }
 
 func GetDefaultDockerConfig() *api.DockerConfig {