Skip to content
This repository has been archived by the owner on Jan 18, 2020. It is now read-only.

Updated plugins from experimental UC #312

Merged
merged 5 commits into from
Oct 10, 2018
Merged

Conversation

jglick
Copy link
Contributor

@jglick jglick commented Oct 8, 2018

Pulls in JEP-210 beta releases. @svanoort @dwnusbaum

node ./prepare-essentials propose-updates
node ./prepare-essentials propose-updates --uc ./update-center.json

propose-experimental-updates: update-center-experimental.json
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just offering an alternate target. Really this stuff should of course be replaced with usage of the updater logic from incrementals-tools so that we get a single consistent way to update everything to whatever is in master.

try {
ok = compareVersions(needed[artifactId].version, plugin.version) == 1;
} catch (x) {
logger.warn(`Cannot compare ${artifactId}:${needed[artifactId].version} to ${plugin.version}: ${x}`);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whatever Node library you are using to compare versions, it is hopeless in the face of the version numbers actually used in Jenkins components. We want to be using jenkinsci/lib-version-number instead. (As, for example, incrementals-tools does.)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I BTW, https://issues.jenkins-ci.org/browse/JENKINS-53767 is probably worth looking at. I agree at some point we'd need a central tool to do all this.

@@ -204,6 +211,7 @@ class ManifestResolver {
return false;
}).forEach((dep) => {
envNeeds[dep] = deps[dep];
// TODO this is printing bogus messages like: The docker-cloud environment also needs docker-plugin
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure what this is all about.

const manifest = Manifest.loadFile();
manifest.getPlugins().forEach((plugin) => {
manifest.getActualPlugins().forEach((plugin) => {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The updater was failing to show updates to what was actually being delivered in status! It only listed newer versions of plugins which were already in spec. As I have noted before, this is not helpful, since for example workflow-aggregator itself does nothing and should have been removed as of Jenkins 2; we need to actually enumerate the plugins we want to ship.

(compareVersions(updated.version, plugin.version) == 1)) {
logger.info(`The update center has a newer ${plugin.artifactId}: ${updated.version}`);
if (updated == null) {
logger.warn(`No such plugin ${plugin.artifactId}`);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hit this at one point, not sure why.

version: '2.7'
- groupId: org.jenkins-ci.plugins.workflow
artifactId: workflow-support
version: 2.21-beta-1
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This block is what I was actually trying to accomplish: pull in JEP-210.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Though I'm not against supporting using the experimental UC, I think it can be useful in some cases, why didn't you just use Incrementals to bump to latest of these?

@@ -8,7 +8,7 @@ spec:
plugins:
- groupId: io.jenkins
artifactId: configuration-as-code
version: '1.0'
version: '1.1'
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just updating everything while I am here.

version: 1.29.3
- groupId: org.jenkins-ci.plugins
artifactId: github-api
version: '1.92'
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems the YAML serializer is more clever than we might like: x.y is quoted as a string but x.y.z is not, even though for our purposes we know all versions ought to be loaded as strings. JavaScript 🤷‍♂️

@@ -87,6 +129,15 @@ spec:
- groupId: org.jenkins-ci.plugins
artifactId: jira
version: 3.0.2
- groupId: org.jenkins-ci.ui
artifactId: ace-editor
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, we sort the status when we write it out; should we not also sort the spec?

(My advice remains to get rid of the separate spec and status sections.)

@jenkinsadmin
Copy link

Build failed; the context from the latest run is:

Expand to view
[Pipeline] [jenkinsciinfra/evergreen-backend] }
[jenkinsciinfra/evergreen-backend] Failed in branch jenkinsciinfra/evergreen-backend
[Pipeline] // parallel
[Pipeline] }
[Pipeline] // stage
[Pipeline] stage
[Pipeline] { (Test images)
Stage "Test images" skipped due to earlier failure(s)
[Pipeline] parallel
[Pipeline] [Docker Cloud image] { (Branch: Docker Cloud image)
[Pipeline] [AWS Cloud image (smokes)] { (Branch: AWS Cloud image (smokes))
[Pipeline] [Docker Cloud image] stage
[Pipeline] [Docker Cloud image] { (Docker Cloud image)
[Pipeline] [AWS Cloud image (smokes)] stage
[Pipeline] [AWS Cloud image (smokes)] { (AWS Cloud image (smokes))
Stage "Docker Cloud image" skipped due to earlier failure(s)
Stage "AWS Cloud image (smokes)" skipped due to earlier failure(s)
[Pipeline] [Docker Cloud image] }
[Pipeline] [AWS Cloud image (smokes)] }
[Pipeline] [Docker Cloud image] // stage
[Pipeline] [AWS Cloud image (smokes)] // stage
[Pipeline] [Docker Cloud image] }
[Docker Cloud image] Failed in branch Docker Cloud image
[Pipeline] [AWS Cloud image (smokes)] }
[AWS Cloud image (smokes)] Failed in branch AWS Cloud image (smokes)
[Pipeline] // parallel
[Pipeline] }
[Pipeline] // stage
[Pipeline] stage
[Pipeline] { (Publish jenkins/evergreen)
Stage "Publish jenkins/evergreen" skipped due to earlier failure(s)
[Pipeline] }
[Pipeline] // stage
[Pipeline] }
[Pipeline] // timestamps
[Pipeline] }
[Pipeline] // timeout
[Pipeline] }
[Pipeline] // withEnv
[Pipeline] }
[Pipeline] // withEnv
[Pipeline] }
[Pipeline] // node
[Pipeline] End of Pipeline

GitHub has been notified of this commit’s build result

ERROR: script returned exit code 2
Finished: FAILURE

Powered by the Comment Logger

@batmat batmat self-requested a review October 10, 2018 08:04
Copy link
Collaborator

@batmat batmat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. It makes sense to me to support experimental UC in addition to the main one, though I wonder why we shouldn't anyway just jump to just using Incrementals.

But well, supporting this is probably good as we'll always find trickier cases where we'd like to consume experimental releases from non-incrementalified plugins, and having it is a good thing.

version: '2.7'
- groupId: org.jenkins-ci.plugins.workflow
artifactId: workflow-support
version: 2.21-beta-1
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Though I'm not against supporting using the experimental UC, I think it can be useful in some cases, why didn't you just use Incrementals to bump to latest of these?

try {
ok = compareVersions(needed[artifactId].version, plugin.version) == 1;
} catch (x) {
logger.warn(`Cannot compare ${artifactId}:${needed[artifactId].version} to ${plugin.version}: ${x}`);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I BTW, https://issues.jenkins-ci.org/browse/JENKINS-53767 is probably worth looking at. I agree at some point we'd need a central tool to do all this.

@jglick
Copy link
Contributor Author

jglick commented Oct 10, 2018

support experimental UC in addition to the main one, though I wonder why we shouldn't anyway just jump to just using Incrementals

That would be my preference. I was making the minimal change I needed to get this particular set of updates in (with repeatable tooling rather than one-off edits). As noted in #312 (comment) I am not aware of existing tooling to do this.

@jglick
Copy link
Contributor Author

jglick commented Oct 10, 2018

cases where we'd like to consume experimental releases from non-incrementalified plugins

Well, you could always just file a PR to

mvn incrementals:incrementalify

and ship the result. :-)

@rtyler
Copy link
Member

rtyler commented Oct 10, 2018

Going to bring these in before some other updates just to keep the pipes moving

@rtyler rtyler merged commit 4c6ce7c into jenkins-infra:master Oct 10, 2018
@jglick jglick deleted the updates branch October 11, 2018 14:52
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants