-
Notifications
You must be signed in to change notification settings - Fork 444
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
OCPBUGS-1357: Fix install-plugins.sh to not override locked plugin versions #1482
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -147,6 +147,7 @@ copy_reference_file() { | |
|
||
REF_DIR=${REF:-/opt/openshift/plugins} | ||
FAILED="$REF_DIR/failed-plugins.txt" | ||
WARNING="$REF_DIR/warning-plugins.txt" | ||
BUNDLE_PLUGINS=${BUNDLE_PLUGINS:-/opt/openshift/plugins/bundle-plugins.txt} | ||
JENKINS_WAR=${JENKINS_WAR:-/usr/lib/jenkins/jenkins.war} | ||
JENKINS_UC=${JENKINS_UC:-https://updates.jenkins.io} | ||
|
@@ -172,9 +173,8 @@ function download() { | |
plugin="$1" | ||
version="${2:-latest}" | ||
ignoreLockFile="${3:-}" | ||
lock="$(getLockFile "$plugin")" | ||
|
||
if [[ $ignoreLockFile ]] || mkdir "$lock" &>/dev/null; then | ||
if [[ $ignoreLockFile ]] || ! test -f $(getLockFile $plugin); then | ||
if ! doDownload "$plugin" "$version"; then | ||
# some plugin don't follow the rules about artifact ID | ||
# typically: docker-plugin | ||
|
@@ -194,6 +194,13 @@ function download() { | |
fi | ||
|
||
resolveDependencies "$plugin" | ||
else | ||
lockFile=$(getLockFile "$plugin") | ||
lockedVersion=$(cat $lockFile) | ||
echo "Plugin $plugin locked to version $lockedVersion, ignoring, requested version $version" | ||
if versionLT "${lockedVersion}" "${version}"; then | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. what will we do with the warning message? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The problem is that sometimes the versions are like |
||
echo "Manual update from ${plugin}:${lockedVersion} to ${plugin}:${version} in base-plugins.txt may be needed." >> $WARNING | ||
fi | ||
fi | ||
} | ||
|
||
|
@@ -310,7 +317,7 @@ function resolveDependencies() { | |
# download the dependence; passing "true" is needed for "download" to replace the existing dependency | ||
if versionLT "${versionInstalled}" "${minVersion}"; then | ||
echo "Upgrading bundled dependency $d ($minVersion > $versionInstalled)" | ||
download "$plugin" "$minVersion" "true" | ||
download "$plugin" "$minVersion" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Was the bug here? I don't see a signature change to the download function, however, it was used only with 2 arguments previously, the 2nd one being a boolean. no regression ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since this was passing |
||
else | ||
echo "Skipping already bundled dependency $d ($minVersion <= $versionInstalled)" | ||
fi | ||
|
@@ -328,7 +335,7 @@ function resolveDependencies() { | |
# version of the plugin | ||
if versionLT "${previouslyDownloadedVersion}" "${minVersion}"; then | ||
echo "Upgrading previously downloaded plugin $plugin at $previouslyDownloadedVersion to $minVersion" | ||
download "$plugin" "$minVersion" "true" | ||
download "$plugin" "$minVersion" | ||
fi | ||
fi | ||
done | ||
|
@@ -411,7 +418,7 @@ main() { | |
continue | ||
fi | ||
echo "Locking $plugin" | ||
mkdir "$(getLockFile "${plugin%%:*}")" | ||
echo "$(versionFromPlugin $plugin)" > "$(getLockFile "${plugin%%:*}")" | ||
done | ||
|
||
echo -e "\nAnalyzing war: $JENKINS_WAR" | ||
|
@@ -449,8 +456,25 @@ main() { | |
echo "Installed plugins:" | ||
installedPlugins | ||
|
||
echo -e "\nVerifying Locked Plugins in Bundle..." | ||
for plugin in `cat $@ | grep -v ^#`; do | ||
if [ -z $plugin ]; then | ||
continue | ||
fi | ||
if grep -q "^${plugin}$" $BUNDLE_PLUGINS; then | ||
echo "Found $plugin" | ||
else | ||
echo "Missing $plugin" | ||
echo "Plugin $plugin not found in $BUNDLE_PLUGINS" >> $FAILED | ||
fi | ||
done | ||
|
||
if [[ -f $WARNING ]]; then | ||
echo -e "\nSome warnings were encountered!\n$(<"$WARNING")" >&2 | ||
fi | ||
|
||
if [[ -f $FAILED ]]; then | ||
echo -e "\nSome plugins failed to download!\n$(<"$FAILED")" >&2 | ||
echo -e "\nSome errors were encountered!\n$(<"$FAILED")" >&2 | ||
exit 1 | ||
fi | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is there any reason to change the lockfile from a directory to a file?
I don't know the historical reason of having a directory, but, if we can minimize changes to this old code, that would be more reinsuring.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am storing the locked plugin version in the
<plugin>.lock
file for use when checking the versions that are being requested.