diff --git a/Godeps/_workspace/src/github.com/GoogleCloudPlatform/kubernetes/pkg/api/validation/validation.go b/Godeps/_workspace/src/github.com/GoogleCloudPlatform/kubernetes/pkg/api/validation/validation.go index 5e64bb80a0cd..80b059bce6bf 100644 --- a/Godeps/_workspace/src/github.com/GoogleCloudPlatform/kubernetes/pkg/api/validation/validation.go +++ b/Godeps/_workspace/src/github.com/GoogleCloudPlatform/kubernetes/pkg/api/validation/validation.go @@ -378,7 +378,7 @@ func ValidatePod(pod *api.Pod) errs.ValidationErrorList { allErrs = append(allErrs, errs.NewFieldInvalid("namespace", pod.Namespace, "")) } allErrs = append(allErrs, ValidatePodSpec(&pod.Spec).Prefix("spec")...) - allErrs = append(allErrs, validateLabels(pod.Labels, "labels")...) + allErrs = append(allErrs, ValidateLabels(pod.Labels, "labels")...) return allErrs } @@ -394,11 +394,11 @@ func ValidatePodSpec(spec *api.PodSpec) errs.ValidationErrorList { allErrs = append(allErrs, validateContainers(spec.Containers, allVolumes).Prefix("containers")...) allErrs = append(allErrs, validateRestartPolicy(&spec.RestartPolicy).Prefix("restartPolicy")...) allErrs = append(allErrs, validateDNSPolicy(&spec.DNSPolicy).Prefix("dnsPolicy")...) - allErrs = append(allErrs, validateLabels(spec.NodeSelector, "nodeSelector")...) + allErrs = append(allErrs, ValidateLabels(spec.NodeSelector, "nodeSelector")...) return allErrs } -func validateLabels(labels map[string]string, field string) errs.ValidationErrorList { +func ValidateLabels(labels map[string]string, field string) errs.ValidationErrorList { allErrs := errs.ValidationErrorList{} for k := range labels { if !util.IsQualifiedName(k) { @@ -458,9 +458,9 @@ func ValidateService(service *api.Service, lister ServiceLister, ctx api.Context } if service.Spec.Selector != nil { - allErrs = append(allErrs, validateLabels(service.Spec.Selector, "spec.selector")...) + allErrs = append(allErrs, ValidateLabels(service.Spec.Selector, "spec.selector")...) } - allErrs = append(allErrs, validateLabels(service.Labels, "labels")...) + allErrs = append(allErrs, ValidateLabels(service.Labels, "labels")...) if service.Spec.CreateExternalLoadBalancer { services, err := lister.ListServices(ctx) @@ -496,7 +496,7 @@ func ValidateReplicationController(controller *api.ReplicationController) errs.V allErrs = append(allErrs, errs.NewFieldInvalid("namespace", controller.Namespace, "")) } allErrs = append(allErrs, ValidateReplicationControllerSpec(&controller.Spec).Prefix("spec")...) - allErrs = append(allErrs, validateLabels(controller.Labels, "labels")...) + allErrs = append(allErrs, ValidateLabels(controller.Labels, "labels")...) return allErrs } @@ -533,7 +533,7 @@ func ValidateReplicationControllerSpec(spec *api.ReplicationControllerSpec) errs // ValidatePodTemplateSpec validates the spec of a pod template func ValidatePodTemplateSpec(spec *api.PodTemplateSpec) errs.ValidationErrorList { allErrs := errs.ValidationErrorList{} - allErrs = append(allErrs, validateLabels(spec.Labels, "labels")...) + allErrs = append(allErrs, ValidateLabels(spec.Labels, "labels")...) allErrs = append(allErrs, ValidatePodSpec(&spec.Spec).Prefix("spec")...) allErrs = append(allErrs, ValidateReadOnlyPersistentDisks(spec.Spec.Volumes).Prefix("spec.volumes")...) return allErrs @@ -577,7 +577,7 @@ func ValidateMinion(minion *api.Node) errs.ValidationErrorList { if len(minion.Name) == 0 { allErrs = append(allErrs, errs.NewFieldRequired("name", minion.Name)) } - allErrs = append(allErrs, validateLabels(minion.Labels, "labels")...) + allErrs = append(allErrs, ValidateLabels(minion.Labels, "labels")...) return allErrs } diff --git a/Godeps/_workspace/src/github.com/GoogleCloudPlatform/kubernetes/pkg/client/cache/poller.go b/Godeps/_workspace/src/github.com/GoogleCloudPlatform/kubernetes/pkg/client/cache/poller.go index 70fec9361da4..deb708320de6 100644 --- a/Godeps/_workspace/src/github.com/GoogleCloudPlatform/kubernetes/pkg/client/cache/poller.go +++ b/Godeps/_workspace/src/github.com/GoogleCloudPlatform/kubernetes/pkg/client/cache/poller.go @@ -57,14 +57,22 @@ func NewPoller(getFunc GetFunc, period time.Duration, store Store) *Poller { // Run begins polling. It starts a goroutine and returns immediately. func (p *Poller) Run() { - go util.Forever(func() { - e, err := p.getFunc() - if err != nil { - glog.Errorf("failed to list: %v", err) - return - } - p.sync(e) - }, p.period) + go util.Forever(p.run, p.period) +} + +// RunUntil begins polling. It starts a goroutine and returns immediately. +// It will stop when the stopCh is closed. +func (p *Poller) RunUntil(stopCh <-chan struct{}) { + go util.Until(p.run, p.period, stopCh) +} + +func (p *Poller) run() { + e, err := p.getFunc() + if err != nil { + glog.Errorf("failed to list: %v", err) + return + } + p.sync(e) } func (p *Poller) sync(e Enumerator) { diff --git a/Godeps/_workspace/src/github.com/GoogleCloudPlatform/kubernetes/pkg/client/cache/reflector.go b/Godeps/_workspace/src/github.com/GoogleCloudPlatform/kubernetes/pkg/client/cache/reflector.go index 56ecdecc50e7..68ae3f22b057 100644 --- a/Godeps/_workspace/src/github.com/GoogleCloudPlatform/kubernetes/pkg/client/cache/reflector.go +++ b/Godeps/_workspace/src/github.com/GoogleCloudPlatform/kubernetes/pkg/client/cache/reflector.go @@ -72,6 +72,12 @@ func (r *Reflector) Run() { go util.Forever(func() { r.listAndWatch() }, r.period) } +// RunUntil starts a watch and handles watch events. Will restart the watch if it is closed. +// RunUntil starts a goroutine and returns immediately. It will exit when stopCh is closed. +func (r *Reflector) RunUntil(stopCh <-chan struct{}) { + go util.Until(func() { r.listAndWatch() }, r.period, stopCh) +} + func (r *Reflector) listAndWatch() { var resourceVersion string diff --git a/Makefile b/Makefile index 116ccff298a7..e58d5d2d08a0 100644 --- a/Makefile +++ b/Makefile @@ -77,3 +77,11 @@ clean: rm -rf $(OUT_DIR) $(OUT_PKG_DIR) .PHONY: clean +# Build an official release of OpenShift, including the official images. +# +# Example: +# make clean +release: clean + hack/build-release.sh + hack/build-images.sh +.PHONY: release diff --git a/cmd/openshift/openshift.go b/cmd/openshift/openshift.go index 949911b1c8e1..09e548d7b4a6 100644 --- a/cmd/openshift/openshift.go +++ b/cmd/openshift/openshift.go @@ -5,9 +5,12 @@ import ( "path/filepath" "github.com/openshift/origin/pkg/cmd/openshift" + "github.com/openshift/origin/pkg/cmd/util/serviceability" ) func main() { + serviceability.BehaviorOnPanic(os.Getenv("OPENSHIFT_ON_PANIC")) + basename := filepath.Base(os.Args[0]) command := openshift.CommandFor(basename) if err := command.Execute(); err != nil { diff --git a/examples/jenkins/jenkins-config.json b/examples/jenkins/jenkins-config.json index 4a2db1e06713..be2c84960c4f 100644 --- a/examples/jenkins/jenkins-config.json +++ b/examples/jenkins/jenkins-config.json @@ -19,7 +19,7 @@ }, { "metadata": { - "id":"jenkins" + "name":"jenkins" }, "kind":"DeploymentConfig", "apiVersion":"v1beta1", diff --git a/examples/sample-app/application-template-custombuild.json b/examples/sample-app/application-template-custombuild.json index 42cb1aa8a293..9b09a7acd72a 100644 --- a/examples/sample-app/application-template-custombuild.json +++ b/examples/sample-app/application-template-custombuild.json @@ -1,7 +1,7 @@ { "metadata":{ "name": "ruby-helloworld-sample", - }, + }, "kind": "Template", "apiVersion": "v1beta1", "description": "This example shows how to create a simple ruby application in openshift origin v3", @@ -44,10 +44,9 @@ { "metadata":{ "name": "origin-ruby-sample", - }, + }, "kind": "ImageRepository", "apiVersion": "v1beta1", - "dockerImageRepository": "172.30.17.3:5001/custom/origin-ruby-sample", "labels": { "name": "origin-ruby-sample" } @@ -58,7 +57,6 @@ }, "kind": "ImageRepository", "apiVersion": "v1beta1", - "dockerImageRepository": "172.30.17.3:5001/openshift/origin-custom-docker-builder", "labels": { "name": "custom-docker-builder" } @@ -85,8 +83,10 @@ { "type": "imageChange", "imageChange": { - "image": "172.30.17.3:5001/openshift/origin-custom-docker-builder", - "imageRepositoryRef": { "name": "origin-custom-docker-builder"}, + "image": "openshift/origin-custom-docker-builder", + "from": { + "name": "origin-custom-docker-builder" + }, "tag":"latest" } } @@ -107,8 +107,9 @@ } }, "output": { - "imageTag": "custom/origin-ruby-sample:latest", - "registry": "172.30.17.3:5001" + "to": { + "name": "origin-ruby-sample" + } }, }, "labels": { @@ -129,7 +130,9 @@ "containerNames": [ "ruby-helloworld" ], - "repositoryName": "172.30.17.3:5001/custom/origin-ruby-sample", + "from": { + "name": "origin-ruby-sample" + }, "tag": "latest" } } @@ -150,7 +153,7 @@ "containers": [ { "name": "ruby-helloworld", - "image": "172.30.17.3:5001/custom/origin-ruby-sample", + "image": "origin-ruby-sample", "env": [ { "name": "ADMIN_USERNAME", diff --git a/examples/sample-app/application-template-dockerbuild.json b/examples/sample-app/application-template-dockerbuild.json index 71eda96a31e2..23ccf0382644 100644 --- a/examples/sample-app/application-template-dockerbuild.json +++ b/examples/sample-app/application-template-dockerbuild.json @@ -1,7 +1,7 @@ { "metadata":{ "name": "ruby-helloworld-sample", - }, + }, "kind": "Template", "apiVersion": "v1beta1", "description": "This example shows how to create a simple ruby application in openshift origin v3", @@ -44,10 +44,9 @@ { "metadata":{ "name": "origin-ruby-sample", - }, + }, "kind": "ImageRepository", "apiVersion": "v1beta1", - "dockerImageRepository": "172.30.17.3:5001/docker/origin-ruby-sample", "labels": { "name": "origin-ruby-sample" } @@ -58,7 +57,6 @@ }, "kind": "ImageRepository", "apiVersion": "v1beta1", - "dockerImageRepository": "172.30.17.3:5001/openshift/ruby-20-centos", "labels": { "name": "ruby-20-centos" } @@ -85,8 +83,10 @@ { "type": "imageChange", "imageChange": { - "image": "172.30.17.3:5001/openshift/ruby-20-centos", - "imageRepositoryRef": { "name": "ruby-20-centos"}, + "image": "openshift/ruby-20-centos", + "from": { + "name": "ruby-20-centos" + }, "tag":"latest" } } @@ -102,8 +102,9 @@ "type": "Docker" }, "output": { - "imageTag": "docker/origin-ruby-sample:latest", - "registry": "172.30.17.3:5001" + "to": { + "name": "origin-ruby-sample" + }, }, }, "labels": { @@ -124,7 +125,9 @@ "containerNames": [ "ruby-helloworld" ], - "repositoryName": "172.30.17.3:5001/docker/origin-ruby-sample", + "from": { + "name": "origin-ruby-sample" + }, "tag": "latest" } } @@ -145,7 +148,7 @@ "containers": [ { "name": "ruby-helloworld", - "image": "172.30.17.3:5001/docker/origin-ruby-sample", + "image": "origin-ruby-sample", "env": [ { "name": "ADMIN_USERNAME", diff --git a/examples/sample-app/application-template-stibuild.json b/examples/sample-app/application-template-stibuild.json index c539b31f2222..58ddbf0e2d30 100644 --- a/examples/sample-app/application-template-stibuild.json +++ b/examples/sample-app/application-template-stibuild.json @@ -47,7 +47,6 @@ }, "kind": "ImageRepository", "apiVersion": "v1beta1", - "dockerImageRepository": "172.30.17.3:5001/test/origin-ruby-sample", "labels": { "name": "origin-ruby-sample" } @@ -58,7 +57,6 @@ }, "kind": "ImageRepository", "apiVersion": "v1beta1", - "dockerImageRepository": "172.30.17.3:5001/openshift/ruby-20-centos", "labels": { "name": "ruby-20-centos" } @@ -85,8 +83,8 @@ { "type": "imageChange", "imageChange": { - "image": "172.30.17.3:5001/openshift/ruby-20-centos", - "imageRepositoryRef": { "name": "ruby-20-centos"}, + "image": "openshift/ruby-20-centos", + "from": { "name": "ruby-20-centos"}, "tag":"latest" } } @@ -106,8 +104,9 @@ } }, "output": { - "imageTag": "test/origin-ruby-sample:latest", - "registry": "172.30.17.3:5001" + "to": { + "name": "origin-ruby-sample" + } }, }, "labels": { @@ -128,7 +127,9 @@ "containerNames": [ "ruby-helloworld" ], - "repositoryName": "172.30.17.3:5001/test/origin-ruby-sample", + "from": { + "name": "origin-ruby-sample" + }, "tag": "latest" } } @@ -149,7 +150,7 @@ "containers": [ { "name": "ruby-helloworld", - "image": "172.30.17.3:5001/test/origin-ruby-sample", + "image": "origin-ruby-sample", "env": [ { "name": "ADMIN_USERNAME", diff --git a/examples/sample-app/docker-registry-template.json b/examples/sample-app/docker-registry-template.json index 2c2b8d419933..df8e965ccee4 100644 --- a/examples/sample-app/docker-registry-template.json +++ b/examples/sample-app/docker-registry-template.json @@ -33,7 +33,6 @@ "creationTimestamp":null, "id":"docker-registry", "kind":"Service", - "portalIp": "172.30.17.3", "port":5001, "containerPort":5000, "selector":{ diff --git a/hack/test-cmd.sh b/hack/test-cmd.sh index 9bd506ab572b..75be28ae32e0 100755 --- a/hack/test-cmd.sh +++ b/hack/test-cmd.sh @@ -25,6 +25,8 @@ trap cleanup EXIT SIGINT set -e +export OPENSHIFT_ON_PANIC=crash + USE_LOCAL_IMAGES=${USE_LOCAL_IMAGES:-true} ETCD_HOST=${ETCD_HOST:-127.0.0.1} @@ -37,7 +39,7 @@ KUBELET_PORT=${KUBELET_PORT:-10250} ETCD_DATA_DIR=$(mktemp -d /tmp/openshift.local.etcd.XXXX) VOLUME_DIR=$(mktemp -d /tmp/openshift.local.volumes.XXXX) -CERT_DIR=$(mktemp -d /tmp/openshift.local.certificates.XXXX) +CERT_DIR="${CERT_DIR:-$(mktemp -d /tmp/openshift.local.certificates.XXXX)}" # set path so OpenShift is available GO_OUT="${OS_ROOT}/_output/local/go/bin" @@ -48,22 +50,22 @@ out=$(openshift version) echo openshift: $out # Start openshift -openshift start --master="${API_SCHEME}://${API_HOST}:${API_PORT}" --listen="${API_SCHEME}://0.0.0.0:${API_PORT}" --volume-dir="${VOLUME_DIR}" --etcd-dir="${ETCD_DATA_DIR}" --cert-dir="${CERT_DIR}" 1>&2 & +openshift start --master="${API_SCHEME}://${API_HOST}:${API_PORT}" --listen="${API_SCHEME}://${API_HOST}:${API_PORT}" --hostname="${API_HOST}" --volume-dir="${VOLUME_DIR}" --cert-dir="${CERT_DIR}" --etcd-dir="${ETCD_DATA_DIR}" 1>&2 & OS_PID=$! -if [[ "$API_SCHEME" == "https" ]]; then - export CURL_CA_BUNDLE="$CERT_DIR/admin/root.crt" +if [[ "${API_SCHEME}" == "https" ]]; then + export CURL_CA_BUNDLE="${CERT_DIR}/admin/root.crt" fi - -wait_for_url "${KUBELET_SCHEME}://${API_HOST}:${KUBELET_PORT}/healthz" "kubelet: " 1 30 -wait_for_url "${API_SCHEME}://${API_HOST}:${API_PORT}/healthz" "apiserver: " +wait_for_url "http://${API_HOST}:${KUBELET_PORT}/healthz" "kubelet: " 0.2 60 +wait_for_url "${API_SCHEME}://${API_HOST}:${API_PORT}/healthz" "apiserver: " 0.2 60 +wait_for_url "${API_SCHEME}://${API_HOST}:${API_PORT}/api/v1beta1/minions/127.0.0.1" "apiserver(minions): " 0.2 60 # Set KUBERNETES_MASTER for osc export KUBERNETES_MASTER="${API_SCHEME}://${API_HOST}:${API_PORT}" -if [[ "$API_SCHEME" == "https" ]]; then - # Make osc use $CERT_DIR/admin/.kubeconfig, and ignore anything in the running user's $HOME dir - export HOME=$CERT_DIR/admin - export KUBECONFIG=$CERT_DIR/admin/.kubeconfig +if [[ "${API_SCHEME}" == "https" ]]; then + # Make osc use ${CERT_DIR}/admin/.kubeconfig, and ignore anything in the running user's $HOME dir + export HOME="${CERT_DIR}/admin" + export KUBECONFIG="${CERT_DIR}/admin/.kubeconfig" fi # @@ -90,6 +92,10 @@ echo "images: ok" osc get imageRepositories osc create -f test/integration/fixtures/test-image-repository.json +[ -z "$(osc get imageRepositories test -t "{{.status.dockerImageRepository}}")" ] +osc apply -f examples/sample-app/docker-registry-config.json +[ -n "$(osc get imageRepositories test -t "{{.status.dockerImageRepository}}")" ] +osc delete -f examples/sample-app/docker-registry-config.json osc delete imageRepositories test echo "imageRepositories: ok" @@ -121,3 +127,5 @@ echo "start-build: ok" osc cancel-build "${started}" --dump-logs --restart echo "cancel-build: ok" + +osc get minions,pods diff --git a/hack/test-end-to-end.sh b/hack/test-end-to-end.sh index b2d62575a631..e7fcd61a0aaa 100755 --- a/hack/test-end-to-end.sh +++ b/hack/test-end-to-end.sh @@ -25,15 +25,17 @@ source "${OS_ROOT}/hack/util.sh" echo "[INFO] Starting end-to-end test" +export OPENSHIFT_ON_PANIC=crash USE_LOCAL_IMAGES="${USE_LOCAL_IMAGES:-true}" ROUTER_TESTS_ENABLED="${ROUTER_TESTS_ENABLED:-true}" TMPDIR="${TMPDIR:-"/tmp"}" -ETCD_DATA_DIR=$(mktemp -d ${TMPDIR}/openshift.local.etcd.XXXX) -VOLUME_DIR=$(mktemp -d ${TMPDIR}/openshift.local.volumes.XXXX) -CERT_DIR=$(mktemp -d ${TMPDIR}/openshift.local.certificates.XXXX) -LOG_DIR="${LOG_DIR:-$(mktemp -d ${TMPDIR}/openshift.local.logs.XXXX)}" -ARTIFACT_DIR="${ARTIFACT_DIR:-$(mktemp -d ${TMPDIR}/openshift.local.artifacts.XXXX)}" +BASETMPDIR="${BASETMPDIR:-$(mktemp -d ${TMPDIR}/openshift-e2e.XXXX)}" +ETCD_DATA_DIR="${BASETMPDIR}/etcd" +VOLUME_DIR="${BASETMPDIR}/volumes" +CERT_DIR="${BASETMPDIR}/certs" +LOG_DIR="${LOG_DIR:-${BASETMPDIR}/logs}" +ARTIFACT_DIR="${ARTIFACT_DIR:-${BASETMPDIR}/artifacts}" mkdir -p $LOG_DIR mkdir -p $ARTIFACT_DIR API_PORT="${API_PORT:-8443}" @@ -135,37 +137,38 @@ trap teardown EXIT SIGINT # Setup stop_openshift_server echo "[INFO] `openshift version`" -echo "[INFO] Server logs will be at: $LOG_DIR/openshift.log" -echo "[INFO] Test artifacts will be in: $ARTIFACT_DIR" -echo "[INFO] Volumes dir is: $VOLUME_DIR" -echo "[INFO] Certs dir is: $CERT_DIR" +echo "[INFO] Server logs will be at: ${LOG_DIR}/openshift.log" +echo "[INFO] Test artifacts will be in: ${ARTIFACT_DIR}" +echo "[INFO] Volumes dir is: ${VOLUME_DIR}" +echo "[INFO] Certs dir is: ${CERT_DIR}" # Start All-in-one server and wait for health # Specify the scheme and port for the master, but let the IP auto-discover echo "[INFO] Starting OpenShift server" -sudo env "PATH=$PATH" openshift start --listen=$API_SCHEME://0.0.0.0:$API_PORT --volume-dir="${VOLUME_DIR}" --etcd-dir="${ETCD_DATA_DIR}" --cert-dir="${CERT_DIR}" --loglevel=4 &> "${LOG_DIR}/openshift.log" & +sudo env "PATH=${PATH}" openshift start --listen="${API_SCHEME}://0.0.0.0:${API_PORT}" --hostname="127.0.0.1" --volume-dir="${VOLUME_DIR}" --etcd-dir="${ETCD_DATA_DIR}" --cert-dir="${CERT_DIR}" --loglevel=4 &> "${LOG_DIR}/openshift.log" & OS_PID=$! -if [[ "$API_SCHEME" == "https" ]]; then - export CURL_CA_BUNDLE="$CERT_DIR/master/root.crt" +if [[ "${API_SCHEME}" == "https" ]]; then + export CURL_CA_BUNDLE="${CERT_DIR}/master/root.crt" fi -wait_for_url "$KUBELET_SCHEME://$API_HOST:$KUBELET_PORT/healthz" "[INFO] kubelet: " 1 30 -wait_for_url "$API_SCHEME://$API_HOST:$API_PORT/healthz" "[INFO] apiserver: " +wait_for_url "${KUBELET_SCHEME}://${API_HOST}:${KUBELET_PORT}/healthz" "[INFO] kubelet: " 0.5 60 +wait_for_url "${API_SCHEME}://${API_HOST}:${API_PORT}/healthz" "[INFO] apiserver: " 0.5 60 +wait_for_url "${API_SCHEME}://${API_HOST}:${API_PORT}/api/v1beta1/minions/127.0.0.1" "[INFO] apiserver(minions): " 0.2 60 # Set KUBERNETES_MASTER for osc -export KUBERNETES_MASTER=$API_SCHEME://$API_HOST:$API_PORT -if [[ "$API_SCHEME" == "https" ]]; then +export KUBERNETES_MASTER="${API_SCHEME}://${API_HOST}:${API_PORT}" +if [[ "${API_SCHEME}" == "https" ]]; then # Read client cert data in to send to containerized components - sudo chmod 644 $CERT_DIR/openshift-client/* - OPENSHIFT_CA_DATA=$(<$CERT_DIR/openshift-client/root.crt) - OPENSHIFT_CERT_DATA=$(<$CERT_DIR/openshift-client/cert.crt) - OPENSHIFT_KEY_DATA=$(<$CERT_DIR/openshift-client/key.key) - - # Make osc use $CERT_DIR/admin/.kubeconfig, and ignore anything in the running user's $HOME dir - sudo chmod 644 $CERT_DIR/admin/* - export HOME=$CERT_DIR/admin - export KUBECONFIG=$CERT_DIR/admin/.kubeconfig + sudo chmod -R a+rX "${CERT_DIR}/openshift-client/" + OPENSHIFT_CA_DATA="$(cat "${CERT_DIR}/openshift-client/root.crt")" + OPENSHIFT_CERT_DATA="$(cat "${CERT_DIR}/openshift-client/cert.crt")" + OPENSHIFT_KEY_DATA="$(cat "${CERT_DIR}/openshift-client/key.key")" + + # Make osc use ${CERT_DIR}/admin/.kubeconfig, and ignore anything in the running user's $HOME dir + sudo chmod -R a+rwX "${CERT_DIR}/admin/" + export HOME="${CERT_DIR}/admin" + export KUBECONFIG="${CERT_DIR}/admin/.kubeconfig" echo "[INFO] To debug: export KUBECONFIG=$KUBECONFIG" else OPENSHIFT_CA_DATA="" @@ -175,19 +178,19 @@ fi # Deploy private docker registry echo "[INFO] Submitting docker-registry template file for processing" -osc process -n test -f examples/sample-app/docker-registry-template.json -v "OPENSHIFT_MASTER=$API_SCHEME://${CONTAINER_ACCESSIBLE_API_HOST}:$API_PORT,OPENSHIFT_CA_DATA=${OPENSHIFT_CA_DATA},OPENSHIFT_CERT_DATA=${OPENSHIFT_CERT_DATA},OPENSHIFT_KEY_DATA=${OPENSHIFT_KEY_DATA}" > "$ARTIFACT_DIR/docker-registry-config.json" +osc process -f examples/sample-app/docker-registry-template.json -v "OPENSHIFT_MASTER=$API_SCHEME://${CONTAINER_ACCESSIBLE_API_HOST}:$API_PORT,OPENSHIFT_CA_DATA=${OPENSHIFT_CA_DATA},OPENSHIFT_CERT_DATA=${OPENSHIFT_CERT_DATA},OPENSHIFT_KEY_DATA=${OPENSHIFT_KEY_DATA}" > "$ARTIFACT_DIR/docker-registry-config.json" echo "[INFO] Deploying private Docker registry from $ARTIFACT_DIR/docker-registry-config.json" -osc apply -n test -f ${ARTIFACT_DIR}/docker-registry-config.json +osc apply -f ${ARTIFACT_DIR}/docker-registry-config.json echo "[INFO] Waiting for Docker registry pod to start" -wait_for_command "osc get -n test pods | grep registrypod | grep -i Running" $((5*TIME_MIN)) +wait_for_command "osc get pods | grep registrypod | grep -i Running" $((5*TIME_MIN)) echo "[INFO] Waiting for Docker registry service to start" -wait_for_command "osc get -n test services | grep registrypod" +wait_for_command "osc get services | grep registrypod" # services can end up on any IP. Make sure we get the IP we need for the docker registry -DOCKER_REGISTRY_IP=$(osc get -n test -o template --output-version=v1beta1 --template="{{ .portalIP }}" service docker-registry) +DOCKER_REGISTRY_IP=$(osc get -o template --output-version=v1beta1 --template="{{ .portalIP }}" service docker-registry) echo "[INFO] Probing the docker-registry" wait_for_url_timed "http://${DOCKER_REGISTRY_IP}:5001" "[INFO] Docker registry says: " $((2*TIME_MIN)) @@ -206,11 +209,6 @@ osc process -n test -f examples/sample-app/application-template-stibuild.json > osc process -n docker -f examples/sample-app/application-template-dockerbuild.json > "${DOCKER_CONFIG_FILE}" osc process -n custom -f examples/sample-app/application-template-custombuild.json > "${CUSTOM_CONFIG_FILE}" -# substitute the default IP address with the address where we actually ended up -# TODO: make this be unnecessary by fixing images -# This is no longer needed because the docker registry explicitly requests the 172.30.17.3 ip address. -#sed -i "s,172.30.17.3,${DOCKER_REGISTRY_IP},g" "${CONFIG_FILE}" - echo "[INFO] Applying STI application config" osc apply -n test -f "${STI_CONFIG_FILE}" diff --git a/hack/test-go.sh b/hack/test-go.sh index 4f07c3eb81eb..55f79c81145b 100755 --- a/hack/test-go.sh +++ b/hack/test-go.sh @@ -47,6 +47,8 @@ fi OUTPUT_COVERAGE=${OUTPUT_COVERAGE:-""} +export OPENSHIFT_ON_PANIC=crash + if [[ -n "${KUBE_COVER}" && -n "${OUTPUT_COVERAGE}" ]]; then # Iterate over packages to run coverage test_packages=( $test_packages ) diff --git a/hack/test-integration.sh b/hack/test-integration.sh index 28a05d45eaf7..9d5d12451b9e 100755 --- a/hack/test-integration.sh +++ b/hack/test-integration.sh @@ -22,4 +22,5 @@ trap cleanup EXIT SIGINT echo echo Integration test cases ... echo -KUBE_RACE="${KUBE_RACE:--race}" KUBE_COVER=" " "${OS_ROOT}/hack/test-go.sh" test/integration -tags 'integration no-docker' "${@:1}" +# TODO: race is disabled because of origin #731 +KUBE_RACE="${KUBE_RACE:-}" KUBE_COVER=" " "${OS_ROOT}/hack/test-go.sh" test/integration -tags 'integration no-docker' "${@:1}" diff --git a/pkg/build/api/types.go b/pkg/build/api/types.go index c6637ccf058c..84b206de5f44 100644 --- a/pkg/build/api/types.go +++ b/pkg/build/api/types.go @@ -16,6 +16,9 @@ type Build struct { // Status is the current status of the build. Status BuildStatus `json:"status,omitempty"` + // A human readable message indicating details about why the build has this status + Message string `json:"message,omitempty"` + // PodName is the name of the pod that is used to execute the build PodName string `json:"podName,omitempty"` @@ -206,11 +209,22 @@ type STIBuildStrategy struct { // BuildOutput is input to a build strategy and describes the Docker image that the strategy // should produce. type BuildOutput struct { - // ImageTag is the tag to give to the image resulting from the build. - ImageTag string `json:"imageTag,omitempty"` + // To defines an optional ImageRepository to push the output of this build to. The namespace + // may be empty, in which case the named ImageRepository will be retrieved from the namespace + // of the build. Kind must be set to 'ImageRepository' and is the only supported value. If set, + // this field takes priority over DockerImageReference. This value will be used to look up + // a Docker image repository to push to. Failure to find the To will result in a build error. + To *kapi.ObjectReference `json:"to,omitempty"` + + // Tag is the "version name" that will be associated with the output image. This + // field is only used if the To field is set, and is ignored when DockerImageReference is used. + // This value represents a consistent name for a set of related changes (v1, 5.x, 5.5, dev, stable) + // and defaults to the preferred tag for "To" if not specified. + Tag string `json:"tag,omitempty"` - // Registry is the Docker registry which should receive the resulting built image via push. - Registry string `json:"registry,omitempty"` + // DockerImageReference is the full name of an image ([registry/]name[:tag]), and will be the + // value sent to Docker push at the end of a build if the To field is not defined. + DockerImageReference string `json:"dockerImageReference,omitempty"` } // BuildConfigLabel is the key of a Build label whose value is the ID of a BuildConfig @@ -226,7 +240,8 @@ type BuildConfig struct { // are defined, a new build can only occur as a result of an explicit client build creation. Triggers []BuildTriggerPolicy `json:"triggers,omitempty"` - // Parameters holds all the input necessary to produce a new build. + // Parameters holds all the input necessary to produce a new build. A build config may only + // define either the Output.To or Output.DockerImageReference fields, but not both. Parameters BuildParameters `json:"parameters,omitempty"` } @@ -241,8 +256,12 @@ type ImageChangeTrigger struct { // Image is used to specify the value in the BuildConfig to replace with the // immutable image id supplied by the ImageRepository when this trigger fires. Image string `json:"image"` - // ImageRepositoryRef a reference to a Docker image repository to watch for changes. - ImageRepositoryRef *kapi.ObjectReference `json:"imageRepositoryRef"` + // From is a reference to a Docker image repository to watch for changes. This field takes + // precedence over ImageRepositoryRef, which is deprecated and will be removed in v1beta2. The + // Kind may be left blank, in which case it defaults to "ImageRepository". The "Name" is + // the only required subfield - if Namespace is blank, the namespace of the current deployment + // trigger will be used. + From kapi.ObjectReference `json:"from"` // Tag is the name of an image repository tag to watch for changes. Tag string `json:"tag,omitempty"` // LastTriggeredImageID is used internally by the ImageChangeController to save last diff --git a/pkg/build/api/v1beta1/conversion.go b/pkg/build/api/v1beta1/conversion.go index b513d664aed3..01bd40d03c30 100644 --- a/pkg/build/api/v1beta1/conversion.go +++ b/pkg/build/api/v1beta1/conversion.go @@ -2,8 +2,10 @@ package v1beta1 import ( "github.com/GoogleCloudPlatform/kubernetes/pkg/api" + kapi "github.com/GoogleCloudPlatform/kubernetes/pkg/api/v1beta3" "github.com/GoogleCloudPlatform/kubernetes/pkg/conversion" newer "github.com/openshift/origin/pkg/build/api" + image "github.com/openshift/origin/pkg/image/api" ) func init() { @@ -26,5 +28,67 @@ func init() { } return nil }, - ) + // Deprecate ImageTag and Registry, replace with To / Tag / DockerImageReference + func(in *newer.BuildOutput, out *BuildOutput, s conversion.Scope) error { + if err := s.Convert(&in.To, &out.To, 0); err != nil { + return err + } + out.Tag = in.Tag + if len(in.DockerImageReference) > 0 { + out.DockerImageReference = in.DockerImageReference + registry, namespace, name, tag, _ := image.SplitDockerPullSpec(in.DockerImageReference) + out.Registry = registry + out.ImageTag = image.JoinDockerPullSpec("", namespace, name, tag) + } + return nil + }, + func(in *BuildOutput, out *newer.BuildOutput, s conversion.Scope) error { + if err := s.Convert(&in.To, &out.To, 0); err != nil { + return err + } + out.Tag = in.Tag + if len(in.DockerImageReference) > 0 { + out.DockerImageReference = in.DockerImageReference + return nil + } + if len(in.ImageTag) != 0 { + _, namespace, name, tag, err := image.SplitDockerPullSpec(in.ImageTag) + if err != nil { + return err + } + out.DockerImageReference = image.JoinDockerPullSpec(in.Registry, namespace, name, tag) + } + return nil + }, + // Rename ImageRepositoryRef to From + func(in *newer.ImageChangeTrigger, out *ImageChangeTrigger, s conversion.Scope) error { + if err := s.Convert(&in.From, &out.From, 0); err != nil { + return err + } + if len(in.From.Name) != 0 { + out.ImageRepositoryRef = &kapi.ObjectReference{} + if err := s.Convert(&in.From, out.ImageRepositoryRef, conversion.AllowDifferentFieldTypeNames); err != nil { + return err + } + } + out.Tag = in.Tag + out.LastTriggeredImageID = in.LastTriggeredImageID + out.Image = in.Image + return nil + }, + func(in *ImageChangeTrigger, out *newer.ImageChangeTrigger, s conversion.Scope) error { + if in.ImageRepositoryRef != nil { + if err := s.Convert(in.ImageRepositoryRef, &out.From, conversion.AllowDifferentFieldTypeNames); err != nil { + return err + } + } else { + if err := s.Convert(&in.From, &out.From, 0); err != nil { + return err + } + } + out.Tag = in.Tag + out.LastTriggeredImageID = in.LastTriggeredImageID + out.Image = in.Image + return nil + }) } diff --git a/pkg/build/api/v1beta1/conversion_test.go b/pkg/build/api/v1beta1/conversion_test.go index 4e83259e0e3c..fa068c2dcc85 100644 --- a/pkg/build/api/v1beta1/conversion_test.go +++ b/pkg/build/api/v1beta1/conversion_test.go @@ -4,6 +4,8 @@ import ( "testing" "github.com/GoogleCloudPlatform/kubernetes/pkg/api" + kapi "github.com/GoogleCloudPlatform/kubernetes/pkg/api/v1beta3" + newer "github.com/openshift/origin/pkg/build/api" current "github.com/openshift/origin/pkg/build/api/v1beta1" ) @@ -11,15 +13,60 @@ import ( var Convert = api.Scheme.Convert func TestSTIBuildStrategyConversion(t *testing.T) { - var got newer.STIBuildStrategy + var actual newer.STIBuildStrategy oldVersion := current.STIBuildStrategy{ BuilderImage: "testimage", } - err := Convert(&oldVersion, &got) + err := Convert(&oldVersion, &actual) if err != nil { t.Fatalf("unexpected error: %v", err) } - if got.Image != oldVersion.BuilderImage { - t.Error("expected %v, got %v", oldVersion.BuilderImage, got.Image) + if actual.Image != oldVersion.BuilderImage { + t.Errorf("expected %v, actual %v", oldVersion.BuilderImage, actual.Image) + } +} + +func TestImageChangeTriggerFromRename(t *testing.T) { + old := current.ImageChangeTrigger{ + From: kapi.ObjectReference{ + Name: "foo", + }, + ImageRepositoryRef: &kapi.ObjectReference{ + Name: "bar", + }, + } + actual := newer.ImageChangeTrigger{} + if err := Convert(&old, &actual); err != nil { + t.Fatalf("unexpected error: %v", err) + } + if actual.From.Name != "bar" { + t.Error("expected %#v, actual %#v", old, actual) + } + + old = current.ImageChangeTrigger{ + From: kapi.ObjectReference{ + Name: "foo", + }, + } + actual = newer.ImageChangeTrigger{} + if err := Convert(&old, &actual); err != nil { + t.Fatalf("unexpected error: %v", err) + } + if actual.From.Name != "foo" { + t.Error("expected %#v, actual %#v", old, actual) + } + + old = current.ImageChangeTrigger{ + From: kapi.ObjectReference{ + Name: "foo", + }, + ImageRepositoryRef: &kapi.ObjectReference{}, + } + actual = newer.ImageChangeTrigger{} + if err := Convert(&old, &actual); err != nil { + t.Fatalf("unexpected error: %v", err) + } + if actual.From.Name != "" { + t.Errorf("expected %#v, actual %#v", old, actual) } } diff --git a/pkg/build/api/v1beta1/types.go b/pkg/build/api/v1beta1/types.go index 318bf3c793f9..9bae38565890 100644 --- a/pkg/build/api/v1beta1/types.go +++ b/pkg/build/api/v1beta1/types.go @@ -16,6 +16,9 @@ type Build struct { // Status is the current status of the build. Status BuildStatus `json:"status,omitempty"` + // A human readable message indicating details about why the build has this status + Message string `json:"message,omitempty"` + // PodName is the name of the pod that is used to execute the build PodName string `json:"podName,omitempty"` @@ -201,10 +204,30 @@ type STIBuildStrategy struct { // BuildOutput is input to a build strategy and describes the Docker image that the strategy // should produce. type BuildOutput struct { + // To defines an optional ImageRepository to push the output of this build to. The namespace + // may be empty, in which case the ImageRepository will be looked for in the namespace of + // the build. Kind must be set to 'ImageRepository' and is the only supported value. If set, + // this field takes priority over DockerImageReference. This value will be used to look up + // a Docker image repository to push to. + To *kapi.ObjectReference `json:"to,omitempty"` + + // Tag is the "version" that will be set on the remote server when the image is created. This + // field is only used if the To field is set, and is ignored when DockerImageReference is used. + // This value represents a consistent name for a set of related changes (v1, 5.x, 5.5, dev, stable) + // and defaults to the preferred label for "To" if not specified. + Tag string `json:"tag,omitempty"` + + // DockerImageReference is the full name of an image ([registry/]name[:tag]), and will be the + // value sent to Docker push at the end of a build. If set, this field takes priority over + // ImageTag and Registry. + DockerImageReference string `json:"dockerImageReference,omitempty"` + // ImageTag is the tag to give to the image resulting from the build. + // DEPRECATED: use DockerImageReference ImageTag string `json:"imageTag,omitempty"` // Registry is the Docker registry which should receive the resulting built image via push. + // DEPRECATED: use DockerImageReference Registry string `json:"registry,omitempty"` } @@ -221,7 +244,8 @@ type BuildConfig struct { // are defined, a new build can only occur as a result of an explicit client build creation. Triggers []BuildTriggerPolicy `json:"triggers,omitempty"` - // Parameters holds all the input necessary to produce a new build. + // Parameters holds all the input necessary to produce a new build. A build config may only + // define either the Output.To or Output.DockerImageReference fields, but not both. Parameters BuildParameters `json:"parameters,omitempty"` } @@ -235,8 +259,16 @@ type WebHookTrigger struct { type ImageChangeTrigger struct { // Image is used to specify the value in the BuildConfig to replace with the // immutable image id supplied by the ImageRepository when this trigger fires. - Image string `json:"image"` + Image string `json:"image"` + RepositoryName string `json:"repositoryName,omitempty"` + // From is a reference to a Docker image repository to watch for changes. This field takes + // precedence over ImageRepositoryRef, which is deprecated and will be removed in v1beta2. The + // Kind may be left blank, in which case it defaults to "ImageRepository". The "Name" is + // the only required subfield - if Namespace is blank, the namespace of the current deployment + // trigger will be used. + From kapi.ObjectReference `json:"from"` // ImageRepositoryRef a reference to a Docker image repository to watch for changes. + // DEPRECATED: replaced by From ImageRepositoryRef *kapi.ObjectReference `json:"imageRepositoryRef"` // Tag is the name of an image repository tag to watch for changes. Tag string `json:"tag,omitempty"` diff --git a/pkg/build/api/validation/validation.go b/pkg/build/api/validation/validation.go index bd4901c3dee0..3503f7a5c0df 100644 --- a/pkg/build/api/validation/validation.go +++ b/pkg/build/api/validation/validation.go @@ -4,8 +4,11 @@ import ( "net/url" errs "github.com/GoogleCloudPlatform/kubernetes/pkg/api/errors" + "github.com/GoogleCloudPlatform/kubernetes/pkg/api/validation" + "github.com/GoogleCloudPlatform/kubernetes/pkg/util" buildapi "github.com/openshift/origin/pkg/build/api" + imageapi "github.com/openshift/origin/pkg/image/api" ) // ValidateBuild tests required fields for a Build. @@ -13,7 +16,15 @@ func ValidateBuild(build *buildapi.Build) errs.ValidationErrorList { allErrs := errs.ValidationErrorList{} if len(build.Name) == 0 { allErrs = append(allErrs, errs.NewFieldRequired("name", build.Name)) + } else if !util.IsDNSSubdomain(build.Name) { + allErrs = append(allErrs, errs.NewFieldInvalid("name", build.Name, "name must be a valid subdomain")) } + if len(build.Namespace) == 0 { + allErrs = append(allErrs, errs.NewFieldRequired("namespace", build.Namespace)) + } else if !util.IsDNSSubdomain(build.Namespace) { + allErrs = append(allErrs, errs.NewFieldInvalid("namespace", build.Namespace, "namespace must be a valid subdomain")) + } + allErrs = append(allErrs, validation.ValidateLabels(build.Labels, "labels")...) allErrs = append(allErrs, validateBuildParameters(&build.Parameters).Prefix("parameters")...) return allErrs } @@ -23,11 +34,20 @@ func ValidateBuildConfig(config *buildapi.BuildConfig) errs.ValidationErrorList allErrs := errs.ValidationErrorList{} if len(config.Name) == 0 { allErrs = append(allErrs, errs.NewFieldRequired("name", config.Name)) + } else if !util.IsDNSSubdomain(config.Name) { + allErrs = append(allErrs, errs.NewFieldInvalid("name", config.Name, "name must be a valid subdomain")) + } + if len(config.Namespace) == 0 { + allErrs = append(allErrs, errs.NewFieldRequired("namespace", config.Namespace)) + } else if !util.IsDNSSubdomain(config.Namespace) { + allErrs = append(allErrs, errs.NewFieldInvalid("namespace", config.Namespace, "namespace must be a valid subdomain")) } + allErrs = append(allErrs, validation.ValidateLabels(config.Labels, "labels")...) for i := range config.Triggers { allErrs = append(allErrs, validateTrigger(&config.Triggers[i]).PrefixIndex(i).Prefix("triggers")...) } allErrs = append(allErrs, validateBuildParameters(&config.Parameters).Prefix("parameters")...) + allErrs = append(allErrs, validateBuildConfigOutput(&config.Parameters.Output).Prefix("parameters.output")...) return allErrs } @@ -44,10 +64,7 @@ func validateBuildParameters(params *buildapi.BuildParameters) errs.ValidationEr } } - if !isCustomBuild || (isCustomBuild && len(params.Output.ImageTag) != 0) { - allErrs = append(allErrs, validateOutput(¶ms.Output).Prefix("output")...) - } - + allErrs = append(allErrs, validateOutput(¶ms.Output).Prefix("output")...) allErrs = append(allErrs, validateStrategy(¶ms.Strategy).Prefix("strategy")...) return allErrs @@ -85,6 +102,45 @@ func validateRevision(revision *buildapi.SourceRevision) errs.ValidationErrorLis return allErrs } +func validateOutput(output *buildapi.BuildOutput) errs.ValidationErrorList { + allErrs := errs.ValidationErrorList{} + + // TODO: make part of a generic ValidateObjectReference method upstream. + if output.To != nil { + kind, name, namespace := output.To.Kind, output.To.Name, output.To.Namespace + if len(kind) == 0 { + kind = "ImageRepository" + output.To.Kind = kind + } + if kind != "ImageRepository" { + allErrs = append(allErrs, errs.NewFieldInvalid("to.kind", kind, "the target of build output must be 'ImageRepository'")) + } + if len(name) == 0 { + allErrs = append(allErrs, errs.NewFieldRequired("to.name", name)) + } else if !util.IsDNSSubdomain(name) { + allErrs = append(allErrs, errs.NewFieldInvalid("to.name", name, "name must be a valid subdomain")) + } + if len(namespace) != 0 && !util.IsDNSSubdomain(namespace) { + allErrs = append(allErrs, errs.NewFieldInvalid("to.namespace", namespace, "namespace must be a valid subdomain")) + } + } + + if len(output.DockerImageReference) != 0 { + if _, _, _, _, err := imageapi.SplitDockerPullSpec(output.DockerImageReference); err != nil { + allErrs = append(allErrs, errs.NewFieldInvalid("dockerImageReference", output.DockerImageReference, err.Error())) + } + } + return allErrs +} + +func validateBuildConfigOutput(output *buildapi.BuildOutput) errs.ValidationErrorList { + allErrs := errs.ValidationErrorList{} + if len(output.DockerImageReference) != 0 && output.To != nil { + allErrs = append(allErrs, errs.NewFieldInvalid("dockerImageReference", output.DockerImageReference, "only one of 'dockerImageReference' and 'to' may be set")) + } + return allErrs +} + func validateStrategy(strategy *buildapi.BuildStrategy) errs.ValidationErrorList { allErrs := errs.ValidationErrorList{} @@ -128,14 +184,6 @@ func validateSTIStrategy(strategy *buildapi.STIBuildStrategy) errs.ValidationErr return allErrs } -func validateOutput(output *buildapi.BuildOutput) errs.ValidationErrorList { - allErrs := errs.ValidationErrorList{} - if len(output.ImageTag) == 0 { - allErrs = append(allErrs, errs.NewFieldRequired("imageTag", output.ImageTag)) - } - return allErrs -} - func validateTrigger(trigger *buildapi.BuildTriggerPolicy) errs.ValidationErrorList { allErrs := errs.ValidationErrorList{} if len(trigger.Type) == 0 { @@ -189,12 +237,10 @@ func validateImageChange(imageChange *buildapi.ImageChangeTrigger) errs.Validati if len(imageChange.Image) == 0 { allErrs = append(allErrs, errs.NewFieldRequired("image", "")) } - if imageChange.ImageRepositoryRef == nil { - allErrs = append(allErrs, errs.NewFieldRequired("imageRepositoryRef", "")) - } else if len(imageChange.ImageRepositoryRef.Name) == 0 { - nestedErrs := errs.ValidationErrorList{errs.NewFieldRequired("name", "")} - nestedErrs.Prefix("imageRepositoryRef") - allErrs = append(allErrs, nestedErrs...) + if len(imageChange.From.Name) == 0 { + allErrs = append(allErrs, errs.NewFieldRequired("from", "")) + } else if len(imageChange.From.Name) == 0 { + allErrs = append(allErrs, errs.ValidationErrorList{errs.NewFieldRequired("name", "")}.Prefix("from")...) } return allErrs } diff --git a/pkg/build/api/validation/validation_test.go b/pkg/build/api/validation/validation_test.go index 7bc991685d29..4827db06db41 100644 --- a/pkg/build/api/validation/validation_test.go +++ b/pkg/build/api/validation/validation_test.go @@ -11,7 +11,7 @@ import ( func TestBuildValdationSuccess(t *testing.T) { build := &buildapi.Build{ - ObjectMeta: kapi.ObjectMeta{Name: "buildID"}, + ObjectMeta: kapi.ObjectMeta{Name: "buildid", Namespace: "default"}, Parameters: buildapi.BuildParameters{ Source: buildapi.BuildSource{ Type: buildapi.BuildSourceGit, @@ -26,7 +26,7 @@ func TestBuildValdationSuccess(t *testing.T) { }, }, Output: buildapi.BuildOutput{ - ImageTag: "repository/data", + DockerImageReference: "repository/data", }, }, Status: buildapi.BuildStatusNew, @@ -38,7 +38,7 @@ func TestBuildValdationSuccess(t *testing.T) { func TestBuildValidationFailure(t *testing.T) { build := &buildapi.Build{ - ObjectMeta: kapi.ObjectMeta{Name: ""}, + ObjectMeta: kapi.ObjectMeta{Name: "", Namespace: ""}, Parameters: buildapi.BuildParameters{ Source: buildapi.BuildSource{ Type: buildapi.BuildSourceGit, @@ -53,19 +53,19 @@ func TestBuildValidationFailure(t *testing.T) { }, }, Output: buildapi.BuildOutput{ - ImageTag: "repository/data", + DockerImageReference: "repository/data", }, }, Status: buildapi.BuildStatusNew, } - if result := ValidateBuild(build); len(result) != 1 { + if result := ValidateBuild(build); len(result) != 2 { t.Errorf("Unexpected validation result: %v", result) } } func TestBuildConfigValidationSuccess(t *testing.T) { buildConfig := &buildapi.BuildConfig{ - ObjectMeta: kapi.ObjectMeta{Name: "configId"}, + ObjectMeta: kapi.ObjectMeta{Name: "config-id", Namespace: "namespace"}, Parameters: buildapi.BuildParameters{ Source: buildapi.BuildSource{ Type: buildapi.BuildSourceGit, @@ -79,9 +79,7 @@ func TestBuildConfigValidationSuccess(t *testing.T) { ContextDir: "context", }, }, - Output: buildapi.BuildOutput{ - ImageTag: "repository/data", - }, + Output: buildapi.BuildOutput{}, }, } if result := ValidateBuildConfig(buildConfig); len(result) > 0 { @@ -91,7 +89,7 @@ func TestBuildConfigValidationSuccess(t *testing.T) { func TestBuildConfigValidationFailure(t *testing.T) { buildConfig := &buildapi.BuildConfig{ - ObjectMeta: kapi.ObjectMeta{Name: ""}, + ObjectMeta: kapi.ObjectMeta{Name: "", Namespace: "foo"}, Parameters: buildapi.BuildParameters{ Source: buildapi.BuildSource{ Type: buildapi.BuildSourceGit, @@ -106,7 +104,7 @@ func TestBuildConfigValidationFailure(t *testing.T) { }, }, Output: buildapi.BuildOutput{ - ImageTag: "repository/data", + DockerImageReference: "repository/data", }, }, } @@ -115,6 +113,35 @@ func TestBuildConfigValidationFailure(t *testing.T) { } } +func TestBuildConfigValidationOutputFailure(t *testing.T) { + buildConfig := &buildapi.BuildConfig{ + ObjectMeta: kapi.ObjectMeta{Name: ""}, + Parameters: buildapi.BuildParameters{ + Source: buildapi.BuildSource{ + Type: buildapi.BuildSourceGit, + Git: &buildapi.GitBuildSource{ + URI: "http://github.com/my/repository", + }, + }, + Strategy: buildapi.BuildStrategy{ + Type: buildapi.DockerBuildStrategyType, + DockerStrategy: &buildapi.DockerBuildStrategy{ + ContextDir: "context", + }, + }, + Output: buildapi.BuildOutput{ + DockerImageReference: "repository/data", + To: &kapi.ObjectReference{ + Name: "other", + }, + }, + }, + } + if result := ValidateBuildConfig(buildConfig); len(result) != 3 { + t.Errorf("Unexpected validation result %v", result) + } +} + func TestValidateSource(t *testing.T) { errorCases := map[string]*buildapi.BuildSource{ string(errs.ValidationErrorTypeRequired) + "git.uri": { @@ -144,52 +171,150 @@ func TestValidateSource(t *testing.T) { } func TestValidateBuildParameters(t *testing.T) { - errorCases := map[string]*buildapi.BuildParameters{ - string(errs.ValidationErrorTypeRequired) + "output.imageTag": { - Source: buildapi.BuildSource{ - Type: buildapi.BuildSourceGit, - Git: &buildapi.GitBuildSource{ - URI: "http://github.com/my/repository", + errorCases := []struct { + err string + *buildapi.BuildParameters + }{ + { + string(errs.ValidationErrorTypeInvalid) + "output.dockerImageReference", + &buildapi.BuildParameters{ + Source: buildapi.BuildSource{ + Type: buildapi.BuildSourceGit, + Git: &buildapi.GitBuildSource{ + URI: "http://github.com/my/repository", + }, + }, + Strategy: buildapi.BuildStrategy{ + Type: buildapi.DockerBuildStrategyType, + DockerStrategy: &buildapi.DockerBuildStrategy{ + ContextDir: "context", + }, + }, + Output: buildapi.BuildOutput{ + DockerImageReference: "some/long/value/with/no/meaning", }, }, - Strategy: buildapi.BuildStrategy{ - Type: buildapi.DockerBuildStrategyType, - DockerStrategy: &buildapi.DockerBuildStrategy{ - ContextDir: "context", + }, + { + string(errs.ValidationErrorTypeInvalid) + "output.to.kind", + &buildapi.BuildParameters{ + Source: buildapi.BuildSource{ + Type: buildapi.BuildSourceGit, + Git: &buildapi.GitBuildSource{ + URI: "http://github.com/my/repository", + }, + }, + Strategy: buildapi.BuildStrategy{ + Type: buildapi.DockerBuildStrategyType, + DockerStrategy: &buildapi.DockerBuildStrategy{ + ContextDir: "context", + }, + }, + Output: buildapi.BuildOutput{ + To: &kapi.ObjectReference{ + Kind: "Foo", + Name: "test", + }, }, }, - Output: buildapi.BuildOutput{ - ImageTag: "", + }, + { + string(errs.ValidationErrorTypeRequired) + "output.to.name", + &buildapi.BuildParameters{ + Source: buildapi.BuildSource{ + Type: buildapi.BuildSourceGit, + Git: &buildapi.GitBuildSource{ + URI: "http://github.com/my/repository", + }, + }, + Strategy: buildapi.BuildStrategy{ + Type: buildapi.DockerBuildStrategyType, + DockerStrategy: &buildapi.DockerBuildStrategy{ + ContextDir: "context", + }, + }, + Output: buildapi.BuildOutput{ + To: &kapi.ObjectReference{}, + }, }, }, - string(errs.ValidationErrorTypeRequired) + "strategy.stiStrategy.image": { - Source: buildapi.BuildSource{ - Type: buildapi.BuildSourceGit, - Git: &buildapi.GitBuildSource{ - URI: "http://github.com/my/repository", + { + string(errs.ValidationErrorTypeInvalid) + "output.to.name", + &buildapi.BuildParameters{ + Source: buildapi.BuildSource{ + Type: buildapi.BuildSourceGit, + Git: &buildapi.GitBuildSource{ + URI: "http://github.com/my/repository", + }, + }, + Strategy: buildapi.BuildStrategy{ + Type: buildapi.DockerBuildStrategyType, + DockerStrategy: &buildapi.DockerBuildStrategy{ + ContextDir: "context", + }, + }, + Output: buildapi.BuildOutput{ + To: &kapi.ObjectReference{ + Name: "not_a_valid_subdomain", + Namespace: "subdomain", + }, }, }, - Output: buildapi.BuildOutput{ - ImageTag: "repository/data", + }, + { + string(errs.ValidationErrorTypeInvalid) + "output.to.namespace", + &buildapi.BuildParameters{ + Source: buildapi.BuildSource{ + Type: buildapi.BuildSourceGit, + Git: &buildapi.GitBuildSource{ + URI: "http://github.com/my/repository", + }, + }, + Strategy: buildapi.BuildStrategy{ + Type: buildapi.DockerBuildStrategyType, + DockerStrategy: &buildapi.DockerBuildStrategy{ + ContextDir: "context", + }, + }, + Output: buildapi.BuildOutput{ + To: &kapi.ObjectReference{ + Name: "test", + Namespace: "not_a_valid_subdomain", + }, + }, }, - Strategy: buildapi.BuildStrategy{ - Type: buildapi.STIBuildStrategyType, - STIStrategy: &buildapi.STIBuildStrategy{ - Image: "", + }, + { + string(errs.ValidationErrorTypeRequired) + "strategy.stiStrategy.image", + &buildapi.BuildParameters{ + Source: buildapi.BuildSource{ + Type: buildapi.BuildSourceGit, + Git: &buildapi.GitBuildSource{ + URI: "http://github.com/my/repository", + }, + }, + Output: buildapi.BuildOutput{ + DockerImageReference: "repository/data", + }, + Strategy: buildapi.BuildStrategy{ + Type: buildapi.STIBuildStrategyType, + STIStrategy: &buildapi.STIBuildStrategy{ + Image: "", + }, }, }, }, } - for desc, config := range errorCases { - errors := validateBuildParameters(config) + for _, config := range errorCases { + errors := validateBuildParameters(config.BuildParameters) if len(errors) != 1 { - t.Errorf("%s: Unexpected validation result: %v", desc, errors) + t.Errorf("%s: Unexpected validation result: %v", config.err, errors) } err := errors[0].(*errs.ValidationError) errDesc := string(err.Type) + err.Field - if desc != errDesc { - t.Errorf("Unexpected validation result for %s: expected %s, got %s", err.Field, desc, errDesc) + if config.err != errDesc { + t.Errorf("Unexpected validation result for %s: expected %s, got %s", err.Field, config.err, errDesc) } } } diff --git a/pkg/build/builder/cmd/builder.go b/pkg/build/builder/cmd/builder.go index 075625c51ff7..c62438e994b8 100644 --- a/pkg/build/builder/cmd/builder.go +++ b/pkg/build/builder/cmd/builder.go @@ -1,15 +1,16 @@ package cmd import ( - "encoding/json" "log" "os" "github.com/fsouza/go-dockerclient" + "github.com/openshift/origin/pkg/api/latest" "github.com/openshift/origin/pkg/build/api" bld "github.com/openshift/origin/pkg/build/builder" "github.com/openshift/origin/pkg/build/builder/cmd/dockercfg" dockerutil "github.com/openshift/origin/pkg/cmd/util/docker" + image "github.com/openshift/origin/pkg/image/api" ) const DefaultDockerEndpoint = "unix:///var/run/docker.sock" @@ -32,11 +33,22 @@ func run(builderFactory factoryFunc) { } buildStr := os.Getenv("BUILD") build := api.Build{} - err = json.Unmarshal([]byte(buildStr), &build) - if err != nil { + if err := latest.Codec.DecodeInto([]byte(buildStr), &build); err != nil { log.Fatalf("Unable to parse build: %v", err) } - authcfg, authPresent := dockercfg.NewHelper().GetDockerAuth(build.Parameters.Output.Registry) + + var ( + authcfg docker.AuthConfiguration + authPresent bool + ) + if len(build.Parameters.Output.DockerImageReference) != 0 { + registry, _, _, _, err := image.SplitDockerPullSpec(build.Parameters.Output.DockerImageReference) + if err != nil { + log.Fatalf("Output does not have a valid Docker image reference: %v", err) + } + authcfg, authPresent = dockercfg.NewHelper().GetDockerAuth(registry) + } + b := builderFactory(client, endpoint, authcfg, authPresent, &build) if err = b.Build(); err != nil { log.Fatalf("Build error: %v", err) diff --git a/pkg/build/builder/docker.go b/pkg/build/builder/docker.go index 21eb3c4b4c32..a41d2c3a66b1 100644 --- a/pkg/build/builder/docker.go +++ b/pkg/build/builder/docker.go @@ -64,9 +64,10 @@ func (d *DockerBuilder) Build() error { if err = d.dockerBuild(buildDir); err != nil { return err } - defer removeImage(d.dockerClient, imageTag(d.build)) - if d.build.Parameters.Output.Registry != "" || d.authPresent { - return pushImage(d.dockerClient, imageTag(d.build), d.auth) + tag := d.build.Parameters.Output.DockerImageReference + defer removeImage(d.dockerClient, tag) + if len(d.build.Parameters.Output.DockerImageReference) != 0 { + return pushImage(d.dockerClient, tag, d.auth) } return nil } @@ -175,5 +176,5 @@ func (d *DockerBuilder) dockerBuild(dir string) error { } noCache = d.build.Parameters.Strategy.DockerStrategy.NoCache } - return buildImage(d.dockerClient, dir, noCache, imageTag(d.build), d.tar) + return buildImage(d.dockerClient, dir, noCache, d.build.Parameters.Output.DockerImageReference, d.tar) } diff --git a/pkg/build/builder/sti.go b/pkg/build/builder/sti.go index f37b8a17015d..95c63d0ef8e8 100644 --- a/pkg/build/builder/sti.go +++ b/pkg/build/builder/sti.go @@ -29,11 +29,12 @@ func NewSTIBuilder(client DockerClient, dockerSocket string, authCfg docker.Auth // Build executes the STI build func (s *STIBuilder) Build() error { + tag := s.build.Parameters.Output.DockerImageReference request := &stiapi.Request{ BaseImage: s.build.Parameters.Strategy.STIStrategy.Image, DockerSocket: s.dockerSocket, Source: s.build.Parameters.Source.Git.URI, - Tag: imageTag(s.build), + Tag: tag, ScriptsURL: s.build.Parameters.Strategy.STIStrategy.Scripts, Environment: getBuildEnvVars(s.build), Clean: s.build.Parameters.Strategy.STIStrategy.Clean, @@ -48,12 +49,12 @@ func (s *STIBuilder) Build() error { if err != nil { return err } - defer removeImage(s.dockerClient, imageTag(s.build)) + defer removeImage(s.dockerClient, tag) if _, err = builder.Build(); err != nil { return err } - if s.build.Parameters.Output.Registry != "" || s.authPresent { - return pushImage(s.dockerClient, imageTag(s.build), s.auth) + if len(s.build.Parameters.Output.DockerImageReference) != 0 { + return pushImage(s.dockerClient, tag, s.auth) } return nil } diff --git a/pkg/build/builder/util.go b/pkg/build/builder/util.go index ee983a16370f..c3870a77d51f 100644 --- a/pkg/build/builder/util.go +++ b/pkg/build/builder/util.go @@ -1,9 +1,6 @@ package builder import ( - "fmt" - "strings" - "github.com/openshift/origin/pkg/build/api" ) @@ -11,16 +8,6 @@ type Builder interface { Build() error } -// imageTag returns the tag to be used for the build. If a registry has been -// specified, it will prepend the registry to the name -func imageTag(build *api.Build) string { - tag := build.Parameters.Output.ImageTag - if !strings.HasPrefix(tag, build.Parameters.Output.Registry) { - tag = fmt.Sprintf("%s/%s", build.Parameters.Output.Registry, tag) - } - return tag -} - // getBuildEnvVars returns a map with the environment variables that should be added // to the built image func getBuildEnvVars(build *api.Build) map[string]string { diff --git a/pkg/build/builder/util_test.go b/pkg/build/builder/util_test.go index a0da9b0c90de..993368354d2a 100644 --- a/pkg/build/builder/util_test.go +++ b/pkg/build/builder/util_test.go @@ -17,7 +17,7 @@ func TestImageTag(t *testing.T) { build: api.Build{ Parameters: api.BuildParameters{ Output: api.BuildOutput{ - ImageTag: "test/tag", + DockerImageReference: "test/tag", }, }, }, @@ -27,19 +27,7 @@ func TestImageTag(t *testing.T) { build: api.Build{ Parameters: api.BuildParameters{ Output: api.BuildOutput{ - ImageTag: "test/tag", - Registry: "registry-server.test:5000", - }, - }, - }, - expected: "registry-server.test:5000/test/tag", - }, - { - build: api.Build{ - Parameters: api.BuildParameters{ - Output: api.BuildOutput{ - ImageTag: "registry-server.test:5000/test/tag", - Registry: "registry-server.test:5000", + DockerImageReference: "registry-server.test:5000/test/tag", }, }, }, @@ -47,7 +35,7 @@ func TestImageTag(t *testing.T) { }, } for _, x := range tests { - result := imageTag(&x.build) + result := x.build.Parameters.Output.DockerImageReference if result != x.expected { t.Errorf("Unexpected imageTag result. Expected: %s, Actual: %s", result, x.expected) diff --git a/pkg/build/controller/controller.go b/pkg/build/controller/controller.go index f7a15f423afb..4ce6dc2aa787 100644 --- a/pkg/build/controller/controller.go +++ b/pkg/build/controller/controller.go @@ -11,6 +11,7 @@ import ( "github.com/GoogleCloudPlatform/kubernetes/pkg/util" buildapi "github.com/openshift/origin/pkg/build/api" + imageapi "github.com/openshift/origin/pkg/image/api" ) // BuildController watches build resources and manages their state @@ -21,6 +22,8 @@ type BuildController struct { BuildUpdater buildUpdater PodManager podManager BuildStrategy BuildStrategy + + ImageRepositoryClient imageRepositoryClient } // BuildStrategy knows how to create a pod spec for a pod which can execute a build. @@ -37,6 +40,10 @@ type podManager interface { DeletePod(namespace string, pod *kapi.Pod) error } +type imageRepositoryClient interface { + GetImageRepository(namespace, name string) (*imageapi.ImageRepository, error) +} + // Run begins watching and syncing build jobs onto the cluster. func (bc *BuildController) Run() { go util.Forever(func() { bc.HandleBuild(bc.NextBuild()) }, 0) @@ -51,39 +58,78 @@ func (bc *BuildController) HandleBuild(build *buildapi.Build) { return } - nextStatus := buildapi.BuildStatusFailed - build.PodName = fmt.Sprintf("build-%s", build.Name) + if err := bc.nextBuildStatus(build); err != nil { + // TODO: all build errors should be retried, and build error should not be a permanent status change. + // Instead, we should requeue this build request using the same backoff logic as the scheduler. + // BuildStatusError should be reserved for meaning "permanently errored, no way to try again". + glog.V(4).Infof("Build failed with error %s/%s: %#v", build.Namespace, build.Name, err) + build.Status = buildapi.BuildStatusError + build.Message = err.Error() + } + if _, err := bc.BuildUpdater.UpdateBuild(build.Namespace, build); err != nil { + glog.V(2).Infof("Failed to record changes to build %s/%s: %#v", build.Namespace, build.Name, err) + } +} + +// nextBuildStatus updates build with any appropriate changes, or returns an error if +// the change cannot occur. When returning nil, be sure to set build.Status and optionally +// build.Message. +func (bc *BuildController) nextBuildStatus(build *buildapi.Build) error { // If a cancelling event was triggered for the build, update build status. if build.Cancelled { - glog.V(2).Infof("Cancelling build %s.", build.Name) - nextStatus = buildapi.BuildStatusCancelled - - } else { - - var podSpec *kapi.Pod - var err error - if podSpec, err = bc.BuildStrategy.CreateBuildPod(build); err != nil { - glog.V(2).Infof("Strategy failed to create build pod definition: %v", err) - nextStatus = buildapi.BuildStatusFailed - } else { - - if _, err := bc.PodManager.CreatePod(build.Namespace, podSpec); err != nil { - if !errors.IsAlreadyExists(err) { - glog.V(2).Infof("Failed to create pod for build %s: %#v", build.Name, err) - nextStatus = buildapi.BuildStatusFailed - } - } else { - glog.V(2).Infof("Created build pod: %#v", podSpec) - nextStatus = buildapi.BuildStatusPending + glog.V(4).Infof("Cancelling build %s.", build.Name) + build.Status = buildapi.BuildStatusCancelled + return nil + } + + // lookup the destination from the referenced image repository + spec := build.Parameters.Output.DockerImageReference + if ref := build.Parameters.Output.To; ref != nil { + // TODO: security, ensure that the reference image stream is actually visible + namespace := ref.Namespace + if len(namespace) == 0 { + namespace = build.Namespace + } + + repo, err := bc.ImageRepositoryClient.GetImageRepository(namespace, ref.Name) + if err != nil { + if errors.IsNotFound(err) { + return fmt.Errorf("the referenced output image repository %s/%s does not exist", namespace, ref.Name) } + return fmt.Errorf("the referenced output repo %s/%s could not be found by %s/%s: %v", namespace, ref.Name, build.Namespace, build.Name, err) } + spec = repo.Status.DockerImageRepository } - build.Status = nextStatus - if _, err := bc.BuildUpdater.UpdateBuild(build.Namespace, build); err != nil { - glog.V(2).Infof("Failed to update build %s: %#v", build.Name, err) + // set the expected build parameters, which will be saved if no error occurs + build.Status = buildapi.BuildStatusPending + build.PodName = fmt.Sprintf("build-%s", build.Name) + + // override DockerImageReference in the strategy for the copy we send to the server + copy, err := kapi.Scheme.Copy(build) + if err != nil { + return fmt.Errorf("unable to copy build: %v", err) + } + buildCopy := copy.(*buildapi.Build) + buildCopy.Parameters.Output.DockerImageReference = spec + + // invoke the strategy to get a build pod + podSpec, err := bc.BuildStrategy.CreateBuildPod(buildCopy) + if err != nil { + return fmt.Errorf("the strategy failed to create a build pod for %s/%s: %v", build.Namespace, build.Name, err) + } + + if _, err := bc.PodManager.CreatePod(build.Namespace, podSpec); err != nil { + if errors.IsAlreadyExists(err) { + glog.V(4).Infof("Build pod already existed: %#v", podSpec) + return nil + } + return fmt.Errorf("failed to create pod for build %s/%s: s", build.Namespace, build.Name, err) } + + glog.V(4).Infof("Created pod for build: %#v", podSpec) + return nil } func (bc *BuildController) HandlePod(pod *kapi.Pod) { diff --git a/pkg/build/controller/controller_test.go b/pkg/build/controller/controller_test.go index ffe89eadd046..0d25408b98d4 100644 --- a/pkg/build/controller/controller_test.go +++ b/pkg/build/controller/controller_test.go @@ -9,6 +9,7 @@ import ( buildapi "github.com/openshift/origin/pkg/build/api" buildtest "github.com/openshift/origin/pkg/build/controller/test" + imageapi "github.com/openshift/origin/pkg/image/api" ) type okBuildUpdater struct{} @@ -23,9 +24,12 @@ func (ebu *errBuildUpdater) UpdateBuild(namespace string, build *buildapi.Build) return &buildapi.Build{}, errors.New("UpdateBuild error!") } -type okStrategy struct{} +type okStrategy struct { + build *buildapi.Build +} func (os *okStrategy) CreateBuildPod(build *buildapi.Build) (*kapi.Pod, error) { + os.build = build return &kapi.Pod{}, nil } @@ -65,10 +69,34 @@ func (_ *errExistsPodManager) DeletePod(namespace string, pod *kapi.Pod) error { return kerrors.NewNotFound("kind", "name") } -func mockBuildAndController(status buildapi.BuildStatus) (build *buildapi.Build, controller *BuildController) { +type okImageRepositoryClient struct{} + +func (_ *okImageRepositoryClient) GetImageRepository(namespace, name string) (*imageapi.ImageRepository, error) { + return &imageapi.ImageRepository{ + ObjectMeta: kapi.ObjectMeta{Name: name, Namespace: namespace}, + Status: imageapi.ImageRepositoryStatus{ + DockerImageRepository: "image/repo", + }, + }, nil +} + +type errImageRepositoryClient struct{} + +func (_ *errImageRepositoryClient) GetImageRepository(namespace, name string) (*imageapi.ImageRepository, error) { + return nil, errors.New("GetImageRepository error!") +} + +type errNotFoundImageRepositoryClient struct{} + +func (_ *errNotFoundImageRepositoryClient) GetImageRepository(namespace, name string) (*imageapi.ImageRepository, error) { + return nil, kerrors.NewNotFound("ImageRepository", name) +} + +func mockBuildAndController(status buildapi.BuildStatus, output buildapi.BuildOutput) (build *buildapi.Build, controller *BuildController) { build = &buildapi.Build{ ObjectMeta: kapi.ObjectMeta{ - Name: "dataBuild", + Name: "data-build", + Namespace: "namespace", Labels: map[string]string{ "name": "dataBuild", }, @@ -86,20 +114,19 @@ func mockBuildAndController(status buildapi.BuildStatus) (build *buildapi.Build, ContextDir: "contextimage", }, }, - Output: buildapi.BuildOutput{ - ImageTag: "repository/dataBuild", - }, + Output: output, }, Status: status, PodName: "-the-pod-id", } controller = &BuildController{ - BuildStore: buildtest.NewFakeBuildStore(build), - BuildUpdater: &okBuildUpdater{}, - PodManager: &okPodManager{}, - NextBuild: func() *buildapi.Build { return nil }, - NextPod: func() *kapi.Pod { return nil }, - BuildStrategy: &okStrategy{}, + BuildStore: buildtest.NewFakeBuildStore(build), + BuildUpdater: &okBuildUpdater{}, + PodManager: &okPodManager{}, + NextBuild: func() *buildapi.Build { return nil }, + NextPod: func() *kapi.Pod { return nil }, + BuildStrategy: &okStrategy{}, + ImageRepositoryClient: &okImageRepositoryClient{}, } return } @@ -124,60 +151,134 @@ func TestHandleBuild(t *testing.T) { type handleBuildTest struct { inStatus buildapi.BuildStatus outStatus buildapi.BuildStatus + buildOutput buildapi.BuildOutput buildStrategy BuildStrategy buildUpdater buildUpdater + imageClient imageRepositoryClient podManager podManager + outputSpec string } tests := []handleBuildTest{ { // 0 inStatus: buildapi.BuildStatusNew, outStatus: buildapi.BuildStatusPending, + buildOutput: buildapi.BuildOutput{ + DockerImageReference: "repository/dataBuild", + }, }, { // 1 inStatus: buildapi.BuildStatusPending, outStatus: buildapi.BuildStatusPending, + buildOutput: buildapi.BuildOutput{ + DockerImageReference: "repository/dataBuild", + }, }, { // 2 inStatus: buildapi.BuildStatusRunning, outStatus: buildapi.BuildStatusRunning, + buildOutput: buildapi.BuildOutput{ + DockerImageReference: "repository/dataBuild", + }, }, { // 3 inStatus: buildapi.BuildStatusComplete, outStatus: buildapi.BuildStatusComplete, + buildOutput: buildapi.BuildOutput{ + DockerImageReference: "repository/dataBuild", + }, }, { // 4 inStatus: buildapi.BuildStatusFailed, outStatus: buildapi.BuildStatusFailed, + buildOutput: buildapi.BuildOutput{ + DockerImageReference: "repository/dataBuild", + }, }, { // 5 inStatus: buildapi.BuildStatusError, outStatus: buildapi.BuildStatusError, + buildOutput: buildapi.BuildOutput{ + DockerImageReference: "repository/dataBuild", + }, }, { // 6 inStatus: buildapi.BuildStatusNew, - outStatus: buildapi.BuildStatusFailed, + outStatus: buildapi.BuildStatusError, buildStrategy: &errStrategy{}, + buildOutput: buildapi.BuildOutput{ + DockerImageReference: "repository/dataBuild", + }, }, { // 7 inStatus: buildapi.BuildStatusNew, - outStatus: buildapi.BuildStatusFailed, + outStatus: buildapi.BuildStatusError, podManager: &errPodManager{}, + buildOutput: buildapi.BuildOutput{ + DockerImageReference: "repository/dataBuild", + }, }, { // 8 inStatus: buildapi.BuildStatusNew, - outStatus: buildapi.BuildStatusFailed, + outStatus: buildapi.BuildStatusPending, podManager: &errExistsPodManager{}, + buildOutput: buildapi.BuildOutput{ + DockerImageReference: "repository/dataBuild", + }, }, { // 9 inStatus: buildapi.BuildStatusNew, outStatus: buildapi.BuildStatusPending, buildUpdater: &errBuildUpdater{}, + buildOutput: buildapi.BuildOutput{ + DockerImageReference: "repository/dataBuild", + }, + }, + { // 10 + inStatus: buildapi.BuildStatusNew, + outStatus: buildapi.BuildStatusPending, + buildOutput: buildapi.BuildOutput{ + To: &kapi.ObjectReference{ + Name: "foo", + }, + }, + outputSpec: "image/repo", + }, + { // 11 + inStatus: buildapi.BuildStatusNew, + outStatus: buildapi.BuildStatusPending, + buildOutput: buildapi.BuildOutput{ + To: &kapi.ObjectReference{ + Name: "foo", + Namespace: "bar", + }, + }, + outputSpec: "image/repo", + }, + { // 12 + inStatus: buildapi.BuildStatusNew, + outStatus: buildapi.BuildStatusError, // TODO: this should be a retry + imageClient: &errNotFoundImageRepositoryClient{}, + buildOutput: buildapi.BuildOutput{ + To: &kapi.ObjectReference{ + Name: "foo", + }, + }, + }, + { // 13 + inStatus: buildapi.BuildStatusNew, + outStatus: buildapi.BuildStatusError, + imageClient: &errImageRepositoryClient{}, + buildOutput: buildapi.BuildOutput{ + To: &kapi.ObjectReference{ + Name: "foo", + }, + }, }, } for i, tc := range tests { - build, ctrl := mockBuildAndController(tc.inStatus) + build, ctrl := mockBuildAndController(tc.inStatus, tc.buildOutput) if tc.buildStrategy != nil { ctrl.BuildStrategy = tc.buildStrategy } @@ -187,12 +288,24 @@ func TestHandleBuild(t *testing.T) { if tc.podManager != nil { ctrl.PodManager = tc.podManager } + if tc.imageClient != nil { + ctrl.ImageRepositoryClient = tc.imageClient + } ctrl.HandleBuild(build) if build.Status != tc.outStatus { t.Errorf("(%d) Expected %s, got %s!", i, tc.outStatus, build.Status) } + if tc.inStatus != buildapi.BuildStatusError && build.Status == buildapi.BuildStatusError && len(build.Message) == 0 { + t.Errorf("(%d) errored build should set message: %#v", i, build) + } + if len(tc.outputSpec) != 0 { + build := ctrl.BuildStrategy.(*okStrategy).build + if build.Parameters.Output.DockerImageReference != tc.outputSpec { + t.Errorf("(%d) expected build sent to strategy to have docker spec %q: %#v", i, tc.outputSpec, build) + } + } } } @@ -253,7 +366,7 @@ func TestHandlePod(t *testing.T) { } for i, tc := range tests { - build, ctrl := mockBuildAndController(tc.inStatus) + build, ctrl := mockBuildAndController(tc.inStatus, buildapi.BuildOutput{}) pod := mockPod(tc.podStatus, tc.exitCode) if tc.matchID { build.PodName = pod.Name @@ -311,7 +424,7 @@ func TestCancelBuild(t *testing.T) { } for i, tc := range tests { - build, ctrl := mockBuildAndController(tc.inStatus) + build, ctrl := mockBuildAndController(tc.inStatus, buildapi.BuildOutput{}) pod := mockPod(tc.podStatus, tc.exitCode) ctrl.CancelBuild(build, pod) diff --git a/pkg/build/controller/factory/factory.go b/pkg/build/controller/factory/factory.go index 0a790bd69b57..f6af56a9ff65 100644 --- a/pkg/build/controller/factory/factory.go +++ b/pkg/build/controller/factory/factory.go @@ -27,16 +27,18 @@ type BuildControllerFactory struct { DockerBuildStrategy *strategy.DockerBuildStrategy STIBuildStrategy *strategy.STIBuildStrategy CustomBuildStrategy *strategy.CustomBuildStrategy + // Stop may be set to allow controllers created by this factory to be terminated. + Stop <-chan struct{} buildStore cache.Store } func (factory *BuildControllerFactory) Create() *controller.BuildController { factory.buildStore = cache.NewStore() - cache.NewReflector(&buildLW{client: factory.Client}, &buildapi.Build{}, factory.buildStore).Run() + cache.NewReflector(&buildLW{client: factory.Client}, &buildapi.Build{}, factory.buildStore).RunUntil(factory.Stop) buildQueue := cache.NewFIFO() - cache.NewReflector(&buildLW{client: factory.Client}, &buildapi.Build{}, buildQueue).Run() + cache.NewReflector(&buildLW{client: factory.Client}, &buildapi.Build{}, buildQueue).RunUntil(factory.Stop) // Kubernetes does not currently synchronize Pod status in storage with a Pod's container // states. Because of this, we can't receive events related to container (and thus Pod) @@ -47,17 +49,23 @@ func (factory *BuildControllerFactory) Create() *controller.BuildController { // TODO: Find a way to get watch events for Pod/container status updates. The polling // strategy is horribly inefficient and should be addressed upstream somehow. podQueue := cache.NewFIFO() - cache.NewPoller(factory.pollPods, 10*time.Second, podQueue).Run() + cache.NewPoller(factory.pollPods, 10*time.Second, podQueue).RunUntil(factory.Stop) + client := ControllerClient{factory.KubeClient, factory.Client} return &controller.BuildController{ - BuildStore: factory.buildStore, - BuildUpdater: ClientBuildUpdater{factory.Client}, - PodManager: ClientPodManager{factory.KubeClient}, + BuildStore: factory.buildStore, + BuildUpdater: client, + ImageRepositoryClient: client, + PodManager: client, NextBuild: func() *buildapi.Build { - return buildQueue.Pop().(*buildapi.Build) + build := buildQueue.Pop().(*buildapi.Build) + panicIfStopped(factory.Stop, "build controller stopped") + return build }, NextPod: func() *kapi.Pod { - return podQueue.Pop().(*kapi.Pod) + pod := podQueue.Pop().(*kapi.Pod) + panicIfStopped(factory.Stop, "build controller stopped") + return pod }, BuildStrategy: &typeBasedFactoryStrategy{ DockerBuildStrategy: factory.DockerBuildStrategy, @@ -79,15 +87,16 @@ type ImageChangeControllerFactory struct { // image is available func (factory *ImageChangeControllerFactory) Create() *controller.ImageChangeController { queue := cache.NewFIFO() - cache.NewReflector(&imageRepositoryLW{factory.Client}, &imageapi.ImageRepository{}, queue).Run() + cache.NewReflector(&imageRepositoryLW{factory.Client}, &imageapi.ImageRepository{}, queue).RunUntil(factory.Stop) store := cache.NewStore() - cache.NewReflector(&buildConfigLW{client: factory.Client}, &buildapi.BuildConfig{}, store).Run() + cache.NewReflector(&buildConfigLW{client: factory.Client}, &buildapi.BuildConfig{}, store).RunUntil(factory.Stop) + client := ControllerClient{nil, factory.Client} return &controller.ImageChangeController{ BuildConfigStore: store, - BuildConfigUpdater: &ClientBuildConfigUpdater{factory.Client}, - BuildCreator: &ClientBuildCreator{factory.Client}, + BuildConfigUpdater: client, + BuildCreator: client, NextImageRepository: func() *imageapi.ImageRepository { repo := queue.Pop().(*imageapi.ImageRepository) panicIfStopped(factory.Stop, "build image change controller stopped") @@ -210,38 +219,29 @@ func (lw *imageRepositoryLW) Watch(resourceVersion string) (watch.Interface, err return lw.client.ImageRepositories(kapi.NamespaceAll).Watch(labels.Everything(), labels.Everything(), resourceVersion) } -// ClientPodManager is a PodManager which delegates to the Kubernetes client interface. -type ClientPodManager struct { +// ControllerClient implements the common interfaces needed for build controllers +type ControllerClient struct { KubeClient kclient.Interface + Client osclient.Interface } // CreatePod creates a pod using the Kubernetes client. -func (c ClientPodManager) CreatePod(namespace string, pod *kapi.Pod) (*kapi.Pod, error) { +func (c ControllerClient) CreatePod(namespace string, pod *kapi.Pod) (*kapi.Pod, error) { return c.KubeClient.Pods(namespace).Create(pod) } // DeletePod destroys a pod using the Kubernetes client. -func (c ClientPodManager) DeletePod(namespace string, pod *kapi.Pod) error { +func (c ControllerClient) DeletePod(namespace string, pod *kapi.Pod) error { return c.KubeClient.Pods(namespace).Delete(pod.Name) } -// ClientBuildUpdater is a buildUpdater which delegates to the OpenShift client interfaces. -type ClientBuildUpdater struct { - Client osclient.Interface -} - // UpdateBuild updates build using the OpenShift client. -func (c ClientBuildUpdater) UpdateBuild(namespace string, build *buildapi.Build) (*buildapi.Build, error) { +func (c ControllerClient) UpdateBuild(namespace string, build *buildapi.Build) (*buildapi.Build, error) { return c.Client.Builds(namespace).Update(build) } -// ClientBuildCreator is a buildCreator which delegates to the OpenShift client interfaces. -type ClientBuildCreator struct { - Client osclient.Interface -} - -// UpdateBuild updates build using the OpenShift client. -func (c *ClientBuildCreator) CreateBuild(config *buildapi.BuildConfig, imageSubstitutions map[string]string) error { +// CreateBuild updates build using the OpenShift client. +func (c ControllerClient) CreateBuild(config *buildapi.BuildConfig, imageSubstitutions map[string]string) error { build := buildutil.GenerateBuildFromConfig(config, nil) for originalImage, newImage := range imageSubstitutions { buildutil.SubstituteImageReferences(build, originalImage, newImage) @@ -253,13 +253,13 @@ func (c *ClientBuildCreator) CreateBuild(config *buildapi.BuildConfig, imageSubs return nil } -// ClientBuildConfigUpdater is a buildConfigUpdater which delegates to the OpenShift client interfaces. -type ClientBuildConfigUpdater struct { - Client osclient.Interface -} - // UpdateBuildConfig updates buildConfig using the OpenShift client. -func (c *ClientBuildConfigUpdater) UpdateBuildConfig(buildConfig *buildapi.BuildConfig) error { +func (c ControllerClient) UpdateBuildConfig(buildConfig *buildapi.BuildConfig) error { _, err := c.Client.BuildConfigs(buildConfig.Namespace).Update(buildConfig) return err } + +// GetImageRepository retrieves an image repository by namespace and name +func (c ControllerClient) GetImageRepository(namespace, name string) (*imageapi.ImageRepository, error) { + return c.Client.ImageRepositories(namespace).Get(name) +} diff --git a/pkg/build/controller/image_change_controller.go b/pkg/build/controller/image_change_controller.go index c2c14a8387a0..6576de32ae12 100644 --- a/pkg/build/controller/image_change_controller.go +++ b/pkg/build/controller/image_change_controller.go @@ -42,6 +42,7 @@ func (c *ImageChangeController) HandleImageRepo() { glog.V(4).Infof("Build image change controller detected imagerepo change %s", imageRepo.DockerImageRepository) imageSubstitutions := make(map[string]string) + // TODO: this is inefficient for _, bc := range c.BuildConfigStore.List() { config := bc.(*buildapi.BuildConfig) glog.V(4).Infof("Detecting changed images for buildConfig %s", config.Name) @@ -52,10 +53,13 @@ func (c *ImageChangeController) HandleImageRepo() { if trigger.Type != buildapi.ImageChangeBuildTriggerType { continue } + icTrigger := trigger.ImageChange + if icTrigger.From.Name != imageRepo.Name { + continue + } // for every ImageChange trigger, record the image it substitutes for and get the latest // image id from the imagerepository. We will substitute all images in the buildconfig // with the latest values from the imagerepositories. - icTrigger := trigger.ImageChange tag := icTrigger.Tag if len(tag) == 0 { tag = buildapi.DefaultImageTag @@ -66,7 +70,7 @@ func (c *ImageChangeController) HandleImageRepo() { } // (must be different) to trigger a build - if icTrigger.ImageRepositoryRef.Name == imageRepo.Name && icTrigger.LastTriggeredImageID != imageID { + if icTrigger.LastTriggeredImageID != imageID { imageSubstitutions[icTrigger.Image] = imageRepo.DockerImageRepository + ":" + imageID shouldTriggerBuild = true icTrigger.LastTriggeredImageID = imageID diff --git a/pkg/build/controller/image_change_controller_test.go b/pkg/build/controller/image_change_controller_test.go index f677302b3000..c3c88f310acb 100644 --- a/pkg/build/controller/image_change_controller_test.go +++ b/pkg/build/controller/image_change_controller_test.go @@ -51,7 +51,7 @@ func mockBuildConfig(baseImage, triggerImage, repoName, repoTag string) *buildap Type: buildapi.ImageChangeBuildTriggerType, ImageChange: &buildapi.ImageChangeTrigger{ Image: triggerImage, - ImageRepositoryRef: &kapi.ObjectReference{ + From: kapi.ObjectReference{ Name: repoName, }, Tag: repoTag, @@ -66,7 +66,7 @@ func appendTrigger(buildcfg *buildapi.BuildConfig, triggerImage, repoName, repoT Type: buildapi.ImageChangeBuildTriggerType, ImageChange: &buildapi.ImageChangeTrigger{ Image: triggerImage, - ImageRepositoryRef: &kapi.ObjectReference{ + From: kapi.ObjectReference{ Name: repoName, }, Tag: repoTag, diff --git a/pkg/build/controller/strategy/custom.go b/pkg/build/controller/strategy/custom.go index 1e96f94aeb45..8c0bd3377e21 100644 --- a/pkg/build/controller/strategy/custom.go +++ b/pkg/build/controller/strategy/custom.go @@ -1,10 +1,10 @@ package strategy import ( - "encoding/json" "errors" kapi "github.com/GoogleCloudPlatform/kubernetes/pkg/api" + "github.com/GoogleCloudPlatform/kubernetes/pkg/runtime" "github.com/golang/glog" buildapi "github.com/openshift/origin/pkg/build/api" @@ -13,18 +13,22 @@ import ( // CustomBuildStrategy creates a build using a custom builder image. type CustomBuildStrategy struct { UseLocalImages bool + // Codec is the codec to use for encoding the output pod. + // IMPORTANT: This may break backwards compatibility when + // it changes. + Codec runtime.Codec } // CreateBuildPod creates the pod to be used for the Custom build func (bs *CustomBuildStrategy) CreateBuildPod(build *buildapi.Build) (*kapi.Pod, error) { - buildJSON, err := json.Marshal(build) + data, err := bs.Codec.Encode(build) if err != nil { return nil, err } strategy := build.Parameters.Strategy.CustomStrategy containerEnv := []kapi.EnvVar{ - {Name: "BUILD", Value: string(buildJSON)}, + {Name: "BUILD", Value: string(data)}, {Name: "SOURCE_REPOSITORY", Value: build.Parameters.Source.Git.URI}, } @@ -61,7 +65,9 @@ func (bs *CustomBuildStrategy) CreateBuildPod(build *buildapi.Build) (*kapi.Pod, }, } - setupBuildEnv(build, pod) + if err := setupBuildEnv(build, pod); err != nil { + return nil, err + } if bs.UseLocalImages { pod.Spec.Containers[0].ImagePullPolicy = kapi.PullIfNotPresent diff --git a/pkg/build/controller/strategy/custom_test.go b/pkg/build/controller/strategy/custom_test.go index 53590e6ae3da..30935df97bc6 100644 --- a/pkg/build/controller/strategy/custom_test.go +++ b/pkg/build/controller/strategy/custom_test.go @@ -1,17 +1,18 @@ package strategy import ( - "encoding/json" "testing" kapi "github.com/GoogleCloudPlatform/kubernetes/pkg/api" + "github.com/openshift/origin/pkg/api/v1beta1" buildapi "github.com/openshift/origin/pkg/build/api" ) func TestCustomCreateBuildPod(t *testing.T) { strategy := CustomBuildStrategy{ UseLocalImages: true, + Codec: v1beta1.Codec, } expectedBad := mockCustomBuild() @@ -41,7 +42,7 @@ func TestCustomCreateBuildPod(t *testing.T) { if actual.Spec.RestartPolicy.Never == nil { t.Errorf("Expected never, got %#v", actual.Spec.RestartPolicy) } - buildJSON, _ := json.Marshal(expected) + buildJSON, _ := v1beta1.Codec.Encode(expected) errorCases := map[int][]string{ 0: {"BUILD", string(buildJSON)}, } @@ -94,8 +95,7 @@ func mockCustomBuild() *buildapi.Build { }, }, Output: buildapi.BuildOutput{ - ImageTag: "repository/customBuild", - Registry: "docker-registry", + DockerImageReference: "docker-registry/repository/customBuild", }, }, Status: buildapi.BuildStatusNew, diff --git a/pkg/build/controller/strategy/docker.go b/pkg/build/controller/strategy/docker.go index 78c05e945ed8..14fe280f762d 100644 --- a/pkg/build/controller/strategy/docker.go +++ b/pkg/build/controller/strategy/docker.go @@ -1,9 +1,8 @@ package strategy import ( - "encoding/json" - kapi "github.com/GoogleCloudPlatform/kubernetes/pkg/api" + "github.com/GoogleCloudPlatform/kubernetes/pkg/runtime" buildapi "github.com/openshift/origin/pkg/build/api" ) @@ -12,12 +11,16 @@ import ( type DockerBuildStrategy struct { Image string UseLocalImages bool + // Codec is the codec to use for encoding the output pod. + // IMPORTANT: This may break backwards compatibility when + // it changes. + Codec runtime.Codec } // CreateBuildPod creates the pod to be used for the Docker build // TODO: Make the Pod definition configurable func (bs *DockerBuildStrategy) CreateBuildPod(build *buildapi.Build) (*kapi.Pod, error) { - buildJSON, err := json.Marshal(build) + data, err := bs.Codec.Encode(build) if err != nil { return nil, err } @@ -32,7 +35,7 @@ func (bs *DockerBuildStrategy) CreateBuildPod(build *buildapi.Build) (*kapi.Pod, Name: "docker-build", Image: bs.Image, Env: []kapi.EnvVar{ - {Name: "BUILD", Value: string(buildJSON)}, + {Name: "BUILD", Value: string(data)}, }, // TODO: run unprivileged https://github.com/openshift/origin/issues/662 Privileged: true, diff --git a/pkg/build/controller/strategy/docker_test.go b/pkg/build/controller/strategy/docker_test.go index 314b57a5348e..4666c12cf3d6 100644 --- a/pkg/build/controller/strategy/docker_test.go +++ b/pkg/build/controller/strategy/docker_test.go @@ -1,11 +1,11 @@ package strategy import ( - "encoding/json" "testing" kapi "github.com/GoogleCloudPlatform/kubernetes/pkg/api" + "github.com/openshift/origin/pkg/api/v1beta1" buildapi "github.com/openshift/origin/pkg/build/api" ) @@ -13,6 +13,7 @@ func TestDockerCreateBuildPod(t *testing.T) { strategy := DockerBuildStrategy{ Image: "docker-test-image", UseLocalImages: true, + Codec: v1beta1.Codec, } expected := mockDockerBuild() @@ -38,7 +39,7 @@ func TestDockerCreateBuildPod(t *testing.T) { if len(container.Env) != 1 { t.Fatalf("Expected 1 elements in Env table, got %d", len(container.Env)) } - buildJSON, _ := json.Marshal(expected) + buildJSON, _ := v1beta1.Codec.Encode(expected) errorCases := map[int][]string{ 0: {"BUILD", string(buildJSON)}, } @@ -71,8 +72,7 @@ func mockDockerBuild() *buildapi.Build { DockerStrategy: &buildapi.DockerBuildStrategy{ContextDir: "my/test/dir"}, }, Output: buildapi.BuildOutput{ - ImageTag: "repository/dockerBuild", - Registry: "docker-registry", + DockerImageReference: "docker-registry/repository/dockerBuild", }, }, Status: buildapi.BuildStatusNew, diff --git a/pkg/build/controller/strategy/sti.go b/pkg/build/controller/strategy/sti.go index b22c04fc9bd7..433d7aa8553f 100644 --- a/pkg/build/controller/strategy/sti.go +++ b/pkg/build/controller/strategy/sti.go @@ -1,10 +1,10 @@ package strategy import ( - "encoding/json" "io/ioutil" kapi "github.com/GoogleCloudPlatform/kubernetes/pkg/api" + "github.com/GoogleCloudPlatform/kubernetes/pkg/runtime" buildapi "github.com/openshift/origin/pkg/build/api" ) @@ -14,6 +14,10 @@ type STIBuildStrategy struct { Image string TempDirectoryCreator TempDirectoryCreator UseLocalImages bool + // Codec is the codec to use for encoding the output pod. + // IMPORTANT: This may break backwards compatibility when + // it changes. + Codec runtime.Codec } type TempDirectoryCreator interface { @@ -31,7 +35,7 @@ var STITempDirectoryCreator = &tempDirectoryCreator{} // CreateBuildPod creates a pod that will execute the STI build // TODO: Make the Pod definition configurable func (bs *STIBuildStrategy) CreateBuildPod(build *buildapi.Build) (*kapi.Pod, error) { - buildJSON, err := json.Marshal(build) + data, err := bs.Codec.Encode(build) if err != nil { return nil, err } @@ -45,7 +49,7 @@ func (bs *STIBuildStrategy) CreateBuildPod(build *buildapi.Build) (*kapi.Pod, er Name: "sti-build", Image: bs.Image, Env: []kapi.EnvVar{ - {Name: "BUILD", Value: string(buildJSON)}, + {Name: "BUILD", Value: string(data)}, }, // TODO: run unprivileged https://github.com/openshift/origin/issues/662 Privileged: true, diff --git a/pkg/build/controller/strategy/sti_test.go b/pkg/build/controller/strategy/sti_test.go index 9514f52f7ed3..6bc0e66b212d 100644 --- a/pkg/build/controller/strategy/sti_test.go +++ b/pkg/build/controller/strategy/sti_test.go @@ -1,11 +1,11 @@ package strategy import ( - "encoding/json" "testing" kapi "github.com/GoogleCloudPlatform/kubernetes/pkg/api" + "github.com/openshift/origin/pkg/api/v1beta1" buildapi "github.com/openshift/origin/pkg/build/api" ) @@ -20,6 +20,7 @@ func TestSTICreateBuildPod(t *testing.T) { Image: "sti-test-image", TempDirectoryCreator: &FakeTempDirCreator{}, UseLocalImages: true, + Codec: v1beta1.Codec, } expected := mockSTIBuild() @@ -44,7 +45,7 @@ func TestSTICreateBuildPod(t *testing.T) { if len(container.Env) != 1 { t.Fatalf("Expected 1 elements in Env table, got %d", len(container.Env)) } - buildJSON, _ := json.Marshal(expected) + buildJSON, _ := v1beta1.Codec.Encode(expected) errorCases := map[int][]string{ 0: {"BUILD", string(buildJSON)}, } @@ -80,8 +81,7 @@ func mockSTIBuild() *buildapi.Build { }, }, Output: buildapi.BuildOutput{ - ImageTag: "repository/stiBuild", - Registry: "docker-registry", + DockerImageReference: "docker-registry/repository/stiBuild", }, }, Status: buildapi.BuildStatusNew, diff --git a/pkg/build/controller/strategy/util.go b/pkg/build/controller/strategy/util.go index 7dff7fbe884e..d98451392a19 100644 --- a/pkg/build/controller/strategy/util.go +++ b/pkg/build/controller/strategy/util.go @@ -6,6 +6,7 @@ import ( kapi "github.com/GoogleCloudPlatform/kubernetes/pkg/api" buildapi "github.com/openshift/origin/pkg/build/api" + imageapi "github.com/openshift/origin/pkg/image/api" ) // dockerSocketPath is the default path for the Docker socket inside the builder @@ -65,7 +66,7 @@ func setupDockerConfig(podSpec *kapi.Pod) { // setupBuildEnv injects human-friendly environment variables which provides // useful information about the current build. -func setupBuildEnv(build *buildapi.Build, pod *kapi.Pod) { +func setupBuildEnv(build *buildapi.Build, pod *kapi.Pod) error { vars := []kapi.EnvVar{} switch build.Parameters.Source.Type { @@ -75,10 +76,17 @@ func setupBuildEnv(build *buildapi.Build, pod *kapi.Pod) { default: // Do nothing for unknown source types } - vars = append(vars, kapi.EnvVar{"OUTPUT_IMAGE", build.Parameters.Output.ImageTag}) - vars = append(vars, kapi.EnvVar{"OUTPUT_REGISTRY", build.Parameters.Output.Registry}) + + registry, namespace, name, tag, err := imageapi.SplitDockerPullSpec(build.Parameters.Output.DockerImageReference) + if err != nil { + return err + } + outputImage := imageapi.JoinDockerPullSpec("", namespace, name, tag) + vars = append(vars, kapi.EnvVar{"OUTPUT_IMAGE", outputImage}) + vars = append(vars, kapi.EnvVar{"OUTPUT_REGISTRY", registry}) if len(pod.Spec.Containers) > 0 { pod.Spec.Containers[0].Env = append(pod.Spec.Containers[0].Env, vars...) } + return nil } diff --git a/pkg/build/controller/strategy/util_test.go b/pkg/build/controller/strategy/util_test.go index 56430ee2e597..203b53c5df16 100644 --- a/pkg/build/controller/strategy/util_test.go +++ b/pkg/build/controller/strategy/util_test.go @@ -51,3 +51,38 @@ func TestSetupDockerSocketHostSocket(t *testing.T) { t.Error("Expected privileged to be false") } } + +func TestSetupBuildEnvFails(t *testing.T) { + build := mockCustomBuild() + containerEnv := []kapi.EnvVar{ + {Name: "BUILD", Value: ""}, + {Name: "SOURCE_REPOSITORY", Value: build.Parameters.Source.Git.URI}, + } + pod := &kapi.Pod{ + ObjectMeta: kapi.ObjectMeta{ + Name: build.PodName, + }, + Spec: kapi.PodSpec{ + Containers: []kapi.Container{ + { + Name: "custom-build", + Image: build.Parameters.Strategy.CustomStrategy.Image, + Env: containerEnv, + // TODO: run unprivileged https://github.com/openshift/origin/issues/662 + Privileged: true, + }, + }, + RestartPolicy: kapi.RestartPolicy{ + Never: &kapi.RestartPolicyNever{}, + }, + }, + } + if err := setupBuildEnv(build, pod); err != nil { + t.Errorf("unexpected error: %v", err) + } + + build.Parameters.Output.DockerImageReference = "" + if err := setupBuildEnv(build, pod); err == nil { + t.Errorf("unexpected non-error: %v", err) + } +} diff --git a/pkg/build/registry/build/rest_test.go b/pkg/build/registry/build/rest_test.go index eae4d662a8d6..4a317ae5b6c7 100644 --- a/pkg/build/registry/build/rest_test.go +++ b/pkg/build/registry/build/rest_test.go @@ -342,7 +342,7 @@ func TestBuildRESTValidatesUpdate(t *testing.T) { }, }, Output: api.BuildOutput{ - ImageTag: "repository/dataBuild", + DockerImageReference: "repository/data-build", }, }, }, @@ -365,10 +365,10 @@ func TestBuildRESTValidatesUpdate(t *testing.T) { func mockBuild() *api.Build { return &api.Build{ ObjectMeta: kapi.ObjectMeta{ - Name: "dataBuild", + Name: "data-build", Namespace: kapi.NamespaceDefault, Labels: map[string]string{ - "name": "dataBuild", + "name": "data-build", }, }, Parameters: api.BuildParameters{ @@ -385,7 +385,7 @@ func mockBuild() *api.Build { }, }, Output: api.BuildOutput{ - ImageTag: "repository/dataBuild", + DockerImageReference: "repository/data-build", }, }, Status: api.BuildStatusPending, diff --git a/pkg/build/registry/buildconfig/rest_test.go b/pkg/build/registry/buildconfig/rest_test.go index 2dda9e3bcad7..3078d3bf84a1 100644 --- a/pkg/build/registry/buildconfig/rest_test.go +++ b/pkg/build/registry/buildconfig/rest_test.go @@ -254,10 +254,10 @@ func TestCreateBuildConfig(t *testing.T) { func mockBuildConfig() *api.BuildConfig { return &api.BuildConfig{ ObjectMeta: kapi.ObjectMeta{ - Name: "dataBuild", + Name: "data-build", Namespace: kapi.NamespaceDefault, Labels: map[string]string{ - "name": "dataBuild", + "name": "data-build", }, }, Parameters: api.BuildParameters{ @@ -274,7 +274,7 @@ func mockBuildConfig() *api.BuildConfig { }, }, Output: api.BuildOutput{ - ImageTag: "repository/dataBuild", + DockerImageReference: "repository/data-build", }, }, } @@ -350,11 +350,11 @@ func TestBuildConfigRESTValidatesCreate(t *testing.T) { }, }, Output: api.BuildOutput{ - ImageTag: "data/image", + DockerImageReference: "data/image", }, }, }, - "blank ImageTag": { + "blank DockerImageReference": { ObjectMeta: kapi.ObjectMeta{Name: "abc"}, Parameters: api.BuildParameters{ Source: api.BuildSource{ @@ -364,7 +364,7 @@ func TestBuildConfigRESTValidatesCreate(t *testing.T) { }, }, Output: api.BuildOutput{ - ImageTag: "", + DockerImageReference: "", }, }, }, @@ -384,7 +384,7 @@ func TestBuildConfigRESTValidatesCreate(t *testing.T) { }, }, Output: api.BuildOutput{ - ImageTag: "data/image", + DockerImageReference: "data/image", }, }, }, @@ -414,7 +414,7 @@ func TestBuildRESTValidatesUpdate(t *testing.T) { }, }, Output: api.BuildOutput{ - ImageTag: "data/image", + DockerImageReference: "data/image", }, }, }, @@ -434,11 +434,11 @@ func TestBuildRESTValidatesUpdate(t *testing.T) { }, }, Output: api.BuildOutput{ - ImageTag: "data/image", + DockerImageReference: "data/image", }, }, }, - "blank ImageTag": { + "blank DockerImageReference": { ObjectMeta: kapi.ObjectMeta{Name: "abc"}, Parameters: api.BuildParameters{ Source: api.BuildSource{ @@ -448,7 +448,7 @@ func TestBuildRESTValidatesUpdate(t *testing.T) { }, }, Output: api.BuildOutput{ - ImageTag: "", + DockerImageReference: "", }, }, }, @@ -468,7 +468,7 @@ func TestBuildRESTValidatesUpdate(t *testing.T) { }, }, Output: api.BuildOutput{ - ImageTag: "data/image", + DockerImageReference: "data/image", }, }, }, diff --git a/pkg/build/registry/buildlog/rest.go b/pkg/build/registry/buildlog/rest.go index 466108906cf3..ba8fefbbe50b 100644 --- a/pkg/build/registry/buildlog/rest.go +++ b/pkg/build/registry/buildlog/rest.go @@ -51,6 +51,8 @@ func (r *REST) ResourceLocation(ctx kapi.Context, id string) (string, error) { return "", errors.NewFieldNotFound("Build", id) } + // TODO: these must be status errors, not field errors + // TODO: choose a more appropriate "try again later" status code, like 202 if len(build.PodName) == 0 { return "", errors.NewFieldRequired("Build.PodName", build.PodName) } diff --git a/pkg/build/registry/etcd/etcd_test.go b/pkg/build/registry/etcd/etcd_test.go index 240e57694cad..065811639f66 100644 --- a/pkg/build/registry/etcd/etcd_test.go +++ b/pkg/build/registry/etcd/etcd_test.go @@ -113,7 +113,7 @@ func TestEtcdCreateBuild(t *testing.T) { }, }, Output: api.BuildOutput{ - ImageTag: "repository/dataBuild", + DockerImageReference: "repository/dataBuild", }, }, Status: api.BuildStatusPending, @@ -335,7 +335,7 @@ func TestEtcdCreateBuildConfig(t *testing.T) { }, }, Output: api.BuildOutput{ - ImageTag: "repository/dataBuild", + DockerImageReference: "repository/dataBuild", }, }, }) diff --git a/pkg/build/util/generate_test.go b/pkg/build/util/generate_test.go index 8bc4604f81da..10fea89fdcd8 100644 --- a/pkg/build/util/generate_test.go +++ b/pkg/build/util/generate_test.go @@ -330,8 +330,7 @@ func mockCustomStrategy() api.BuildStrategy { func mockOutput() api.BuildOutput { return api.BuildOutput{ - Registry: "http://localhost:5000", - ImageTag: "test/image-tag", + DockerImageReference: "http://localhost:5000/test/image-tag", } } diff --git a/pkg/cmd/cli/describe/describer.go b/pkg/cmd/cli/describe/describer.go index 18b005878d83..d04538fef606 100644 --- a/pkg/cmd/cli/describe/describer.go +++ b/pkg/cmd/cli/describe/describer.go @@ -86,8 +86,8 @@ func (d *BuildDescriber) DescribeParameters(p buildapi.BuildParameters, out *tab formatString(out, "Ref", p.Source.Git.Ref) } } - formatString(out, "Output Image", p.Output.ImageTag) - formatString(out, "Output Registry", p.Output.Registry) + formatString(out, "Output To", p.Output.To) + formatString(out, "Output Spec", p.Output.DockerImageReference) if p.Revision != nil && p.Revision.Type == buildapi.BuildSourceGit && p.Revision.Git != nil { formatString(out, "Git Commit", p.Revision.Git.Commit) d.DescribeUser(out, "Revision Author", p.Revision.Git.Author) @@ -133,7 +133,7 @@ func (d *BuildConfigDescriber) DescribeTriggers(bc *buildapi.BuildConfig, host s if trigger.Type != buildapi.ImageChangeBuildTriggerType { continue } - formatString(out, "Image Repository Trigger", trigger.ImageChange.ImageRepositoryRef.Name) + formatString(out, "Image Repository Trigger", trigger.ImageChange.From.Name) formatString(out, "- Tag", trigger.ImageChange.Tag) formatString(out, "- Image", trigger.ImageChange.Image) formatString(out, "- LastTriggeredImageID", trigger.ImageChange.LastTriggeredImageID) @@ -242,7 +242,7 @@ func (d *ImageRepositoryDescriber) Describe(namespace, name string) (string, err return tabbedString(func(out *tabwriter.Writer) error { formatMeta(out, imageRepository.ObjectMeta) formatString(out, "Tags", formatLabels(imageRepository.Tags)) - formatString(out, "Registry", imageRepository.DockerImageRepository) + formatString(out, "Registry", imageRepository.Status.DockerImageRepository) return nil }) } diff --git a/pkg/cmd/cli/describe/printer.go b/pkg/cmd/cli/describe/printer.go index 53d43639fedc..a8778405a658 100644 --- a/pkg/cmd/cli/describe/printer.go +++ b/pkg/cmd/cli/describe/printer.go @@ -119,7 +119,7 @@ func printImageRepository(repo *imageapi.ImageRepository, w io.Writer) error { } tags = strings.Join(t, ",") } - _, err := fmt.Fprintf(w, "%s\t%s\t%s\n", repo.Name, repo.DockerImageRepository, tags) + _, err := fmt.Fprintf(w, "%s\t%s\t%s\n", repo.Name, repo.Status.DockerImageRepository, tags) return err } diff --git a/pkg/cmd/server/etcd/etcd.go b/pkg/cmd/server/etcd/etcd.go index b8f13a870c0e..9d98f1114000 100644 --- a/pkg/cmd/server/etcd/etcd.go +++ b/pkg/cmd/server/etcd/etcd.go @@ -1,6 +1,8 @@ package etcd import ( + "time" + "github.com/GoogleCloudPlatform/kubernetes/pkg/util" etcdconfig "github.com/coreos/etcd/config" "github.com/coreos/etcd/etcd" @@ -29,6 +31,6 @@ func (c *Config) Run() { glog.Infof("Started etcd at %s", config.Addr) server.Run() glog.Fatalf("etcd died, exiting.") - }, 0) + }, 500*time.Millisecond) <-server.ReadyNotify() } diff --git a/pkg/cmd/server/origin/master.go b/pkg/cmd/server/origin/master.go index 21f20855225d..2351851f9974 100644 --- a/pkg/cmd/server/origin/master.go +++ b/pkg/cmd/server/origin/master.go @@ -44,7 +44,6 @@ import ( osclient "github.com/openshift/origin/pkg/client" cmdutil "github.com/openshift/origin/pkg/cmd/util" "github.com/openshift/origin/pkg/cmd/util/clientcmd" - deployapi "github.com/openshift/origin/pkg/deploy/api" deploycontrollerfactory "github.com/openshift/origin/pkg/deploy/controller/factory" deployconfiggenerator "github.com/openshift/origin/pkg/deploy/generator" deployregistry "github.com/openshift/origin/pkg/deploy/registry/deploy" @@ -65,6 +64,7 @@ import ( projectregistry "github.com/openshift/origin/pkg/project/registry/project" routeetcd "github.com/openshift/origin/pkg/route/registry/etcd" routeregistry "github.com/openshift/origin/pkg/route/registry/route" + "github.com/openshift/origin/pkg/service" templateregistry "github.com/openshift/origin/pkg/template/registry" "github.com/openshift/origin/pkg/user" useretcd "github.com/openshift/origin/pkg/user/registry/etcd" @@ -191,27 +191,37 @@ func (c *MasterConfig) EnsureCORSAllowedOrigins(origins []string) { } func (c *MasterConfig) InstallAPI(container *restful.Container) []string { + defaultRegistry := env("OPENSHIFT_DEFAULT_REGISTRY", "${DOCKER_REGISTRY_SERVICE_HOST}:${DOCKER_REGISTRY_SERVICE_PORT}") + svcCache := service.NewServiceResolverCache(c.KubeClient().Services(api.NamespaceDefault).Get) + defaultRegistryFunc, err := svcCache.Defer(defaultRegistry) + if err != nil { + glog.Fatalf("OPENSHIFT_DEFAULT_REGISTRY variable is invalid %q: %v", defaultRegistry, err) + } + buildEtcd := buildetcd.New(c.EtcdHelper) - imageEtcd := imageetcd.New(c.EtcdHelper) + imageEtcd := imageetcd.New(c.EtcdHelper, imageetcd.DefaultRegistryFunc(defaultRegistryFunc)) deployEtcd := deployetcd.New(c.EtcdHelper) routeEtcd := routeetcd.New(c.EtcdHelper) projectEtcd := projectetcd.New(c.EtcdHelper) userEtcd := useretcd.New(c.EtcdHelper, user.NewDefaultUserInitStrategy()) oauthEtcd := oauthetcd.New(c.EtcdHelper) - osclient, kclient := c.DeploymentConfigControllerClients() + // TODO: with sharding, this needs to be changed deployConfigGenerator := &deployconfiggenerator.DeploymentConfigGenerator{ - DeploymentInterface: &oldClientDeploymentInterface{kclient}, - DeploymentConfigInterface: deployEtcd, - ImageRepositoryInterface: imageEtcd, + Client: deployconfiggenerator.Client{ + DCFn: deployEtcd.GetDeploymentConfig, + IRFn: imageEtcd.GetImageRepository, + LIRFn2: imageEtcd.ListImageRepositories, + }, Codec: latest.Codec, } - - deployRollbackGenerator := &deployrollback.RollbackGenerator{} - rollbackDeploymentGetter := &clientDeploymentInterface{kclient} - rollbackDeploymentConfigGetter := &clientDeploymentConfigInterface{osclient} - - defaultRegistry := env("OPENSHIFT_DEFAULT_REGISTRY", "") + _, kclient := c.DeploymentConfigControllerClients() + deployRollback := &deployrollback.RollbackGenerator{} + deployRollbackClient := deployrollback.Client{ + DCFn: deployEtcd.GetDeploymentConfig, + RCFn: clientDeploymentInterface{kclient}.GetDeployment, + GRFn: deployRollback.GenerateRollback, + } // initialize OpenShift API storage := map[string]apiserver.RESTStorage{ @@ -220,14 +230,14 @@ func (c *MasterConfig) InstallAPI(container *restful.Container) []string { "buildLogs": buildlogregistry.NewREST(buildEtcd, c.BuildLogClient()), "images": image.NewREST(imageEtcd), - "imageRepositories": imagerepository.NewREST(imageEtcd, defaultRegistry), + "imageRepositories": imagerepository.NewREST(imageEtcd), "imageRepositoryMappings": imagerepositorymapping.NewREST(imageEtcd, imageEtcd), "imageRepositoryTags": imagerepositorytag.NewREST(imageEtcd, imageEtcd), "deployments": deployregistry.NewREST(deployEtcd), "deploymentConfigs": deployconfigregistry.NewREST(deployEtcd), "generateDeploymentConfigs": deployconfiggenerator.NewREST(deployConfigGenerator, v1beta1.Codec), - "deploymentConfigRollbacks": deployrollback.NewREST(deployRollbackGenerator, rollbackDeploymentGetter, rollbackDeploymentConfigGetter, latest.Codec), + "deploymentConfigRollbacks": deployrollback.NewREST(deployRollbackClient, latest.Codec), "templateConfigs": templateregistry.NewREST(), @@ -460,14 +470,20 @@ func (c *MasterConfig) RunBuildController() { DockerBuildStrategy: &buildstrategy.DockerBuildStrategy{ Image: dockerImage, UseLocalImages: useLocalImages, + // TODO: this will be set to --storage-version (the internal schema we use) + Codec: v1beta1.Codec, }, STIBuildStrategy: &buildstrategy.STIBuildStrategy{ Image: stiImage, TempDirectoryCreator: buildstrategy.STITempDirectoryCreator, UseLocalImages: useLocalImages, + // TODO: this will be set to --storage-version (the internal schema we use) + Codec: v1beta1.Codec, }, CustomBuildStrategy: &buildstrategy.CustomBuildStrategy{ UseLocalImages: useLocalImages, + // TODO: this will be set to --storage-version (the internal schema we use) + Codec: v1beta1.Codec, }, } @@ -569,26 +585,10 @@ func (c ClientWebhookInterface) GetBuildConfig(namespace, name string) (*buildap return c.Client.BuildConfigs(namespace).Get(name) } -type oldClientDeploymentInterface struct { - KubeClient kclient.Interface -} - -func (c *oldClientDeploymentInterface) GetDeployment(ctx api.Context, name string) (*api.ReplicationController, error) { - return c.KubeClient.ReplicationControllers(api.Namespace(ctx)).Get(name) -} - type clientDeploymentInterface struct { KubeClient kclient.Interface } -func (c *clientDeploymentInterface) GetDeployment(namespace, name string) (*api.ReplicationController, error) { - return c.KubeClient.ReplicationControllers(namespace).Get(name) -} - -type clientDeploymentConfigInterface struct { - Client osclient.Interface -} - -func (c *clientDeploymentConfigInterface) GetDeploymentConfig(namespace, name string) (*deployapi.DeploymentConfig, error) { - return c.Client.DeploymentConfigs(namespace).Get(name) +func (c clientDeploymentInterface) GetDeployment(ctx api.Context, name string) (*api.ReplicationController, error) { + return c.KubeClient.ReplicationControllers(api.Namespace(ctx)).Get(name) } diff --git a/pkg/cmd/util/serviceability/panic.go b/pkg/cmd/util/serviceability/panic.go new file mode 100644 index 000000000000..1e35ed33acdb --- /dev/null +++ b/pkg/cmd/util/serviceability/panic.go @@ -0,0 +1,16 @@ +package serviceability + +import ( + "github.com/golang/glog" + + "github.com/GoogleCloudPlatform/kubernetes/pkg/util" +) + +// BehaviorOnPanic is a helper for setting the crash mode of OpenShift when a panic is caught. +func BehaviorOnPanic(mode string) { + switch mode { + case "crash": + glog.V(4).Infof("OpenShift will terminate as soon as a panic occurs.") + util.ReallyCrash = true + } +} diff --git a/pkg/deploy/api/types.go b/pkg/deploy/api/types.go index ae4a1504b805..429ec85880cd 100644 --- a/pkg/deploy/api/types.go +++ b/pkg/deploy/api/types.go @@ -172,7 +172,14 @@ type DeploymentTriggerImageChangeParams struct { // ContainerNames is used to restrict tag updates to the specified set of container names in a pod. ContainerNames []string `json:"containerNames,omitempty"` // RepositoryName is the identifier for a Docker image repository to watch for changes. + // DEPRECATED: will be removed in v1beta2. RepositoryName string `json:"repositoryName,omitempty"` + // From is a reference to a Docker image repository to watch for changes. This field takes + // precedence over RepositoryName, which is deprecated and will be removed in v1beta2. The + // Kind may be left blank, in which case it defaults to "ImageRepository". The "Name" is + // the only required subfield - if Namespace is blank, the namespace of the current deployment + // trigger will be used. + From kapi.ObjectReference `json:"from"` // Tag is the name of an image repository tag to watch for changes. Tag string `json:"tag,omitempty"` } diff --git a/pkg/deploy/api/v1beta1/types.go b/pkg/deploy/api/v1beta1/types.go index 127996f19f7f..e38999b9e949 100644 --- a/pkg/deploy/api/v1beta1/types.go +++ b/pkg/deploy/api/v1beta1/types.go @@ -2,7 +2,7 @@ package v1beta1 import ( v1beta1 "github.com/GoogleCloudPlatform/kubernetes/pkg/api/v1beta1" - v1beta3 "github.com/GoogleCloudPlatform/kubernetes/pkg/api/v1beta3" + kapi "github.com/GoogleCloudPlatform/kubernetes/pkg/api/v1beta3" ) // A deployment represents a single configuration of a pod deployed into the cluster, and may @@ -11,8 +11,8 @@ import ( // DEPRECATED: This type longer drives any system behavior. Deployments are now represented directly // by ReplicationControllers. Use DeploymentConfig to drive deployments. type Deployment struct { - v1beta3.TypeMeta `json:",inline"` - v1beta3.ObjectMeta `json:"metadata,omitempty"` + kapi.TypeMeta `json:",inline"` + kapi.ObjectMeta `json:"metadata,omitempty"` // Strategy describes how a deployment is executed. Strategy DeploymentStrategy `json:"strategy,omitempty"` @@ -70,7 +70,7 @@ type CustomDeploymentStrategyParams struct { // Image specifies a Docker image which can carry out a deployment. Image string `json:"image,omitempty"` // Environment holds the environment which will be given to the container for Image. - Environment []v1beta3.EnvVar `json:"environment,omitempty"` + Environment []kapi.EnvVar `json:"environment,omitempty"` // Command is optional and overrides CMD in the container Image. Command []string `json:"command,omitempty"` } @@ -78,9 +78,9 @@ type CustomDeploymentStrategyParams struct { // A DeploymentList is a collection of deployments. // DEPRECATED: Like Deployment, this is no longer used. type DeploymentList struct { - v1beta3.TypeMeta `json:",inline"` - v1beta3.ListMeta `json:"metadata,omitempty"` - Items []Deployment `json:"items"` + kapi.TypeMeta `json:",inline"` + kapi.ListMeta `json:"metadata,omitempty"` + Items []Deployment `json:"items"` } // These constants represent keys used for correlating objects related to deployments. @@ -120,8 +120,8 @@ const ( // state of the DeploymentConfig. Each change to the DeploymentConfig which should result in // a new deployment results in an increment of LatestVersion. type DeploymentConfig struct { - v1beta3.TypeMeta `json:",inline"` - v1beta3.ObjectMeta `json:"metadata,omitempty"` + kapi.TypeMeta `json:",inline"` + kapi.ObjectMeta `json:"metadata,omitempty"` // Triggers determine how updates to a DeploymentConfig result in new deployments. If no triggers // are defined, a new deployment can only occur as a result of an explicit client update to the // DeploymentConfig with a new LatestVersion. @@ -173,7 +173,14 @@ type DeploymentTriggerImageChangeParams struct { // ContainerNames is used to restrict tag updates to the specified set of container names in a pod. ContainerNames []string `json:"containerNames,omitempty"` // RepositoryName is the identifier for a Docker image repository to watch for changes. + // DEPRECATED: will be removed in v1beta2. RepositoryName string `json:"repositoryName,omitempty"` + // From is a reference to a Docker image repository to watch for changes. This field takes + // precedence over RepositoryName, which is deprecated and will be removed in v1beta2. The + // Kind may be left blank, in which case it defaults to "ImageRepository". The "Name" is + // the only required subfield - if Namespace is blank, the namespace of the current deployment + // trigger will be used. + From kapi.ObjectReference `json:"from"` // Tag is the name of an image repository tag to watch for changes. Tag string `json:"tag,omitempty"` } @@ -203,14 +210,14 @@ type DeploymentCauseImageTrigger struct { // A DeploymentConfigList is a collection of deployment configs. type DeploymentConfigList struct { - v1beta3.TypeMeta `json:",inline"` - v1beta3.ListMeta `json:"metadata,omitempty"` - Items []DeploymentConfig `json:"items"` + kapi.TypeMeta `json:",inline"` + kapi.ListMeta `json:"metadata,omitempty"` + Items []DeploymentConfig `json:"items"` } // DeploymentConfigRollback provides the input to rollback generation. type DeploymentConfigRollback struct { - v1beta3.TypeMeta `json:",inline"` + kapi.TypeMeta `json:",inline"` // Spec defines the options to rollback generation. Spec DeploymentConfigRollbackSpec `json:"spec"` } @@ -218,7 +225,7 @@ type DeploymentConfigRollback struct { // DeploymentConfigRollbackSpec represents the options for rollback generation. type DeploymentConfigRollbackSpec struct { // From points to a ReplicationController which is a deployment. - From v1beta3.ObjectReference `json:"from"` + From kapi.ObjectReference `json:"from"` // IncludeTriggers specifies whether to include config Triggers. IncludeTriggers bool `json:"includeTriggers` // IncludeTemplate specifies whether to include the PodTemplateSpec. diff --git a/pkg/deploy/api/validation/validation.go b/pkg/deploy/api/validation/validation.go index 32a2200815ae..38b329d10d47 100644 --- a/pkg/deploy/api/validation/validation.go +++ b/pkg/deploy/api/validation/validation.go @@ -3,6 +3,8 @@ package validation import ( "github.com/GoogleCloudPlatform/kubernetes/pkg/api/errors" "github.com/GoogleCloudPlatform/kubernetes/pkg/api/validation" + "github.com/GoogleCloudPlatform/kubernetes/pkg/util" + deployapi "github.com/openshift/origin/pkg/deploy/api" ) @@ -11,25 +13,42 @@ import ( // upstream and fix when it goes in. func ValidateDeployment(deployment *deployapi.Deployment) errors.ValidationErrorList { - result := validateDeploymentStrategy(&deployment.Strategy).Prefix("strategy") - controllerStateErrors := validation.ValidateReplicationControllerSpec(&deployment.ControllerTemplate) - result = append(result, controllerStateErrors.Prefix("controllerTemplate")...) - - return result + errs := validateDeploymentStrategy(&deployment.Strategy).Prefix("strategy") + if len(deployment.Name) == 0 { + errs = append(errs, errors.NewFieldRequired("name", deployment.Name)) + } else if !util.IsDNSSubdomain(deployment.Name) { + errs = append(errs, errors.NewFieldInvalid("name", deployment.Name, "name must be a valid subdomain")) + } + if len(deployment.Namespace) == 0 { + errs = append(errs, errors.NewFieldRequired("namespace", deployment.Namespace)) + } else if !util.IsDNSSubdomain(deployment.Namespace) { + errs = append(errs, errors.NewFieldInvalid("namespace", deployment.Namespace, "namespace must be a valid subdomain")) + } + errs = append(errs, validation.ValidateLabels(deployment.Labels, "labels")...) + errs = append(errs, validation.ValidateReplicationControllerSpec(&deployment.ControllerTemplate).Prefix("controllerTemplate")...) + return errs } func ValidateDeploymentConfig(config *deployapi.DeploymentConfig) errors.ValidationErrorList { - result := errors.ValidationErrorList{} + errs := errors.ValidationErrorList{} + if len(config.Name) == 0 { + errs = append(errs, errors.NewFieldRequired("name", config.Name)) + } else if !util.IsDNSSubdomain(config.Name) { + errs = append(errs, errors.NewFieldInvalid("name", config.Name, "name must be a valid subdomain")) + } + if len(config.Namespace) == 0 { + errs = append(errs, errors.NewFieldRequired("namespace", config.Namespace)) + } else if !util.IsDNSSubdomain(config.Namespace) { + errs = append(errs, errors.NewFieldInvalid("namespace", config.Namespace, "namespace must be a valid subdomain")) + } + errs = append(errs, validation.ValidateLabels(config.Labels, "labels")...) for i := range config.Triggers { - result = append(result, validateTrigger(&config.Triggers[i]).PrefixIndex(i).Prefix("triggers")...) + errs = append(errs, validateTrigger(&config.Triggers[i]).PrefixIndex(i).Prefix("triggers")...) } - - result = append(result, validateDeploymentStrategy(&config.Template.Strategy).Prefix("template.strategy")...) - controllerStateErrors := validation.ValidateReplicationControllerSpec(&config.Template.ControllerTemplate) - result = append(result, controllerStateErrors.Prefix("template.controllerTemplate")...) - - return result + errs = append(errs, validateDeploymentStrategy(&config.Template.Strategy).Prefix("template.strategy")...) + errs = append(errs, validation.ValidateReplicationControllerSpec(&config.Template.ControllerTemplate).Prefix("template.controllerTemplate")...) + return errs } func ValidateDeploymentConfigRollback(rollback *deployapi.DeploymentConfigRollback) errors.ValidationErrorList { @@ -51,62 +70,82 @@ func ValidateDeploymentConfigRollback(rollback *deployapi.DeploymentConfigRollba } func validateDeploymentStrategy(strategy *deployapi.DeploymentStrategy) errors.ValidationErrorList { - result := errors.ValidationErrorList{} + errs := errors.ValidationErrorList{} if len(strategy.Type) == 0 { - result = append(result, errors.NewFieldRequired("type", "")) + errs = append(errs, errors.NewFieldRequired("type", "")) } switch strategy.Type { case deployapi.DeploymentStrategyTypeCustom: if strategy.CustomParams == nil { - result = append(result, errors.NewFieldRequired("customParams", "")) + errs = append(errs, errors.NewFieldRequired("customParams", "")) } else { - result = append(result, validateCustomParams(strategy.CustomParams).Prefix("customParams")...) + errs = append(errs, validateCustomParams(strategy.CustomParams).Prefix("customParams")...) } } - return result + return errs } func validateCustomParams(params *deployapi.CustomDeploymentStrategyParams) errors.ValidationErrorList { - result := errors.ValidationErrorList{} + errs := errors.ValidationErrorList{} if len(params.Image) == 0 { - result = append(result, errors.NewFieldRequired("image", "")) + errs = append(errs, errors.NewFieldRequired("image", "")) } - return result + return errs } func validateTrigger(trigger *deployapi.DeploymentTriggerPolicy) errors.ValidationErrorList { - result := errors.ValidationErrorList{} + errs := errors.ValidationErrorList{} if len(trigger.Type) == 0 { - result = append(result, errors.NewFieldRequired("type", "")) + errs = append(errs, errors.NewFieldRequired("type", "")) } if trigger.Type == deployapi.DeploymentTriggerOnImageChange { if trigger.ImageChangeParams == nil { - result = append(result, errors.NewFieldRequired("imageChangeParams", nil)) + errs = append(errs, errors.NewFieldRequired("imageChangeParams", nil)) } else { - result = append(result, validateImageChangeParams(trigger.ImageChangeParams).Prefix("imageChangeParams")...) + errs = append(errs, validateImageChangeParams(trigger.ImageChangeParams).Prefix("imageChangeParams")...) } } - return result + return errs } func validateImageChangeParams(params *deployapi.DeploymentTriggerImageChangeParams) errors.ValidationErrorList { - result := errors.ValidationErrorList{} + errs := errors.ValidationErrorList{} + + if len(params.From.Name) != 0 { + if len(params.From.Kind) == 0 { + params.From.Kind = "ImageRepository" + } + if params.From.Kind != "ImageRepository" { + errs = append(errs, errors.NewFieldInvalid("from.kind", params.From.Kind, "only 'ImageRepository' is allowed")) + } - if len(params.RepositoryName) == 0 { - result = append(result, errors.NewFieldRequired("repositoryName", "")) + if !util.IsDNSSubdomain(params.From.Name) { + errs = append(errs, errors.NewFieldInvalid("from.name", params.From.Name, "name must be a valid subdomain")) + } + if len(params.From.Namespace) != 0 && !util.IsDNSSubdomain(params.From.Namespace) { + errs = append(errs, errors.NewFieldInvalid("from.namespace", params.From.Namespace, "namespace must be a valid subdomain")) + } + + if len(params.RepositoryName) != 0 { + errs = append(errs, errors.NewFieldInvalid("repositoryName", params.RepositoryName, "only one of 'from', 'repository' name may be specified")) + } + } else { + if len(params.RepositoryName) == 0 { + errs = append(errs, errors.NewFieldRequired("from", "")) + } } if len(params.ContainerNames) == 0 { - result = append(result, errors.NewFieldRequired("containerNames", "")) + errs = append(errs, errors.NewFieldRequired("containerNames", "")) } - return result + return errs } diff --git a/pkg/deploy/api/validation/validation_test.go b/pkg/deploy/api/validation/validation_test.go index 3c0809491042..ebbfa3b1cd6d 100644 --- a/pkg/deploy/api/validation/validation_test.go +++ b/pkg/deploy/api/validation/validation_test.go @@ -24,6 +24,7 @@ func manualTrigger() []api.DeploymentTriggerPolicy { func TestValidateDeploymentOK(t *testing.T) { errs := ValidateDeployment(&api.Deployment{ + ObjectMeta: kapi.ObjectMeta{Name: "foo", Namespace: "bar"}, Strategy: test.OkStrategy(), ControllerTemplate: test.OkControllerTemplate(), }) @@ -40,6 +41,7 @@ func TestValidateDeploymentMissingFields(t *testing.T) { }{ "missing strategy.type": { api.Deployment{ + ObjectMeta: kapi.ObjectMeta{Name: "foo", Namespace: "bar"}, Strategy: api.DeploymentStrategy{}, ControllerTemplate: test.OkControllerTemplate(), }, @@ -66,8 +68,9 @@ func TestValidateDeploymentMissingFields(t *testing.T) { func TestValidateDeploymentConfigOK(t *testing.T) { errs := ValidateDeploymentConfig(&api.DeploymentConfig{ - Triggers: manualTrigger(), - Template: test.OkDeploymentTemplate(), + ObjectMeta: kapi.ObjectMeta{Name: "foo", Namespace: "bar"}, + Triggers: manualTrigger(), + Template: test.OkDeploymentTemplate(), }) if len(errs) > 0 { @@ -81,8 +84,42 @@ func TestValidateDeploymentConfigMissingFields(t *testing.T) { T errors.ValidationErrorType F string }{ + "missing name": { + api.DeploymentConfig{ + ObjectMeta: kapi.ObjectMeta{Name: "", Namespace: "bar"}, + Template: test.OkDeploymentTemplate(), + }, + errors.ValidationErrorTypeRequired, + "name", + }, + "missing namespace": { + api.DeploymentConfig{ + ObjectMeta: kapi.ObjectMeta{Name: "foo", Namespace: ""}, + Template: test.OkDeploymentTemplate(), + }, + errors.ValidationErrorTypeRequired, + "namespace", + }, + "invalid name": { + api.DeploymentConfig{ + ObjectMeta: kapi.ObjectMeta{Name: "-foo", Namespace: "bar"}, + Template: test.OkDeploymentTemplate(), + }, + errors.ValidationErrorTypeInvalid, + "name", + }, + "invalid namespace": { + api.DeploymentConfig{ + ObjectMeta: kapi.ObjectMeta{Name: "foo", Namespace: "-bar"}, + Template: test.OkDeploymentTemplate(), + }, + errors.ValidationErrorTypeInvalid, + "namespace", + }, + "missing trigger.type": { api.DeploymentConfig{ + ObjectMeta: kapi.ObjectMeta{Name: "foo", Namespace: "bar"}, Triggers: []api.DeploymentTriggerPolicy{ { ImageChangeParams: &api.DeploymentTriggerImageChangeParams{ @@ -95,8 +132,9 @@ func TestValidateDeploymentConfigMissingFields(t *testing.T) { errors.ValidationErrorTypeRequired, "triggers[0].type", }, - "missing Trigger imageChangeParams.repositoryName": { + "missing Trigger imageChangeParams.from": { api.DeploymentConfig{ + ObjectMeta: kapi.ObjectMeta{Name: "foo", Namespace: "bar"}, Triggers: []api.DeploymentTriggerPolicy{ { Type: api.DeploymentTriggerOnImageChange, @@ -108,10 +146,31 @@ func TestValidateDeploymentConfigMissingFields(t *testing.T) { Template: test.OkDeploymentTemplate(), }, errors.ValidationErrorTypeRequired, + "triggers[0].imageChangeParams.from", + }, + "both fields illegal Trigger imageChangeParams.repositoryName": { + api.DeploymentConfig{ + ObjectMeta: kapi.ObjectMeta{Name: "foo", Namespace: "bar"}, + Triggers: []api.DeploymentTriggerPolicy{ + { + Type: api.DeploymentTriggerOnImageChange, + ImageChangeParams: &api.DeploymentTriggerImageChangeParams{ + ContainerNames: []string{"foo"}, + RepositoryName: "name", + From: kapi.ObjectReference{ + Name: "other", + }, + }, + }, + }, + Template: test.OkDeploymentTemplate(), + }, + errors.ValidationErrorTypeInvalid, "triggers[0].imageChangeParams.repositoryName", }, "missing Trigger imageChangeParams.containerNames": { api.DeploymentConfig{ + ObjectMeta: kapi.ObjectMeta{Name: "foo", Namespace: "bar"}, Triggers: []api.DeploymentTriggerPolicy{ { Type: api.DeploymentTriggerOnImageChange, @@ -127,7 +186,8 @@ func TestValidateDeploymentConfigMissingFields(t *testing.T) { }, "missing strategy.type": { api.DeploymentConfig{ - Triggers: manualTrigger(), + ObjectMeta: kapi.ObjectMeta{Name: "foo", Namespace: "bar"}, + Triggers: manualTrigger(), Template: api.DeploymentTemplate{ Strategy: api.DeploymentStrategy{ CustomParams: test.OkCustomParams(), @@ -140,7 +200,8 @@ func TestValidateDeploymentConfigMissingFields(t *testing.T) { }, "missing strategy.customParams": { api.DeploymentConfig{ - Triggers: manualTrigger(), + ObjectMeta: kapi.ObjectMeta{Name: "foo", Namespace: "bar"}, + Triggers: manualTrigger(), Template: api.DeploymentTemplate{ Strategy: api.DeploymentStrategy{ Type: api.DeploymentStrategyTypeCustom, @@ -153,7 +214,8 @@ func TestValidateDeploymentConfigMissingFields(t *testing.T) { }, "missing template.strategy.customParams.image": { api.DeploymentConfig{ - Triggers: manualTrigger(), + ObjectMeta: kapi.ObjectMeta{Name: "foo", Namespace: "bar"}, + Triggers: manualTrigger(), Template: api.DeploymentTemplate{ Strategy: api.DeploymentStrategy{ Type: api.DeploymentStrategyTypeCustom, diff --git a/pkg/deploy/controller/config_change_controller.go b/pkg/deploy/controller/config_change_controller.go index 3be34f09ff86..32423031dae8 100644 --- a/pkg/deploy/controller/config_change_controller.go +++ b/pkg/deploy/controller/config_change_controller.go @@ -57,7 +57,7 @@ func (dc *DeploymentConfigChangeController) HandleDeploymentConfig() { return } - latestDeploymentID := deployutil.LatestDeploymentIDForConfig(config) + latestDeploymentID := deployutil.LatestDeploymentNameForConfig(config) obj, exists := dc.DeploymentStore.Get(latestDeploymentID) if !exists { @@ -67,14 +67,17 @@ func (dc *DeploymentConfigChangeController) HandleDeploymentConfig() { deployment := obj.(*kapi.ReplicationController) - if deployedConfig, err := deployutil.DecodeDeploymentConfig(deployment, dc.Codec); err == nil { - if deployutil.PodSpecsEqual(config.Template.ControllerTemplate.Template.Spec, deployedConfig.Template.ControllerTemplate.Template.Spec) { - glog.V(4).Infof("Ignoring updated config %s with LatestVersion=%d because it matches deployed config %s", config.Name, config.LatestVersion, deployment.Name) - return - } - } else { + deployedConfig, err := deployutil.DecodeDeploymentConfig(deployment, dc.Codec) + if err != nil { glog.V(0).Infof("Error decoding deploymentConfig from deployment %s: %v", deployment.Name, err) + return + } + + if deployutil.PodSpecsEqual(config.Template.ControllerTemplate.Template.Spec, deployedConfig.Template.ControllerTemplate.Template.Spec) { + glog.V(4).Infof("Ignoring updated config %s with LatestVersion=%d because it matches deployed config %s", config.Name, config.LatestVersion, deployment.Name) + return } + glog.V(4).Infof("Diff:\n%s", util.ObjectDiff(config.Template.ControllerTemplate.Template.Spec, deployedConfig.Template.ControllerTemplate.Template.Spec)) dc.generateDeployment(config, deployment) } @@ -86,6 +89,10 @@ func (dc *DeploymentConfigChangeController) generateDeployment(config *deployapi return } + if newConfig.LatestVersion == config.LatestVersion { + newConfig.LatestVersion++ + } + if deployment != nil { glog.V(4).Infof("Updating config %s (LatestVersion: %d -> %d) to advance existing deployment %s", config.Name, config.LatestVersion, newConfig.LatestVersion, deployment.Name) } diff --git a/pkg/deploy/controller/deployment_config_controller.go b/pkg/deploy/controller/deployment_config_controller.go index 85197c6cc38d..a003ab11db81 100644 --- a/pkg/deploy/controller/deployment_config_controller.go +++ b/pkg/deploy/controller/deployment_config_controller.go @@ -70,7 +70,7 @@ func (c *DeploymentConfigController) shouldDeploy(config *deployapi.DeploymentCo return false, nil } - latestDeploymentID := deployutil.LatestDeploymentIDForConfig(config) + latestDeploymentID := deployutil.LatestDeploymentNameForConfig(config) deployment, err := c.DeploymentInterface.GetDeployment(config.Namespace, latestDeploymentID) if err != nil { diff --git a/pkg/deploy/controller/factory/factory.go b/pkg/deploy/controller/factory/factory.go index 3e923e32f504..fd7a4b1571fe 100644 --- a/pkg/deploy/controller/factory/factory.go +++ b/pkg/deploy/controller/factory/factory.go @@ -29,7 +29,7 @@ type DeploymentConfigControllerFactory struct { func (factory *DeploymentConfigControllerFactory) Create() *controller.DeploymentConfigController { queue := cache.NewFIFO() - cache.NewReflector(&deploymentConfigLW{factory.Client}, &deployapi.DeploymentConfig{}, queue).Run() + cache.NewReflector(&deploymentConfigLW{factory.Client}, &deployapi.DeploymentConfig{}, queue).RunUntil(factory.Stop) return &controller.DeploymentConfigController{ DeploymentInterface: &ClientDeploymentInterface{factory.KubeClient}, @@ -68,10 +68,10 @@ type DeploymentControllerFactory struct { func (factory *DeploymentControllerFactory) Create() *controller.DeploymentController { deploymentQueue := cache.NewFIFO() - cache.NewReflector(&deploymentLW{client: factory.KubeClient, field: labels.Everything()}, &kapi.ReplicationController{}, deploymentQueue).Run() + cache.NewReflector(&deploymentLW{client: factory.KubeClient, field: labels.Everything()}, &kapi.ReplicationController{}, deploymentQueue).RunUntil(factory.Stop) factory.deploymentStore = cache.NewStore() - cache.NewReflector(&deploymentLW{client: factory.KubeClient, field: labels.Everything()}, &kapi.ReplicationController{}, factory.deploymentStore).Run() + cache.NewReflector(&deploymentLW{client: factory.KubeClient, field: labels.Everything()}, &kapi.ReplicationController{}, factory.deploymentStore).RunUntil(factory.Stop) // Kubernetes does not currently synchronize Pod status in storage with a Pod's container // states. Because of this, we can't receive events related to container (and thus Pod) @@ -82,7 +82,7 @@ func (factory *DeploymentControllerFactory) Create() *controller.DeploymentContr // TODO: Find a way to get watch events for Pod/container status updates. The polling // strategy is horribly inefficient and should be addressed upstream somehow. podQueue := cache.NewFIFO() - cache.NewPoller(factory.pollPods, 10*time.Second, podQueue).Run() + cache.NewPoller(factory.pollPods, 10*time.Second, podQueue).RunUntil(factory.Stop) return &controller.DeploymentController{ ContainerCreator: factory, @@ -201,10 +201,10 @@ type DeploymentConfigChangeControllerFactory struct { func (factory *DeploymentConfigChangeControllerFactory) Create() *controller.DeploymentConfigChangeController { queue := cache.NewFIFO() - cache.NewReflector(&deploymentConfigLW{factory.Client}, &deployapi.DeploymentConfig{}, queue).Run() + cache.NewReflector(&deploymentConfigLW{factory.Client}, &deployapi.DeploymentConfig{}, queue).RunUntil(factory.Stop) store := cache.NewStore() - cache.NewReflector(&deploymentLW{client: factory.KubeClient, field: labels.Everything()}, &kapi.ReplicationController{}, store).Run() + cache.NewReflector(&deploymentLW{client: factory.KubeClient, field: labels.Everything()}, &kapi.ReplicationController{}, store).RunUntil(factory.Stop) return &controller.DeploymentConfigChangeController{ ChangeStrategy: &ClientDeploymentConfigInterface{factory.Client}, @@ -229,10 +229,10 @@ type ImageChangeControllerFactory struct { func (factory *ImageChangeControllerFactory) Create() *controller.ImageChangeController { queue := cache.NewFIFO() - cache.NewReflector(&imageRepositoryLW{factory.Client}, &imageapi.ImageRepository{}, queue).Run() + cache.NewReflector(&imageRepositoryLW{factory.Client}, &imageapi.ImageRepository{}, queue).RunUntil(factory.Stop) store := cache.NewStore() - cache.NewReflector(&deploymentConfigLW{factory.Client}, &deployapi.DeploymentConfig{}, store).Run() + cache.NewReflector(&deploymentConfigLW{factory.Client}, &deployapi.DeploymentConfig{}, store).RunUntil(factory.Stop) return &controller.ImageChangeController{ DeploymentConfigInterface: &ClientDeploymentConfigInterface{factory.Client}, diff --git a/pkg/deploy/generator/config_generator.go b/pkg/deploy/generator/config_generator.go index af1696f90d77..145d5c4cd7fb 100644 --- a/pkg/deploy/generator/config_generator.go +++ b/pkg/deploy/generator/config_generator.go @@ -3,16 +3,14 @@ package generator import ( "fmt" - "github.com/golang/glog" - kapi "github.com/GoogleCloudPlatform/kubernetes/pkg/api" "github.com/GoogleCloudPlatform/kubernetes/pkg/api/errors" "github.com/GoogleCloudPlatform/kubernetes/pkg/labels" "github.com/GoogleCloudPlatform/kubernetes/pkg/runtime" "github.com/GoogleCloudPlatform/kubernetes/pkg/util" + kerrors "github.com/GoogleCloudPlatform/kubernetes/pkg/util/errors" deployapi "github.com/openshift/origin/pkg/deploy/api" - deployutil "github.com/openshift/origin/pkg/deploy/util" imageapi "github.com/openshift/origin/pkg/image/api" ) @@ -20,138 +18,223 @@ import ( // and produces a DeploymentConfig which represents a potential future DeploymentConfig. If the generated // state differs from the input state, the LatestVersion field of the output is incremented. type DeploymentConfigGenerator struct { - DeploymentInterface deploymentInterface - DeploymentConfigInterface deploymentConfigInterface - ImageRepositoryInterface imageRepositoryInterface - Codec runtime.Codec + Client GeneratorClient + Codec runtime.Codec } -type deploymentInterface interface { - GetDeployment(ctx kapi.Context, id string) (*kapi.ReplicationController, error) +type GeneratorClient interface { + GetDeploymentConfig(ctx kapi.Context, name string) (*deployapi.DeploymentConfig, error) + GetImageRepository(ctx kapi.Context, name string) (*imageapi.ImageRepository, error) + // LEGACY: used, to scan all repositories for a DockerImageReference. Will be removed + // when we drop support for reference by DockerImageReference. + ListImageRepositories(ctx kapi.Context) (*imageapi.ImageRepositoryList, error) } -type deploymentConfigInterface interface { - GetDeploymentConfig(ctx kapi.Context, id string) (*deployapi.DeploymentConfig, error) +type Client struct { + DCFn func(ctx kapi.Context, name string) (*deployapi.DeploymentConfig, error) + IRFn func(ctx kapi.Context, name string) (*imageapi.ImageRepository, error) + LIRFn func(ctx kapi.Context) (*imageapi.ImageRepositoryList, error) + LIRFn2 func(ctx kapi.Context, label labels.Selector) (*imageapi.ImageRepositoryList, error) } -type imageRepositoryInterface interface { - ListImageRepositories(ctx kapi.Context, labels labels.Selector) (*imageapi.ImageRepositoryList, error) +func (c Client) GetDeploymentConfig(ctx kapi.Context, name string) (*deployapi.DeploymentConfig, error) { + return c.DCFn(ctx, name) +} +func (c Client) GetImageRepository(ctx kapi.Context, name string) (*imageapi.ImageRepository, error) { + return c.IRFn(ctx, name) +} +func (c Client) ListImageRepositories(ctx kapi.Context) (*imageapi.ImageRepositoryList, error) { + if c.LIRFn2 != nil { + return c.LIRFn2(ctx, labels.Everything()) + } + return c.LIRFn(ctx) } // Generate returns a potential future DeploymentConfig based on the DeploymentConfig specified -// by deploymentConfigID. -func (g *DeploymentConfigGenerator) Generate(ctx kapi.Context, deploymentConfigID string) (*deployapi.DeploymentConfig, error) { - glog.V(4).Infof("Generating new deployment config from deploymentConfig %v", deploymentConfigID) - - deploymentConfig, err := g.DeploymentConfigInterface.GetDeploymentConfig(ctx, deploymentConfigID) +// by namespace and name +func (g *DeploymentConfigGenerator) Generate(ctx kapi.Context, name string) (*deployapi.DeploymentConfig, error) { + dc, err := g.Client.GetDeploymentConfig(ctx, name) if err != nil { - glog.V(4).Infof("Error getting deploymentConfig for id %v", deploymentConfigID) return nil, err } - deploymentID := deployutil.LatestDeploymentIDForConfig(deploymentConfig) - - deployment, err := g.DeploymentInterface.GetDeployment(ctx, deploymentID) - if err != nil && !errors.IsNotFound(err) { - glog.V(2).Infof("Error getting deployment: %#v", err) - return nil, err + refs, legacy := findReferences(dc) + if errs := retrieveReferences(g.Client, ctx, refs, legacy); len(errs) > 0 { + return nil, kerrors.NewAggregate(errs) + } + indexed := referencesByIndex(refs, legacy) + changed, errs := replaceReferences(dc, indexed) + if len(errs) > 0 { + return nil, kerrors.NewAggregate(errs) + } + if changed || dc.LatestVersion == 0 { + dc.LatestVersion++ } - deploymentExists := !errors.IsNotFound(err) - configPodTemplateSpec := deploymentConfig.Template.ControllerTemplate.Template + return dc, nil +} - referencedRepoNames := referencedRepoNames(deploymentConfig) - referencedRepos := imageReposByDockerImageRepo(ctx, g.ImageRepositoryInterface, referencedRepoNames) +type refKey struct { + namespace string + name string +} - for _, repoName := range referencedRepoNames.List() { - params := deployutil.ParamsForImageChangeTrigger(deploymentConfig, repoName) - repo, ok := referencedRepos[params.RepositoryName] - if !ok { - return nil, fmt.Errorf("Config references unknown ImageRepository '%s'", params.RepositoryName) +type triggerEntry struct { + positions []int + repo *imageapi.ImageRepository +} + +type triggersByRef map[refKey]*triggerEntry +type triggersByName map[string]*triggerEntry + +// findReferences looks up triggers with references and maps them back to their position in the trigger array. +func findReferences(dc *deployapi.DeploymentConfig) (refs triggersByRef, legacy triggersByName) { + refs, legacy = make(triggersByRef), make(triggersByName) + + for i := range dc.Triggers { + trigger := &dc.Triggers[i] + if trigger.Type != deployapi.DeploymentTriggerOnImageChange { + continue } - // TODO: If the tag is missing, what's the correct reaction? - tag, tagExists := repo.Tags[params.Tag] - if !tagExists { - glog.V(4).Infof("No tag %s found for repository %s (potentially invalid DeploymentConfig status)", tag, repoName) + // use the object reference to find the image repository + if from := &trigger.ImageChangeParams.From; len(from.Name) != 0 { + k := refKey{ + namespace: from.Namespace, + name: from.Name, + } + if len(k.namespace) == 0 { + k.namespace = dc.Namespace + } + trigger, ok := refs[k] + if !ok { + trigger = &triggerEntry{} + refs[k] = trigger + } + trigger.positions = append(trigger.positions, i) continue } - newImage := repo.DockerImageRepository + ":" + tag - updateContainers(configPodTemplateSpec, util.NewStringSet(params.ContainerNames...), newImage) + // use the old way of looking up the name + // DEPRECATED: this path will be removed soon + if k := trigger.ImageChangeParams.RepositoryName; len(k) != 0 { + trigger, ok := legacy[k] + if !ok { + trigger = &triggerEntry{} + legacy[k] = trigger + } + trigger.positions = append(trigger.positions, i) + continue + } } + return +} + +// retrieveReferences loads the repositories referenced by a deployment config +func retrieveReferences(client GeneratorClient, ctx kapi.Context, refs triggersByRef, legacy triggersByName) []error { + errs := []error{} - if deploymentExists { - if deployedConfig, err := deployutil.DecodeDeploymentConfig(deployment, g.Codec); err == nil { - if !deployutil.PodSpecsEqual(configPodTemplateSpec.Spec, deployedConfig.Template.ControllerTemplate.Template.Spec) { - deploymentConfig.LatestVersion++ - // reset the details of the deployment trigger for this deploymentConfig - deploymentConfig.Details = nil - glog.V(4).Infof("Incremented deploymentConfig %s to %d due to template inequality with deployed config", deploymentConfig.Name, deploymentConfig.LatestVersion) - } else { - glog.V(4).Infof("No diff detected between deploymentConfig %s and deployed config %s", deploymentConfig.Name, deployedConfig.Name) + // fetch repositories directly + for k, v := range refs { + repo, err := client.GetImageRepository(kapi.WithNamespace(ctx, k.namespace), k.name) + if err != nil { + errs = append(errs, err) + continue + } + v.repo = repo + } + + // look for legacy references that we've already loaded + // DEPRECATED: remove all code below this line when the reference logic is removed + missing := make(triggersByName) + for k, v := range legacy { + for _, ref := range refs { + if ref.repo.Status.DockerImageRepository == k { + v.repo = ref.repo + break } - } else { - glog.V(0).Infof("Failed to decode DeploymentConfig from deployment %s: %v", deployment.Name, err) } - } else { - if deploymentConfig.LatestVersion == 0 { - // If the latest version is zero, and the generation's being called, bump it. - deploymentConfig.LatestVersion = 1 - // reset the details of the deployment trigger for this deploymentConfig - deploymentConfig.Details = nil - glog.V(4).Infof("Set deploymentConfig %s to version %d for initial deployment", deploymentConfig.Name, deploymentConfig.LatestVersion) + if v.repo == nil { + missing[k] = v } } - return deploymentConfig, nil -} - -func updateContainers(template *kapi.PodTemplateSpec, containers util.StringSet, newImage string) { - for i, container := range template.Spec.Containers { - if !containers.Has(container.Name) { - continue + // if we haven't loaded the references, do the more expensive list all + if len(missing) != 0 { + repos, err := client.ListImageRepositories(ctx) + if err != nil { + errs = append(errs, err) + return errs } - // TODO: If we grow beyond this single mutation, diffing hashes of - // a clone of the original config vs the mutation would be more generic. - if newImage != container.Image { - template.Spec.Containers[i].Image = newImage + for k, ref := range missing { + for i := range repos.Items { + repo := &repos.Items[i] + if repo.DockerImageRepository == k { + ref.repo = repo + break + } + } + if ref.repo == nil { + errs = append(errs, errors.NewFieldNotFound("dockerImageRepository", k)) + } } } + return errs } -func imageReposByDockerImageRepo(ctx kapi.Context, imageRepoInterface imageRepositoryInterface, filter *util.StringSet) map[string]imageapi.ImageRepository { - repos := make(map[string]imageapi.ImageRepository) +type reposByIndex map[int]*imageapi.ImageRepository - imageRepos, err := imageRepoInterface.ListImageRepositories(ctx, labels.Everything()) - if err != nil { - glog.V(2).Infof("Error listing imageRepositories: %#v", err) - return repos +func referencesByIndex(refs triggersByRef, legacy triggersByName) reposByIndex { + repos := make(reposByIndex) + for _, v := range refs { + for _, i := range v.positions { + repos[i] = v.repo + } } - - for _, repo := range imageRepos.Items { - if filter.Has(repo.DockerImageRepository) { - repos[repo.DockerImageRepository] = repo + for _, v := range legacy { + for _, i := range v.positions { + repos[i] = v.repo } } - return repos } -// Returns the image repositories names a config has triggers registered for -func referencedRepoNames(config *deployapi.DeploymentConfig) *util.StringSet { - repoIDs := &util.StringSet{} - - if config == nil || config.Triggers == nil { - return repoIDs - } +func replaceReferences(dc *deployapi.DeploymentConfig, repos reposByIndex) (changed bool, errs []error) { + template := dc.Template.ControllerTemplate.Template + for i, repo := range repos { + params := dc.Triggers[i].ImageChangeParams - for _, trigger := range config.Triggers { - if trigger.Type == deployapi.DeploymentTriggerOnImageChange { - repoIDs.Insert(trigger.ImageChangeParams.RepositoryName) + // lookup image id + tag := params.Tag + if len(tag) == 0 { + // TODO: replace with "preferred tag" from repo + tag = "latest" + } + id, ok := repo.Tags[tag] + if !ok { + errs = append(errs, fmt.Errorf("image repository %s/%s does not have tag %q", repo.Namespace, repo.Name)) + continue + } + if len(repo.Status.DockerImageRepository) == 0 { + errs = append(errs, fmt.Errorf("image repository %s/%s does not have a Docker image repository reference set and can't be used in a deployment config trigger", repo.Namespace, repo.Name)) + continue + } + // TODO: this assumes that tag value is the image id + image := fmt.Sprintf("%s:%s", repo.Status.DockerImageRepository, id) + + // update containers + names := util.NewStringSet(params.ContainerNames...) + for i := range template.Spec.Containers { + container := &template.Spec.Containers[i] + if !names.Has(container.Name) { + continue + } + if container.Image != image { + container.Image = image + changed = true + } } } - - return repoIDs + return } diff --git a/pkg/deploy/generator/config_generator_test.go b/pkg/deploy/generator/config_generator_test.go index b378a177c308..41c3a0b519be 100644 --- a/pkg/deploy/generator/config_generator_test.go +++ b/pkg/deploy/generator/config_generator_test.go @@ -1,11 +1,14 @@ package generator import ( + "strings" "testing" + "speter.net/go/exp/math/dec/inf" + kapi "github.com/GoogleCloudPlatform/kubernetes/pkg/api" kerrors "github.com/GoogleCloudPlatform/kubernetes/pkg/api/errors" - "github.com/GoogleCloudPlatform/kubernetes/pkg/labels" + "github.com/GoogleCloudPlatform/kubernetes/pkg/api/resource" api "github.com/openshift/origin/pkg/api/latest" deployapi "github.com/openshift/origin/pkg/deploy/api" @@ -17,8 +20,8 @@ import ( func TestGenerateFromMissingDeploymentConfig(t *testing.T) { generator := &DeploymentConfigGenerator{ Codec: api.Codec, - DeploymentConfigInterface: &testDeploymentConfigInterface{ - GetDeploymentConfigFunc: func(id string) (*deployapi.DeploymentConfig, error) { + Client: Client{ + DCFn: func(ctx kapi.Context, id string) (*deployapi.DeploymentConfig, error) { return nil, kerrors.NewNotFound("deploymentConfig", id) }, }, @@ -38,20 +41,42 @@ func TestGenerateFromMissingDeploymentConfig(t *testing.T) { func TestGenerateFromConfigWithoutTagChange(t *testing.T) { generator := &DeploymentConfigGenerator{ Codec: api.Codec, - DeploymentConfigInterface: &testDeploymentConfigInterface{ - GetDeploymentConfigFunc: func(id string) (*deployapi.DeploymentConfig, error) { + Client: Client{ + DCFn: func(ctx kapi.Context, id string) (*deployapi.DeploymentConfig, error) { return deploytest.OkDeploymentConfig(1), nil }, - }, - ImageRepositoryInterface: &testImageRepositoryInterface{ - ListImageRepositoriesFunc: func(labels labels.Selector) (*imageapi.ImageRepositoryList, error) { + LIRFn: func(ctx kapi.Context) (*imageapi.ImageRepositoryList, error) { return okImageRepoList(), nil }, }, - DeploymentInterface: &testDeploymentInterface{ - GetDeploymentFunc: func(id string) (*kapi.ReplicationController, error) { - deployment, _ := deployutil.MakeDeployment(deploytest.OkDeploymentConfig(1), kapi.Codec) - return deployment, nil + } + + config, err := generator.Generate(kapi.NewDefaultContext(), "deploy1") + + if err != nil { + t.Fatalf("Unexpected error: %v", err) + } + + if config == nil { + t.Fatalf("Expected non-nil config") + } + + if config.LatestVersion != 1 { + t.Fatalf("Expected config LatestVersion=1, got %d", config.LatestVersion) + } +} + +func TestGenerateFromZeroConfigWithoutTagChange(t *testing.T) { + dc := basicDeploymentConfig() + dc.LatestVersion = 0 + generator := &DeploymentConfigGenerator{ + Codec: api.Codec, + Client: Client{ + DCFn: func(ctx kapi.Context, id string) (*deployapi.DeploymentConfig, error) { + return dc, nil + }, + LIRFn: func(ctx kapi.Context) (*imageapi.ImageRepositoryList, error) { + return okImageRepoList(), nil }, }, } @@ -74,21 +99,14 @@ func TestGenerateFromConfigWithoutTagChange(t *testing.T) { func TestGenerateFromConfigWithNoDeployment(t *testing.T) { generator := &DeploymentConfigGenerator{ Codec: api.Codec, - DeploymentConfigInterface: &testDeploymentConfigInterface{ - GetDeploymentConfigFunc: func(id string) (*deployapi.DeploymentConfig, error) { + Client: Client{ + DCFn: func(ctx kapi.Context, id string) (*deployapi.DeploymentConfig, error) { return deploytest.OkDeploymentConfig(1), nil }, - }, - ImageRepositoryInterface: &testImageRepositoryInterface{ - ListImageRepositoriesFunc: func(labels labels.Selector) (*imageapi.ImageRepositoryList, error) { + LIRFn: func(ctx kapi.Context) (*imageapi.ImageRepositoryList, error) { return okImageRepoList(), nil }, }, - DeploymentInterface: &testDeploymentInterface{ - GetDeploymentFunc: func(id string) (*kapi.ReplicationController, error) { - return nil, kerrors.NewNotFound("replicationController", id) - }, - }, } config, err := generator.Generate(kapi.NewDefaultContext(), "deploy2") @@ -109,22 +127,69 @@ func TestGenerateFromConfigWithNoDeployment(t *testing.T) { func TestGenerateFromConfigWithUpdatedImageRef(t *testing.T) { generator := &DeploymentConfigGenerator{ Codec: api.Codec, - DeploymentConfigInterface: &testDeploymentConfigInterface{ - GetDeploymentConfigFunc: func(id string) (*deployapi.DeploymentConfig, error) { + Client: Client{ + DCFn: func(ctx kapi.Context, id string) (*deployapi.DeploymentConfig, error) { return deploytest.OkDeploymentConfig(1), nil }, - }, - ImageRepositoryInterface: &testImageRepositoryInterface{ - ListImageRepositoriesFunc: func(labels labels.Selector) (*imageapi.ImageRepositoryList, error) { + LIRFn: func(ctx kapi.Context) (*imageapi.ImageRepositoryList, error) { list := okImageRepoList() list.Items[0].Tags["tag1"] = "ref2" return list, nil }, }, - DeploymentInterface: &testDeploymentInterface{ - GetDeploymentFunc: func(id string) (*kapi.ReplicationController, error) { - deployment, _ := deployutil.MakeDeployment(deploytest.OkDeploymentConfig(1), kapi.Codec) - return deployment, nil + } + + config, err := generator.Generate(kapi.NewDefaultContext(), "deploy1") + + if err != nil { + t.Fatalf("Unexpected error: %v", err) + } + + if config == nil { + t.Fatalf("Expected non-nil config") + } + + if config.LatestVersion != 2 { + t.Fatalf("Expected config LatestVersion=2, got %d", config.LatestVersion) + } + + expected := "registry:8080/repo1:ref2" + actual := config.Template.ControllerTemplate.Template.Spec.Containers[0].Image + if expected != actual { + t.Fatalf("Expected container image %s, got %s", expected, actual) + } +} + +func TestGenerateReportsErrorWhenRepoHasNoImage(t *testing.T) { + generator := &DeploymentConfigGenerator{ + Codec: api.Codec, + Client: Client{ + DCFn: func(ctx kapi.Context, name string) (*deployapi.DeploymentConfig, error) { + return referenceDeploymentConfig(), nil + }, + IRFn: func(ctx kapi.Context, name string) (*imageapi.ImageRepository, error) { + return &emptyImageRepo().Items[0], nil + }, + }, + } + _, err := generator.Generate(kapi.NewDefaultContext(), "deploy1") + if err == nil { + t.Fatalf("Unexpected non-error") + } + if !strings.Contains(err.Error(), "image repository /imageRepo1 does not have a Docker") { + t.Errorf("unexpected error message: %v", err) + } +} + +func TestGenerateDeploymentConfigWithFrom(t *testing.T) { + generator := &DeploymentConfigGenerator{ + Codec: api.Codec, + Client: Client{ + DCFn: func(ctx kapi.Context, name string) (*deployapi.DeploymentConfig, error) { + return referenceDeploymentConfig(), nil + }, + IRFn: func(ctx kapi.Context, name string) (*imageapi.ImageRepository, error) { + return &internalImageRepo().Items[0], nil }, }, } @@ -143,46 +208,148 @@ func TestGenerateFromConfigWithUpdatedImageRef(t *testing.T) { t.Fatalf("Expected config LatestVersion=2, got %d", config.LatestVersion) } - expected := "registry:8080/repo1:ref2" + expected := "internal/namespace/imageRepo1:ref1" actual := config.Template.ControllerTemplate.Template.Spec.Containers[0].Image if expected != actual { t.Fatalf("Expected container image %s, got %s", expected, actual) } } -type testDeploymentInterface struct { - GetDeploymentFunc func(id string) (*kapi.ReplicationController, error) +func okImageRepoList() *imageapi.ImageRepositoryList { + return &imageapi.ImageRepositoryList{ + Items: []imageapi.ImageRepository{ + { + ObjectMeta: kapi.ObjectMeta{Name: "imageRepo1"}, + DockerImageRepository: "registry:8080/repo1", + Tags: map[string]string{ + "tag1": "ref1", + }, + Status: imageapi.ImageRepositoryStatus{ + DockerImageRepository: "registry:8080/repo1", + }, + }, + }, + } } -func (i *testDeploymentInterface) GetDeployment(ctx kapi.Context, id string) (*kapi.ReplicationController, error) { - return i.GetDeploymentFunc(id) +func basicPodTemplate() *kapi.PodTemplateSpec { + return &kapi.PodTemplateSpec{ + Spec: kapi.PodSpec{ + Containers: []kapi.Container{ + { + Name: "container1", + Image: "registry:8080/repo1:ref1", + CPU: resource.Quantity{Amount: inf.NewDec(0, 3), Format: "DecimalSI"}, + Memory: resource.Quantity{Amount: inf.NewDec(0, 0), Format: "DecimalSI"}, + }, + { + Name: "container2", + Image: "registry:8080/repo1:ref2", + CPU: resource.Quantity{Amount: inf.NewDec(0, 3), Format: "DecimalSI"}, + Memory: resource.Quantity{Amount: inf.NewDec(0, 0), Format: "DecimalSI"}, + }, + }, + }, + } } -type testDeploymentConfigInterface struct { - GetDeploymentConfigFunc func(id string) (*deployapi.DeploymentConfig, error) +func basicDeploymentConfig() *deployapi.DeploymentConfig { + return &deployapi.DeploymentConfig{ + ObjectMeta: kapi.ObjectMeta{Name: "deploy1"}, + LatestVersion: 1, + Triggers: []deployapi.DeploymentTriggerPolicy{ + { + Type: deployapi.DeploymentTriggerOnImageChange, + ImageChangeParams: &deployapi.DeploymentTriggerImageChangeParams{ + ContainerNames: []string{ + "container1", + }, + RepositoryName: "registry:8080/repo1", + Tag: "tag1", + }, + }, + }, + Template: deployapi.DeploymentTemplate{ + ControllerTemplate: kapi.ReplicationControllerSpec{ + Template: basicPodTemplate(), + }, + }, + } } -func (i *testDeploymentConfigInterface) GetDeploymentConfig(ctx kapi.Context, id string) (*deployapi.DeploymentConfig, error) { - return i.GetDeploymentConfigFunc(id) +func referenceDeploymentConfig() *deployapi.DeploymentConfig { + return &deployapi.DeploymentConfig{ + ObjectMeta: kapi.ObjectMeta{Name: "deploy1"}, + LatestVersion: 1, + Triggers: []deployapi.DeploymentTriggerPolicy{ + { + Type: deployapi.DeploymentTriggerOnImageChange, + ImageChangeParams: &deployapi.DeploymentTriggerImageChangeParams{ + ContainerNames: []string{ + "container1", + }, + From: kapi.ObjectReference{ + Name: "repo1", + }, + Tag: "tag1", + }, + }, + }, + Template: deployapi.DeploymentTemplate{ + ControllerTemplate: kapi.ReplicationControllerSpec{ + Template: basicPodTemplate(), + }, + }, + } } -type testImageRepositoryInterface struct { - ListImageRepositoriesFunc func(labels labels.Selector) (*imageapi.ImageRepositoryList, error) +func basicDeployment() *kapi.ReplicationController { + config := basicDeploymentConfig() + encodedConfig, _ := deployutil.EncodeDeploymentConfig(config, api.Codec) + + return &kapi.ReplicationController{ + ObjectMeta: kapi.ObjectMeta{ + Name: deployutil.LatestDeploymentNameForConfig(config), + Annotations: map[string]string{ + deployapi.DeploymentConfigAnnotation: config.Name, + deployapi.DeploymentStatusAnnotation: string(deployapi.DeploymentStatusNew), + deployapi.DeploymentEncodedConfigAnnotation: encodedConfig, + }, + Labels: config.Labels, + }, + Spec: kapi.ReplicationControllerSpec{ + Template: basicPodTemplate(), + }, + } } -func (i *testImageRepositoryInterface) ListImageRepositories(ctx kapi.Context, labels labels.Selector) (*imageapi.ImageRepositoryList, error) { - return i.ListImageRepositoriesFunc(labels) +func internalImageRepo() *imageapi.ImageRepositoryList { + return &imageapi.ImageRepositoryList{ + Items: []imageapi.ImageRepository{ + { + ObjectMeta: kapi.ObjectMeta{Name: "imageRepo1"}, + Tags: map[string]string{ + "tag1": "ref1", + }, + Status: imageapi.ImageRepositoryStatus{ + DockerImageRepository: "internal/namespace/imageRepo1", + }, + }, + }, + } } -func okImageRepoList() *imageapi.ImageRepositoryList { +func emptyImageRepo() *imageapi.ImageRepositoryList { return &imageapi.ImageRepositoryList{ Items: []imageapi.ImageRepository{ { - ObjectMeta: kapi.ObjectMeta{Name: "imageRepo1"}, - DockerImageRepository: "registry:8080/repo1", + ObjectMeta: kapi.ObjectMeta{Name: "imageRepo1"}, Tags: map[string]string{ "tag1": "ref1", }, + Status: imageapi.ImageRepositoryStatus{ + DockerImageRepository: "", + }, }, }, } diff --git a/pkg/deploy/registry/deployconfig/rest.go b/pkg/deploy/registry/deployconfig/rest.go index 287fa1544d90..4be79ea453e8 100644 --- a/pkg/deploy/registry/deployconfig/rest.go +++ b/pkg/deploy/registry/deployconfig/rest.go @@ -118,6 +118,6 @@ func (s *REST) Update(ctx kapi.Context, obj runtime.Object) (<-chan apiserver.RE if err != nil { return nil, err } - return deploymentConfig, nil + return s.Get(ctx, deploymentConfig.Name) }), nil } diff --git a/pkg/deploy/rollback/rest.go b/pkg/deploy/rollback/rest.go index dfcd83d9f5f3..eac02d0a24de 100644 --- a/pkg/deploy/rollback/rest.go +++ b/pkg/deploy/rollback/rest.go @@ -15,34 +15,39 @@ import ( // REST provides a rollback generation endpoint. Only the Create method is implemented. type REST struct { - generator GeneratorClient - deploymentGetter DeploymentGetter - deploymentConfigGetter DeploymentConfigGetter - codec runtime.Codec + generator GeneratorClient + codec runtime.Codec } // GeneratorClient defines a local interface to a rollback generator for testability. type GeneratorClient interface { - Generate(from, to *deployapi.DeploymentConfig, spec *deployapi.DeploymentConfigRollbackSpec) (*deployapi.DeploymentConfig, error) + GenerateRollback(from, to *deployapi.DeploymentConfig, spec *deployapi.DeploymentConfigRollbackSpec) (*deployapi.DeploymentConfig, error) + GetDeployment(ctx kapi.Context, name string) (*kapi.ReplicationController, error) + GetDeploymentConfig(ctx kapi.Context, name string) (*deployapi.DeploymentConfig, error) } -// DeploymentGetter is a local interface to ReplicationControllers for testability. -type DeploymentGetter interface { - GetDeployment(namespace, name string) (*kapi.ReplicationController, error) +// Client provides an implementation of Generator client +type Client struct { + GRFn func(from, to *deployapi.DeploymentConfig, spec *deployapi.DeploymentConfigRollbackSpec) (*deployapi.DeploymentConfig, error) + RCFn func(ctx kapi.Context, name string) (*kapi.ReplicationController, error) + DCFn func(ctx kapi.Context, name string) (*deployapi.DeploymentConfig, error) } -// DeploymentConfigGetter is a local interface to DeploymentConfigs for testability. -type DeploymentConfigGetter interface { - GetDeploymentConfig(namespace, name string) (*deployapi.DeploymentConfig, error) +func (c Client) GetDeploymentConfig(ctx kapi.Context, name string) (*deployapi.DeploymentConfig, error) { + return c.DCFn(ctx, name) +} +func (c Client) GetDeployment(ctx kapi.Context, name string) (*kapi.ReplicationController, error) { + return c.RCFn(ctx, name) +} +func (c Client) GenerateRollback(from, to *deployapi.DeploymentConfig, spec *deployapi.DeploymentConfigRollbackSpec) (*deployapi.DeploymentConfig, error) { + return c.GRFn(from, to, spec) } // NewREST safely creates a new REST. -func NewREST(generator GeneratorClient, deploymentGetter DeploymentGetter, configGetter DeploymentConfigGetter, codec runtime.Codec) apiserver.RESTStorage { +func NewREST(generator GeneratorClient, codec runtime.Codec) apiserver.RESTStorage { return &REST{ - generator: generator, - deploymentGetter: deploymentGetter, - deploymentConfigGetter: configGetter, - codec: codec, + generator: generator, + codec: codec, } } @@ -61,33 +66,28 @@ func (s *REST) Create(ctx kapi.Context, obj runtime.Object) (<-chan apiserver.RE return nil, kerrors.NewInvalid("deploymentConfigRollback", "", errs) } - namespace, namespaceOk := kapi.NamespaceFrom(ctx) - if !namespaceOk { - return nil, fmt.Errorf("namespace %s is invalid", ctx.Value) - } - // Roll back "from" the current deployment "to" a target deployment - var from, to *deployapi.DeploymentConfig - var err error // Find the target ("to") deployment and decode the DeploymentConfig - if targetDeployment, err := s.deploymentGetter.GetDeployment(namespace, rollback.Spec.From.Name); err != nil { + targetDeployment, err := s.generator.GetDeployment(ctx, rollback.Spec.From.Name) + if err != nil { // TODO: correct error type? return nil, kerrors.NewBadRequest(fmt.Sprintf("Couldn't get specified deployment: %v", err)) - } else { - if to, err = deployutil.DecodeDeploymentConfig(targetDeployment, s.codec); err != nil { - // TODO: correct error type? - return nil, kerrors.NewBadRequest(fmt.Sprintf("deploymentConfig on target deployment is invalid: %v", err)) - } + } + to, err := deployutil.DecodeDeploymentConfig(targetDeployment, s.codec) + if err != nil { + // TODO: correct error type? + return nil, kerrors.NewBadRequest(fmt.Sprintf("deploymentConfig on target deployment is invalid: %v", err)) } // Find the current ("from") version of the target deploymentConfig - if from, err = s.deploymentConfigGetter.GetDeploymentConfig(namespace, to.Name); err != nil { + from, err := s.generator.GetDeploymentConfig(ctx, to.Name) + if err != nil { // TODO: correct error type? - return nil, kerrors.NewBadRequest(fmt.Sprintf("Couldn't find current deploymentConfig %s/%s: %v", namespace, to.Name, err)) + return nil, kerrors.NewBadRequest(fmt.Sprintf("Couldn't find current deploymentConfig %s/%s: %v", targetDeployment.Namespace, to.Name, err)) } return apiserver.MakeAsync(func() (runtime.Object, error) { - return s.generator.Generate(from, to, &rollback.Spec) + return s.generator.GenerateRollback(from, to, &rollback.Spec) }), nil } diff --git a/pkg/deploy/rollback/rest_test.go b/pkg/deploy/rollback/rest_test.go index 6016496b9cb2..df95536a2785 100644 --- a/pkg/deploy/rollback/rest_test.go +++ b/pkg/deploy/rollback/rest_test.go @@ -16,7 +16,6 @@ import ( func TestCreateError(t *testing.T) { rest := REST{} - obj, err := rest.Create(kapi.NewDefaultContext(), &deployapi.DeploymentConfig{}) if err == nil { @@ -30,7 +29,6 @@ func TestCreateError(t *testing.T) { func TestCreateInvalid(t *testing.T) { rest := REST{} - obj, err := rest.Create(kapi.NewDefaultContext(), &deployapi.DeploymentConfigRollback{}) if err == nil { @@ -44,23 +42,19 @@ func TestCreateInvalid(t *testing.T) { func TestCreateOk(t *testing.T) { rest := REST{ - generator: &testGenerator{ - generateFunc: func(from, to *deployapi.DeploymentConfig, spec *deployapi.DeploymentConfigRollbackSpec) (*deployapi.DeploymentConfig, error) { + generator: Client{ + GRFn: func(from, to *deployapi.DeploymentConfig, spec *deployapi.DeploymentConfigRollbackSpec) (*deployapi.DeploymentConfig, error) { return &deployapi.DeploymentConfig{}, nil }, - }, - codec: api.Codec, - deploymentGetter: &testDeploymentGetter{ - GetDeploymentFunc: func(namespace, name string) (*kapi.ReplicationController, error) { + RCFn: func(ctx kapi.Context, name string) (*kapi.ReplicationController, error) { deployment, _ := deployutil.MakeDeployment(deploytest.OkDeploymentConfig(1), kapi.Codec) return deployment, nil }, - }, - deploymentConfigGetter: &testDeploymentConfigGetter{ - GetDeploymentConfigFunc: func(namespace, name string) (*deployapi.DeploymentConfig, error) { + DCFn: func(ctx kapi.Context, name string) (*deployapi.DeploymentConfig, error) { return deploytest.OkDeploymentConfig(1), nil }, }, + codec: api.Codec, } channel, err := rest.Create(kapi.NewDefaultContext(), &deployapi.DeploymentConfigRollback{ @@ -92,23 +86,19 @@ func TestCreateOk(t *testing.T) { func TestCreateGeneratorError(t *testing.T) { rest := REST{ - generator: &testGenerator{ - generateFunc: func(from, to *deployapi.DeploymentConfig, spec *deployapi.DeploymentConfigRollbackSpec) (*deployapi.DeploymentConfig, error) { + generator: Client{ + GRFn: func(from, to *deployapi.DeploymentConfig, spec *deployapi.DeploymentConfigRollbackSpec) (*deployapi.DeploymentConfig, error) { return nil, errors.New("something terrible happened") }, - }, - codec: api.Codec, - deploymentGetter: &testDeploymentGetter{ - GetDeploymentFunc: func(namespace, name string) (*kapi.ReplicationController, error) { + RCFn: func(ctx kapi.Context, name string) (*kapi.ReplicationController, error) { deployment, _ := deployutil.MakeDeployment(deploytest.OkDeploymentConfig(1), kapi.Codec) return deployment, nil }, - }, - deploymentConfigGetter: &testDeploymentConfigGetter{ - GetDeploymentConfigFunc: func(namespace, name string) (*deployapi.DeploymentConfig, error) { + DCFn: func(ctx kapi.Context, name string) (*deployapi.DeploymentConfig, error) { return deploytest.OkDeploymentConfig(1), nil }, }, + codec: api.Codec, } channel, err := rest.Create(kapi.NewDefaultContext(), &deployapi.DeploymentConfigRollback{ @@ -144,24 +134,21 @@ func TestCreateGeneratorError(t *testing.T) { func TestCreateMissingDeployment(t *testing.T) { rest := REST{ - generator: &testGenerator{ - generateFunc: func(from, to *deployapi.DeploymentConfig, spec *deployapi.DeploymentConfigRollbackSpec) (*deployapi.DeploymentConfig, error) { + generator: Client{ + GRFn: func(from, to *deployapi.DeploymentConfig, spec *deployapi.DeploymentConfigRollbackSpec) (*deployapi.DeploymentConfig, error) { t.Fatal("unexpected call to generator") return nil, errors.New("something terrible happened") }, - }, - codec: api.Codec, - deploymentGetter: &testDeploymentGetter{ - GetDeploymentFunc: func(namespace, name string) (*kapi.ReplicationController, error) { + RCFn: func(ctx kapi.Context, name string) (*kapi.ReplicationController, error) { return nil, kerrors.NewNotFound("replicationController", name) }, - }, - deploymentConfigGetter: &testDeploymentConfigGetter{ - GetDeploymentConfigFunc: func(namespace, name string) (*deployapi.DeploymentConfig, error) { + DCFn: func(ctx kapi.Context, name string) (*deployapi.DeploymentConfig, error) { + namespace, _ := kapi.NamespaceFrom(ctx) t.Fatalf("unexpected call to GetDeploymentConfig(%s/%s)", namespace, name) return nil, kerrors.NewNotFound("deploymentConfig", name) }, }, + codec: api.Codec, } channel, err := rest.Create(kapi.NewDefaultContext(), &deployapi.DeploymentConfigRollback{ @@ -184,27 +171,24 @@ func TestCreateMissingDeployment(t *testing.T) { func TestCreateInvalidDeployment(t *testing.T) { rest := REST{ - generator: &testGenerator{ - generateFunc: func(from, to *deployapi.DeploymentConfig, spec *deployapi.DeploymentConfigRollbackSpec) (*deployapi.DeploymentConfig, error) { + generator: Client{ + GRFn: func(from, to *deployapi.DeploymentConfig, spec *deployapi.DeploymentConfigRollbackSpec) (*deployapi.DeploymentConfig, error) { t.Fatal("unexpected call to generator") return nil, errors.New("something terrible happened") }, - }, - codec: api.Codec, - deploymentGetter: &testDeploymentGetter{ - GetDeploymentFunc: func(namespace, name string) (*kapi.ReplicationController, error) { + RCFn: func(ctx kapi.Context, name string) (*kapi.ReplicationController, error) { // invalidate the encoded config deployment, _ := deployutil.MakeDeployment(deploytest.OkDeploymentConfig(1), kapi.Codec) deployment.Annotations[deployapi.DeploymentEncodedConfigAnnotation] = "" return deployment, nil }, - }, - deploymentConfigGetter: &testDeploymentConfigGetter{ - GetDeploymentConfigFunc: func(namespace, name string) (*deployapi.DeploymentConfig, error) { + DCFn: func(ctx kapi.Context, name string) (*deployapi.DeploymentConfig, error) { + namespace, _ := kapi.NamespaceFrom(ctx) t.Fatalf("unexpected call to GetDeploymentConfig(%s/%s)", namespace, name) return nil, kerrors.NewNotFound("deploymentConfig", name) }, }, + codec: api.Codec, } channel, err := rest.Create(kapi.NewDefaultContext(), &deployapi.DeploymentConfigRollback{ @@ -227,24 +211,20 @@ func TestCreateInvalidDeployment(t *testing.T) { func TestCreateMissingDeploymentConfig(t *testing.T) { rest := REST{ - generator: &testGenerator{ - generateFunc: func(from, to *deployapi.DeploymentConfig, spec *deployapi.DeploymentConfigRollbackSpec) (*deployapi.DeploymentConfig, error) { + generator: Client{ + GRFn: func(from, to *deployapi.DeploymentConfig, spec *deployapi.DeploymentConfigRollbackSpec) (*deployapi.DeploymentConfig, error) { t.Fatal("unexpected call to generator") return nil, errors.New("something terrible happened") }, - }, - codec: api.Codec, - deploymentGetter: &testDeploymentGetter{ - GetDeploymentFunc: func(namespace, name string) (*kapi.ReplicationController, error) { + RCFn: func(ctx kapi.Context, name string) (*kapi.ReplicationController, error) { deployment, _ := deployutil.MakeDeployment(deploytest.OkDeploymentConfig(1), kapi.Codec) return deployment, nil }, - }, - deploymentConfigGetter: &testDeploymentConfigGetter{ - GetDeploymentConfigFunc: func(namespace, name string) (*deployapi.DeploymentConfig, error) { + DCFn: func(ctx kapi.Context, name string) (*deployapi.DeploymentConfig, error) { return nil, kerrors.NewNotFound("deploymentConfig", name) }, }, + codec: api.Codec, } channel, err := rest.Create(kapi.NewDefaultContext(), &deployapi.DeploymentConfigRollback{ @@ -267,30 +247,6 @@ func TestCreateMissingDeploymentConfig(t *testing.T) { func TestNew(t *testing.T) { // :) - rest := NewREST(&testGenerator{}, &testDeploymentGetter{}, &testDeploymentConfigGetter{}, api.Codec) + rest := NewREST(Client{}, api.Codec) rest.New() } - -type testGenerator struct { - generateFunc func(from, to *deployapi.DeploymentConfig, spec *deployapi.DeploymentConfigRollbackSpec) (*deployapi.DeploymentConfig, error) -} - -func (g *testGenerator) Generate(from, to *deployapi.DeploymentConfig, spec *deployapi.DeploymentConfigRollbackSpec) (*deployapi.DeploymentConfig, error) { - return g.generateFunc(from, to, spec) -} - -type testDeploymentGetter struct { - GetDeploymentFunc func(namespace, name string) (*kapi.ReplicationController, error) -} - -func (i *testDeploymentGetter) GetDeployment(namespace, name string) (*kapi.ReplicationController, error) { - return i.GetDeploymentFunc(namespace, name) -} - -type testDeploymentConfigGetter struct { - GetDeploymentConfigFunc func(namespace, name string) (*deployapi.DeploymentConfig, error) -} - -func (i *testDeploymentConfigGetter) GetDeploymentConfig(namespace, name string) (*deployapi.DeploymentConfig, error) { - return i.GetDeploymentConfigFunc(namespace, name) -} diff --git a/pkg/deploy/rollback/rollback_generator.go b/pkg/deploy/rollback/rollback_generator.go index b0bf41714e09..6a642639e8dd 100644 --- a/pkg/deploy/rollback/rollback_generator.go +++ b/pkg/deploy/rollback/rollback_generator.go @@ -10,13 +10,12 @@ import ( // RollbackGenerator generates a new DeploymentConfig by merging a pair of DeploymentConfigs // in a configurable way. -type RollbackGenerator struct { -} +type RollbackGenerator struct{} // Generate creates a new DeploymentConfig by merging to onto from based on the options provided // by spec. The LatestVersion of the result is unconditionally incremented, as rollback candidates are // should be possible to be deployed manually regardless of other system behavior such as triggering. -func (g *RollbackGenerator) Generate(from, to *deployapi.DeploymentConfig, spec *deployapi.DeploymentConfigRollbackSpec) (*deployapi.DeploymentConfig, error) { +func (g *RollbackGenerator) GenerateRollback(from, to *deployapi.DeploymentConfig, spec *deployapi.DeploymentConfigRollbackSpec) (*deployapi.DeploymentConfig, error) { rollback := &deployapi.DeploymentConfig{} if err := kapi.Scheme.Convert(&from, &rollback); err != nil { diff --git a/pkg/deploy/rollback/rollback_generator_test.go b/pkg/deploy/rollback/rollback_generator_test.go index 10a84f9035fc..6fadc278c29b 100644 --- a/pkg/deploy/rollback/rollback_generator_test.go +++ b/pkg/deploy/rollback/rollback_generator_test.go @@ -47,7 +47,7 @@ func TestGeneration(t *testing.T) { for _, spec := range rollbackSpecs { t.Logf("testing spec %#v", spec) - if rollback, err := generator.Generate(from, to, spec); err != nil { + if rollback, err := generator.GenerateRollback(from, to, spec); err != nil { t.Fatalf("Unexpected error: %v", err) } else { if hasStrategyDiff(from, rollback) && !spec.IncludeStrategy { diff --git a/pkg/deploy/util/util.go b/pkg/deploy/util/util.go index 3cab436d7cdd..ad3f7a3b7de4 100644 --- a/pkg/deploy/util/util.go +++ b/pkg/deploy/util/util.go @@ -5,80 +5,25 @@ import ( "fmt" "hash/adler32" "strconv" - "strings" "github.com/golang/glog" "github.com/GoogleCloudPlatform/kubernetes/pkg/api" "github.com/GoogleCloudPlatform/kubernetes/pkg/api/resource" "github.com/GoogleCloudPlatform/kubernetes/pkg/runtime" - "github.com/GoogleCloudPlatform/kubernetes/pkg/util" deployapi "github.com/openshift/origin/pkg/deploy/api" ) -// LatestDeploymentIDForConfig returns a stable identifier for config based on its version. -func LatestDeploymentIDForConfig(config *deployapi.DeploymentConfig) string { +// LatestDeploymentNameForConfig returns a stable identifier for config based on its version. +func LatestDeploymentNameForConfig(config *deployapi.DeploymentConfig) string { return config.Name + "-" + strconv.Itoa(config.LatestVersion) } -func ParamsForImageChangeTrigger(config *deployapi.DeploymentConfig, repoName string) *deployapi.DeploymentTriggerImageChangeParams { - if config == nil || config.Triggers == nil { - return nil - } - - for _, trigger := range config.Triggers { - if trigger.Type == deployapi.DeploymentTriggerOnImageChange && trigger.ImageChangeParams.RepositoryName == repoName { - return trigger.ImageChangeParams - } - } - - return nil -} - -// Set a-b -func Difference(a, b util.StringSet) util.StringSet { - diff := util.StringSet{} - - if a == nil || b == nil { - return diff - } - - for _, s := range a.List() { - if !b.Has(s) { - diff.Insert(s) - } - } - - return diff -} - -// Returns a map of referenced image name to image version -func ReferencedImages(deployment *deployapi.Deployment) map[string]string { - result := make(map[string]string) - - if deployment == nil { - return result - } - - for _, container := range deployment.ControllerTemplate.Template.Spec.Containers { - name, version := ParseContainerImage(container.Image) - result[name] = version - } - - return result -} - -func ParseContainerImage(image string) (string, string) { - tokens := strings.Split(image, ":") - return tokens[0], tokens[1] -} - // HashPodSpecs hashes a PodSpec into a uint64. // TODO: Resources are currently ignored due to the formats not surviving encoding/decoding // in a consistent manner (e.g. 0 is represented sometimes as 0.000) func HashPodSpec(t api.PodSpec) uint64 { - for i := range t.Containers { t.Containers[i].CPU = resource.Quantity{} t.Containers[i].Memory = resource.Quantity{} @@ -135,7 +80,7 @@ func MakeDeployment(config *deployapi.DeploymentConfig, codec runtime.Codec) (*a deployment := &api.ReplicationController{ ObjectMeta: api.ObjectMeta{ - Name: LatestDeploymentIDForConfig(config), + Name: LatestDeploymentNameForConfig(config), Annotations: map[string]string{ deployapi.DeploymentConfigAnnotation: config.Name, deployapi.DeploymentStatusAnnotation: string(deployapi.DeploymentStatusNew), diff --git a/pkg/image/api/helper.go b/pkg/image/api/helper.go index 057bc31b20c6..1885607caefd 100644 --- a/pkg/image/api/helper.go +++ b/pkg/image/api/helper.go @@ -7,8 +7,8 @@ import ( "github.com/fsouza/go-dockerclient" ) -// dockerDefaultNamespace is the value for namespace when a single segment name is provided. -const dockerDefaultNamespace = "library" +// DockerDefaultNamespace is the value for namespace when a single segment name is provided. +const DockerDefaultNamespace = "library" // SplitDockerPullSpec breaks a Docker pull specification into its components, or returns // an error if those components are not valid. Attempts to match as closely as possible the @@ -18,9 +18,6 @@ func SplitDockerPullSpec(spec string) (registry, namespace, name, tag string, er if err != nil { return } - if len(namespace) == 0 { - namespace = dockerDefaultNamespace - } return } @@ -51,12 +48,15 @@ func SplitOpenShiftPullSpec(spec string) (registry, namespace, name, tag string, // string. Attempts to match as closely as possible the Docker spec up to 1.3. Future API // revisions may change the pull syntax. func JoinDockerPullSpec(registry, namespace, name, tag string) string { - if len(namespace) == 0 { - namespace = dockerDefaultNamespace - } if len(tag) != 0 { tag = ":" + tag } + if len(namespace) == 0 { + if len(registry) == 0 { + return fmt.Sprintf("%s%s", name, tag) + } + namespace = DockerDefaultNamespace + } if len(registry) == 0 { return fmt.Sprintf("%s/%s%s", namespace, name, tag) } diff --git a/pkg/image/api/helper_test.go b/pkg/image/api/helper_test.go index 083a930603b4..4818cdfe7cba 100644 --- a/pkg/image/api/helper_test.go +++ b/pkg/image/api/helper_test.go @@ -11,9 +11,8 @@ func TestSplitDockerPullSpec(t *testing.T) { Err bool }{ { - From: "foo", - Namespace: "library", - Name: "foo", + From: "foo", + Name: "foo", }, { From: "bar/foo", diff --git a/pkg/image/api/v1beta1/types.go b/pkg/image/api/v1beta1/types.go index a2e6d65c9fb0..d038e255a685 100644 --- a/pkg/image/api/v1beta1/types.go +++ b/pkg/image/api/v1beta1/types.go @@ -54,7 +54,7 @@ type ImageRepository struct { type ImageRepositoryStatus struct { // Represents the effective location this repository may be accessed at. May be empty until the server // determines where the repository is located - DockerImageRepository string `json:"dockerImageRepository,omitempty"` + DockerImageRepository string `json:"dockerImageRepository"` } // TODO add metadata overrides diff --git a/pkg/image/registry/etcd/etcd.go b/pkg/image/registry/etcd/etcd.go index 8c0b1bc2faf4..1b2291afc76b 100644 --- a/pkg/image/registry/etcd/etcd.go +++ b/pkg/image/registry/etcd/etcd.go @@ -26,15 +26,32 @@ const ( ImageRepositoriesPath string = "/imageRepositories" ) +// DefaultRegistry returns the default Docker registry (host or host:port), or false if it is not available. +type DefaultRegistry interface { + DefaultRegistry() (string, bool) +} + +// DefaultRegistryFunc implements DefaultRegistry for a simple function. +type DefaultRegistryFunc func() (string, bool) + +// DefaultRegistry implements the DefaultRegistry interface for a function. +func (fn DefaultRegistryFunc) DefaultRegistry() (string, bool) { + return fn() +} + // Etcd implements ImageRegistry and ImageRepositoryRegistry backed by etcd. type Etcd struct { tools.EtcdHelper + defaultRegistry DefaultRegistry } -// New returns a new etcd registry. -func New(helper tools.EtcdHelper) *Etcd { +// New returns a new etcd registry. Default registry is the value that will be +// applied to the Status.DockerImageRepository field if the repository does not +// have a specified DockerImageRepository. +func New(helper tools.EtcdHelper, defaultRegistry DefaultRegistry) *Etcd { return &Etcd{ - EtcdHelper: helper, + EtcdHelper: helper, + defaultRegistry: defaultRegistry, } } @@ -160,6 +177,7 @@ func (r *Etcd) ListImageRepositories(ctx kapi.Context, selector labels.Selector) filtered := []api.ImageRepository{} for _, item := range list.Items { if selector.Matches(labels.Set(item.Labels)) { + r.fillRepository(&item) filtered = append(filtered, item) } } @@ -185,7 +203,7 @@ func (r *Etcd) GetImageRepository(ctx kapi.Context, id string) (*api.ImageReposi if err = r.ExtractObj(key, &repo, false); err != nil { return nil, etcderr.InterpretGetError(err, "imageRepository", id) } - return &repo, nil + return r.fillRepository(&repo), nil } // WatchImageRepositories begins watching for new, changed, or deleted ImageRepositories. @@ -205,7 +223,11 @@ func (r *Etcd) WatchImageRepositories(ctx kapi.Context, label, field labels.Sele "name": repo.Name, "dockerImageRepository": repo.DockerImageRepository, } - return label.Matches(labels.Set(repo.Labels)) && field.Matches(fields) + if !label.Matches(labels.Set(repo.Labels)) || !field.Matches(fields) { + return false + } + r.fillRepository(repo) + return true }) } @@ -238,3 +260,21 @@ func (r *Etcd) DeleteImageRepository(ctx kapi.Context, id string) error { err = r.Delete(key, false) return etcderr.InterpretDeleteError(err, "imageRepository", id) } + +// fillRepository sets the status information of a repository +func (r *Etcd) fillRepository(repo *api.ImageRepository) *api.ImageRepository { + var value string + if len(repo.DockerImageRepository) != 0 { + value = repo.DockerImageRepository + } else { + registry, ok := r.defaultRegistry.DefaultRegistry() + if ok { + if len(repo.Namespace) == 0 { + repo.Namespace = kapi.NamespaceDefault + } + value = api.JoinDockerPullSpec(registry, repo.Namespace, repo.Name, "") + } + } + repo.Status.DockerImageRepository = value + return repo +} diff --git a/pkg/image/registry/etcd/etcd_test.go b/pkg/image/registry/etcd/etcd_test.go index c8608ab8ed6f..9fc6223d6157 100644 --- a/pkg/image/registry/etcd/etcd_test.go +++ b/pkg/image/registry/etcd/etcd_test.go @@ -55,8 +55,15 @@ func makeTestDefaultImageRepositoriesListKey() string { return makeTestImageRepositoriesListKey(kapi.NamespaceDefault) } +var ( + testDefaultRegistry = DefaultRegistryFunc(func() (string, bool) { return "test", true }) + noDefaultRegistry = DefaultRegistryFunc(func() (string, bool) { + return "", false + }) +) + func NewTestEtcd(client tools.EtcdClient) *Etcd { - return New(tools.EtcdHelper{client, latest.Codec, tools.RuntimeVersionAdapter{latest.ResourceVersioner}}) + return New(tools.EtcdHelper{client, latest.Codec, tools.RuntimeVersionAdapter{latest.ResourceVersioner}}, noDefaultRegistry) } func TestEtcdListImagesEmpty(t *testing.T) { @@ -510,12 +517,13 @@ func TestEtcdListImageRepositoriesEverything(t *testing.T) { E: nil, } registry := NewTestEtcd(fakeClient) + registry.defaultRegistry = testDefaultRegistry repos, err := registry.ListImageRepositories(kapi.NewDefaultContext(), labels.Everything()) if err != nil { t.Errorf("unexpected error: %v", err) } - if len(repos.Items) != 2 || repos.Items[0].Name != "foo" || repos.Items[1].Name != "bar" { + if len(repos.Items) != 2 || repos.Items[0].Name != "foo" || repos.Items[1].Name != "bar" || repos.Items[1].Status.DockerImageRepository != "test/default/bar" { t.Errorf("Unexpected images list: %#v", repos) } } @@ -802,6 +810,8 @@ func TestEtcdWatchImageRepositories(t *testing.T) { fakeClient.WaitForWatchCompletion() for testIndex, repo := range tt.repos { + // Set this value to avoid duplication in tests + repo.Status.DockerImageRepository = repo.DockerImageRepository repoBytes, _ := latest.Codec.Encode(repo) fakeClient.WatchResponse <- &etcd.Response{ Action: "set", @@ -822,7 +832,7 @@ func TestEtcdWatchImageRepositories(t *testing.T) { t.Errorf("Expected %v, got %v", e, a) } if e, a := repo, event.Object; !reflect.DeepEqual(e, a) { - t.Errorf("Expected %v, got %v", e, a) + t.Errorf("Expected %#v, got %#v", e, a) } case <-time.After(50 * time.Millisecond): if tt.expected[testIndex] { diff --git a/pkg/image/registry/imagerepository/rest.go b/pkg/image/registry/imagerepository/rest.go index b3dee36c591a..9f07178f4ca4 100644 --- a/pkg/image/registry/imagerepository/rest.go +++ b/pkg/image/registry/imagerepository/rest.go @@ -16,17 +16,13 @@ import ( // REST implements the RESTStorage interface in terms of an Registry. type REST struct { - registry Registry - defaultRegistry string + registry Registry } -// NewREST returns a new REST. Default registry is the prefix that will be -// applied to the Status.DockerImageRepository field if the repository does not -// have a real DockerImageRepository. -func NewREST(registry Registry, defaultRegistry string) apiserver.RESTStorage { +// NewREST returns a new REST. +func NewREST(registry Registry) apiserver.RESTStorage { return &REST{ - registry: registry, - defaultRegistry: defaultRegistry, + registry: registry, } } @@ -41,24 +37,12 @@ func (*REST) NewList() runtime.Object { // List retrieves a list of ImageRepositories that match selector. func (s *REST) List(ctx kapi.Context, selector, fields labels.Selector) (runtime.Object, error) { - imageRepositories, err := s.registry.ListImageRepositories(ctx, selector) - if err != nil { - return nil, err - } - for i := range imageRepositories.Items { - s.fillRepository(&imageRepositories.Items[i]) - } - return imageRepositories, err + return s.registry.ListImageRepositories(ctx, selector) } // Get retrieves an ImageRepository by id. func (s *REST) Get(ctx kapi.Context, id string) (runtime.Object, error) { - repo, err := s.registry.GetImageRepository(ctx, id) - if err != nil { - return nil, err - } - s.fillRepository(repo) - return repo, nil + return s.registry.GetImageRepository(ctx, id) } // Watch begins watching for new, changed, or deleted ImageRepositories. @@ -114,14 +98,3 @@ func (s *REST) Delete(ctx kapi.Context, id string) (<-chan apiserver.RESTResult, return &kapi.Status{Status: kapi.StatusSuccess}, s.registry.DeleteImageRepository(ctx, id) }), nil } - -// fillRepository sets the status information of a repository -func (s *REST) fillRepository(repo *api.ImageRepository) { - var value string - if len(repo.DockerImageRepository) != 0 { - value = repo.DockerImageRepository - } else { - value = api.JoinDockerPullSpec(s.defaultRegistry, repo.Namespace, repo.Name, "") - } - repo.Status.DockerImageRepository = value -} diff --git a/pkg/image/registry/imagerepository/rest_test.go b/pkg/image/registry/imagerepository/rest_test.go index 500f1ef74955..5540525c6215 100644 --- a/pkg/image/registry/imagerepository/rest_test.go +++ b/pkg/image/registry/imagerepository/rest_test.go @@ -8,7 +8,6 @@ import ( "testing" kapi "github.com/GoogleCloudPlatform/kubernetes/pkg/api" - "github.com/GoogleCloudPlatform/kubernetes/pkg/apiserver" kclient "github.com/GoogleCloudPlatform/kubernetes/pkg/client" "github.com/GoogleCloudPlatform/kubernetes/pkg/labels" "github.com/openshift/origin/pkg/image/api" @@ -18,10 +17,12 @@ import ( func TestGetImageRepositoryError(t *testing.T) { mockRepositoryRegistry := test.NewImageRepositoryRegistry() mockRepositoryRegistry.Err = fmt.Errorf("test error") - storage := REST{registry: mockRepositoryRegistry} + storage := REST{ + registry: mockRepositoryRegistry, + } image, err := storage.Get(kapi.NewDefaultContext(), "image1") - if image != nil { + if image != (*api.ImageRepository)(nil) { t.Errorf("Unexpected non-nil image: %#v", image) } if err != mockRepositoryRegistry.Err { @@ -35,7 +36,9 @@ func TestGetImageRepositoryOK(t *testing.T) { ObjectMeta: kapi.ObjectMeta{Name: "foo"}, DockerImageRepository: "openshift/ruby-19-centos", } - storage := REST{registry: mockRepositoryRegistry} + storage := REST{ + registry: mockRepositoryRegistry, + } repo, err := storage.Get(kapi.NewDefaultContext(), "foo") if repo == nil { @@ -62,7 +65,7 @@ func TestListImageRepositoriesError(t *testing.T) { t.Errorf("Expected %#v, Got %#v", mockRepositoryRegistry.Err, err) } - if imageRepositories != nil { + if imageRepositories != (*api.ImageRepositoryList)(nil) { t.Errorf("Unexpected non-nil imageRepositories list: %#v", imageRepositories) } } @@ -122,9 +125,11 @@ func TestListImageRepositoriesPopulatedList(t *testing.T) { func TestCreateImageRepositoryOK(t *testing.T) { mockRepositoryRegistry := test.NewImageRepositoryRegistry() - storage := NewREST(mockRepositoryRegistry, "test") + storage := REST{ + registry: mockRepositoryRegistry, + } - channel, err := storage.(apiserver.RESTCreater).Create(kapi.NewDefaultContext(), &api.ImageRepository{ObjectMeta: kapi.ObjectMeta{Name: "foo"}}) + channel, err := storage.Create(kapi.NewDefaultContext(), &api.ImageRepository{ObjectMeta: kapi.ObjectMeta{Name: "foo"}}) if err != nil { t.Fatalf("Unexpected non-nil error: %#v", err) } @@ -143,15 +148,14 @@ func TestCreateImageRepositoryOK(t *testing.T) { if repo.DockerImageRepository != "" { t.Errorf("unexpected repository: %#v", repo) } - if repo.Status.DockerImageRepository != "test/default/foo" { - t.Errorf("unexpected Status values: %#v", repo) - } } func TestCreateRegistryErrorSaving(t *testing.T) { mockRepositoryRegistry := test.NewImageRepositoryRegistry() mockRepositoryRegistry.Err = fmt.Errorf("foo") - storage := REST{registry: mockRepositoryRegistry} + storage := REST{ + registry: mockRepositoryRegistry, + } channel, err := storage.Create(kapi.NewDefaultContext(), &api.ImageRepository{ObjectMeta: kapi.ObjectMeta{Name: "foo"}}) if err != nil { @@ -182,7 +186,9 @@ func TestUpdateImageRepositoryMissingID(t *testing.T) { func TestUpdateRegistryErrorSaving(t *testing.T) { mockRepositoryRegistry := test.NewImageRepositoryRegistry() mockRepositoryRegistry.Err = fmt.Errorf("foo") - storage := REST{registry: mockRepositoryRegistry} + storage := REST{ + registry: mockRepositoryRegistry, + } channel, err := storage.Update(kapi.NewDefaultContext(), &api.ImageRepository{ ObjectMeta: kapi.ObjectMeta{Name: "bar"}, @@ -202,7 +208,9 @@ func TestUpdateRegistryErrorSaving(t *testing.T) { func TestUpdateImageRepositoryOK(t *testing.T) { mockRepositoryRegistry := test.NewImageRepositoryRegistry() - storage := REST{registry: mockRepositoryRegistry} + storage := REST{ + registry: mockRepositoryRegistry, + } channel, err := storage.Update(kapi.NewDefaultContext(), &api.ImageRepository{ ObjectMeta: kapi.ObjectMeta{Name: "bar"}, @@ -222,7 +230,9 @@ func TestUpdateImageRepositoryOK(t *testing.T) { func TestDeleteImageRepository(t *testing.T) { mockRepositoryRegistry := test.NewImageRepositoryRegistry() - storage := REST{registry: mockRepositoryRegistry} + storage := REST{ + registry: mockRepositoryRegistry, + } channel, err := storage.Delete(kapi.NewDefaultContext(), "foo") if err != nil { @@ -254,7 +264,9 @@ func TestCreateImageRepositoryConflictingNamespace(t *testing.T) { func TestUpdateImageRepositoryConflictingNamespace(t *testing.T) { mockRepositoryRegistry := test.NewImageRepositoryRegistry() - storage := REST{registry: mockRepositoryRegistry} + storage := REST{ + registry: mockRepositoryRegistry, + } channel, err := storage.Update(kapi.WithNamespace(kapi.NewContext(), "legal-name"), &api.ImageRepository{ ObjectMeta: kapi.ObjectMeta{Name: "bar", Namespace: "some-value"}, diff --git a/pkg/image/registry/imagerepositorymapping/rest.go b/pkg/image/registry/imagerepositorymapping/rest.go index d8c9873b3cb2..fce2041303ca 100644 --- a/pkg/image/registry/imagerepositorymapping/rest.go +++ b/pkg/image/registry/imagerepositorymapping/rest.go @@ -87,6 +87,9 @@ func (s *REST) Create(ctx kapi.Context, obj runtime.Object) (<-chan apiserver.RE // findRepositoryForMapping retrieves an ImageRepository whose DockerImageRepository matches dockerRepo. func (s *REST) findRepositoryForMapping(ctx kapi.Context, mapping *api.ImageRepositoryMapping) (*api.ImageRepository, error) { + if len(mapping.Name) > 0 { + return s.imageRepositoryRegistry.GetImageRepository(ctx, mapping.Name) + } if len(mapping.DockerImageRepository) != 0 { //TODO make this more efficient list, err := s.imageRepositoryRegistry.ListImageRepositories(ctx, labels.Everything()) @@ -102,7 +105,7 @@ func (s *REST) findRepositoryForMapping(ctx kapi.Context, mapping *api.ImageRepo errors.NewFieldNotFound("dockerImageRepository", mapping.DockerImageRepository), }) } - return s.imageRepositoryRegistry.GetImageRepository(ctx, mapping.Name) + return nil, errors.NewNotFound("ImageRepository", "") } // Update is not supported. diff --git a/pkg/service/environmentresolvercache.go b/pkg/service/environmentresolvercache.go new file mode 100644 index 000000000000..80c952663f0f --- /dev/null +++ b/pkg/service/environmentresolvercache.go @@ -0,0 +1,142 @@ +package service + +import ( + "fmt" + "os" + "strconv" + "strings" + "sync" + + "github.com/GoogleCloudPlatform/kubernetes/pkg/api" +) + +type ServiceRetriever interface { + Get(name string) (*api.Service, error) +} + +type serviceEntry struct { + host string + port string +} + +type ResolverCacheFunc func(name string) (*api.Service, error) + +type ServiceResolverCache struct { + fill ResolverCacheFunc + cache map[string]serviceEntry + lock sync.RWMutex +} + +func NewServiceResolverCache(fill ResolverCacheFunc) *ServiceResolverCache { + return &ServiceResolverCache{ + cache: make(map[string]serviceEntry), + fill: fill, + } +} + +func (c *ServiceResolverCache) get(name string) (host, port string, ok bool) { + // check + c.lock.RLock() + entry, found := c.cache[name] + c.lock.RUnlock() + if found { + return entry.host, entry.port, true + } + + // fill the cache + c.lock.Lock() + defer c.lock.Unlock() + if entry, found := c.cache[name]; found { + return entry.host, entry.port, true + } + service, err := c.fill(name) + if err != nil { + return + } + host, port, ok = service.Spec.PortalIP, strconv.Itoa(service.Spec.Port), true + c.cache[name] = serviceEntry{ + host: host, + port: port, + } + return +} + +func toServiceName(envName string) string { + return strings.TrimSpace(strings.ToLower(strings.Replace(envName, "_", "-", -1))) +} + +func recognizeVariable(name string) (service string, host bool, ok bool) { + switch { + case strings.HasSuffix(name, "_SERVICE_HOST"): + service = toServiceName(strings.TrimSuffix(name, "_SERVICE_HOST")) + host = true + case strings.HasSuffix(name, "_SERVICE_PORT"): + service = toServiceName(strings.TrimSuffix(name, "_SERVICE_PORT")) + default: + return "", false, false + } + if len(service) == 0 { + return "", false, false + } + ok = true + return +} + +func (c *ServiceResolverCache) resolve(name string) (string, bool) { + service, isHost, ok := recognizeVariable(name) + if !ok { + return "", false + } + host, port, ok := c.get(service) + if !ok { + return "", false + } + if isHost { + return host, true + } + return port, true +} + +// Defer takes a string (with optional variables) and an expansion function and returns +// a function that can be called to get the value. This method will optimize the +// expansion away in the event that no expansion is necessary. +func (c *ServiceResolverCache) Defer(env string) (func() (string, bool), error) { + hasExpansion := false + invalid := []string{} + os.Expand(env, func(name string) string { + hasExpansion = true + if _, _, ok := recognizeVariable(name); !ok { + invalid = append(invalid, name) + } + return "" + }) + if len(invalid) != 0 { + return nil, fmt.Errorf("invalid variable name(s): %s", strings.Join(invalid, ", ")) + } + if !hasExpansion { + return func() (string, bool) { return env, true }, nil + } + + // only load the value once + lock := sync.Mutex{} + loaded := false + return func() (string, bool) { + lock.Lock() + defer lock.Unlock() + if loaded { + return env, true + } + resolved := true + expand := os.Expand(env, func(s string) string { + s, ok := c.resolve(s) + resolved = resolved && ok + return s + }) + if !resolved { + return "", false + } + loaded = true + env = expand + return env, true + }, nil +} diff --git a/pkg/service/environmentresolvercache_test.go b/pkg/service/environmentresolvercache_test.go new file mode 100644 index 000000000000..550976259ccf --- /dev/null +++ b/pkg/service/environmentresolvercache_test.go @@ -0,0 +1,83 @@ +package service + +import ( + "testing" + + "github.com/GoogleCloudPlatform/kubernetes/pkg/api" + "github.com/GoogleCloudPlatform/kubernetes/pkg/api/errors" + "github.com/GoogleCloudPlatform/kubernetes/pkg/client" +) + +func TestServiceResolverCacheEmpty(t *testing.T) { + fakeClient := &client.Fake{} + cache := NewServiceResolverCache(fakeClient.Services("default").Get) + if v, ok := cache.resolve("FOO_SERVICE_HOST"); v != "" || !ok { + t.Errorf("unexpected cache item") + } + if len(fakeClient.Actions) != 1 { + t.Errorf("unexpected client actions: %#v", fakeClient.Actions) + } + cache.resolve("FOO_SERVICE_HOST") + if len(fakeClient.Actions) != 1 { + t.Errorf("unexpected cache miss: %#v", fakeClient.Actions) + } + cache.resolve("FOO_SERVICE_PORT") + if len(fakeClient.Actions) != 1 { + t.Errorf("unexpected cache miss: %#v", fakeClient.Actions) + } +} + +type fakeRetriever struct { + service *api.Service + err error +} + +func (r fakeRetriever) Get(name string) (*api.Service, error) { + return r.service, r.err +} + +func TestServiceResolverCache(t *testing.T) { + c := fakeRetriever{ + err: errors.NewNotFound("Service", "bar"), + } + cache := NewServiceResolverCache(c.Get) + if v, ok := cache.resolve("FOO_SERVICE_HOST"); v != "" || ok { + t.Errorf("unexpected cache item") + } + + c = fakeRetriever{ + service: &api.Service{ + Spec: api.ServiceSpec{ + PortalIP: "127.0.0.1", + Port: 80, + }, + }, + } + cache = NewServiceResolverCache(c.Get) + if v, ok := cache.resolve("FOO_SERVICE_HOST"); v != "127.0.0.1" || !ok { + t.Errorf("unexpected cache item") + } + if v, ok := cache.resolve("FOO_SERVICE_PORT"); v != "80" || !ok { + t.Errorf("unexpected cache item") + } + if _, err := cache.Defer("${UNKNOWN}"); err == nil { + t.Errorf("unexpected non-error") + } + fn, err := cache.Defer("test") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if v, ok := fn(); v != "test" || !ok { + t.Errorf("unexpected cache item") + } + fn, err = cache.Defer("${FOO_SERVICE_HOST}") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if v, ok := fn(); v != "127.0.0.1" || !ok { + t.Errorf("unexpected cache item") + } + if v, ok := fn(); v != "127.0.0.1" || !ok { + t.Errorf("unexpected cache item") + } +} diff --git a/test/integration/auth_proxy_test.go b/test/integration/auth_proxy_test.go index c2469b91440b..7be46ffdbf03 100644 --- a/test/integration/auth_proxy_test.go +++ b/test/integration/auth_proxy_test.go @@ -9,8 +9,6 @@ import ( "net/url" "testing" - "github.com/golang/glog" - kapi "github.com/GoogleCloudPlatform/kubernetes/pkg/api" klatest "github.com/GoogleCloudPlatform/kubernetes/pkg/api/latest" kclient "github.com/GoogleCloudPlatform/kubernetes/pkg/client" @@ -86,15 +84,17 @@ func TestFrontProxyOnAuthorize(t *testing.T) { mux := http.NewServeMux() server.Install(mux, origin.OpenShiftOAuthAPIPrefix) oauthServer := httptest.NewServer(http.Handler(mux)) - glog.Infof("oauth server is on %v\n", oauthServer.URL) + defer oauthServer.Close() + t.Logf("oauth server is on %v\n", oauthServer.URL) // set up a front proxy guarding the oauth server proxyHTTPHandler := NewBasicAuthChallenger("TestRegistryAndServer", validUsers, NewXRemoteUserProxyingHandler(oauthServer.URL)) proxyServer := httptest.NewServer(proxyHTTPHandler) - glog.Infof("proxy server is on %v\n", proxyServer.URL) + defer proxyServer.Close() + t.Logf("proxy server is on %v\n", proxyServer.URL) // need to prime clients so that we can get back a code. the client must be valid - createClient(oauthEtcd, &oauthapi.Client{ObjectMeta: kapi.ObjectMeta{Name: "test"}, Secret: "secret", RedirectURIs: []string{oauthServer.URL}}) + createClient(t, oauthEtcd, &oauthapi.Client{ObjectMeta: kapi.ObjectMeta{Name: "test"}, Secret: "secret", RedirectURIs: []string{oauthServer.URL}}) // our simple URL to get back a code. We want to go through the front proxy rawAuthorizeRequest := proxyServer.URL + origin.OpenShiftOAuthAPIPrefix + "/authorize?response_type=code&client_id=test" @@ -114,7 +114,7 @@ func TestFrontProxyOnAuthorize(t *testing.T) { // and manually handling redirects and setting our auth information every time for the front proxy redirectedUrls := make([]url.URL, 10) httpClient := http.Client{ - CheckRedirect: getRedirectMethod(&redirectedUrls), + CheckRedirect: getRedirectMethod(t, &redirectedUrls), Transport: kclient.NewBasicAuthRoundTripper("sanefarmer", "who?", http.DefaultTransport), } @@ -134,21 +134,21 @@ func TestFrontProxyOnAuthorize(t *testing.T) { if len(foundCode) == 0 { t.Errorf("Did not find code in any redirect: %v", redirectedUrls) } else { - glog.Infof("Found code %v\n", foundCode) + t.Logf("Found code %v\n", foundCode) } } -func createClient(oauthEtcd oauthclient.Registry, client *oauthapi.Client) { +func createClient(t *testing.T, oauthEtcd oauthclient.Registry, client *oauthapi.Client) { if err := oauthEtcd.CreateClient(client); err != nil { - glog.Errorf("Error creating client: %v due to %v\n", client, err) + t.Errorf("Error creating client: %v due to %v\n", client, err) } } type checkRedirect func(req *http.Request, via []*http.Request) error -func getRedirectMethod(redirectRecord *[]url.URL) checkRedirect { +func getRedirectMethod(t *testing.T, redirectRecord *[]url.URL) checkRedirect { return func(req *http.Request, via []*http.Request) error { - glog.Infof("Going to %v\n", req.URL) + t.Logf("Going to %v\n", req.URL) *redirectRecord = append(*redirectRecord, *req.URL) if len(via) >= 10 { diff --git a/test/integration/basic_auth_test.go b/test/integration/basic_auth_test.go index c66600349b30..f1eb2fc0422a 100644 --- a/test/integration/basic_auth_test.go +++ b/test/integration/basic_auth_test.go @@ -3,13 +3,11 @@ package integration import ( "log" "net/http" - - "github.com/golang/glog" ) func ExampleNewBasicAuthChallenger() { challenger := NewBasicAuthChallenger("realm", []User{{"username", "password", "Brave Butcher", "cowardly_butcher@example.org"}}, NewIdentifyingHandler()) http.Handle("/", challenger) - glog.Infoln("Auth server listening on http://localhost:1234") + log.Printf("Auth server listening on http://localhost:1234") log.Fatal(http.ListenAndServe(":1234", nil)) } diff --git a/test/integration/buildcfgclient_test.go b/test/integration/buildcfgclient_test.go index c44da1fa97ba..665e2e32c5b9 100644 --- a/test/integration/buildcfgclient_test.go +++ b/test/integration/buildcfgclient_test.go @@ -18,6 +18,7 @@ func init() { func TestListBuildConfigs(t *testing.T) { deleteAllEtcdKeys() openshift := NewTestBuildOpenshift(t) + defer openshift.Close() buildConfigs, err := openshift.Client.BuildConfigs(testNamespace).List(labels.Everything(), labels.Everything()) if err != nil { @@ -31,6 +32,7 @@ func TestListBuildConfigs(t *testing.T) { func TestCreateBuildConfig(t *testing.T) { deleteAllEtcdKeys() openshift := NewTestBuildOpenshift(t) + defer openshift.Close() buildConfig := mockBuildConfig() expected, err := openshift.Client.BuildConfigs(testNamespace).Create(buildConfig) @@ -53,6 +55,7 @@ func TestCreateBuildConfig(t *testing.T) { func TestUpdateBuildConfig(t *testing.T) { deleteAllEtcdKeys() openshift := NewTestBuildOpenshift(t) + defer openshift.Close() buildConfig := mockBuildConfig() actual, err := openshift.Client.BuildConfigs(testNamespace).Create(buildConfig) @@ -71,6 +74,7 @@ func TestUpdateBuildConfig(t *testing.T) { func TestDeleteBuildConfig(t *testing.T) { deleteAllEtcdKeys() openshift := NewTestBuildOpenshift(t) + defer openshift.Close() buildConfig := mockBuildConfig() actual, err := openshift.Client.BuildConfigs(testNamespace).Create(buildConfig) @@ -85,12 +89,14 @@ func TestDeleteBuildConfig(t *testing.T) { func TestWatchBuildConfigs(t *testing.T) { deleteAllEtcdKeys() openshift := NewTestBuildOpenshift(t) + defer openshift.Close() buildConfig := mockBuildConfig() watch, err := openshift.Client.BuildConfigs(testNamespace).Watch(labels.Everything(), labels.Everything(), "0") if err != nil { t.Fatalf("Unexpected error: %v", err) } + defer watch.Stop() expected, err := openshift.Client.BuildConfigs(testNamespace).Create(buildConfig) if err != nil { @@ -135,7 +141,7 @@ func mockBuildConfig() *buildapi.BuildConfig { }, }, Output: buildapi.BuildOutput{ - ImageTag: "namespace/builtimage", + DockerImageReference: "namespace/builtimage", }, }, } @@ -144,6 +150,7 @@ func mockBuildConfig() *buildapi.BuildConfig { func TestBuildConfigClient(t *testing.T) { deleteAllEtcdKeys() openshift := NewTestBuildOpenshift(t) + defer openshift.Close() buildConfigs, err := openshift.Client.BuildConfigs(testNamespace).List(labels.Everything(), labels.Everything()) if err != nil { @@ -175,7 +182,7 @@ func TestBuildConfigClient(t *testing.T) { }, }, Output: buildapi.BuildOutput{ - ImageTag: "namespace/builtimage", + DockerImageReference: "namespace/builtimage", }, }, } diff --git a/test/integration/buildclient_test.go b/test/integration/buildclient_test.go index aef04111be3b..c70e72f8818b 100644 --- a/test/integration/buildclient_test.go +++ b/test/integration/buildclient_test.go @@ -5,6 +5,7 @@ package integration import ( "net/http" "net/http/httptest" + "sync" "testing" kapi "github.com/GoogleCloudPlatform/kubernetes/pkg/api" @@ -15,7 +16,6 @@ import ( "github.com/GoogleCloudPlatform/kubernetes/pkg/labels" "github.com/GoogleCloudPlatform/kubernetes/pkg/master" "github.com/GoogleCloudPlatform/kubernetes/plugin/pkg/admission/admit" - "github.com/golang/glog" "github.com/openshift/origin/pkg/api/latest" buildapi "github.com/openshift/origin/pkg/build/api" @@ -36,6 +36,7 @@ func init() { func TestListBuilds(t *testing.T) { deleteAllEtcdKeys() openshift := NewTestBuildOpenshift(t) + defer openshift.Close() builds, err := openshift.Client.Builds(testNamespace).List(labels.Everything(), labels.Everything()) if err != nil { @@ -49,6 +50,7 @@ func TestListBuilds(t *testing.T) { func TestCreateBuild(t *testing.T) { deleteAllEtcdKeys() openshift := NewTestBuildOpenshift(t) + defer openshift.Close() build := mockBuild() expected, err := openshift.Client.Builds(testNamespace).Create(build) @@ -71,6 +73,7 @@ func TestCreateBuild(t *testing.T) { func TestDeleteBuild(t *testing.T) { deleteAllEtcdKeys() openshift := NewTestBuildOpenshift(t) + defer openshift.Close() build := mockBuild() actual, err := openshift.Client.Builds(testNamespace).Create(build) @@ -85,12 +88,14 @@ func TestDeleteBuild(t *testing.T) { func TestWatchBuilds(t *testing.T) { deleteAllEtcdKeys() openshift := NewTestBuildOpenshift(t) + defer openshift.Close() build := mockBuild() watch, err := openshift.Client.Builds(testNamespace).Watch(labels.Everything(), labels.Everything(), "0") if err != nil { t.Fatalf("Unexpected error: %v", err) } + defer watch.Stop() expected, err := openshift.Client.Builds(testNamespace).Create(build) if err != nil { @@ -127,7 +132,7 @@ func mockBuild() *buildapi.Build { }, }, Output: buildapi.BuildOutput{ - ImageTag: "namespace/builtimage", + DockerImageReference: "namespace/builtimage", }, }, } @@ -137,11 +142,17 @@ type testBuildOpenshift struct { Client *osclient.Client server *httptest.Server whPrefix string + stop chan struct{} + lock sync.Mutex } func NewTestBuildOpenshift(t *testing.T) *testBuildOpenshift { - openshift := &testBuildOpenshift{} + openshift := &testBuildOpenshift{ + stop: make(chan struct{}), + } + openshift.lock.Lock() + defer openshift.lock.Unlock() etcdClient := newEtcdClient() etcdHelper, _ := master.NewEtcdHelper(etcdClient, klatest.Version) @@ -155,7 +166,7 @@ func NewTestBuildOpenshift(t *testing.T) *testBuildOpenshift { kubeletClient, err := kclient.NewKubeletClient(&kclient.KubeletConfig{Port: 10250}) if err != nil { - glog.Fatalf("Unable to configure Kubelet client: %v", err) + t.Fatalf("Unable to configure Kubelet client: %v", err) } kmaster := master.New(&master.Config{ @@ -200,6 +211,7 @@ func NewTestBuildOpenshift(t *testing.T) *testBuildOpenshift { TempDirectoryCreator: buildstrategy.STITempDirectoryCreator, UseLocalImages: false, }, + Stop: openshift.stop, } factory.Create().Run() @@ -207,6 +219,14 @@ func NewTestBuildOpenshift(t *testing.T) *testBuildOpenshift { return openshift } +func (t *testBuildOpenshift) Close() { + t.lock.Lock() + defer t.lock.Unlock() + close(t.stop) + t.server.CloseClientConnections() + t.server.Close() +} + type clientWebhookInterface struct { Client osclient.Interface } diff --git a/test/integration/cli_get_token_test.go b/test/integration/cli_get_token_test.go index ca4ecea53168..d716c0d88eaf 100644 --- a/test/integration/cli_get_token_test.go +++ b/test/integration/cli_get_token_test.go @@ -9,7 +9,6 @@ import ( "strings" "testing" - "github.com/golang/glog" "github.com/spf13/pflag" klatest "github.com/GoogleCloudPlatform/kubernetes/pkg/api/latest" @@ -80,7 +79,8 @@ func TestGetToken(t *testing.T) { mux := http.NewServeMux() server.Install(mux, origin.OpenShiftOAuthAPIPrefix) oauthServer := httptest.NewServer(http.Handler(mux)) - glog.Infof("oauth server is on %v\n", oauthServer.URL) + defer oauthServer.Close() + t.Logf("oauth server is on %v\n", oauthServer.URL) // create the default oauth clients with redirects to our server origin.CreateOrUpdateDefaultOAuthClients(oauthServer.URL, oauthEtcd) diff --git a/test/integration/deploy_trigger_test.go b/test/integration/deploy_trigger_test.go index d656f69f20cb..9ce975d95cbc 100644 --- a/test/integration/deploy_trigger_test.go +++ b/test/integration/deploy_trigger_test.go @@ -3,11 +3,12 @@ package integration import ( - "flag" "net/http" "net/http/httptest" "testing" + "github.com/golang/glog" + kapi "github.com/GoogleCloudPlatform/kubernetes/pkg/api" klatest "github.com/GoogleCloudPlatform/kubernetes/pkg/api/latest" "github.com/GoogleCloudPlatform/kubernetes/pkg/apiserver" @@ -51,22 +52,31 @@ func TestSuccessfulManualDeployment(t *testing.T) { config := manualDeploymentConfig() var err error - watch, err := openshift.KubeClient.ReplicationControllers(testNamespace).Watch(labels.Everything(), labels.Everything(), "0") + dc, err := openshift.Client.DeploymentConfigs(testNamespace).Create(config) if err != nil { - t.Fatalf("Couldn't subscribe to Deployments: %v", err) + t.Fatalf("Couldn't create DeploymentConfig: %v %#v", err, config) } - if _, err := openshift.Client.DeploymentConfigs(testNamespace).Create(config); err != nil { - t.Fatalf("Couldn't create DeploymentConfig: %v %#v", err, config) + watch, err := openshift.KubeClient.ReplicationControllers(testNamespace).Watch(labels.Everything(), labels.Everything(), dc.ResourceVersion) + if err != nil { + t.Fatalf("Couldn't subscribe to Deployments: %v", err) } + defer watch.Stop() - if config, err = openshift.Client.DeploymentConfigs(testNamespace).Generate(config.Name); err != nil { + config, err = openshift.Client.DeploymentConfigs(testNamespace).Generate(config.Name) + if err != nil { t.Fatalf("Error generating config: %v", err) } + if config.LatestVersion != 1 { + t.Fatalf("Generated deployment should have version 1: %#v", config) + } + glog.Infof("config(1): %#v", config) - if _, err := openshift.Client.DeploymentConfigs(testNamespace).Update(config); err != nil { + new, err := openshift.Client.DeploymentConfigs(testNamespace).Update(config) + if err != nil { t.Fatalf("Couldn't create updated DeploymentConfig: %v %#v", err, config) } + glog.Infof("config(2): %#v", new) event := <-watch.ResultChan() if e, a := watchapi.Added, event.Type; e != a { @@ -77,6 +87,9 @@ func TestSuccessfulManualDeployment(t *testing.T) { if e, a := config.Name, deployment.Annotations[deployapi.DeploymentConfigAnnotation]; e != a { t.Fatalf("Expected deployment annotated with deploymentConfig '%s', got '%s'", e, a) } + if e, a := "1", deployment.Annotations[deployapi.DeploymentVersionAnnotation]; e != a { + t.Fatalf("Deployment annotation version does not match: %#v", deployment) + } } func TestSimpleImageChangeTrigger(t *testing.T) { @@ -99,6 +112,75 @@ func TestSimpleImageChangeTrigger(t *testing.T) { if err != nil { t.Fatalf("Couldn't subscribe to Deployments %v", err) } + defer watch.Stop() + + if imageRepo, err = openshift.Client.ImageRepositories(testNamespace).Create(imageRepo); err != nil { + t.Fatalf("Couldn't create ImageRepository: %v", err) + } + + if _, err := openshift.Client.DeploymentConfigs(testNamespace).Create(config); err != nil { + t.Fatalf("Couldn't create DeploymentConfig: %v", err) + } + + if config, err = openshift.Client.DeploymentConfigs(testNamespace).Generate(config.Name); err != nil { + t.Fatalf("Error generating config: %v", err) + } + + if _, err := openshift.Client.DeploymentConfigs(testNamespace).Update(config); err != nil { + t.Fatalf("Couldn't create updated DeploymentConfig: %v", err) + } + + event := <-watch.ResultChan() + if e, a := watchapi.Added, event.Type; e != a { + t.Fatalf("expected watch event type %s, got %s", e, a) + } + deployment := event.Object.(*kapi.ReplicationController) + + if e, a := config.Name, deployment.Annotations[deployapi.DeploymentConfigAnnotation]; e != a { + t.Fatalf("Expected deployment annotated with deploymentConfig '%s', got '%s'", e, a) + } + + imageRepo.Tags["latest"] = "ref-2" + + if _, err = openshift.Client.ImageRepositories(testNamespace).Update(imageRepo); err != nil { + t.Fatalf("Error updating imageRepo: %v", err) + } + + event = <-watch.ResultChan() + if e, a := watchapi.Added, event.Type; e != a { + t.Fatalf("expected watch event type %s, got %s", e, a) + } + newDeployment := event.Object.(*kapi.ReplicationController) + + if newDeployment.Name == deployment.Name { + t.Fatalf("expected new deployment; old=%s, new=%s", deployment.Name, newDeployment.Name) + } +} + +func TestSimpleImageChangeTriggerFrom(t *testing.T) { + deleteAllEtcdKeys() + openshift := NewTestOpenshift(t) + defer openshift.Close() + + imageRepo := &imageapi.ImageRepository{ + ObjectMeta: kapi.ObjectMeta{Name: "test-image-repo"}, + Tags: map[string]string{ + "latest": "ref-1", + }, + } + + config := imageChangeDeploymentConfig() + config.Triggers[0].ImageChangeParams.RepositoryName = "" + config.Triggers[0].ImageChangeParams.From = kapi.ObjectReference{ + Name: "test-image-repo", + } + var err error + + watch, err := openshift.KubeClient.ReplicationControllers(testNamespace).Watch(labels.Everything(), labels.Everything(), "0") + if err != nil { + t.Fatalf("Couldn't subscribe to Deployments %v", err) + } + defer watch.Stop() if imageRepo, err = openshift.Client.ImageRepositories(testNamespace).Create(imageRepo); err != nil { t.Fatalf("Couldn't create ImageRepository: %v", err) @@ -141,6 +223,9 @@ func TestSimpleImageChangeTrigger(t *testing.T) { if newDeployment.Name == deployment.Name { t.Fatalf("expected new deployment; old=%s, new=%s", deployment.Name, newDeployment.Name) } + if a, e := newDeployment.Spec.Template.Spec.Containers[0].Image, "registry:3000/integration-test/test-image-repo:ref-2"; e != a { + t.Fatalf("new deployment isn't pointing to the right image: %s %s", e, a) + } } func TestSimpleConfigChangeTrigger(t *testing.T) { @@ -155,6 +240,7 @@ func TestSimpleConfigChangeTrigger(t *testing.T) { if err != nil { t.Fatalf("Couldn't subscribe to Deployments %v", err) } + defer watch.Stop() // submit the initial deployment config if _, err := openshift.Client.DeploymentConfigs(testNamespace).Create(config); err != nil { @@ -228,7 +314,6 @@ type testOpenshift struct { } func NewTestOpenshift(t *testing.T) *testOpenshift { - flag.Set("v", "4") t.Logf("Starting test openshift") openshift := &testOpenshift{ @@ -263,20 +348,22 @@ func NewTestOpenshift(t *testing.T) *testOpenshift { interfaces, _ := latest.InterfacesFor(latest.Version) - imageEtcd := imageetcd.New(etcdHelper) + imageEtcd := imageetcd.New(etcdHelper, imageetcd.DefaultRegistryFunc(func() (string, bool) { return "registry:3000", true })) deployEtcd := deployetcd.New(etcdHelper) deployConfigGenerator := &deployconfiggenerator.DeploymentConfigGenerator{ - Codec: latest.Codec, - DeploymentInterface: &clientDeploymentInterface{kubeClient}, - DeploymentConfigInterface: deployEtcd, - ImageRepositoryInterface: imageEtcd, + Client: deployconfiggenerator.Client{ + DCFn: deployEtcd.GetDeploymentConfig, + IRFn: imageEtcd.GetImageRepository, + LIRFn2: imageEtcd.ListImageRepositories, + }, + Codec: latest.Codec, } buildEtcd := buildetcd.New(etcdHelper) storage := map[string]apiserver.RESTStorage{ "images": image.NewREST(imageEtcd), - "imageRepositories": imagerepository.NewREST(imageEtcd, ""), + "imageRepositories": imagerepository.NewREST(imageEtcd), "imageRepositoryMappings": imagerepositorymapping.NewREST(imageEtcd, imageEtcd), "deployments": deployregistry.NewREST(deployEtcd), "deploymentConfigs": deployconfigregistry.NewREST(deployEtcd), @@ -333,6 +420,7 @@ func NewTestOpenshift(t *testing.T) *testOpenshift { TempDirectoryCreator: buildstrategy.STITempDirectoryCreator, UseLocalImages: false, }, + Stop: openshift.stop, } bcFactory.Create().Run() diff --git a/test/integration/fixtures/test-image-repository.json b/test/integration/fixtures/test-image-repository.json index 62c257129f88..0755d177cc3f 100644 --- a/test/integration/fixtures/test-image-repository.json +++ b/test/integration/fixtures/test-image-repository.json @@ -7,7 +7,6 @@ "color": "blue" } }, - "dockerImageRepository": "openshift/ruby-19-centos", "tags": { "latest": "foo", "another": "bar", diff --git a/test/integration/fixtures/test-mapping.json b/test/integration/fixtures/test-mapping.json index d13158682abb..ae0327709fcf 100644 --- a/test/integration/fixtures/test-mapping.json +++ b/test/integration/fixtures/test-mapping.json @@ -1,7 +1,9 @@ { + "metadata": { + "name": "test" + }, "kind": "ImageRepositoryMapping", "apiVersion": "v1beta1", - "dockerImageRepository": "openshift/ruby-19-centos", "image": { "kind": "Image", "version": "v1beta1", @@ -9,7 +11,7 @@ "name": "abcd1234" }, "dockerImageReference": "openshift/ruby-19-centos:latest", - "meta": { + "dockerImageMetadata": { "Architecture": "amd64", "Author": "Michal Fojtik \u003cmfojtik@redhat.com\u003e", "Comment": "", diff --git a/test/integration/imagechange_buildtrigger_test.go b/test/integration/imagechange_buildtrigger_test.go index 5a01d5d4c641..bbf94e204a98 100644 --- a/test/integration/imagechange_buildtrigger_test.go +++ b/test/integration/imagechange_buildtrigger_test.go @@ -84,7 +84,7 @@ func TestSimpleImageChangeBuildTrigger(t *testing.T) { func imageChangeBuildConfig() *buildapi.BuildConfig { buildcfg := &buildapi.BuildConfig{ ObjectMeta: kapi.ObjectMeta{ - Name: "testBuildCfg", + Name: "test-build-cfg", }, Parameters: buildapi.BuildParameters{ Source: buildapi.BuildSource{ @@ -101,7 +101,7 @@ func imageChangeBuildConfig() *buildapi.BuildConfig { }, }, Output: buildapi.BuildOutput{ - ImageTag: "tag", + DockerImageReference: "foo:tag", }, }, Triggers: []buildapi.BuildTriggerPolicy{ @@ -109,7 +109,7 @@ func imageChangeBuildConfig() *buildapi.BuildConfig { Type: buildapi.ImageChangeBuildTriggerType, ImageChange: &buildapi.ImageChangeTrigger{ Image: "registry:8080/openshift/test-image", - ImageRepositoryRef: &kapi.ObjectReference{ + From: kapi.ObjectReference{ Name: "test-image-repo", }, Tag: "latest", diff --git a/test/integration/imageclient_test.go b/test/integration/imageclient_test.go index e9bee8fd1701..1b874e0a51b6 100644 --- a/test/integration/imageclient_test.go +++ b/test/integration/imageclient_test.go @@ -18,7 +18,6 @@ import ( "github.com/GoogleCloudPlatform/kubernetes/pkg/master" "github.com/GoogleCloudPlatform/kubernetes/pkg/util" "github.com/GoogleCloudPlatform/kubernetes/plugin/pkg/admission/admit" - "github.com/golang/glog" "github.com/openshift/origin/pkg/api/latest" osclient "github.com/openshift/origin/pkg/client" @@ -193,15 +192,19 @@ type testImageOpenshift struct { Client *osclient.Client server *httptest.Server dockerServer *httptest.Server + stop chan struct{} } func (o *testImageOpenshift) Close() { + close(o.stop) o.server.Close() o.dockerServer.Close() } func NewTestImageOpenShift(t *testing.T) *testImageOpenshift { - openshift := &testImageOpenshift{} + openshift := &testImageOpenshift{ + stop: make(chan struct{}), + } etcdClient := newEtcdClient() etcdHelper, _ := master.NewEtcdHelper(etcdClient, klatest.Version) @@ -219,7 +222,7 @@ func NewTestImageOpenShift(t *testing.T) *testImageOpenshift { kubeletClient, err := kclient.NewKubeletClient(&kclient.KubeletConfig{Port: 10250}) if err != nil { - glog.Fatalf("Unable to configure Kubelet client: %v", err) + t.Fatalf("Unable to configure Kubelet client: %v", err) } kmaster := master.New(&master.Config{ @@ -232,11 +235,11 @@ func NewTestImageOpenShift(t *testing.T) *testImageOpenshift { interfaces, _ := latest.InterfacesFor(latest.Version) - imageEtcd := imageetcd.New(etcdHelper) + imageEtcd := imageetcd.New(etcdHelper, imageetcd.DefaultRegistryFunc(func() (string, bool) { return openshift.dockerServer.URL, true })) storage := map[string]apiserver.RESTStorage{ "images": image.NewREST(imageEtcd), - "imageRepositories": imagerepository.NewREST(imageEtcd, openshift.dockerServer.URL), + "imageRepositories": imagerepository.NewREST(imageEtcd), "imageRepositoryMappings": imagerepositorymapping.NewREST(imageEtcd, imageEtcd), "imageRepositoryTags": imagerepositorytag.NewREST(imageEtcd, imageEtcd), } diff --git a/test/integration/oauthstorage_test.go b/test/integration/oauthstorage_test.go index 887959f4e01a..25b1aa017041 100644 --- a/test/integration/oauthstorage_test.go +++ b/test/integration/oauthstorage_test.go @@ -80,6 +80,7 @@ func TestOAuthStorage(t *testing.T) { mux := http.NewServeMux() oauthServer.Install(mux, "") server := httptest.NewServer(mux) + defer server.Close() ch := make(chan *osincli.AccessData, 1) var oaclient *osincli.Client diff --git a/test/integration/userclient_test.go b/test/integration/userclient_test.go index 32ac9eb92f04..cfed45470edd 100644 --- a/test/integration/userclient_test.go +++ b/test/integration/userclient_test.go @@ -45,6 +45,7 @@ func TestUserInitialization(t *testing.T) { } server := httptest.NewServer(apiserver.Handle(storage, v1beta1.Codec, "/osapi", "v1beta1", interfaces.MetadataAccessor, admit.NewAlwaysAdmit())) + defer server.Close() mapping := api.UserIdentityMapping{ Identity: api.Identity{ diff --git a/test/integration/utils.go b/test/integration/utils.go index 25112100626c..0c49c8ac65ba 100644 --- a/test/integration/utils.go +++ b/test/integration/utils.go @@ -3,12 +3,16 @@ package integration import ( + "flag" "fmt" + "log" "math/rand" "os" "github.com/coreos/go-etcd/etcd" "github.com/golang/glog" + + "github.com/GoogleCloudPlatform/kubernetes/pkg/capabilities" ) const testNamespace = "integration-test" @@ -24,6 +28,17 @@ func newEtcdClient() *etcd.Client { return etcd.NewClient(etcdServers) } +func init() { + capabilities.SetForTests(capabilities.Capabilities{ + AllowPrivileged: true, + }) + flag.Set("v", "5") +} + +func logEtcd() { + etcd.SetLogger(log.New(os.Stderr, "go-etcd", log.LstdFlags)) +} + func requireEtcd() { if _, err := newEtcdClient().Get("/", false, false); err != nil { glog.Fatalf("unable to connect to etcd for integration testing: %v", err) diff --git a/test/integration/webhookgithub_test.go b/test/integration/webhookgithub_test.go index d0bba3e8c638..8460cdf274c8 100644 --- a/test/integration/webhookgithub_test.go +++ b/test/integration/webhookgithub_test.go @@ -22,6 +22,7 @@ func init() { func TestWebhookGithubPush(t *testing.T) { deleteAllEtcdKeys() openshift := NewTestBuildOpenshift(t) + defer openshift.Close() // create buildconfig buildConfig := &buildapi.BuildConfig{ @@ -50,7 +51,7 @@ func TestWebhookGithubPush(t *testing.T) { }, }, Output: buildapi.BuildOutput{ - ImageTag: "namespace/builtimage", + DockerImageReference: "namespace/builtimage", }, }, } @@ -78,6 +79,7 @@ func TestWebhookGithubPush(t *testing.T) { func TestWebhookGithubPing(t *testing.T) { deleteAllEtcdKeys() openshift := NewTestBuildOpenshift(t) + defer openshift.Close() // create buildconfig buildConfig := &buildapi.BuildConfig{ @@ -106,7 +108,7 @@ func TestWebhookGithubPing(t *testing.T) { }, }, Output: buildapi.BuildOutput{ - ImageTag: "namespace/builtimage", + DockerImageReference: "namespace/builtimage", }, }, }