From 5e3fd039d39ccc01a6b1888cdd67367d192ab325 Mon Sep 17 00:00:00 2001 From: oatmealraisin Date: Mon, 23 Jan 2017 17:36:24 -0500 Subject: [PATCH] Changed splitting index of Volume specs to first : This allows us to specify file modes when Docker mounts, such as :Z or :rw --- cmd/s2i/main.go | 4 +- hack/test-stirunimage.sh | 87 +++++++++++++++++---------------- pkg/api/describe/describer.go | 9 +++- pkg/api/types.go | 2 +- pkg/build/strategies/sti/sti.go | 2 +- 5 files changed, 58 insertions(+), 46 deletions(-) diff --git a/cmd/s2i/main.go b/cmd/s2i/main.go index e8f75ba0f..9a805f9a4 100644 --- a/cmd/s2i/main.go +++ b/cmd/s2i/main.go @@ -4,6 +4,7 @@ import ( "bytes" "flag" "fmt" + "io" "math/rand" "os" "path/filepath" @@ -31,7 +32,6 @@ import ( "github.com/openshift/source-to-image/pkg/tar" "github.com/openshift/source-to-image/pkg/util" "github.com/openshift/source-to-image/pkg/version" - "io" ) // glog is a placeholder until the builders pass an output stream down @@ -187,7 +187,7 @@ $ s2i build . centos/ruby-22-centos7 hello-world-app buildCmd.Flags().StringVarP(&(cfg.Description), "description", "", "", "Specify the description of the application") buildCmd.Flags().VarP(&(cfg.AllowedUIDs), "allowed-uids", "u", "Specify a range of allowed user ids for the builder and runtime images") buildCmd.Flags().VarP(&(cfg.Injections), "inject", "i", "Specify a directory to inject into the assemble container") - buildCmd.Flags().VarP(&(cfg.BuildVolumes), "volume", "v", "Specify a volume to mount into the assemble container") + buildCmd.Flags().StringArrayVarP(&(cfg.BuildVolumes), "volume", "v", []string{}, "Specify a volume to mount into the assemble container") buildCmd.Flags().StringSliceVar(&(cfg.DropCapabilities), "cap-drop", []string{}, "Specify a comma-separated list of capabilities to drop when running Docker containers") buildCmd.Flags().StringVarP(&(oldDestination), "location", "l", "", "DEPRECATED: Specify a destination location for untar operation") diff --git a/hack/test-stirunimage.sh b/hack/test-stirunimage.sh index 9a22c9404..a41e51146 100755 --- a/hack/test-stirunimage.sh +++ b/hack/test-stirunimage.sh @@ -8,14 +8,14 @@ export PATH="$PWD/_output/local/bin/$(go env GOHOSTOS)/$(go env GOHOSTARCH):$PAT function time_now() { - date +%s000 + date +%s000 } mkdir -p /tmp/sti WORK_DIR=$(mktemp -d /tmp/sti/test-work.XXXX) S2I_WORK_DIR=${WORK_DIR} if [[ "$OSTYPE" == "cygwin" ]]; then - S2I_WORK_DIR=$(cygpath -w ${WORK_DIR}) + S2I_WORK_DIR=$(cygpath -w ${WORK_DIR}) fi mkdir -p ${WORK_DIR} NEEDKILL="yes" @@ -25,39 +25,39 @@ function cleanup() set +e #some failures will exit the shell script before check_result() can dump the logs (ssh seems to be such a case) if [ -a "${WORK_DIR}/ran-clean" ]; then - echo "Cleaning up working dir ${WORK_DIR}" + echo "Cleaning up working dir ${WORK_DIR}" else - echo "Dumping logs since did not run successfully before cleanup of ${WORK_DIR} ..." - cat ${WORK_DIR}/*.log + echo "Dumping logs since did not run successfully before cleanup of ${WORK_DIR} ..." + cat ${WORK_DIR}/*.log fi rm -rf ${WORK_DIR} # use sigint so that s2i post processing will remove docker container if [ -n "${NEEDKILL}" ]; then - if [ -n "${S2I_PID}" ]; then - kill -2 "${S2I_PID}" - fi + if [ -n "${S2I_PID}" ]; then + kill -2 "${S2I_PID}" + fi fi echo echo "Complete" } function check_result() { - local result=$1 - if [ $result -eq 0 ]; then - echo - echo "TEST PASSED" - echo - if [ -n "${2}" ]; then - rm $2 - fi - else - echo - echo "TEST FAILED ${result}" - echo - cat $2 - cleanup - exit $result - fi + local result=$1 + if [ $result -eq 0 ]; then + echo + echo "TEST PASSED" + echo + if [ -n "${2}" ]; then + rm $2 + fi + else + echo + echo "TEST FAILED ${result}" + echo + cat $2 + cleanup + exit $result + fi } function test_debug() { @@ -69,6 +69,7 @@ function test_debug() { trap cleanup EXIT SIGINT echo "working dir: ${WORK_DIR}" +echo "s2i working dir: ${S2I_WORK_DIR}" pushd ${WORK_DIR} test_debug "cloning source into working dir" @@ -86,6 +87,10 @@ test_debug "s2i build with relative path with file://" s2i build file://./cakephp-ex openshift/php-55-centos7 test --loglevel=5 &> "${WORK_DIR}/s2i-rel-proto.log" check_result $? "${WORK_DIR}/s2i-rel-proto.log" +test_debug "s2i build with volume options" +s2i build cakephp-ex openshift/php-55-centos7 test --volume "${WORK_DIR}:/home/:z" --loglevel=5 &> "${WORK_DIR}/s2i-volume-correct.log" +check_result $? "${WORK_DIR}/s2i-volume-correct.log" + popd test_debug "s2i build with absolute path with file://" @@ -145,23 +150,23 @@ set +e while [[ $(time_now) -lt $expire ]]; do grep "as a result of the --run=true option" "${WORK_DIR}/s2i-run.log" if [ $? -eq 0 ]; then - echo "[INFO] Success running command s2i --run=true" - - # use sigint so that s2i post processing will remove docker container - kill -2 "${S2I_PID}" - NEEDKILL="" - sleep 30 - docker ps -a | grep test-jee-app - - if [ $? -eq 1 ]; then - echo "[INFO] Success terminating associated docker container" - touch "${WORK_DIR}/ran-clean" - exit 0 - else - echo "[INFO] Associated docker container still found, review docker ps -a output above, and here is the dump of ${WORK_DIR}/s2i-run.log" - cat "${WORK_DIR}/s2i-run.log" - exit 1 - fi + echo "[INFO] Success running command s2i --run=true" + + # use sigint so that s2i post processing will remove docker container + kill -2 "${S2I_PID}" + NEEDKILL="" + sleep 30 + docker ps -a | grep test-jee-app + + if [ $? -eq 1 ]; then + echo "[INFO] Success terminating associated docker container" + touch "${WORK_DIR}/ran-clean" + exit 0 + else + echo "[INFO] Associated docker container still found, review docker ps -a output above, and here is the dump of ${WORK_DIR}/s2i-run.log" + cat "${WORK_DIR}/s2i-run.log" + exit 1 + fi fi sleep 1 done diff --git a/pkg/api/describe/describer.go b/pkg/api/describe/describer.go index 8d706f0a6..1f7743cfc 100644 --- a/pkg/api/describe/describer.go +++ b/pkg/api/describe/describer.go @@ -5,6 +5,7 @@ import ( "fmt" "io" "os" + "runtime" "strings" "text/tabwriter" @@ -78,12 +79,18 @@ func Config(config *api.Config) string { if len(config.BuildVolumes) > 0 { result := []string{} for _, i := range config.BuildVolumes { - result = append(result, fmt.Sprintf("%s->%s", i.Source, i.Destination)) + if runtime.GOOS == "windows" { + // We need to avoid the colon in the Windows drive letter + result = append(result, i[0:2]+strings.Replace(i[3:], ":", "->", 1)) + } else { + result = append(result, strings.Replace(i, ":", "->", 1)) + } } fmt.Fprintf(out, "Bind mounts:\t%s\n", strings.Join(result, ",")) } return nil }) + if err != nil { fmt.Printf("error: %v", err) } diff --git a/pkg/api/types.go b/pkg/api/types.go index 2c400d7a4..996f86420 100644 --- a/pkg/api/types.go +++ b/pkg/api/types.go @@ -232,7 +232,7 @@ type Config struct { // BuildVolumes specifies a list of volumes to mount to container running the // build. - BuildVolumes VolumeList + BuildVolumes []string // Labels specify labels and their values to be applied to the resulting image. Label keys // must have non-zero length. The labels defined here override generated labels in case diff --git a/pkg/build/strategies/sti/sti.go b/pkg/build/strategies/sti/sti.go index 8397b1127..ba36b3f9a 100644 --- a/pkg/build/strategies/sti/sti.go +++ b/pkg/build/strategies/sti/sti.go @@ -577,7 +577,7 @@ func (builder *STI) Execute(command string, user string, config *api.Config) err NetworkMode: string(config.DockerNetworkMode), CGroupLimits: config.CGroupLimits, CapDrop: config.DropCapabilities, - Binds: config.BuildVolumes.AsBinds(), + Binds: config.BuildVolumes, } // If there are injections specified, override the original assemble script