1
0
mirror of https://github.com/openshift/source-to-image.git synced 2026-02-05 12:44:54 +01:00

miscellaneous changes associated with s2i windows work

- add missing function documentation
- fix govet shadow warnings
- fix spelling errors
- add .atom-build.yml to .gitignore
- don't mess up bash completions if build fails
- re-enable test-go.sh race checker as documented
- remove extraneous goroutines in tests
- use ExcludeRegExp in integration tests
- remove finished containers in integration tests
This commit is contained in:
Jim Minter
2016-10-28 12:10:59 +01:00
parent e646826939
commit 1a0ef1eb7e
18 changed files with 128 additions and 116 deletions

3
.gitignore vendored
View File

@@ -1,4 +1,5 @@
/_output
/.vagrant
/.atom-build.yml
/.project
/.vagrant
*~

View File

@@ -74,7 +74,7 @@ To run tests without the go race detector, which is on by default, use:
A line coverage report is run by default when testing a single package.
To create a coverage report for all packages:
$ OUTPUT_COVERAGE=true hack/test-go.sh pkg/build/strategies/sti
$ OUTPUT_COVERAGE=/tmp/reportdir hack/test-go.sh
### Integration tests

View File

@@ -212,11 +212,15 @@ func newCmdRebuild(cfg *api.Config) *cobra.Command {
os.Exit(1)
}
if r, err := os.Open(cfg.DockerCfgPath); err == nil {
var auths *docker.AuthConfigurations
r, err := os.Open(cfg.DockerCfgPath)
if err == nil {
defer r.Close()
cfg.PullAuthentication = docker.LoadAndGetImageRegistryAuth(r, cfg.Tag)
auths = docker.LoadImageRegistryAuth(r)
}
cfg.PullAuthentication = docker.GetImageRegistryAuth(auths, cfg.Tag)
pr, err := docker.GetRebuildImage(cfg)
checkErr(err)
err = build.GenerateConfigFromLabels(cfg, pr)
@@ -226,12 +230,7 @@ func newCmdRebuild(cfg *api.Config) *cobra.Command {
cfg.Tag = args[1]
}
// Attempt to read the .dockercfg and extract the authentication for
// docker pull
if r, err := os.Open(cfg.DockerCfgPath); err == nil {
defer r.Close()
cfg.PullAuthentication = docker.LoadAndGetImageRegistryAuth(r, cfg.BuilderImage)
}
cfg.PullAuthentication = docker.GetImageRegistryAuth(auths, cfg.BuilderImage)
if len(cfg.BuilderPullPolicy) == 0 {
cfg.BuilderPullPolicy = api.DefaultBuilderPullPolicy

View File

@@ -26,26 +26,8 @@ find_test_dirs() {
\) -name '*_test.go' -print0 | xargs -0n1 dirname | sort -u | xargs -n1 printf "${S2I_GO_PACKAGE}/%s\n"
}
# -covermode=atomic becomes default with -race in Go >=1.3
if [ -z ${S2I_COVER+x} ]; then
S2I_COVER=""
fi
OUTPUT_COVERAGE=${OUTPUT_COVERAGE:-""}
if [ -n "${OUTPUT_COVERAGE}" ]; then
if [ -z ${S2I_RACE+x} ]; then
S2I_RACE="-race"
fi
if [ -z "${S2I_COVER}" ]; then
S2I_COVER="-cover -covermode=atomic"
fi
fi
if [ -z ${S2I_RACE+x} ]; then
S2I_RACE=""
fi
S2I_RACE=${S2I_RACE:--race}
S2I_COVER=${S2I_COVER:--covermode=atomic}
S2I_TIMEOUT=${S2I_TIMEOUT:--timeout 60s}
if [ "${1-}" != "" ]; then
@@ -54,6 +36,8 @@ else
test_packages=`find_test_dirs`
fi
OUTPUT_COVERAGE=${OUTPUT_COVERAGE:-""}
if [[ -n "${S2I_COVER}" && -n "${OUTPUT_COVERAGE}" ]]; then
# Iterate over packages to run coverage
test_packages=( $test_packages )
@@ -69,7 +53,8 @@ if [[ -n "${S2I_COVER}" && -n "${OUTPUT_COVERAGE}" ]]; then
find $OUTPUT_COVERAGE -name profile.out | xargs sed '/^mode: atomic$/d' >> ${OUTPUT_COVERAGE}/profiles.out
go tool cover "-html=${OUTPUT_COVERAGE}/profiles.out" -o "${OUTPUT_COVERAGE}/coverage.html"
rm -rf $OUTPUT_COVERAGE/$S2I_GO_PACKAGE
# remove ${OUTPUT_COVERAGE}/github.com
rm -rf $OUTPUT_COVERAGE/${S2I_GO_PACKAGE%%/*}
else
go test $S2I_RACE $S2I_TIMEOUT $S2I_COVER "${@:2}" $test_packages
fi

View File

@@ -10,12 +10,12 @@ source "${S2I_ROOT}/hack/common.sh"
cd "${S2I_ROOT}"
mv contrib/bash/s2i contrib/bash/s2i-proposed
trap "mv contrib/bash/s2i-proposed contrib/bash/s2i" exit
hack/update-generated-completions.sh
ret=0
diff -Naupr contrib/bash/s2i contrib/bash/s2i-proposed || ret=$?
mv contrib/bash/s2i-proposed contrib/bash/s2i
if [[ $ret -eq 0 ]]
then
echo "SUCCESS: Generated completions up to date."

View File

@@ -247,7 +247,8 @@ func (step *startRuntimeImageAndUploadFilesStep) execute(ctx *postExecutorStepCo
for _, script := range []string{api.AssembleRuntime, api.Run} {
// scripts must be inside of "scripts" subdir, see createCommandForExecutingRunScript()
destinationDir := filepath.Join(artifactsDir, "scripts")
if err = step.copyScriptIfNeeded(script, destinationDir); err != nil {
err = step.copyScriptIfNeeded(script, destinationDir)
if err != nil {
return err
}
}
@@ -262,7 +263,8 @@ func (step *startRuntimeImageAndUploadFilesStep) execute(ctx *postExecutorStepCo
useExternalAssembleScript := step.builder.externalScripts[api.AssembleRuntime]
if !useExternalAssembleScript {
// script already inside of the image
scriptsURL, err := step.docker.GetScriptsURL(image)
var scriptsURL string
scriptsURL, err = step.docker.GetScriptsURL(image)
if err != nil {
return err
}
@@ -317,16 +319,18 @@ func (step *startRuntimeImageAndUploadFilesStep) execute(ctx *postExecutorStepCo
}
glog.V(5).Infof("Uploading directory %q -> %q", artifactsDir, workDir)
if err = step.docker.UploadToContainerWithCallback(artifactsDir, workDir, containerID, setStandardPerms, true); err != nil {
onStartErr := step.docker.UploadToContainerWithCallback(artifactsDir, workDir, containerID, setStandardPerms, true)
if onStartErr != nil {
return fmt.Errorf("Couldn't upload directory (%q -> %q) into container %s: %v", artifactsDir, workDir, containerID, err)
}
glog.V(5).Infof("Uploading file %q -> %q", lastFilePath, lastFileDstPath)
if err := step.docker.UploadToContainerWithCallback(lastFilePath, lastFileDstPath, containerID, setStandardPerms, true); err != nil {
onStartErr = step.docker.UploadToContainerWithCallback(lastFilePath, lastFileDstPath, containerID, setStandardPerms, true)
if onStartErr != nil {
return fmt.Errorf("Couldn't upload file (%q -> %q) into container %s: %v", lastFilePath, lastFileDstPath, containerID, err)
}
return err
return onStartErr
}
go dockerpkg.StreamContainerIO(outReader, nil, func(a ...interface{}) { glog.V(0).Info(a...) })

View File

@@ -139,7 +139,9 @@ func New(config *api.Config, overrides build.Overrides) (*STI, error) {
// which would lead to replacing this quick short circuit (so this change is tactical)
builder.source = overrides.Downloader
if builder.source == nil && !config.Usage {
downloader, sourceURL, err := scm.DownloaderForSource(config.Source, config.ForceCopy)
var downloader build.Downloader
var sourceURL string
downloader, sourceURL, err = scm.DownloaderForSource(config.Source, config.ForceCopy)
if err != nil {
return nil, err
}
@@ -253,7 +255,8 @@ func (builder *STI) Prepare(config *api.Config) error {
// user didn't specify mapping, let's take it from the runtime image then
if len(builder.config.RuntimeArtifacts) == 0 {
mapping, err := builder.docker.GetAssembleInputFiles(config.RuntimeImage)
var mapping string
mapping, err = builder.docker.GetAssembleInputFiles(config.RuntimeImage)
if err != nil {
builder.result.BuildInfo.FailureReason = utilstatus.NewFailureReason(utilstatus.ReasonInvalidArtifactsMapping, utilstatus.ReasonMessageInvalidArtifactsMapping)
return err

View File

@@ -141,6 +141,7 @@ type stiDocker struct {
pullAuth dockertypes.AuthConfig
}
// InspectImage returns the image information and its raw representation.
func (d stiDocker) InspectImage(name string) (*dockertypes.ImageInspect, error) {
ctx, cancel := getDefaultContext()
defer cancel()

View File

@@ -152,17 +152,6 @@ func authConfigs(confs map[string]dockerConfig) (*AuthConfigurations, error) {
// end block of 3 methods borrowed from go-dockerclient
// LoadAndGetImageRegistryAuth loads the set of client auth objects from a docker config file
// and returns the appropriate client auth object for a given image name.
func LoadAndGetImageRegistryAuth(dockerCfg io.Reader, imageName string) api.AuthConfig {
auths, err := NewAuthConfigurations(dockerCfg)
if err != nil {
glog.V(0).Infof("error: Unable to load docker config: %v", err)
return api.AuthConfig{}
}
return GetImageRegistryAuth(auths, imageName)
}
// StreamContainerIO takes data from the Reader and redirects to the log function (typically we pass in
// glog.Error for stderr and glog.Info for stdout. The caller should wrap glog functions in a closure
// to ensure accurate line numbers are reported: https://github.com/openshift/source-to-image/issues/558 .

View File

@@ -180,14 +180,14 @@ func TestBasicDelKeepMix(t *testing.T) {
/*
Per the docker user guide, with a docker ignore list of:
LICENCSE.*
!LICENCSE.md
LICENSE.*
!LICENSE.md
*.md
LICENSE.MD will NOT be kept, as *.md overrides !LICENCSE.md
LICENSE.MD will NOT be kept, as *.md overrides !LICENSE.md
*/
func TestExcludeOverride(t *testing.T) {
baseTest(t, []string{"LICENCSE.*\n", "!LICENCSE.md\n", "*.md"}, []string{"LICENCSE.foo", "LICENCSE.md"}, []string{"foo.bar"})
baseTest(t, []string{"LICENSE.*\n", "!LICENSE.md\n", "*.md"}, []string{"LICENSE.foo", "LICENSE.md"}, []string{"foo.bar"})
}
func TestExclusionWithWildcard(t *testing.T) {

View File

@@ -39,7 +39,8 @@ func TestIsValidGitRepository(t *testing.T) {
}
if err != nil {
if e, ok := err.(errors.Error); !ok || e.ErrorCode != errors.EmptyGitRepositoryError {
var e errors.Error
if e, ok = err.(errors.Error); !ok || e.ErrorCode != errors.EmptyGitRepositoryError {
t.Errorf("isValidGitRepository returned an unexpected error: %q, expecting EmptyGitRepositoryError", err.Error())
}
} else {

View File

@@ -8,7 +8,6 @@ import (
"os"
"path/filepath"
"regexp"
"sync"
"testing"
"time"
@@ -385,22 +384,12 @@ func TestExtractTarStream(t *testing.T) {
t.Fatalf("Cannot create temp directory: %v", err)
}
defer os.RemoveAll(destDir)
wg := sync.WaitGroup{}
wg.Add(2)
th := New()
go func() {
defer wg.Done()
if err := createTestTar(testFiles, writer); err != nil {
t.Fatal(err)
}
writer.Close()
writer.CloseWithError(createTestTar(testFiles, writer))
}()
go func() {
defer wg.Done()
th.ExtractTarStream(destDir, reader)
}()
wg.Wait()
verifyDirectory(t, destDir, testFiles)
}
@@ -411,22 +400,10 @@ func TestExtractTarStreamTimeout(t *testing.T) {
t.Fatalf("Cannot create temp directory: %v", err)
}
defer os.RemoveAll(destDir)
wg := sync.WaitGroup{}
wg.Add(2)
th := New()
th.(*stiTar).timeout = 10 * time.Millisecond
go func() {
defer wg.Done()
time.Sleep(20 * time.Millisecond)
writer.Close()
}()
extractError := make(chan error, 1)
go func() {
defer wg.Done()
extractError <- th.ExtractTarStream(destDir, reader)
}()
wg.Wait()
err = <-extractError
time.AfterFunc(20*time.Millisecond, func() { writer.Close() })
err = th.ExtractTarStream(destDir, reader)
if e, ok := err.(errors.Error); err == nil || (ok && e.ErrorCode != errors.TarTimeoutError) {
t.Errorf("Did not get the expected timeout error. err = %v", err)
}

View File

@@ -4,11 +4,11 @@ import (
"io/ioutil"
"net/url"
"os"
"os/exec"
"path/filepath"
"testing"
"github.com/openshift/source-to-image/pkg/api"
"github.com/openshift/source-to-image/pkg/util"
)
// FakeGit provides a fake Git
@@ -90,23 +90,18 @@ func (f *FakeGit) GetInfo(repo string) *api.SourceInfo {
// CreateLocalGitDirectory creates a git directory with a commit
func CreateLocalGitDirectory(t *testing.T) string {
cr := util.NewCommandRunner()
dir := CreateEmptyLocalGitDirectory(t)
f, err := os.Create(filepath.Join(dir, "testfile"))
if err != nil {
t.Fatal(err)
}
f.Close()
cmd := exec.Command("git", "add", ".")
cmd.Dir = dir
err = cmd.Run()
err = cr.RunWithOptions(util.CommandOpts{Dir: dir}, "git", "add", ".")
if err != nil {
t.Fatal(err)
}
cmd = exec.Command("git", "commit", "-m", "testcommit")
cmd.Env = os.Environ()
cmd.Env = append(cmd.Env, "GIT_AUTHOR_NAME=test", "GIT_AUTHOR_EMAIL=test@test", "GIT_COMMITTER_NAME=test", "GIT_COMMITTER_EMAIL=test@test")
cmd.Dir = dir
err = cmd.Run()
err = cr.RunWithOptions(util.CommandOpts{Dir: dir, EnvAppend: []string{"GIT_AUTHOR_NAME=test", "GIT_AUTHOR_EMAIL=test@test", "GIT_COMMITTER_NAME=test", "GIT_COMMITTER_EMAIL=test@test"}}, "git", "commit", "-m", "testcommit")
if err != nil {
t.Fatal(err)
}
@@ -116,13 +111,13 @@ func CreateLocalGitDirectory(t *testing.T) string {
// CreateEmptyLocalGitDirectory creates a git directory with no checkin yet
func CreateEmptyLocalGitDirectory(t *testing.T) string {
cr := util.NewCommandRunner()
dir, err := ioutil.TempDir(os.TempDir(), "gitdir-s2i-test")
if err != nil {
t.Fatal(err)
}
cmd := exec.Command("git", "init")
cmd.Dir = dir
err = cmd.Run()
err = cr.RunWithOptions(util.CommandOpts{Dir: dir}, "git", "init")
if err != nil {
t.Fatal(err)
}
@@ -132,13 +127,13 @@ func CreateEmptyLocalGitDirectory(t *testing.T) string {
// CreateLocalGitDirectoryWithSubmodule creates a git directory with a submodule
func CreateLocalGitDirectoryWithSubmodule(t *testing.T) string {
cr := util.NewCommandRunner()
submodule := CreateLocalGitDirectory(t)
defer os.RemoveAll(submodule)
dir := CreateEmptyLocalGitDirectory(t)
cmd := exec.Command("git", "submodule", "add", submodule, "submodule")
cmd.Dir = dir
err := cmd.Run()
err := cr.RunWithOptions(util.CommandOpts{Dir: dir}, "git", "submodule", "add", submodule, "submodule")
if err != nil {
t.Fatal(err)
}

View File

@@ -2,6 +2,7 @@ package util
import (
"io"
"os"
"os/exec"
)
@@ -11,6 +12,7 @@ type CommandOpts struct {
Stdout io.Writer
Stderr io.Writer
Dir string
EnvAppend []string
}
// CommandRunner executes OS commands with the given parameters and options
@@ -39,6 +41,10 @@ func (c *runner) RunWithOptions(opts CommandOpts, name string, arg ...string) er
if opts.Dir != "" {
cmd.Dir = opts.Dir
}
if len(opts.EnvAppend) > 0 {
cmd.Env = os.Environ()
cmd.Env = append(cmd.Env, opts.EnvAppend...)
}
return cmd.Run()
}

View File

@@ -46,10 +46,13 @@ type fs struct {
runner CommandRunner
}
// Stat returns a FileInfo describing the named file.
func (h *fs) Stat(path string) (os.FileInfo, error) {
return os.Stat(path)
}
// ReadDir reads the directory named by dirname and returns a list of directory
// entries sorted by filename.
func (h *fs) ReadDir(path string) ([]os.FileInfo, error) {
return ioutil.ReadDir(path)
}
@@ -193,7 +196,8 @@ func (h *fs) Open(filename string) (io.ReadCloser, error) {
return os.Open(filename)
}
// Write opens a file and writes data to it, returning error if such occurred
// WriteFile opens a file and writes data to it, returning error if such
// occurred
func (h *fs) WriteFile(filename string, data []byte) error {
return ioutil.WriteFile(filename, data, 0700)
}

View File

@@ -51,16 +51,47 @@ var (
// discard is a Logger that outputs nothing.
type discard struct{}
func (discard) Is(level int32) bool { return false }
func (discarding discard) V(level int32) VerboseLogger { return discarding }
func (discard) Infof(_ string, _ ...interface{}) {}
func (discard) Info(_ ...interface{}) {}
func (discard) Errorf(_ string, _ ...interface{}) {}
func (discard) Error(_ ...interface{}) {}
func (discard) Warningf(_ string, _ ...interface{}) {}
func (discard) Warning(_ ...interface{}) {}
func (discard) Fatalf(_ string, _ ...interface{}) {}
func (discard) Fatal(_ ...interface{}) {}
// Is returns whether the current logging level is greater than or equal to the parameter.
func (discard) Is(level int32) bool {
return false
}
// V will returns a logger which will discard output if the specified level is greater than the current logging level.
func (discarding discard) V(level int32) VerboseLogger {
return discarding
}
// Infof records an info log entry.
func (discard) Infof(string, ...interface{}) {
}
// Info records an info log entry.
func (discard) Info(...interface{}) {
}
// Errorf records an error log entry.
func (discard) Errorf(string, ...interface{}) {
}
// Error records an error log entry.
func (discard) Error(...interface{}) {
}
// Warningf records an warning log entry.
func (discard) Warningf(string, ...interface{}) {
}
// Warning records an warning log entry.
func (discard) Warning(...interface{}) {
}
// Fatalf records a fatal log entry.
func (discard) Fatalf(string, ...interface{}) {
}
// Fatal records a fatal log entry.
func (discard) Fatal(...interface{}) {
}
// FileLogger logs the provided messages at level or below to the writer, or delegates
// to glog.

View File

@@ -83,6 +83,7 @@ func (b *rangeBuilder) setBound(num int, err error, bound **int) *rangeBuilder {
return b
}
// Range returns the completed Range from the rangeBuilder.
func (b *rangeBuilder) Range() (*Range, error) {
if b.err != nil {
return nil, b.err

View File

@@ -22,6 +22,7 @@ import (
"github.com/openshift/source-to-image/pkg/api"
"github.com/openshift/source-to-image/pkg/build/strategies"
"github.com/openshift/source-to-image/pkg/docker"
"github.com/openshift/source-to-image/pkg/tar"
"github.com/openshift/source-to-image/pkg/util"
"golang.org/x/net/context"
)
@@ -252,6 +253,7 @@ func (i *integrationTest) exerciseCleanAllowedUIDsBuild(tag, imageName string, e
Tag: tag,
Incremental: false,
ScriptsURL: "",
ExcludeRegExp: tar.DefaultExclusionPattern.String(),
}
config.AllowedUIDs.Set("1-")
_, _, err := strategies.GetStrategy(config)
@@ -309,7 +311,9 @@ func (i *integrationTest) exerciseCleanBuild(tag string, verifyCallback bool, im
Tag: buildTag,
Incremental: false,
CallbackURL: callbackURL,
ScriptsURL: scriptsURL}
ScriptsURL: scriptsURL,
ExcludeRegExp: tar.DefaultExclusionPattern.String(),
}
b, _, err := strategies.GetStrategy(config)
if err != nil {
@@ -333,8 +337,8 @@ func (i *integrationTest) exerciseCleanBuild(tag string, verifyCallback bool, im
if setTag {
i.checkForImage(tag)
containerID := i.createContainer(tag)
defer i.removeContainer(containerID)
i.checkBasicBuildState(containerID, resp.WorkingDir)
i.removeContainer(containerID)
}
// Check if we receive back an ImageID when we are expecting to
@@ -393,6 +397,7 @@ func (i *integrationTest) exerciseInjectionBuild(tag, imageName string, injectio
Source: TestSource,
Tag: tag,
Injections: injectionList,
ExcludeRegExp: tar.DefaultExclusionPattern.String(),
}
builder, _, err := strategies.GetStrategy(config)
if err != nil {
@@ -435,6 +440,7 @@ func (i *integrationTest) exerciseIncrementalBuild(tag, imageName string, remove
Tag: tag,
Incremental: false,
RemovePreviousImage: removePreviousImage,
ExcludeRegExp: tar.DefaultExclusionPattern.String(),
}
builder, _, err := strategies.GetStrategy(config)
@@ -459,6 +465,7 @@ func (i *integrationTest) exerciseIncrementalBuild(tag, imageName string, remove
Incremental: true,
RemovePreviousImage: removePreviousImage,
PreviousImagePullPolicy: api.PullIfNotPresent,
ExcludeRegExp: tar.DefaultExclusionPattern.String(),
}
builder, _, err = strategies.GetStrategy(config)
@@ -551,7 +558,13 @@ func (i *integrationTest) runInContainer(image string, command []string) int {
defer cancel()
exitCode, err := i.engineClient.ContainerWait(ctx, container.ID)
if err != nil {
i.t.Errorf("Couldn't start container: %s", container.ID)
i.t.Errorf("Couldn't wait for container: %s", container.ID)
}
ctx, cancel = getDefaultContext()
defer cancel()
err = i.engineClient.ContainerRemove(ctx, container.ID, dockertypes.ContainerRemoveOptions{})
if err != nil {
i.t.Errorf("Couldn't remove container: %s", container.ID)
}
return exitCode
}
@@ -561,10 +574,12 @@ func (i *integrationTest) removeContainer(cID string) {
defer cancel()
removeOpts := dockertypes.ContainerRemoveOptions{
RemoveVolumes: true,
RemoveLinks: true,
Force: true,
}
i.engineClient.ContainerRemove(ctx, cID, removeOpts)
err := i.engineClient.ContainerRemove(ctx, cID, removeOpts)
if err != nil {
i.t.Errorf("Couldn't remove container %s: %s", cID, err)
}
}
func (i *integrationTest) fileExists(cID string, filePath string) {