Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jenkins Log URL in OpenShift Console #11

Closed
hrishin opened this issue Feb 7, 2018 · 28 comments
Closed

Jenkins Log URL in OpenShift Console #11

hrishin opened this issue Feb 7, 2018 · 28 comments
Assignees

Comments

@hrishin
Copy link
Member

hrishin commented Feb 7, 2018

Looks like there is a need to accommodate the change in Jenkins Log URL due to the introduction of proxy.

Right now URL may be getting generated from
Used here: https://github.com/openshift/jenkins-sync-plugin/blob/master/src/main/java/io/fabric8/jenkins/openshiftsync/BuildSyncRunListener.java#L274
Generated here: https://github.com/openshift/jenkins-sync-plugin/blob/master/src/main/java/io/fabric8/jenkins/openshiftsync/OpenShiftUtils.java#L393

Instead of this, the plugin should be able to pick the URL from ENV VAR's.

cc: @vpavlin @hferentschik @pradeepto @chmouel @rupalibehera

@vpavlin
Copy link

vpavlin commented Feb 7, 2018

If I am not mistaken, this is the template we use for Jenkins deployment: https://github.com/fabric8-services/fabric8-tenant-jenkins/blob/master/apps/jenkins/src/main/fabric8/openshift-deployment.yml

@aslakknutsen can you confirm?

If that's the case, and we get this issue resolved - i.e. ENV var added, we'll need to also update the template to provide that ENV var

@hferentschik
Copy link

A couple of questions, does the fabric8/jenkins-openshift have the required plugins already installed? Are they getting added and configured when the image is built?

Is it really the right thing to do, to let the plugin code itself read the environment variable? Is this the default approach for Jenkins plugins? Do Jenkins plugins not handle configuration differently, aka in a config file? So, in this case, would it not be the responsibility of the boot script of this image to read the injected Jenkins URL and write it into the config?

The last time I looked into this Jenkins image, I got a bit lost in terms of how it is structured and bootstrapped. Where is this happening (in terms of repository and code)?

@hferentschik
Copy link

Also to be sure, we are on the same page here. What we are after is updating the link as marked in this screenshot - https://www.awesomescreenshot.com/image/3144831/e3fa9e616b4061440bb2e61f868d1f6d

Will the change we are discussing here achieve this?

@vpavlin
Copy link

vpavlin commented Feb 7, 2018

As far as my understanding of the code it will - the link is taken from annotations, annotations are generated by sync plugin when the OpenShift build artifact is created - so if we can influence what is generated and stored in annotations, the link in console will change.

One point though - we'll need all users to update their Jenkins for this to take effect

@hferentschik
Copy link

One point though - we'll need all users to update their Jenkins for this to take effect

You mean existing users need to make sure that they get the Jenkins image with the changes discussed above? Is this something we should do via a migration as well? Updating the Jenkins image version in respective deployment config? Then there would be some work needed from us.

@rupalibehera
Copy link
Member

@hferentschik, the plugins which are customized or not available in the update center is here https://github.com/fabric8io/openshift-jenkins-s2i-config/tree/master/plugins. The plugins which are directly installed from the update center is https://github.com/fabric8io/openshift-jenkins-s2i-config/blob/master/plugins.txt they are installed when the image is built https://github.com/fabric8io/openshift-jenkins-s2i-config/blob/master/Jenkinsfile#L21

@rupalibehera
Copy link
Member

@hrishin , we should discuss this with @gabemontero too as this may affect our work on #10 reducing the delta and our aim to merge with upstream finally.

@hferentschik
Copy link

Ok, so the sync plugin seems to be added as part of the customized plugins. It looks like the plugin is added as binary? How is that working? How do you know the version of the binary?

@rupalibehera
Copy link
Member

I am sorry to say but this a manual step -> we do a mvn clean install in jenkins-sync-plugin and then run https://github.com/fabric8io/openshift-jenkins-s2i-config/blob/master/copy-jenkins-sync-plugin.sh which basically just copies the binary to the right folder.

@rupalibehera
Copy link
Member

@hrishin , we can do the change in our current fork and let's deploy it as it looks like be a bit urgent.

@pradeepto @chmouel should we include this issue in our current sprint ?

@hferentschik
Copy link

Sure, but would you not at least add some sort of version string? In a perfect world you would not even add this as a binary, but this aside how do you identify the version of this plugin? How do you track down problems, if you don't even know what version you added to the repository? It does not even seem to be mentioned in the git commit.

@rupalibehera
Copy link
Member

rupalibehera commented Feb 7, 2018

we did not wanted to maintain this fork and wanted reduce the delta as quick as possible but upstream PR reviewing and merging really took long long time. and we had time crunch due to deadlines and from then it is like this. hopefully in coming few sprints we just use the upstream one.

@hrishin
Copy link
Member Author

hrishin commented Feb 7, 2018

@vpavlin @hferentschik @rupalibehera

The last time I looked into this Jenkins image, I got a bit lost in terms of how it is structured and bootstrapped. Where is this happening (in terms of repository and code)?

FYI, Ideally, it should get fixed in the upstream project to always get available in all OpenShift Jenkins images but this will take some time due to #10.
On other hands, we can get it fixed in downstream (fork) and comparatively that will be quick.

In terms of code organization, both repos. (upstream and downstream) looks identical to generate console URL. So fixing in fork should not be an issue.

Yes, to display the console log URLs, OpenShift front-end application might be querying the build resources which in turn created by Jenkin sync plugin. While creating these build resources Jenkins sync plugin injects the console log URLS as a metadata annotation key called "openshift.io/jenkins-log-url".

Key questions:

  1. How many ENV vars? Assuming a single ENV var is enough for this issue. URL is subject to change sometimes. It could be better to use one more ENV var for URL suffix as well.
  2. What data ENV var going to contain? Just proxy hostname/IP or other data as well ( like protocol i.e HTTP or HTTPS or other)
  3. What could be ENV var names?

@vpavlin
Copy link

vpavlin commented Feb 7, 2018

How many ENV vars? Assuming a single ENV var is enough for this issue. URL is subject to change sometimes. It could be better to use one more ENV var for URL suffix as well.

I'd go with one variable - this would be very specific case where you want to override the OpenShift generated URL with a specific hostname from my point of view

What data ENV var going to contain? Just proxy hostname/IP or other data as well ( like protocol i.e HTTP or HTTPS or other)

As the code figures out the protocol automatically for generated URLs, we should probably provide it here in ENV var - e.g. https://jenkins.openshift.io

What could be ENV var names?

JENKINS_ROOT_URL ?

@hrishin
Copy link
Member Author

hrishin commented Feb 7, 2018

@rupalibehera

we can do the change in our current fork and let's deploy it as it looks like a bit urgent.

Sure, we can.

@hrishin
Copy link
Member Author

hrishin commented Feb 7, 2018

@vpavlin

What could be ENV var names?
JENKINS_ROOT_URL ?

looks good or JENKINS_PROXY_URL sounds better?

@hrishin
Copy link
Member Author

hrishin commented Feb 7, 2018

@rupalibehera

we should discuss this with @gabemontero too as this may affect our work on #10 reducing the delta and our aim to merge with upstream finally.

Yes we should but intimate @gabemontero about this change. Still, this change is going to be backward compatible with existing state.

@vpavlin
Copy link

vpavlin commented Feb 7, 2018

looks good or JENKINS_PROXY_URL sounds better?

That ties this potentially general change to our specific use case where we use a single proxy to route to many jenkins instances. Not sure if there really is a different use case at the moment, but I'd keep the name generic

@hrishin hrishin self-assigned this Feb 7, 2018
@hferentschik
Copy link

TBH, I think this is not quite the right way of doing this, but it might be the fastest indeed. So let's go ahead and do this.

@hferentschik
Copy link

@hrishin, any update on this? Are there any problems creating the fix and updated Jenkins image?

@hrishin
Copy link
Member Author

hrishin commented Feb 8, 2018

TBH, I think this is not quite the right way of doing this, but it might be the fastest indeed. So let's go ahead and do this.

@hferentschik yes it's out of sync with upstream. Apart from this what things could you see?

Any update on this? Are there any problems creating the fix and updated Jenkins image?

Not a problem, just a note: will fix the issue in job-to-bc branch and use that to update the JenkinsImage. In the previous case as well team has leveraged on this branch only.

@gabemontero
Copy link

Hey @hrishin and @rupalibehera - finally was able to bring this up. I think I've assimilated the particulars, but please bear with me and provide course corrections as needed.

My initial take - we should employ an union of the various sources, with an order of precedence. For example, in the case of the jenkins root url:

  1. if an openshift service for jenkins exists, that takes precedence
  2. if the service does not exist, try the well defined env var

My first instinct was to not have the order in the other direction, because we typically have design take of "config/settings in openshift, if running in openshift, take precedence".

But let me know if the fabric/OSIO scenarios dictate otherwise, where you want the env var to be first choice, and we can discuss the pros / cons.

Thoughts?

@hrishin
Copy link
Member Author

hrishin commented Feb 9, 2018

Thanks for having look into this issue @gabemontero !

My initial take - we should employ an union of the various sources, with an order of precedence. For example, in the case of the jenkins root url:

if an openshift service for jenkins exists, that takes precedence
if the service does not exist, try the well defined env var

In this fabric8/OSIO case, precedence is given to ENV VAR containing the proxy URL that will override the OpenShift service hostname/host URL.
https://github.com/fabric8io/jenkins-sync-plugin/pull/13/files#diff-b9ed119c66ed02d31e9cb3377f727207R263

@vpavlin
Copy link

vpavlin commented Feb 9, 2018

But let me know if the fabric/OSIO scenarios dictate otherwise, where you want the env var to be first choice, and we can discuss the pros / cons.

Exactly - we need the ENV var to have precedence to override whatever OpenShift provides.

The reason is that our proxy uses Jenkins route thus we cannot remove the route or the service for Jenkins - as we would not be able to access it from proxy. At the same time, we need to use Proxy URL as a root URL of Jenkins to generate proper links in console - going through the Proxy.

I think this use case is strictly specific to fabric8/OSIO due to our need for Jenkins idling - if this is solved systematically directly in OpenShift in the future, there might not be a need for this ENV var anymore.

Hope it helps

@hferentschik
Copy link

@hrishin

Apart from this what things could you see?

My concern with this is that I don't believe that taking configuration settings for plugins from the environment is the way things are done in Jenkins. Injecting parameters into the environment is coming from the container world and yes, from an OpenShift perspective, this URL will be injected into the Jenkins image. This does not mean that all programs suddenly should read their configuration from the environment. That's not how this works. If you are packaging existing applications, Jenkins, Wildfly, Kafka (you name it) into a container, you are not going to change how these tools are used standalone (at least not usually). For these tools, it should be transparent whether they run standalone or in a container. It is part of the image creation/setup to bridge any potential gaps between settings being injected as environment variables and the configuration a given tool needs.

Say you want to run this Jenkins instance standalone with this plugin. Now you need to start setting environment variables as well. That makes no sense. Hence I was saying that most likely the plugin should just offer the default way of configuring plugins in Jenkins and the Jenkins image bootstrapping should create the required config.

I am not an expert on Jenkins. Maybe it is common for plugins to read env variables in which case my argument is of course moot.

@gabemontero
Copy link

First, wrt @hferentschik concerns, a couple of points:

  • there is extensive use of env vars in Jenkins, both the core and plugins, for config and tuning; they even have utilities for getting information from either env vars or the servlet context ... so sure in some sense your argument as you say is moot
  • that said, at least from my perpsective, that points you made wrt where jenkins is run, container vs. bare metal, etc. are still pertinent, and are still in line I believe with my earlier point around being able to pick from multiple places and have a well defined order of precedence for our users to understand ... aside from env vars or other config injected in a pod, the user may have explicitly configured jenkins for example with a given root URL (i.e. and entry in the config.xml)

To @vpavlin and @hrishin - ok, yeah, after refreshing my memory a bit with proxy in particular, Jenkins tends to work through env overrides with how the git plugin and other items deals with proxies.

So sure, let's do the order of precedence with env vars ahead of openshift injected config ... we should document this in the plugin readme though minimally (and I may end up citing something about it in the openshift doc) ... @hrishin - if you could go ahead and update your PR with a commit for updating the README, that would be great. Otherwise, I'll try to remember to look for it during my upcoming PR review (hoping to find time Monday) and ask for the if I don't see it ;-)

Lastly, as is often the case in our industry, issues like this tend to cluster ;-) ... we just a customer support case where they had log url annotations incorrect because in a multi-project scenario (i.e. sync plugin monitoring different project) we were looking for the jenkins root url in the service/route in the build's namespace, not the jenkins namesapce. So I'm going to be hacking in his space next week as well. I'll certainly tag you @hrishin / @rupalibehera in my PR, and we can coordinate as needed wrt OSIO getting caught back up with "upstream".

thanks

@hrishin
Copy link
Member Author

hrishin commented Feb 12, 2018

Sure @gabemontero
Could you please review the PR #13 ?

@pradeepto
Copy link

pradeepto commented Feb 15, 2018

Closing this. Reopen if need be,

Edit: Apparently I can't.

@hrishin hrishin closed this as completed Feb 19, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants