-
Notifications
You must be signed in to change notification settings - Fork 567
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
Ensure jenkins.repo is deleted if repo is not used. (As the official RPM... #114
Conversation
…RPM package deploy/install jenkins.repo)
Hum i'm a bit lost about the error generated on Travis... Can someone point me on what exactly is going wrong? |
@Zophar78 I haven't a clue wtf Travis is on about. That said, the contribution looks good but we're trying to ensure rspec-puppet examples exist for new Puppet code in the module. The options we have are:
|
Thank you for a pull request! Please check this document for how the Jenkins project handles pull requests |
Something to note: I'm not sure we want to be removing this file. If we've got the flag set to false, it won't be installed, and thus, you might have dropped your own repo in, named jenkins.repo. I'm not sure why you would have an issue w/ the repo file, since it's installed by the module, not by the RPM, as far a I know. |
Let me join the conversation as we're working together with the guy who made the PR. The issue we have here at ${DAYJOB} is that we're using RedHat Satellite to manage nodes subscription to repositories, and we want our nodes to rely only on repositories exposed through Satellite (without talking about the fact that our nodes can't access external repositories). The problem is that the jenkins rpm do contain a repository file, so the current flag is quite useless in our case : One solution I can see would be to ensure that puppet removes the file after package installation if jenkins::params::repo is set to 0, would it be something acceptable from your side ? It's a blocking point for us, and we would like to avoid forking / dirty-hacking just because of it. Thank you ! |
@matthewbarr What do you think we should do with this pull request? I'm not too smert with RHEL based systems (haven't managed them in years) so I'm not sure what the right path forward here is. |
Hmm.. I'd actually consider this a bug in Jenkins's packaging. They suggest you add the repo manually and use yum to install, so there's really no reason for it to be inside the RPM. (as soon as I can figure out what component to open the bug against, I'll open a bug, & PR) Here's what I think they're using as their spec file: It does not replace an existing jenkins.repo, per L145. So: why don't I want to remove the file as part of our code? What happens if you set repo => 0, and put your own repo in that place? If we don't manage a file, we shouldn't delete something. Also, you may rebuild the package, or otherwise alter it. (hmm... We should actually allow the specification of a custom package name!) I've actually used my own jenkins.repo file, and would have been VERY cranky if it was deleted just because I also set repo => 0. |
@rtyler I have no idea how to get it looked at by the jenkins folks, but it's in there. |
I manually (puppet ensures it is absent outside this module) have to remove the jenkins.repo file myself for the reasons listed above. Personally I don't think RPM packages should manage repo files (unless I'm installing a package specifically for a repo like EPEL). With a puppet module that has the ability to manage the repo (and runs somewhat frequently to ensure its wishes are met) I think it makes sense for it to overrule the desires of the package (which is installed/updated less frequently). This also solves the problem if someone changes the module from true to false for managing the repo by cleaning itself up. |
@jlambert121 See my comments from Apr 19th. I'm completely in agreement that it shouldn't be installed by the RPM, but... I can tell you that I did install a file named jenkins.repo into the correct place, outside the module, w/ different content from the official one. It was THE reason this feature is even present, because I didn't want to fight the module... Thus, removing that file would have made me very unhappy. For now, what you're doing is best- remove the file in code outside the module. And anyone that figures out how to make the Jenkins folks actually look at the PR, that'd be great. |
@matthewbarr I saw your comment and agree about what the package should and shouldn't do. Thank you for your PR to jenkins to remove that from the package - I hope it gets merged and that part of this discussion isn't valid. WRT this and other puppet modules, if I set repo => true today and then repo => false tomorrow, I would assume that it would remove the apt source, yum config, or zyp config because I explicitly told it I didn't want the repo installed. Is that a bad assumption I have on what to expect from a puppet module? I view the jenkins.repo as just a config file (and it is declared as such in the spec) which (to me) means it can be freely modified or removed outside the confines of the package. I (again personal opinion) think it's similar to a config .d directory where removing unused/unneeded config is just part of configuring a service. With your jenkins.repo config specifically, is the reason you installed the repo config outside this module because you use a different/local base url? That seems like a reasonable parameter to add (repo_url) as many organizations (us included) mirror outside repositories and host them internally. If that's a feature that you think would be of value, I'm happy to push a PR for it. Thoughts? |
It was 2 jobs back at this point, but I think that I was using a central organization repo, vs a local jenkins repo. -- Wait, that wouldn't make sense. I must have been using a jenkins specific one. But then you'd have to also add in repo_key, and things like that, and deal w/ it for both Apt & Yum at the very least. I think that might be a bit too ambitious for a small amount of usefulness. We OK to close this now that the PR is merged? |
Based on the fact that this no longer is shipped w/ Jenkins RPM, I'm closing this PR. (The reason not to set a parameter w/ the base URL was that we you needed things like keys and such to match the repo. Some folks would build their own RPMs, even.. That would preclude just switching the base URL.) |
Ensure jenkins.repo is deleted if repo is set to false. (As the official RPM package deploy/install jenkins.repo)
As we use an internal repository (satellite) we don't want the jenkins.repo file deploy during install.