fabric8-pipeline-library icon indicating copy to clipboard operation
fabric8-pipeline-library copied to clipboard

WIP fix: integration tests are now built in the user namespace

Open bartoszmajsak opened this issue 6 years ago • 7 comments

Due to misconfigured arquillian.xml and environment variables in the pipeline library integration tests are currently executed in the user-jenkins namespace which is not intended for that nor has enough resources - see https://github.com/openshiftio/openshift.io/issues/3134#issuecomment-409705409

The idea always was to use user namespace for that purpose. Therefore arquillian.xml is configured in a way that when executing tests in Che it will pick up current namespace (and it happens to be user).

This fix brings the same behaviour to the CI build.

Open points

Before we merge, let's discuss one thing, as the code in the current shape is not something I'm happy about.

Is there a way to set up some config.properties which we can than refer to in the pipeline library?

I know we have a hook .openshiftio/Jenkins.setup.groovy with setupEnvironmentPost(). I was wondering if we can externalized things like profiles and env variables we can pass for the build as mvn arguments, as right now have fixed in the pipeline itself (i.e. -P openshift - which not every project can have). This way we can move away some booster-specific settings for OSIO and do not pollute the code of the pipeline library. I see there is ${config.itestPattern} so was wondering if that concept can be somehow re-used

Fixes https://github.com/openshiftio/openshift.io/issues/3134 https://github.com/openshiftio/openshift.io/issues/763

bartoszmajsak avatar Aug 01 '18 21:08 bartoszmajsak

@bartoszmajsak is it a good idea to keep it into Jenkinsfile only?

hrishin avatar Aug 02 '18 06:08 hrishin

@hrishin can you expand on what you mean?

bartoszmajsak avatar Aug 02 '18 07:08 bartoszmajsak

Based on the discussion today let's recap what we can do with that issue, as I'm not sure we reached the conclusion. Here are the options I can think of:

  • merge - keep it as ugly as it is (we are not making it uglier ;))
  • open it up for customization, where possible solution could be
    • rely on already passed config object and customize on how mavenCI{} and mavenCD{} are invoked (so replace this or this with something like mavenCI{build: '.....'} - requires further analysis of what we want to make customizable - first cut would be those places where we explictly define profiles (so testing and deployment)
      • as a consquence we have to adjust Jenkinsfiles which come with imported projects (through Launcher), as they will have to now define their way of building/testing
    • I think extending Jenkinsfile.setup.groovy which we use for Booster hooks should not be considered as the right path

I think this is an important point to address. My overall concern is that the library as is now, makes it hard for any project to customize, and we all know that even in the mvn world where there is a strong focus on conventions we very frequently break out of it using profiles, properties etc (as this PR demonstrated anyway).

@sthaha @hrishin @gorkem WDYT?

bartoszmajsak avatar Aug 09 '18 13:08 bartoszmajsak

I think extending Jenkinsfile.setup.groovy which we use for Booster hooks should not be considered as the right path

Feels the same. It appears like the the purpose of the setup.groovy is to have pre or post build hooks and things we are referring here is about customizing one of the build steps. right?

At this moment this library is failing about one thing "speration of concern, isolating thing that is subject to vary from project to project like mvn command". How about extract the maven command to projects Jenkinsfile? In that way user would know the build command that build going to execute for given project. This path would require us to change many things accroos consumer of this library, however in log run it could pay off.

Otherwise being pragmatic, I agree with @bartoszmajsak approach to provide customization for mavenCI{}, mavenCD{} and other clsures to accept profiles, builds flags and probably goals (build targets) as well. This closure config would get more precedence over default one.

hrishin avatar Aug 10 '18 02:08 hrishin

@bartoszmajsak voices heard. https://github.com/fabric8io/fabric8-pipeline-library/pull/418

Can you we try to follow https://github.com/fabric8io/fabric8-pipeline-library#maven-canary-release.? The same options are avaliable now with mavenCI{}.

hrishin avatar Aug 10 '18 05:08 hrishin

Thanks @hrishin.

In order to make this fix working we also need to pass this cmd here

https://github.com/fabric8io/fabric8-pipeline-library/blob/c6231c0aa5756559ad594c058cd7195c867e5a0b/vars/mavenCI.groovy#L65-L69

and adjust this line https://github.com/fabric8io/fabric8-pipeline-library/blob/c6231c0aa5756559ad594c058cd7195c867e5a0b/vars/mavenIntegrationTest.groovy#L32

bartoszmajsak avatar Aug 10 '18 06:08 bartoszmajsak

@bartoszmajsak shall we close this PR?

hrishin avatar Aug 24 '18 09:08 hrishin