Fix Git metadata extraction for multi-branch pipelines#493
Conversation
0c16536 to
2164691
Compare
39b77f7 to
245701f
Compare
drodriguezhdez
left a comment
drodriguezhdez
left a comment
There was a problem hiding this comment.
Dropped some comments
| private static void updateGitData(Run<?, ?> run, Map<String, String> environment) { | ||
| GitMetadataAction gitMetadataAction = run.getAction(GitMetadataAction.class); |
There was a problem hiding this comment.
If I understood correctly, if the pipeline has multiple steps that checkouts code, the final git data will use always the latest one.
I guess this already happened with the previous implementation, but I'm wondering if we need to indicate this somewhere (recommend use a single checkout step per pipeline)?
There was a problem hiding this comment.
I'm not sure if it'd be appropriate to ask the users to restructure their pipelines to accommodate our plugin needs.
The root cause of the problem is that CI Visibility assumes there's a 1-to-1 relationship between pipelines and Git repositories, while for Jenkins this is not the case.
I will mention it in the public docs though, that the last checked out repo has higher priority.
| GitMetadataAction gitMetadataAction = run.getAction(GitMetadataAction.class); | ||
| if (gitMetadataAction != null) { | ||
| gitMetadata = GitMetadata.merge(gitMetadata, gitMetadataAction.getMetadata()); | ||
| } | ||
|
|
||
| gitMetadata = GitMetadata.merge(gitMetadata, GitUtils.buildGitMetadataWithJenkinsEnvVars(environment)); | ||
| gitMetadata = GitMetadata.merge(gitMetadata, GitUtils.buildGitMetadataWithUserSuppliedEnvVars(environment)); |
There was a problem hiding this comment.
The GitMetadataAction has been merging all the git info across different steps (SCMCheckout and StepListener). With the last 2 lines, aren't we overriding with the env vars available at BuildListener level? (IIRC there's only one BuildData instance per Run<?,?>)
There was a problem hiding this comment.
BuildData is not persisted anywhere, and is created on demand in several places - not only for CI Visibility traces, but also for the events that the plugin sends.
I agree, however, that if GitMetadataAction is available, it should have precedence over the standard Jenkins variables available to the BuildListener.
I've changed the logic accordingly.
| public static final String JENKINS_EXECUTOR_NUMBER = "jenkins.executor.number"; | ||
| public static final String JENKINS_RESULT = "jenkins.result"; | ||
| public static final String JENKINS_PLUGIN_VERSION = "jenkins.plugin.version"; | ||
| public static final String PIPELINE_DEFINITION_GIT = "pipeline.definition.git"; |
There was a problem hiding this comment.
This tag uses the shared namespace pipeline and it's not compatible with other CI providers.
Could we use jenkins.pipeline.definition.git instead to encapsulate this in the jenkins namespace?
There was a problem hiding this comment.
Agree, updated the tags to have jenkins.pipeline.definition.git prefix
nikita-tkachenko-datadog
added
the
changelog/Fixed
2 participants
Requirements for Contributing to this repository
What does this PR do?
Fixes Git metadata extraction for multi-branch pipelines.
A multi-branch pipeline has its definition (aka "pipeline script") stored in a
Jenkinsfilein a Git repository.Each branch may have its own version of the pipeline definition, which is why Jenkins automatically creates a separate pipeline for each branch in the repo.
When such pipeline is executed, the first step is the pipeline definition checkout.
The plugin is notified of the checkout, so after it finishes the plugin fetches Git metadata of the checkout (repo URL, branch, commit, etc) and associates it with the pipeline.
The problem is that in some cases the pipeline definition contains another explicit Git checkout command.
What happens in this case is: Git checkout of the pipeline definition is done, pipeline starts executing, then another Git checkout of a different repo is done as one of the pipeline steps.
In this case we want the pipeline to be associated with the latter checkout.
This PR ensures this (prior to the changes the first checkout had higher priority).
The original pipeline definition checkout is stored in a separate set of tags (prefixed with
pipeline.definition.git.*) in case this data is needed (it will also be reported in the original set of tags unless there is another Git checkout inside the pipeline).Existing Git metadata extraction logic was too convoluted to incorporate the fix, so it has been rewritten.
The most notable changes are:
GitMetadataandGitCommitMetadataclasses are added that encapsulate repository and commit metadataSourceclass is added, defining where a specificGitMetadatainstance is coming from (possible source are - in the order or increasing priority: Jenkins standard environment variables, pipeline definition Git checkout event, generic Git checkout event, user-provided environment variables)GitMetadata#mergeandGitCommitMetadata#mergeDescription of the Change
Alternate Designs
Possible Drawbacks
Verification Process
Additional Notes
Release Notes
Review checklist (to be filled by reviewers)
changelog/label attached. If applicable it should have thebackward-incompatiblelabel attached.do-not-merge/label attached.kind/andseverity/labels attached at least.