Extract interfaces for objects to be used through the executors widget #9749

Vlatombe · 2024-09-18T11:38:03Z

This is required for CloudBees CI HA support: we provide alternate implementations of these interfaces to represent computers and related objects that exist in different physical replicas of the same logical instance.

This allows to build an aggregated view of computers and executors, some local, some remote.

To avoid wording conflicts with existing concrete classes, the new interfaces are prefixed with I e.g. IComputer. I'm open to alternative naming suggestions, but unfortunately the names that would represent them the most accurately are already taken.

This introduces a new extension point, ComputerSource, allowing to override the list of computers that is returned via ComputerSet. Only the implementation with the highest ordinal among those registered will be picked up. The default implementation returns local Computer instances, and has ordinal -1, allowing a plugin to override it with a simple @Extension without any explicit ordinal being set.

See JENKINS-XXXXX.

Testing done

Proposed changelog entries

N/A

Proposed upgrade guidelines

N/A

Submitter checklist

Give feedback

The Jira issue, if it exists, is well-described.
The changelog entries and upgrade guidelines are appropriate for the audience affected by the change (users or developers, depending on the change) and are in the imperative mood (see examples). Fill in the Proposed upgrade guidelines section only if there are breaking changes or changes that may require extra steps from users during upgrade.
There is automated testing or an explanation as to why this change has no tests.
New public classes, fields, and methods are annotated with @Restricted or have @since TODO Javadocs, as appropriate.
New deprecations are annotated with @Deprecated(since = "TODO") or @Deprecated(forRemoval = true, since = "TODO"), if applicable.
New or substantially changed JavaScript is not defined inline and does not call eval to ease future introduction of Content Security Policy (CSP) directives (see documentation).
For dependency updates, there are links to external changelogs and, if possible, full differentials.
For new APIs and extension points, there is a link to at least one consumer.
Options

Desired reviewers

@mention

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

Maintainer checklist

Give feedback

There are at least two (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 is not blocking the change.
Changelog entries in the pull request title and/or Proposed changelog entries are accurate, human-readable, and in the imperative mood.
Proper changelog labels are set so that the changelog can be generated automatically.
If the change needs additional upgrade steps from users, the upgrade-guide-needed label is set and there is a Proposed upgrade guidelines section in the pull request title (see 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).
Options

This is required for CloudBees CI HA support: we provide alternate implementations of these interfaces to represent computers and related objects that exist in different physical replicas of the same logical instance. This allows to be build an aggregated view of computers and executors, some local, some remote.

Vlatombe · 2024-09-18T11:42:41Z

core/src/main/java/hudson/model/Computer.java

-    /**
-     * A value class to provide a consistent snapshot view of the state of an executor to avoid race conditions
-     * during rendering of the executors list.
-     *
-     * @since 1.607
-     */
-    @Restricted(NoExternalUse.class)
-    public static class DisplayExecutor implements ModelObject {


Moved to its own file.

Vlatombe · 2024-09-18T11:43:37Z

core/src/main/java/hudson/model/Computer.java

-            return executor.equals(that.executor);
-        }
-
-        @Extension(ordinal = Double.MAX_VALUE)


(ignore WS)

Vlatombe · 2024-09-18T11:49:35Z

core/src/main/java/hudson/model/Queue.java

-        /**
-         * Works just like {@link #checkAbortPermission()} except it indicates the status by a return value,
-         * instead of exception.
-         * Also used by default for {@link hudson.model.Queue.Item#hasCancelPermission}.
-         * <p>
-         * NOTE: If you have implemented {@link AccessControlled} this returns by default
-         * {@code return hasPermission(hudson.model.Item.CANCEL);}
-         *
-         * @return false
-         *      if the user doesn't have the permission.
-         */
-        default boolean hasAbortPermission() {
-            if (this instanceof AccessControlled) {
-                return ((AccessControlled) this).hasPermission(CANCEL);
-            }
-            return true;
-        }
-


Moved to ITask

jglick

Minor questions about Javadoc and nullability.

core/src/main/java/hudson/model/Computer.java

core/src/main/java/hudson/model/ComputerSet.java

core/src/main/java/jenkins/model/IComputer.java

core/src/main/java/jenkins/model/ModelObjectWithContextMenu.java

core/src/main/java/jenkins/model/queue/ITask.java

Vlatombe · 2024-09-18T14:34:55Z

core/src/main/java/hudson/model/Computer.java

     */
    @Exported
    public String getIconClassName() {
-        return getIcon();
+        return IComputer.super.getIconClassName();


Only here for @Exported

Vlatombe · 2024-09-18T14:35:06Z

core/src/main/java/hudson/model/queue/SubTask.java

- * <p>
- * Plugins are encouraged to extend from {@link AbstractSubTask}
- * instead of implementing this interface directly, to maintain
- * compatibility with future changes to this interface.
- *


While I'm here... something missed in https://github.com/jenkinsci/jenkins/pull/2879/files#diff-fe633c0d4d59f5cbd8fc0437fc0ac478b781deb33afaba9d6ed142a1154ac1f0R27

jglick

Still unsure about ITask.getUrl (#9749 (comment)).

jglick · 2024-09-18T15:36:52Z

core/src/main/java/hudson/model/queue/SubTask.java

- * <p>
- * Plugins are encouraged to extend from {@link AbstractSubTask}
- * instead of implementing this interface directly, to maintain
- * compatibility with future changes to this interface.
- *


jglick

#9749 (comment) still unclear

core/src/main/java/jenkins/model/IComputer.java

jglick · 2024-09-18T16:19:19Z

test/src/test/java/hudson/model/ProjectTest.java

@@ -953,6 +953,10 @@ public String getDisplayName() {
        }


+        @Override
+        public String getUrl() {
+            return "sometask/";


Maybe there should be a default impl in SubTask for compatibility reasons?

Vlatombe requested review from jglick and a team September 18, 2024 11:38

Vlatombe added the skip-changelog Should not be shown in the changelog label Sep 18, 2024

Vlatombe commented Sep 18, 2024

View reviewed changes

Vlatombe added 2 commits September 18, 2024 13:44

Fix Deprecated annotation

96d4f27

Missing @OverRide annotations

e879007

Vlatombe commented Sep 18, 2024

View reviewed changes

Missing @SInCE TODO

877640d

jglick reviewed Sep 18, 2024

View reviewed changes

Vlatombe added 4 commits September 18, 2024 16:25

No longer true according to AbstractSubTask javadoc

32585b2

Restore old signature for compatibility

7118bc2

Fix reviews

1e715b1

Javadoc

4513cb0

Vlatombe commented Sep 18, 2024

View reviewed changes

Typo

ef81082

Vlatombe requested a review from jglick September 18, 2024 14:37

jglick reviewed Sep 18, 2024

View reviewed changes

core/src/main/java/jenkins/model/IComputer.java Outdated Show resolved Hide resolved

core/src/main/java/jenkins/model/IComputer.java Outdated Show resolved Hide resolved

Fix inconsistency between hasOfflineCause and getOfflineCauseReason

742cee8

jglick approved these changes Sep 18, 2024

View reviewed changes

Remove default implementation of ITask#getUrl

4cf4a80

jglick reviewed Sep 18, 2024

View reviewed changes

Provide default impl in SubTask for compatibility

91f0dd9

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Extract interfaces for objects to be used through the executors widget #9749

Extract interfaces for objects to be used through the executors widget #9749

Vlatombe commented Sep 18, 2024 •

edited

Loading

Submitter checklist

Maintainer checklist

Vlatombe Sep 18, 2024

Vlatombe Sep 18, 2024

Vlatombe Sep 18, 2024

jglick left a comment

Vlatombe Sep 18, 2024

Vlatombe Sep 18, 2024 •

edited

Loading

jglick Sep 18, 2024

jglick left a comment •

edited

Loading

jglick Sep 18, 2024

jglick left a comment

jglick Sep 18, 2024

Extract interfaces for objects to be used through the executors widget #9749

Are you sure you want to change the base?

Extract interfaces for objects to be used through the executors widget #9749

Conversation

Vlatombe commented Sep 18, 2024 • edited Loading

Testing done

Proposed changelog entries

Proposed upgrade guidelines

Submitter checklist

Desired reviewers

Maintainer checklist

Vlatombe Sep 18, 2024

Choose a reason for hiding this comment

Vlatombe Sep 18, 2024

Choose a reason for hiding this comment

Vlatombe Sep 18, 2024

Choose a reason for hiding this comment

jglick left a comment

Choose a reason for hiding this comment

Vlatombe Sep 18, 2024

Choose a reason for hiding this comment

Vlatombe Sep 18, 2024 • edited Loading

Choose a reason for hiding this comment

jglick Sep 18, 2024

Choose a reason for hiding this comment

jglick left a comment • edited Loading

Choose a reason for hiding this comment

jglick Sep 18, 2024

Choose a reason for hiding this comment

jglick left a comment

Choose a reason for hiding this comment

jglick Sep 18, 2024

Choose a reason for hiding this comment

Vlatombe commented Sep 18, 2024 •

edited

Loading

Vlatombe Sep 18, 2024 •

edited

Loading

jglick left a comment •

edited

Loading