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

Display appropriate GUI that accurately displays offline by design #9883

Merged
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 12 additions & 32 deletions core/src/main/java/hudson/model/Computer.java
Original file line number Diff line number Diff line change
Expand Up @@ -355,6 +355,7 @@ public AnnotatedLargeText<Computer> getLogText() {
* null if the system was put offline without given a cause.
*/
@Exported
@Override
public OfflineCause getOfflineCause() {
var node = getNode();
if (node != null) {
Expand All @@ -374,19 +375,7 @@ public boolean hasOfflineCause() {
@Exported
@Override
public String getOfflineCauseReason() {
var offlineCause = getOfflineCause();
if (offlineCause == null) {
return "";
}
// fetch the localized string for "Disconnected By"
String gsub_base = hudson.slaves.Messages.SlaveComputer_DisconnectedBy("", "");
// regex to remove commented reason base string
String gsub1 = "^" + gsub_base + "[\\w\\W]* \\: ";
// regex to remove non-commented reason base string
String gsub2 = "^" + gsub_base + "[\\w\\W]*";

String newString = offlineCause.toString().replaceAll(gsub1, "");
return newString.replaceAll(gsub2, "");
return IComputer.super.getOfflineCauseReason();
}

/**
Expand Down Expand Up @@ -677,6 +666,14 @@ public boolean isTemporarilyOffline() {
return node != null && node.isTemporarilyOffline();
}

/**
* Allows a caller to define an {@link OfflineCause} for a computer that has never been online.
* @since TODO
*/
public void setOfflineCause(OfflineCause cause) {
this.offlineCause = cause;
}

/**
* @deprecated as of 1.320.
* Use {@link #setTemporaryOfflineCause(OfflineCause)}
Expand Down Expand Up @@ -736,37 +733,20 @@ public String getTemporaryOfflineCauseReason() {
@Exported
@Override
public String getIcon() {
// The machine was taken offline by someone
if (isTemporarilyOffline() && getOfflineCause() instanceof OfflineCause.UserCause) return "symbol-computer-disconnected";
// The computer is not accepting tasks, e.g. because the availability demands it being offline.
if (!isAcceptingTasks()) {
return "symbol-computer-not-accepting";
}
// The computer is not connected or it is temporarily offline due to a node monitor
if (isOffline()) return "symbol-computer-offline";
return "symbol-computer";
return IComputer.super.getIcon();
}

/**
* {@inheritDoc}
*
* <p>
* It is both the recommended and default implementation to serve different icons based on {@link #isOffline}.
* @see #getIcon()
*/
@Exported
@Override
public String getIconClassName() {
return IComputer.super.getIconClassName();
}

public String getIconAltText() {
// The machine was taken offline by someone
if (isTemporarilyOffline() && getOfflineCause() instanceof OfflineCause.UserCause) return "[temporarily offline by user]";
// There is a "technical" reason the computer will not accept new builds
if (isOffline() || !isAcceptingTasks()) return "[offline]";
return "[online]";
}

@Exported
@Override public @NonNull String getDisplayName() {
return nodeName;
Expand Down
57 changes: 45 additions & 12 deletions core/src/main/java/hudson/slaves/OfflineCause.java
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
import hudson.model.User;
import java.io.ObjectStreamException;
import java.util.Collections;
import java.util.Date;
import jenkins.agents.IOfflineCause;
import jenkins.model.Jenkins;
import org.jvnet.localizer.Localizable;
import org.kohsuke.accmod.Restricted;
Expand All @@ -51,28 +51,20 @@
* @since 1.320
*/
@ExportedBean
public abstract class OfflineCause {
public abstract class OfflineCause implements IOfflineCause {
protected final long timestamp = System.currentTimeMillis();

/**
* Timestamp in which the event happened.
* {@inheritDoc}
*
* @since 1.612
*/
@Exported
@Override
public long getTimestamp() {
return timestamp;
}

/**
* Same as {@link #getTimestamp()} but in a different type.
*
* @since 1.612
*/
public final @NonNull Date getTime() {
return new Date(timestamp);
}

/**
* @deprecated Only exists for backward compatibility.
* @see Computer#setTemporarilyOffline(boolean)
Expand Down Expand Up @@ -192,6 +184,24 @@
}
return this;
}

@Override
@NonNull
public String getComputerIcon() {
return "symbol-computer-disconnected";
}

@Override
@NonNull
public String getComputerIconAltText() {
return "[temporarily offline by user]";
}

@NonNull
@Override
public String getIcon() {
return "symbol-person";

Check warning on line 203 in core/src/main/java/hudson/slaves/OfflineCause.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Not covered lines

Lines 191-203 are not covered by tests
}
}

public static class ByCLI extends UserCause {
Expand All @@ -212,5 +222,28 @@
public IdleOfflineCause() {
super(hudson.slaves.Messages._RetentionStrategy_Demand_OfflineIdle());
Copy link
Member

Choose a reason for hiding this comment

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

If this wasn't shown before and now is I'd suggest changing the text from computer to agent

Offline because computer was idle doesn't make sense as a computer isn't user facing terminology, (not mentioned in https://www.jenkins.io/doc/book/glossary/), it should be agent instead.

Copy link
Member Author

Choose a reason for hiding this comment

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

This was already shown before, just not before the agent was online, then became offline again as instructed by the retention strategy.

}

@Override
@NonNull
public String getComputerIcon() {
return "symbol-computer-paused";
}

@Override
@NonNull
public String getComputerIconAltText() {
return "[will connect automatically whenever needed]";
}

@Override
@NonNull
public String getIcon() {
return "symbol-pause";
}

@Override
public String getStatusClass() {
return "info";

Check warning on line 246 in core/src/main/java/hudson/slaves/OfflineCause.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Not covered lines

Lines 235-246 are not covered by tests
}
}
}
2 changes: 2 additions & 0 deletions core/src/main/java/hudson/slaves/RetentionStrategy.java
Original file line number Diff line number Diff line change
Expand Up @@ -272,6 +272,8 @@
logger.log(Level.INFO, "Launching computer {0} as it has been in demand for {1}",
new Object[]{c.getName(), Util.getTimeSpanString(demandMilliseconds)});
c.connect(false);
} else if (c.getOfflineCause() == null) {
c.setOfflineCause(new OfflineCause.IdleOfflineCause());

Check warning on line 276 in core/src/main/java/hudson/slaves/RetentionStrategy.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Not covered lines

Lines 275-276 are not covered by tests
}
} else if (c.isIdle()) {
final long idleMilliseconds = System.currentTimeMillis() - c.getIdleStartMilliseconds();
Expand Down
108 changes: 108 additions & 0 deletions core/src/main/java/jenkins/agents/IOfflineCause.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,108 @@
/*
* The MIT License
*
* Copyright 2024 CloudBees, Inc.
*
* Permission is hereby granted, free of charge, to any person obtaining a copy
* of this software and associated documentation files (the "Software"), to deal
* in the Software without restriction, including without limitation the rights
* to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
* copies of the Software, and to permit persons to whom the Software is
* furnished to do so, subject to the following conditions:
*
* The above copyright notice and this permission notice shall be included in
* all copies or substantial portions of the Software.
*
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
* AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
* THE SOFTWARE.
*/
package jenkins.agents;

import edu.umd.cs.findbugs.annotations.NonNull;
import hudson.Util;
import java.util.Date;
import java.util.Objects;
import jenkins.model.IComputer;

/**
* Represents a cause that puts a {@linkplain IComputer#isOffline() computer offline}.
* @since TODO
*/
public interface IOfflineCause {
/**
* @return The icon to use for the computer that has this offline cause. It will be displayed in the build executor status widget, as well as in nodes list screen.
*/
@NonNull
default String getComputerIcon() {
return "symbol-computer-offline";
}

/**
* @return The alt text for the icon returned by {@link #getComputerIcon()}.
*/
@NonNull
default String getComputerIconAltText() {
return "[offline]";

Check warning on line 50 in core/src/main/java/jenkins/agents/IOfflineCause.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Not covered line

Line 50 is not covered by tests
}

/**
* @return The icon to render this offline cause. It will be displayed in the build executor status widget.
*/
@NonNull
default String getIcon() {
return "symbol-error";
}

/**
* @return The reason why this offline cause exists.
* <p>
* For implementers: this can use HTML formatting, so make sure to only include trusted content.
*/
@NonNull
default String getReason() {
// fetch the localized string for "Disconnected By"
String gsub_base = hudson.slaves.Messages.SlaveComputer_DisconnectedBy("", "");
// regex to remove commented reason base string
String gsub1 = "^" + gsub_base + "[\\w\\W]* \\: ";
// regex to remove non-commented reason base string
String gsub2 = "^" + gsub_base + "[\\w\\W]*";
return Objects.requireNonNull(Util.escape(toString().replaceAll(gsub1, "").replaceAll(gsub2, "")));
}

/**
* @return A short message (one word) that summarizes the offline cause.
*
* <p>
* For implementers: this can use HTML formatting, so make sure to only include trusted content.
*/
@NonNull
default String getMessage() {
return Messages.IOfflineCause_offline();

Check warning on line 85 in core/src/main/java/jenkins/agents/IOfflineCause.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Not covered line

Line 85 is not covered by tests
}

/**
* @return the CSS class name that should be used to render the status.
*/
@SuppressWarnings("unused") // jelly
default String getStatusClass() {
return "warning";
}

/**
* Timestamp in which the event happened.
*/
long getTimestamp();

/**
* Same as {@link #getTimestamp()} but in a different type.
*/
@NonNull
default Date getTime() {
return new Date(getTimestamp());
}
}
51 changes: 48 additions & 3 deletions core/src/main/java/jenkins/model/IComputer.java
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
import hudson.security.AccessControlled;
import java.util.List;
import java.util.concurrent.Future;
import jenkins.agents.IOfflineCause;
import org.jenkins.ui.icon.Icon;
import org.jenkins.ui.icon.IconSet;
import org.kohsuke.accmod.Restricted;
Expand Down Expand Up @@ -91,6 +92,12 @@
return Util.fixEmpty(getOfflineCauseReason()) != null;
}

/**
* @return the offline cause if the computer is offline.
* @since TODO
*/
IOfflineCause getOfflineCause();

/**
* If the computer was offline (either temporarily or not),
* this method will return the cause as a string (without user info).
Expand All @@ -101,7 +108,12 @@
* empty string if the system was put offline without given a cause.
*/
@NonNull
String getOfflineCauseReason();
default String getOfflineCauseReason() {
if (getOfflineCause() == null) {
return "";
}
return getOfflineCause().getReason();
}

/**
* @return true if the node is currently connecting to the Jenkins controller.
Expand All @@ -115,12 +127,45 @@
*
* @see #getIconClassName()
*/
String getIcon();
default String getIcon() {
// The computer is not accepting tasks, e.g. because the availability demands it being offline.
if (!isAcceptingTasks()) {

Check warning on line 132 in core/src/main/java/jenkins/model/IComputer.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Partially covered line

Line 132 is only partially covered, one branch is missing
return "symbol-computer-not-accepting";

Check warning on line 133 in core/src/main/java/jenkins/model/IComputer.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Not covered line

Line 133 is not covered by tests
}
var offlineCause = getOfflineCause();
if (offlineCause != null) {
return offlineCause.getComputerIcon();
}
// The computer is not connected or it is temporarily offline due to a node monitor
if (isOffline()) return "symbol-computer-offline";
return "symbol-computer";
}

/**
* Returns the alternative text for the computer icon.
*/
String getIconAltText();
@SuppressWarnings("unused") // jelly
default String getIconAltText() {
if (!isAcceptingTasks()) {

Check warning on line 149 in core/src/main/java/jenkins/model/IComputer.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Partially covered line

Line 149 is only partially covered, one branch is missing
return "[suspended]";

Check warning on line 150 in core/src/main/java/jenkins/model/IComputer.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Not covered line

Line 150 is not covered by tests
}
var offlineCause = getOfflineCause();
if (offlineCause != null) {

Check warning on line 153 in core/src/main/java/jenkins/model/IComputer.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Partially covered line

Line 153 is only partially covered, one branch is missing
return offlineCause.getComputerIconAltText();

Check warning on line 154 in core/src/main/java/jenkins/model/IComputer.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Not covered line

Line 154 is not covered by tests
}
// There is a "technical" reason the computer will not accept new builds
if (isOffline()) return "[offline]";
return "[online]";
}

default String getTooltip() {
var offlineCause = getOfflineCause();
if (offlineCause != null) {
return offlineCause.toString();
} else {
return "";
}
}

/**
* Returns the class name that will be used to look up the icon.
Expand Down
Loading
Loading