Merge pull request #12390 from bparees/commit_env

Merged by openshift-bot

pkg/build/builder/common.go

@@ -48,7 +48,7 @@ type GitClient interface {
 
 // buildInfo returns a slice of KeyValue pairs with build metadata to be
 // inserted into Docker images produced by build.
-func buildInfo(build *api.Build) []KeyValue {
+func buildInfo(build *api.Build, sourceInfo *git.SourceInfo) []KeyValue {
 	kv := []KeyValue{
 		{"OPENSHIFT_BUILD_NAME", build.Name},
 		{"OPENSHIFT_BUILD_NAMESPACE", build.Namespace},
@@ -62,7 +62,10 @@ func buildInfo(build *api.Build) []KeyValue {
 		if build.Spec.Source.Git.Ref != "" {
 			kv = append(kv, KeyValue{"OPENSHIFT_BUILD_REFERENCE", build.Spec.Source.Git.Ref})
 		}
-		if build.Spec.Revision != nil && build.Spec.Revision.Git != nil && build.Spec.Revision.Git.Commit != "" {
+
+		if sourceInfo != nil && len(sourceInfo.CommitID) != 0 {
+			kv = append(kv, KeyValue{"OPENSHIFT_BUILD_COMMIT", sourceInfo.CommitID})
+		} else if build.Spec.Revision != nil && build.Spec.Revision.Git != nil && build.Spec.Revision.Git.Commit != "" {
 			kv = append(kv, KeyValue{"OPENSHIFT_BUILD_COMMIT", build.Spec.Revision.Git.Commit})
 		}
 	}

pkg/build/builder/common_test.go

@@ -9,6 +9,7 @@ import (
 	kapi "k8s.io/kubernetes/pkg/api"
 
 	"github.com/openshift/origin/pkg/build/api"
+	"github.com/openshift/origin/pkg/generate/git"
 )
 
 func TestBuildInfo(t *testing.T) {
@@ -32,15 +33,12 @@ func TestBuildInfo(t *testing.T) {
 						},
 					},
 				},
-				Revision: &api.SourceRevision{
-					Git: &api.GitSourceRevision{
-						Commit: "1575a90c569a7cc0eea84fbd3304d9df37c9f5ee",
-					},
-				},
 			},
 		},
 	}
-	got := buildInfo(b)
+	sourceInfo := &git.SourceInfo{}
+	sourceInfo.CommitID = "1575a90c569a7cc0eea84fbd3304d9df37c9f5ee"
+	got := buildInfo(b, sourceInfo)
 	want := []KeyValue{
 		{"OPENSHIFT_BUILD_NAME", "sample-app"},
 		{"OPENSHIFT_BUILD_NAMESPACE", "default"},
@@ -52,6 +50,17 @@ func TestBuildInfo(t *testing.T) {
 	if !reflect.DeepEqual(got, want) {
 		t.Errorf("buildInfo(%+v) = %+v; want %+v", b, got, want)
 	}
+
+	b.Spec.Revision = &api.SourceRevision{
+		Git: &api.GitSourceRevision{
+			Commit: "1575a90c569a7cc0eea84fbd3304d9df37c9f5ee",
+		},
+	}
+	got = buildInfo(b, nil)
+	if !reflect.DeepEqual(got, want) {
+		t.Errorf("buildInfo(%+v) = %+v; want %+v", b, got, want)
+	}
+
 }
 
 func TestRandomBuildTag(t *testing.T) {

pkg/build/builder/docker.go

@@ -84,7 +84,7 @@ func (d *DockerBuilder) Build() error {
 			utilruntime.HandleError(fmt.Errorf("error: An error occured while updating the build status: %v", updateErr))
 		}
 	}
-	if err = d.addBuildParameters(buildDir); err != nil {
+	if err = d.addBuildParameters(buildDir, sourceInfo); err != nil {
 		return err
 	}
 
@@ -211,7 +211,7 @@ func (d *DockerBuilder) copySecrets(secrets []api.SecretBuildSource, buildDir st
 // addBuildParameters checks if a Image is set to replace the default base image.
 // If that's the case then change the Dockerfile to make the build with the given image.
 // Also append the environment variables and labels in the Dockerfile.
-func (d *DockerBuilder) addBuildParameters(dir string) error {
+func (d *DockerBuilder) addBuildParameters(dir string, sourceInfo *git.SourceInfo) error {
 	dockerfilePath := d.getDockerfilePath(dir)
 	node, err := parseDockerfile(dockerfilePath)
 	if err != nil {
@@ -232,13 +232,13 @@ func (d *DockerBuilder) addBuildParameters(dir string) error {
 	}
 
 	// Append build info as environment variables.
-	err = appendEnv(node, d.buildInfo())
+	err = appendEnv(node, d.buildEnv(sourceInfo))
 	if err != nil {
 		return err
 	}
 
 	// Append build labels.
-	err = appendLabel(node, d.buildLabels(dir))
+	err = appendLabel(node, d.buildLabels(sourceInfo))
 	if err != nil {
 		return err
 	}
@@ -259,10 +259,10 @@ func (d *DockerBuilder) addBuildParameters(dir string) error {
 	return ioutil.WriteFile(dockerfilePath, instructions, fi.Mode())
 }
 
-// buildInfo converts the buildInfo output to a format that appendEnv can
+// buildEnv converts the buildInfo output to a format that appendEnv can
 // consume.
-func (d *DockerBuilder) buildInfo() []dockerfile.KeyValue {
-	bi := buildInfo(d.build)
+func (d *DockerBuilder) buildEnv(sourceInfo *git.SourceInfo) []dockerfile.KeyValue {
+	bi := buildInfo(d.build, sourceInfo)
 	kv := make([]dockerfile.KeyValue, len(bi))
 	for i, item := range bi {
 		kv[i] = dockerfile.KeyValue{Key: item.Key, Value: item.Value}
@@ -272,18 +272,10 @@ func (d *DockerBuilder) buildInfo() []dockerfile.KeyValue {
 
 // buildLabels returns a slice of KeyValue pairs in a format that appendEnv can
 // consume.
-func (d *DockerBuilder) buildLabels(dir string) []dockerfile.KeyValue {
+func (d *DockerBuilder) buildLabels(sourceInfo *git.SourceInfo) []dockerfile.KeyValue {
 	labels := map[string]string{}
-	// TODO: allow source info to be overridden by build
-	sourceInfo := &git.SourceInfo{}
-	if d.build.Spec.Source.Git != nil {
-		var errors []error
-		sourceInfo, errors = d.gitClient.GetInfo(dir)
-		if len(errors) > 0 {
-			for _, e := range errors {
-				glog.V(0).Infof("warning: Unable to retrieve Git info: %v", e.Error())
-			}
-		}
+	if sourceInfo == nil {
+		sourceInfo = &git.SourceInfo{}
 	}
 	if len(d.build.Spec.Source.ContextDir) > 0 {
 		sourceInfo.ContextDir = d.build.Spec.Source.ContextDir

pkg/build/builder/docker_test.go

@@ -176,19 +176,34 @@ func TestDockerfilePath(t *testing.T) {
 		},
 	}
 
+	from := "FROM openshift/origin-base"
+	expected := []string{
+		from,
+		// expected env variables
+		"\"OPENSHIFT_BUILD_NAME\"=\"name\"",
+		"\"OPENSHIFT_BUILD_NAMESPACE\"=\"namespace\"",
+		"\"OPENSHIFT_BUILD_SOURCE\"=\"http://github.com/openshift/origin.git\"",
+		"\"OPENSHIFT_BUILD_COMMIT\"=\"commitid\"",
+		// expected labels
+		"\"io.openshift.build.commit.author\"=\"test user \\u003ctest@email.com\\u003e\"",
+		"\"io.openshift.build.commit.date\"=\"date\"",
+		"\"io.openshift.build.commit.id\"=\"commitid\"",
+		"\"io.openshift.build.commit.ref\"=\"ref\"",
+		"\"io.openshift.build.commit.message\"=\"message\"",
+	}
+
 	for _, test := range tests {
 		buildDir, err := ioutil.TempDir(util.GetBaseDir(), "dockerfile-path")
 		if err != nil {
 			t.Errorf("failed to create tmpdir: %v", err)
 			continue
 		}
 		absoluteDockerfilePath := filepath.Join(buildDir, test.contextDir, test.dockerfilePath)
-		dockerfileContent := "FROM openshift/origin-base"
 		if err = os.MkdirAll(filepath.Dir(absoluteDockerfilePath), os.FileMode(0750)); err != nil {
 			t.Errorf("failed to create directory %s: %v", filepath.Dir(absoluteDockerfilePath), err)
 			continue
 		}
-		if err = ioutil.WriteFile(absoluteDockerfilePath, []byte(dockerfileContent), os.FileMode(0644)); err != nil {
+		if err = ioutil.WriteFile(absoluteDockerfilePath, []byte(from), os.FileMode(0644)); err != nil {
 			t.Errorf("failed to write dockerfile to %s: %v", absoluteDockerfilePath, err)
 			continue
 		}
@@ -214,7 +229,16 @@ func TestDockerfilePath(t *testing.T) {
 				},
 			},
 		}
+		build.Name = "name"
+		build.Namespace = "namespace"
 
+		sourceInfo := &git.SourceInfo{}
+		sourceInfo.AuthorName = "test user"
+		sourceInfo.AuthorEmail = "test@email.com"
+		sourceInfo.Date = "date"
+		sourceInfo.CommitID = "commitid"
+		sourceInfo.Ref = "ref"
+		sourceInfo.Message = "message"
 		dockerClient := &FakeDocker{
 			buildImageFunc: func(opts docker.BuildImageOptions) error {
 				if opts.Dockerfile != test.dockerfilePath {
@@ -233,7 +257,7 @@ func TestDockerfilePath(t *testing.T) {
 
 		// this will validate that the Dockerfile is readable
 		// and append some labels to the Dockerfile
-		if err = dockerBuilder.addBuildParameters(buildDir); err != nil {
+		if err = dockerBuilder.addBuildParameters(buildDir, sourceInfo); err != nil {
 			t.Errorf("failed to add build parameters: %v", err)
 			continue
 		}
@@ -244,9 +268,11 @@ func TestDockerfilePath(t *testing.T) {
 			t.Errorf("failed to read dockerfile %s: %v", absoluteDockerfilePath, err)
 			continue
 		}
-		if !strings.Contains(string(dockerfileData), dockerfileContent) {
-			t.Errorf("Updated Dockerfile content does not contains the original Dockerfile content.\n\nOriginal content:\n%s\n\nUpdated content:\n%s\n", dockerfileContent, string(dockerfileData))
-			continue
+		for _, value := range expected {
+			if !strings.Contains(string(dockerfileData), value) {
+				t.Errorf("Updated Dockerfile content does not contain expected value:\n%s\n\nUpdated content:\n%s\n", value, string(dockerfileData))
+
+			}
 		}
 
 		// check that the docker client is called with the right Dockerfile parameter

pkg/build/builder/sti.go

@@ -22,6 +22,7 @@ import (
 	"github.com/openshift/origin/pkg/build/builder/cmd/dockercfg"
 	"github.com/openshift/origin/pkg/build/controller/strategy"
 	"github.com/openshift/origin/pkg/client"
+	"github.com/openshift/origin/pkg/generate/git"
 )
 
 // builderFactory is the internal interface to decouple S2I-specific code from Origin builder code
@@ -182,7 +183,7 @@ func (s *S2IBuilder) Build() error {
 		Incremental:        incremental,
 		IncrementalFromTag: pushTag,
 
-		Environment:       buildEnvVars(s.build),
+		Environment:       buildEnvVars(s.build, sourceInfo),
 		Labels:            buildLabels(s.build),
 		DockerNetworkMode: getDockerNetworkMode(),
 
@@ -337,8 +338,8 @@ func (d *downloader) Download(config *s2iapi.Config) (*s2iapi.SourceInfo, error)
 // 2. In case of repeated Keys, the last Value takes precedence right here,
 //    instead of deferring what to do with repeated environment variables to the
 //    Docker runtime.
-func buildEnvVars(build *api.Build) s2iapi.EnvironmentList {
-	bi := buildInfo(build)
+func buildEnvVars(build *api.Build, sourceInfo *git.SourceInfo) s2iapi.EnvironmentList {
+	bi := buildInfo(build, sourceInfo)
 	envVars := &s2iapi.EnvironmentList{}
 	for _, item := range bi {
 		envVars.Set(fmt.Sprintf("%s=%s", item.Key, item.Value))

pkg/build/builder/sti_test.go

@@ -169,6 +169,9 @@ func TestBuildEnvVars(t *testing.T) {
 		}, s2iapi.EnvironmentSpec{
 			Name:  "OPENSHIFT_BUILD_SOURCE",
 			Value: "http://localhost/123",
+		}, s2iapi.EnvironmentSpec{
+			Name:  "OPENSHIFT_BUILD_COMMIT",
+			Value: "1575a90c569a7cc0eea84fbd3304d9df37c9f5ee",
 		}, s2iapi.EnvironmentSpec{
 			Name:  "HTTPS_PROXY",
 			Value: "https://test/secure:8443",
@@ -182,7 +185,9 @@ func TestBuildEnvVars(t *testing.T) {
 	mockBuild.Name = "openshift-test-1-build"
 	mockBuild.Namespace = "openshift-demo"
 	mockBuild.Spec.Source.Git = &api.GitBuildSource{URI: "http://localhost/123"}
-	resultedEnvList := buildEnvVars(mockBuild)
+	sourceInfo := &git.SourceInfo{}
+	sourceInfo.CommitID = "1575a90c569a7cc0eea84fbd3304d9df37c9f5ee"
+	resultedEnvList := buildEnvVars(mockBuild, sourceInfo)
 	if !reflect.DeepEqual(expectedEnvList, resultedEnvList) {
 		t.Errorf("Expected EnvironmentList to match: %#v, got %#v", expectedEnvList, resultedEnvList)
 	}