Skip to content
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

[JENKINS-55582] Split instance-identity to a plugin #5304

Closed

Conversation

jglick
Copy link
Member

@jglick jglick commented Feb 23, 2021

Downstream of jenkinsci/instance-identity-plugin#17. Requires #5049 because sshd depends on instance-identity (for real).

Proposed changelog entries

  • Converting the instance-identity module into a (detached) plugin.

Proposed upgrade guidelines

Normally the new plugin will be automatically installed. If you use an “as-code” installation mechanism like a text file with a list of plugins, you may need to add instance-identity (version 3.0 or later) in order to restore this functionality.

Maintainer checklist

Before the changes are marked as ready-for-merge:

  • There are at least 2 approvals for the pull request and no outstanding requests for change
  • Conversations in the pull request are over OR it is explicit that a reviewer does not block the change
  • Changelog entries in the PR title and/or Proposed changelog entries are correct
  • Proper changelog labels are set so that the changelog can be generated automatically
  • If the change needs additional upgrade steps from users, upgrade-guide-needed label is set and there is a Proposed upgrade guidelines section in the PR title. (example)
  • If it would make sense to backport the change to LTS, a Jira issue must exist, be a Bug or Improvement, and be labeled as lts-candidate to be considered (see query).

bouncycastle-api command-launcher
trilead-api instance-identity
jdk-tool instance-identity
bouncycastle-api jdk-tool
Copy link
Member Author

Choose a reason for hiding this comment

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

Trial-and-error.

I still get errors during the setup wizard because ssh-credentials cannot be installed without updating trilead-api. Not sure if that is somehow related to this PR.

Copy link
Member

Choose a reason for hiding this comment

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

yes, sshd PR worked fine through setup wizard.

Appears to be related to this PR

Also I get this on a clean install on startup:

2021-02-25 21:13:53.773+0000 [id=42]	WARNING	h.ExtensionFinder$GuiceFinder$SezpozModule#configure: Failed to load org.jenkinsci.modules.systemd_slave_installer.SlaveInstallerFactoryImpl
java.lang.ClassNotFoundException: org.jenkinsci.main.modules.instance_identity.InstanceIdentity
	at java.base/java.net.URLClassLoader.findClass(URLClassLoader.java:471)
	at java.base/java.lang.ClassLoader.loadClass(ClassLoader.java:589)
	at java.base/java.lang.ClassLoader.loadClass(ClassLoader.java:522)
	at org.eclipse.jetty.webapp.WebAppClassLoader.loadClass(WebAppClassLoader.java:538)
	at java.base/java.lang.ClassLoader.loadClass(ClassLoader.java:522)
Caused: java.lang.NoClassDefFoundError: Lorg/jenkinsci/main/modules/instance_identity/InstanceIdentity;
	at java.base/java.lang.Class.getDeclaredFields0(Native Method)
	at java.base/java.lang.Class.privateGetDeclaredFields(Class.java:3061)
	at java.base/java.lang.Class.getDeclaredFields(Class.java:2248)
	at hudson.ExtensionFinder$GuiceFinder$SezpozModule.resolve(ExtensionFinder.java:497)
	at hudson.ExtensionFinder$GuiceFinder$SezpozModule.resolve(ExtensionFinder.java:482)
	at hudson.ExtensionFinder$GuiceFinder$SezpozModule.configure(ExtensionFinder.java:525)
	at com.google.inject.AbstractModule.configure(AbstractModule.java:62)
	at com.google.inject.spi.Elements$RecordingBinder.install(Elements.java:340)
	at com.google.inject.spi.Elements.getElements(Elements.java:110)
	at com.google.inject.internal.InjectorShell$Builder.build(InjectorShell.java:138)
	at com.google.inject.internal.InternalInjectorCreator.build(InternalInjectorCreator.java:104)
	at com.google.inject.Guice.createInjector(Guice.java:96)
	at com.google.inject.Guice.createInjector(Guice.java:73)
	at hudson.ExtensionFinder$GuiceFinder.<init>(ExtensionFinder.java:285)
	at java.base/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method)
	at java.base/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:62)
	at java.base/jdk.internal.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)
	at java.base/java.lang.reflect.Constructor.newInstance(Constructor.java:490)
	at java.base/java.lang.Class.newInstance(Class.java:584)
	at net.java.sezpoz.IndexItem.instance(IndexItem.java:181)
	at hudson.ExtensionFinder$Sezpoz._find(ExtensionFinder.java:703)
	at hudson.ExtensionFinder$Sezpoz.find(ExtensionFinder.java:689)
	at hudson.ClassicPluginStrategy.findComponents(ClassicPluginStrategy.java:349)
	at hudson.ExtensionList.load(ExtensionList.java:381)
	at hudson.ExtensionList.ensureLoaded(ExtensionList.java:317)
	at hudson.ExtensionList.getComponents(ExtensionList.java:183)
	at jenkins.model.Jenkins$6.onInitMilestoneAttained(Jenkins.java:1161)
	at jenkins.InitReactorRunner$1.onAttained(InitReactorRunner.java:84)
	at org.jvnet.hudson.reactor.ReactorListener$Aggregator.lambda$onAttained$3(ReactorListener.java:102)
	at org.jvnet.hudson.reactor.ReactorListener$Aggregator.run(ReactorListener.java:109)
	at org.jvnet.hudson.reactor.ReactorListener$Aggregator.onAttained(ReactorListener.java:102)
	at org.jvnet.hudson.reactor.Reactor$1.run(Reactor.java:177)
	at org.jvnet.hudson.reactor.Reactor$Node.run(Reactor.java:117)
	at jenkins.security.ImpersonatingExecutorService$1.run(ImpersonatingExecutorService.java:68)
	at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128)
	at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628)
	at java.base/java.lang.Thread.run(Thread.java:834)

Copy link
Member Author

Choose a reason for hiding this comment

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

Will recheck after merge of #5049.

Copy link
Member

Choose a reason for hiding this comment

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

It’s merged?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes I know, but I need time to retest.

@@ -20,7 +20,7 @@ windows-slaves 1.547 1.0
antisamy-markup-formatter 1.553 1.0
matrix-project 1.561 1.0
junit 1.577 1.0
bouncycastle-api 2.16 2.16.0
bouncycastle-api 2.18 2.16.0
Copy link
Member Author

Choose a reason for hiding this comment

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

Dep of new plugin.

@@ -159,7 +159,6 @@ public void moduleClassesShouldBeWhitelisted() throws Exception {
ClassFilterImpl filter = new ClassFilterImpl();
filter.check("org.jenkinsci.main.modules.cli.auth.ssh.UserPropertyImpl");
filter.check("org.jenkinsci.modules.windows_slave_installer.WindowsSlaveInstaller");
filter.check("org.jenkinsci.main.modules.instance_identity.PageDecoratorImpl");
Copy link
Member Author

Choose a reason for hiding this comment

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

No longer a module, so no longer relevant to test.

war/pom.xml Outdated Show resolved Hide resolved
@timja timja added needs-comments-resolved Comments in the PR need to be addressed before it can be considered for merging rfe For changelog: Minor enhancement. use `major-rfe` for changes to be highlighted labels Feb 25, 2021
@jglick
Copy link
Member Author

jglick commented Feb 28, 2021

Nope, cannot do this yet—need to do slave-installer first: https://github.com/jenkinsci/launchd-slave-installer-module/blob/1264c586db41b11cc1647dd3b126f695fee5c6de/src/main/java/org/jenkinsci/modules/launchd_slave_installer/SlaveInstallerFactoryImpl.java#L27 depends on instance-identity. Better suppress instance-identity from the update center.

Turns out doing them all together, as in #3988, was easier than trying to do one module at a time.

@MarkEWaite MarkEWaite added the unresolved-merge-conflict There is a merge conflict with the target branch. label Apr 15, 2021
@jglick
Copy link
Member Author

jglick commented May 3, 2021

Now would like to merge slave-installer impls. Waiting to see if jenkinsci/windows-slave-installer-module#41 gets merged.

@jglick
Copy link
Member Author

jglick commented Dec 8, 2021

Have not had time to work on this.

@basil
Copy link
Member

basil commented May 13, 2022

Completed in #6570.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-comments-resolved Comments in the PR need to be addressed before it can be considered for merging rfe For changelog: Minor enhancement. use `major-rfe` for changes to be highlighted unresolved-merge-conflict There is a merge conflict with the target branch.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants