fabric8-pipeline-library
fabric8-pipeline-library copied to clipboard
fabric8io
Snapshot for standalone planner
Added a new groovy script that will deploy snapshot and report the route URL on the PR. Similar to deployOpenShiftSnapshot
This is being consumed in https://github.com/fabric8-ui/fabric8-planner/pull/2426
@pranavgore09 Several comments inline. AFAIK, it fundamentally boils down to a
wget openShiftTemplate > snapshot.yml
oc apply -f snapshot.yml
I guess a straightforward shell script would look a lot saner than so many levels of indirection caused by this library. The patch looks fine & it seems to fit in with the rest of the code. LGTM except the inline comments.
@jaseemabid @sthaha I have removed container(clients) where it was not required and variables are not modified after assignment.
please review once! thanks.
Just a general thought I wanted to share - this kind of functions should not belong to the "library" as they are very specific to particular projects. That makes them unshareable essentially. So either we could decompose it to more "generic purpose" functions and reuse from within Planner Jenkinsfile or maybe create it's own "planner-pipeline-library" with those bits instead? (I believe one can have more than one @Library reference in the Jenkinsfile itsefl - see here for more details https://jenkins.io/doc/book/pipeline/shared-libraries/.
/cc @pradeepto @joshuawilson
I have questioned this approach of one-off functions since we started using it. I don't know how we can move away from it quickly. So we might just accept this but work toward a better solution.
@joshuawilson timebox it for 1 day for someone in your team and try? I think it's easily done based on the guide I shared above.
thanks, @bartoszmajsak @joshuawilson for taking a look, but when I started working on this, my intention was to deploy snapshot for the standalone planner, over a period of time it changed to a greater extent and as of now that change does not have any static things in it, any specific setup inside. Except the name of the file itself.
essentially it deploys application given in openshiftTemplate. here
The only line is regex matcher that seems to be static, but IMO that is fine. WDYT @jaseemabid @sthaha @bartoszmajsak @joshuawilson about changing its name to something more meaningful for a library
Thanks for explanation @pranavgore09, makes sense :) Nice work! My comment was a more a general statement.
Maybe in this given case we only need to rename the function then. But we do have a lot of other fabric8..Deploy... functions which are project specific - this needs to be addressed as part of https://github.com/openshiftio/openshift.io/issues/2243.
@bartoszmajsak @joshuawilson There is a whole lot of project specific code in the library and I think we all agree that it doesn't make much sense.
We will need to take a look at it as a whole and trim down a lot. For example, https://github.com/fabric8io/fabric8-pipeline-library/issues/372