oc new-app applying language detection when not needed, causing an error.

[GrahamDumpleton]

According to:

• https://docs.openshift.com/container-platform/3.3/dev_guide/new_app.html#advanced-multiple-components-and-grouping

oc new-app can be passed multiple applications to deploy as separate deployments.

There is one caveat on that though as explained in the note in that section.

If a source code repository and a builder image are specified as separate arguments, new-app uses the builder image as the builder for the source code repository. If this is not the intent, simply specify a specific builder image for the source using the ~ separator.

Thus one can have two arguments where one is a builder image and the other is a source code repository. In that case the image would be applied as an S2I builder with the source code repository as input. Thus the three command lines below should all be equivalent:

oc new-app getwarped/s2i-httpd-server https://gitlab.com/graham-dumpleton/static-website-test-1.git --name test1
oc new-app --docker-image=getwarped/s2i-httpd-server https://gitlab.com/graham-dumpleton/static-website-test-1.git --name test1
oc new-app getwarped/s2i-httpd-server~https://gitlab.com/graham-dumpleton/static-website-test-1.git --name test1

That is not the case though as the first and second commands will actually fail.

The problem is that although oc new-app recognises that it needs to run the image as an S2I builder with the source code, before it does that it is running language detection on the source code repository to determine what builder it should use, even though not necessary, as the builder image was provided.

When this language detection runs, if the source code repository doesn't have any of the special files in it that the builtin language builders are looking for, the detection fails and causes oc new-app to fail. As a consequence, the first two commands fail with the error:

error: No language matched the source repository

The third command where ~ is used doesn't suffer this problem as in that case language detection does appear to be disabled.

Worth noting that if any of the special files looked for by language detection is added then it will then run okay, but with the language being detected being ignored anyway as it still uses the builder image given on the command line. Thus the commands:

oc new-app getwarped/s2i-httpd-server~https://gitlab.com/graham-dumpleton/static-website-test-2.git --name test2
oc new-app --docker-image=getwarped/s2i-httpd-server https://gitlab.com/graham-dumpleton/static-website-test-2.git --name test2

work fine, as they have a dummy project.json in the source code repository even though not required by the image builder nor anything to do with dotNet.

In summary, in this two argument case where one is a builder image and the other is a source code repository, language detection should be disabled, in the same way as occurs when using ~.

Version

oc v1.3.1
kubernetes v1.3.0+52492b4
features: Basic-Auth

Server https://127.0.0.1:8443
openshift v1.3.1
kubernetes v1.3.0+52492b4

Steps To Reproduce

See above.

Current Result

Fails with error:

error: No language matched the source repository

Expected Result

Should executed the image as an S2I with source code repository as input.

Additional Information

There is nothing of relevance in log output, even at log level 5 or above.

Most you get is:

I1026 15:40:12.503453   52500 repository.go:355] Executing  git ls-remote https://gitlab.com/graham-dumpleton/static-website-test-1.git
I1026 15:40:14.541400   52500 sourcelookup.go:516] No source ref specified, using shallow git clone
I1026 15:40:14.541453   52500 repository.go:355] Executing  git clone --recursive --depth=1 https://gitlab.com/graham-dumpleton/static-website-test-1.git /var/folders/0p/4vcv19pj5d72m_bx0h40sw340000gp/T/gen754776583
F1026 15:40:18.184447   52500 helpers.go:110] error: No language matched the source repository

The image is believed to have labels defined which identify it as a builder image and works fine when using ~.

                "io.k8s.description": "S2I builder for hosting files with Apache HTTPD server",
                "io.k8s.display-name": "Apache HTTPD Server",
                "io.openshift.expose-services": "8080:http",
                "io.openshift.s2i.scripts-url": "image:///opt/app-root/s2i/bin",
                "io.openshift.tags": "builder,httpd",

[bparees]

I'm pretty sure we can't actually change this behavior now, but we should at least fix the docs to clarify what happens in each of these scenarios (and make sure we actually understand too):

oc new-app non-builder-image https://detectable-repo
oc new-app non-builder-image~https://detectable-repo
oc new-app --docker-image=non-builder-image https://detectable-repo
oc new-app --image-stream=non-builder-image https://detectable-repo

oc new-app non-builder-image https://non-detectable-repo
oc new-app non-builder-image~https://non-detectable-repo
oc new-app --docker-image=non-builder-image https://non-detectable-repo
oc new-app --image-stream=non-builder-image https://non-detectable-repo

oc new-app builder-image https://detectable-repo
oc new-app builder-image~https://detectable-repo
oc new-app --docker-image=builder-image https://detectable-repo
oc new-app --image-stream=builder-image https://detectable-repo

oc new-app builder-image https://non-detectable-repo
oc new-app builder-image~https://non-detectable-repo
oc new-app --docker-image=builder-image https://non-detectable-repo
oc new-app --image-stream=builder-image https://non-detectable-repo

[GrahamDumpleton]

In what scenario would running language detection still be valid when you have that image/repo case and know the image is a builder image, such that you can't change the behaviour? Or are you talking about the bigger picture of how oc new-app works and not just the specific issue I am raising with language detection being run when it shouldn't?

[bparees]

In what scenario would running language detection still be valid when you have that image/repo case and know the image is a builder image, such that you can't change the behaviour?

"oc new-app openshift/jenkins-1-centos7 https://github.com/openshift/nodejs-ex"

(do you want to build nodejs-ex w/ the jenkins image, which is a valid s2i image? or do you want to deploy jenkins and build nodejs-ex with whatever builder we normally recommend for nodejs-ex based on source detection?)

Or are you talking about the bigger picture of how oc new-app works and not just the specific issue I am raising with language detection being run when it shouldn't?

yes

I think we need to run through the matrix I described above, see what the current behavior is for each case, and then we can decide what can/can't be changed or what is/isn't behaving in a useful/consistent way. So let's gather the data and then debate the solution.

[GrahamDumpleton]

The matrix should also be perhaps be expanded to include checking what happens when the --strategy option is also included.

For example, what should:

oc new-app openshift/jenkins-1-centos7 https://github.com/openshift/nodejs-ex --strategy=source

do.

[coreydaley]

@bparees Can you point me to somewhere that clarifies (or can you clarify) the builder vs non builder images? (builder images are s2i images correct?)

[bparees]

@coreydaley yeah builder images are s2i-enabled images. here's the new-app codes that decides if an image is an (s2i) builder image or not:
https://github.com/openshift/origin/blob/master/pkg/generate/app/builder.go#L12-L35

there are a couple things it checks, as you can see.

[szpak]

I found it also problematic when building from source a Gradle-based application. Without pom.xml on oc new-app --image-stream=... --code ... OpenShift is not able to detect repository type. Is there way to force jee (my image supports Gradle)?

[bparees]

@szpak you can't force the source type (and then let new-app pick an image for that source type), unfortunately, but if you know of an image can build your source code, you can just specify that image/imagestream explicitly. --image-stream should work assuming the imagestream you're specifying is marked as a builder. Barring that, you can always use the ~ syntax:

oc new-app mygradebuilder~myrepo.git

does that help?

[szpak]

I've tried it already and it bypassed that validation, however, later on the build was treated as binary deployment and my image tries to find JAR instead of cloning a Git repo and build the code.

Nevertheless, today I tried with remote Git repo instead of local directory and it worked. Thanks.

[bparees]

later on the build was treated as binary deployment and my image tries to find JAR instead of cloning a Git repo and build the code.

hmm, i'd have to see your exact new-app invocation, and the buildconfig json that was produced, to understand why that would be the case.

binary build has multiple meanings unfortunately. if you're not providing a remote git repo, you will need to supply the source code from your local machine via a "binary build" (e.g. oc start-build mybuild --from-dir .), but that should not preclude the source code being compiled into a jar, if you are providing a pom.xml+java source, so i'm not sure why your build would be ignoring your source and only expecting a jar (normally the JEE builder images will both try to build source if a pom.xml is present, or just deploy a .jar file if one is present in the "source")

[bparees]

@mjudeikis very related to the PR you were already working related to not doing source detection when a builder image is provided.

[mjudeikis]

@bparees will check, thanks.

[mjudeikis]

k, I can still see that issue is still there. But before jumping to the code, lets flush right behavior (and update doc after).

oc new-app <command>
command:

Case Num	Command	Expected Behaviour	Current Behaviour
1	non-builder-image https://detectable-repo	Code detection is done. IF builder found - use it, else Fail. Non-builder image is ran as image without build
2	non-builder-image~https://detectable-repo	No detection is done on code repo. Build using image provided
3	--docker-image=non-builder-image https://detectable-repo	Code detection is done. IF builder found - use it, else Fail. Image imported as IS. Non-builder image is ran as image without build
4	--image-stream=non-builder-image https://detectable-repo	If image-stream has "builder" tag - dont do detection and use it to build. Else previous behaviour
5	non-builder-image https://non-detectable-repo	ERROR. Sugges to use ~
6	non-builder-image~https://non-detectable-repo	Build using non-builder image. No code detection
7	--docker-image=non-builder-image https://non-detectable-repo	suggest import-image and ~ syntax
8	--image-stream=non-builder-image https://non-detectable-repo	suggest import-image and ~ syntax
9	builder-image https://detectable-repo	Build using builder image. No code detection
10	builder-image~https://detectable-repo	Build using builder image. No code detection
11	--docker-image=builder-image https://detectable-repo	Import image as IS. Build using builder image. No code detection
12	--image-stream=builder-image https://detectable-repo	Build using builder image, ignore repo code detection
13	builder-image https://non-detectable-repo	Build using builder image. No code detection
14	builder-image~https://non-detectable-repo	Build using builder image. No code detection
15	--docker-image=builder-image https://non-detectable-repo	Import iamge as IS. Build using builder image. No code detection
16	--image-stream=builder-image https://non-detectable-repo	Build using builder image. No code detection

Please Update the table with what do you think is right. And we can check what is valid in the code :)
@bparees

[bparees]

@mjudeikis I think those look fine for expected behaviors. I could quibble with (5) (it's possible we should error in that case and force you to use the ~ syntax) but then it would be unclear what to do for (7) and (8) where you can't use the ~ syntax.

[mjudeikis]

@bparees this is where I was in doubt too. I tend to "put a dot here and say ~ is supported without flags only. I think backend changes will be easier this way too. So if I detect ~ - it will be one logic, and no - other. As If I recall right logic is already complex enough there.

[mjudeikis]

@bparees spent few hours going via code, and all this behavior does not look like easy win without a lot of refactoring. All this logic, where we decide if we should detect code or now, looks flaky at minimum... Was thinking under these lines

1. Add flag: --code-detection=false (default true), which would force all code detection to false, so of anybody want to force to use other image or type (like gradle example), they can. We just interprets inputs and add the to the builder, and does not matter is it imageStream or repos.

2. Update docs with matric from this story and previous one (no git installed bug 1470374 - oc new-app behaviour  #17457 )

3. I tempted to abandon my suggestion for behavior: Code detection is done. IF builder found - use it, else Fail. The non-builder image runs as an image without build and try to keep it simple. High level, code + repo, code detection is done always, expect if ~ is used or --code-detection=false

4. In a longer-term add something like https://github.com/src-d/go-git to code base, which would solve some issue with different behaviors when using this in different environments and code detection. (this is more to bug 1470374 - oc new-app behaviour  #17457) as testing is not possible for it now.

[openshift-bot]

Issues go stale after 90d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.
Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle stale

[GrahamDumpleton]

/remove-lifecycle stale

[openshift-bot]

Issues go stale after 90d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.
Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle stale

[GrahamDumpleton]

/lifecycle frozen