Fail fast when non-existent URL passed to docker build

[soltysh]

This PR is to address issue talked in #115
@smarterclayton @csrwng @mfojtik lemme know what do you think?

The other thing is, I'd like to extend our integration tests a little bit, as it was talked some time ago on IRC with possibility to test e.g. this as well. I'll working on it as well, unless told otherwise.

[ironcladlou]

You need to use something like --max-time here, otherwise you've just shifted the timeout issues to curl.

[mfojtik]

I think we also need to be able to handle redirect (-L option) and deal with invalid SSL certificates (?). Curl provides many useful options for retries/timeouts, worth to explore them as Dan mentioned.

[smarterclayton]

How much more complex does this script get before it should be written in a real programming language?

----- Original Message -----

@@ -14,6 +14,17 @@ if [ -n "$DOCKER_REGISTRY" ]; then
TAG=$DOCKER_REGISTRY/$BUILD_TAG
fi

+if [[ $DOCKER_CONTEXT_URL != "git://"* ]] && [[ $DOCKER_CONTEXT_URL !=
"git@"* ]]; then

• URL=$DOCKER_CONTEXT_URL
• if [[ $URL != "http://"* ]] && [[ $URL != "https://"* ]]; then
• URL="https://"$URL
• fi
• curl -sf $URL > /dev/null

I think we also need to be able to handle redirect (-L option) and deal
with invalid SSL certificates (?). Curl provides many useful options for
retries/timeouts, worth to explore them as Dan mentioned.

---

Reply to this email directly or view it on GitHub:
https://github.com/openshift/origin/pull/140/files#r17983782

[soltysh]

@smarterclayton I know what you mean, I was thinking the same way when I was writing those bytes of code. But on the other hand if docker w/ git does not give us that flexibility, we need to do it by ourselves. Should I take your proposal as a challenge to create such a thing?

[smarterclayton]

Id like to see a proposal for what that might look like before we invest significant effort into it. We should not however mistake the current code for a general purpose solution that captures all corner cases. Curl and bash is NOT a production quality library when used in this was

On Sep 24, 2014, at 4:29 PM, Maciej Szulik notifications@github.com wrote:

In images/builder/docker/docker-builder/build.sh:

@@ -14,6 +14,17 @@ if [ -n "$DOCKER_REGISTRY" ]; then
TAG=$DOCKER_REGISTRY/$BUILD_TAG
fi

+if [[ $DOCKER_CONTEXT_URL != "git://"* ]] && [[ $DOCKER_CONTEXT_URL != "git@"* ]]; then

• URL=$DOCKER_CONTEXT_URL
• if [[ $URL != "http://"* ]] && [[ $URL != "https://"* ]]; then
• URL="https://"$URL
• fi
• curl -sf $URL > /dev/null
  @smarterclayton I know what you mean, I was thinking the same way when I was writing those bytes of code. But on the other hand if docker w/ git does not give us that flexibility, we need to do it by ourselves. Should I take your proposal as a challenge to create such a thing?

—
Reply to this email directly or view it on GitHub.

[soltysh]

@ironcladlou the problem here was that when git was accessing non-existent repository it though that it might be private (priv repos are 404-ed on github) so it asked for credentials. Plain curl will get 404 which is what I wanted to achieve with it. I've added --max-time and --location as @mfojtik suggested. I don't think we need to address wrong cert issue here, as git will have exactly the same problems accessing those.

[smarterclayton]

Can we get the last comment addressed?

[soltysh]

Working on it right now...

[soltysh]

Addressed all the comments.
@smarterclayton @ironcladlou @mfojtik review pls

[smarterclayton]

@csrwng

[csrwng]

Unless we have good reason not to, I'd think it be good to include the --head option so we don't download things twice.

[soltysh]

Rebased and applied @csrwng suggestion.
@smarterclayton after merging please close #115, thx.

[smarterclayton]

LGTM [merge]

[openshift-bot]

Origin Merge Results: SUCCESS (https://ci.openshift.redhat.com/jenkins/job/merge_pull_requests_openshift3/146/) (Image: devenv-fedora_210)

[openshift-bot]

Evaluated for origin up to 2be282a