-
Notifications
You must be signed in to change notification settings - Fork 99
[JENKINS-75902] Don't save incomplete pipeline configs on GitLab API errors while fetching project info #495
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
base: master
Are you sure you want to change the base?
Conversation
…e fetching initial repo info
Hi @Pitxyoki, unfortunately we don't have any developers actively reviewing incoming pull requests for this plugin. We don't want your pull request to be in limbo forever; you can step forward to become a maintainer of this plugin following the instructions here: https://www.jenkins.io/doc/developer/plugin-governance/adopt-a-plugin/ |
Note that this change was not enough - we saw re-occurrences of the original issue. We are now testing with the extra changes added in Pitxyoki#1. |
Hello @basil, I might very well step forward and do that. My main motivation is to get this and related changes into the plugin and release a fixed version, but I may as well help with approving minor changes such as non-functional code quality or very small improvements. I foresee, however, that I will need:
How can we coordinate this more efficiently? Thank you! |
Hi @Pitxyoki, thanks for your interest in adopting this plugin. That motivation is well aligned with that of other new plugin maintainers.
As far as integration with Job DSL's dynamic DSL and Pipeline jobs, I think this is the best documentation we have:
I don't think we have any explicit documentation about integration with the Folders plugin. The GitHub Branch Source plugin should be considered a role model here. Probably many questions can be answered by checking to see how something is implemented there.
Our main concern is not introducing regressions and maintaining backwards compatibility with existing data. For the first, running it on your own Jenkins controller is the best way to make sure there are no regressions. For the second, documentation is here: If you aren't introducing any changes to on-disk saved state, then you don't need to worry about XStream compatibility.
This already has automated plugin release enabled as documented here: So you can simply apply a label to the PR, merge it, and a new release will be created automatically. Let me know if I can help with anything else. You can also join the |
Please see JENKINS-75902 for the original issue description.
Several safety assurances are added to the
GitLabSCMSource
class:GitLabApiException
s are allowed to be thrown instead of hidden byIllegalStateException
s on thegetGitlabProject()
methods.getGitlabProject(gitLabApi)
, at the beginning of theretrieve()
method, is enclosed in a dedicated try-catch. This prevents theconfig.xml
from being saved with incomplete data due to any communication error with GitLab. This should avoid the "loss of pipeline triggering" as described in JENKINS-75902.retrieveActions()
andcreateProbe()
) that communicate with the GitLab API in a similar way were also changed, but just to throwIOException
s in a non-breaking way, compatible with their inherited declarations.There are several line changes where only whitespace/indentation is changed. Unfortunately, GitHub highlights the whole lines. You may be able to see the diff more properly in a better diff editor or your IDE, setting it to ignore whitespace changes.
Please note that these changes were the result of some careful investigation on our part (I work for Feedzai) and we think we have headed in the right direction. We would appreciate any guidance if you find something is not right in our reasoning and/or the patch is inappropriate.
This seems to be related to:
Testing done
Testing this involved deploying the patched plugin onto our running Jenkins and leaving it there for one month.
We have not seen the original issue occur again ever since (we have, however, seen a similar occurrence associated to another plugin [cloudbees-folder] — we will open a respective issue+pull request for that as well).
Since we deployed the patched plugin, we have had GitLab become unreachable, but the problem of the pipelines not being triggered (and their config.xml files inconsistent) did not manifest.
Given the complexity of simulating GitLab failures when this plugin communicates with it, I did not implement automated tests. If there is a straightforward way to implement that, I'll appreciate it if you can guide me on how to do it.
Submitter checklist