The 5 char git SHA breaks install-plugins.sh

mikejoh commented 6 years ago

Hey!

Wall of text.

I've been testing and learning Jenkins and specifically JAC for a couple of weeks now, during one of the Docker image build steps (of the master) we're running the installer for all the Jenkins plugins. I wanted to get rid of that deprecation warning by switching to the install-script.sh script instead:

Step 11/14 : RUN /usr/local/bin/plugins.sh /usr/share/jenkins/plugins.txt
 ---> Running in e48e7cd499ff
WARN: plugins.sh is deprecated, please switch to install-plugins.sh

To test this before changing in the master Dockerfile i jumped into the master container and ran the install-plugins.sh that are included in the jenkins:2.46.3-alpine image that JAC are based on:

bash-4.3$ xargs /usr/local/bin/install-plugins.sh < /usr/share/jenkins/plugins.txt
Creating initial locks...
Analyzing war...
Using version-specific update center: https://updates.jenkins.io/2.46...
Downloading plugins...
... snip ...
Downloading plugin: (409ae) from https://updates.jenkins.io/2.46/latest/(409ae).hpi
... snip ...
Downloading plugin: (409ae)-plugin from https://updates.jenkins.io/2.46/latest/(409ae)-plugin.hpi
Failed to download plugin: (409ae) or (409ae)-plugin

Contents of /usr/share/jenkins/plugins.txt:

...
pretested-integration:2.4.1 (409ae)
...

That 5 char git SHA (set by Maven and appended when building the hpi?) breaks the installer and exits with a:

Some plugins failed to download! Not downloaded: (409ae)

Sure that xargs install run is messing up this a bit by treating the (409ae) as the next plugin to install since we have a whitespace between pretested-integration:2.4.1 and (409ae).. so i downloaded the newest version of install-plugins.sh i could find and ran it in the master container:

bash-4.3# /usr/local/bin/install-plugins_new.sh < /usr/share/jenkins/plugins.txt
Creating initial locks...
Analyzing war...
Registering preinstalled plugins...
Using version-specific update center: https://updates.jenkins.io/2.46...
... snip ...
Downloading plugin: pretested-integration from https://updates.jenkins.io/download/plugins/pretested-integration/2.4.1 (409ae)/pretested-integration.hpi
... snip ...
Failed to download plugin: pretested-integration or pretested-integration-plugin
... snip ...
Some plugins failed to download! Not downloaded: pretested-integration

OK so we don't need to use that xargs workaround anymore, that's good, but we're still failing.

This is what fails in the doDownload() function:

curl --connect-timeout 20 --retry 5 --retry-delay 0 --retry-max-time 60 -f -L 'https://updates.jenkins.io/download/plugins/pretested-integration-plugin/2.4.
1 (409ae)/pretested-integration-plugin.hpi' -o /usr/share/jenkins/ref/plugins/pretested-integration-plugin.jpi

curl: (22) The requested URL returned error: 404 Not Found

No other plugins, of what i've seen have anything similar appended in regards to the 5-char git SHA to the plugin name in the plugins.txt file.

That installer script could easily be doing some input clean-up before using curl and fetching the plugins, so this could in the end be something for Jenkins to fix and not us (Praqma), but i figured that i could start here. :-)

What do you think?

MadsNielsen commented 6 years ago

@mikejoh You're right. We do set this as part of the metadata when building the plugin

https://github.com/Praqma/pretested-integration-plugin/blob/9289150ae8ea061880476effaa36ce3a0094b7f2/pom.xml#L79-L104

I think the best way would be to alter the installer script to clean up those theings before the curl, as you suggested.

buep commented 6 years ago

My comment explains why we have it - should we reconsider that now we started to use the beta-releases instead? Not following strict semver should be allowed in the install script anyway, because there are at least a handful of other plugins adding extra stuff to the version number.

@mikejoh did you make issues or PR for the install-plugin.sh script you can link to here?

mikejoh commented 6 years ago

@buep i created this #644 over at jenkinsci/docker. As soon as i have time i'll continue from there. Thanks for the input.

mikejoh commented 6 years ago

Note to this issue:

In the plugin comment (in pom.xml) regarding custom build numbers it states that the version number should show up in this format :

major.minor.patch-gitSHA

but it actually ends up as:

major.minor.patch (gitSHA)

If the artifact semver follow the stated format in the comments we could mitigate the install-plugins.sh "bug".

buep commented 6 years ago

Do you know @mikejoh if the install-plugins.sh will honor true semantic versions if there are no space between the extra information, nor any parenthesis?

It seems like we could also improve our pom.xml to use real semver compatible versions.

Alternatively if we install-plugin.sh can't work with anything else than major.minor.patch I'm willing to switch to that because the initial reason for using git sha was never finished. We don't have functional testing installing a dev version, and testing it.

buep commented 6 years ago

@MadsNielsen - can you help @mikejoh ?

I would like you to do the following (else let's chat if it doesn't make sense):

see or ask @mikejoh if the install-plugin.sh will work with real semver numbers, where we do not use space https://semver.org/
- if so, fix our pom.xml and versioning to follow semver and release a new version
if install-plugins.sh doesn't work with anything beside major.minor(.path) I'm willing to follow that versioning scheme and release a new version (but please really really make sure it is needed first!)

When releasing, make sure to commit changes with reference to this issues, update wiki page etc.

mikejoh commented 6 years ago

Do you know @mikejoh if the install-plugins.sh will honor true semantic versions if there are no space between the extra information, nor any parenthesis?

Verbosity ahead!

It does not enforce any semantic versioning "policies" or such, it does the bare minimum i would say. If we would version the plugin as stated in that comment in the pom-file (major.minor.patch-gitSHA) we would've passed through both installer scripts but the amount of versions would increase in the plugin download servers if we uploaded a new one per commit (?). That shouldn't be a problem per se, but if we aren't using the git SHA then we'll "solve" this for now by scrapping it.

Let's be clear about that there's room for improvement(s) in the installer script and i will try to compose one or two PRs when i have the time!

About those scripts, the logic for determining the plugin name and version are done quite differently between the installer scripts. The following code causes the download problem for the pretested-integration plugin in install-plugins.sh, see my comments in the code block below:

...
versionFromPlugin() {
    local plugin=$1 # plugin=pretested-integration:2.4.1 (409ae)
    if [[ $plugin =~ .*:.* ]]; then
        echo "${plugin##*:}" # prints "2.4.1 (409ae)"
    else
        echo "latest"
    fi
}
...
main() {
    ...
    echo "Downloading plugins..."
    for plugin in "${plugins[@]}"; do
        pluginVersion=""
       if [[ $plugin =~ .*:.* ]]; then
            pluginVersion=$(versionFromPlugin "${plugin}") # calls the versionFromPlugin function above and sets pluginVersion=2.4.1 (409ae)
            plugin="${plugin%%:*}" # plugin=pretested-integration
       fi
        download "$plugin" "$pluginVersion" "true" & # calls a download function with arguments: "pretested-integration" and "2.4.1 (409ae)"
    done
    ...
}
...

As i mentioned earlier in this issue, that download() call ends up invoking curl with a path containing the "wrong" version number: /download/plugins/pretested-integration-plugin/2.4.1 (409ae)/pretested-integration-plugin.hpi and results in a 404.

I also dug through the plugins.sh just to compare the script logic a bit, it works as intended in that installer because of this code:

...
while read -r spec || [ -n "$spec" ]; do
    plugin=()
    IFS=' ' read -r -a plugin <<< "${spec//:/ }"
    [[ ${plugin[0]} =~ ^# ]] && continue
    [[ ${plugin[0]} =~ ^[[:space:]]*$ ]] && continue
    [[ -z ${plugin[1]} ]] && plugin[1]="latest"
    ...
    echo "Downloading ${plugin[0]}:${plugin[1]}"
    curl --retry 3 --retry-delay 5 -sSL -f "${JENKINS_UC_DOWNLOAD}/plugins/${plugin[0]}/${plugin[1]}/${plugin[0]}.hpi" -o "$REF/${plugin[0]}.jpi"
    ...
done
...

The file containing all of the plugins are read from stdin, the : in the plugin entry are removed and the rest are thrown into an array. Then some regexp are applied on the first element (the actual plugin name), as long as the plugin name part are passed through the regexp:ing the download with curl will happen a couple of lines later.

So our plugin entry pretested-integration:2.4.1 (409ae) ends up as pretested-integration 2.4.1 (409ae) (in an array) and when curl are invoked the $plugin[0] will be pretested-integration and $plugin[1] will be 2.4.1. Last but not least the $plugins[2] will be (409ae), but it's never "used" anywhere. Simple as that!

buep commented 6 years ago

but the amount of versions would increase in the plugin download servers if we uploaded a new one per commit (?).

Just because we use the git sha in the release version, doesn't mean we release on pr. commit. We only release once a while.

but if we aren't using the git SHA then we'll "solve" this for now by scrapping it.

I planned to use it, to solve the problem of developer versions used for functional testing, can't be distinguished. For example if pom.xml says 1.0.0-snapshop all builds will install as 1.0.0-snapshot (timestamp) and thus if I try to run functional testing on a Jenkins master under test with a developer version installed I can't know which version is installed in relation to SHA. That is the reason why SHA was introduced.

mikejoh commented 6 years ago

Just because we use the git sha in the release version, doesn't mean we release on pr. commit. We only release once a while.

For example if pom.xml says 1.0.0-snapshop all builds will install as 1.0.0-snapshot (timestamp) and thus if I try to run functional testing on a Jenkins master under test with a developer version installed I can't know which version is installed in relation to SHA. That is the reason why SHA was introduced.

Makes sense, thanks for explaining!

MadsNielsen commented 6 years ago

I'm trying to figure out where this $buildNumber is actually used, it's not part of the version number at release but it's stored somewhere as part of the metadata for the plugin. The path in the repo does not use this bit of information.

I would just remove this from the POM, as the issue @buep is actually resolved. When you install a locally built pom, and do a manual upload it will display the GIT sha in the update center even without the build number section in the POM. I think the issue we're trying to solve never happen: As in performing a public release of a SNAPSHOT build. As @buep said we can just make a beta now, much cleaner.

So my suggestion: Remove the buildnumber from the POM and do a new release, do you need a 2.4.X release @mikejoh or can you work with a 3.X.X release?

buep commented 6 years ago

I think the issue we're trying to solve never happen: As in performing a public release of a SNAPSHOT build.

Not that will never happen, but I would like to install a version of the plugin to a Jenkins test server so I can run test on the plugin, before releasing it. At that point it will be x.y.z-snapshot build and I need to be able to distinguish them.

That is why the SHA is added, but a buildnumber or something else is also fine. And even having it as part of local builds only is fine.

MadsNielsen commented 6 years ago

Right, if you remove all the elements that has to do with the build number and custom HPI namings, you get this when building the plugin, and installing it locally:

3.0.1-SNAPSHOT (private-9289150a-mads)

That's the 8 first digits of the git sha, private and the username. So the sections are not needed. Will release a new version where this is not included.

buep commented 6 years ago

Great - fine by me. That is a new format, probably from the parent pom. Years ago it wasn't like that.

But cool - go ahead.

Praqma / pretested-integration-plugin

The 5 char git SHA breaks install-plugins.sh #118