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.

Sign up for GitHub

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

Logging improvments. #74

Merged
merged 2 commits into from
Jul 28, 2017
Merged

Logging improvments. #74

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
344aaca
Logging improvments.
pjdarton Jun 28, 2017
9ff20bb
Add javadoc for vSphereCloudLauncher.findOurVsInstance()
pjdarton Jun 28, 2017
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
33 changes: 21 additions & 12 deletions src/main/java/org/jenkinsci/plugins/vSphereCloud.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,7 @@ public class vSphereCloud extends Cloud {

private static java.util.logging.Logger VSLOG = java.util.logging.Logger.getLogger("vsphere-cloud");

private static void InternalLog(Slave slave, SlaveComputer slaveComputer, TaskListener listener, Throwable ex, String format, Object... args) {
final Level logLevel = Level.INFO;
private static void InternalLog(Slave slave, SlaveComputer slaveComputer, TaskListener listener, Throwable ex, Level logLevel, String format, Object... args) {
if (!VSLOG.isLoggable(logLevel) && listener == null)
return;
String s = "";
Expand All @@ -91,44 +90,54 @@ private static void InternalLog(Slave slave, SlaveComputer slaveComputer, TaskLi
}
}

/** Logs an {@link Level#INFO} message (created with {@link String#format(String, Object...)}). */
public static void Log(String format, Object... args) {
InternalLog(null, null, null, null, format, args);
InternalLog(null, null, null, null, Level.INFO, format, args);
}

/** Logs an {@link Level#SEVERE} message (created with {@link String#format(String, Object...)}) and a {@link Throwable} with stacktrace. */
public static void Log(Throwable ex, String format, Object... args) {
InternalLog(null, null, null, ex, format, args);
InternalLog(null, null, null, ex, Level.SEVERE, format, args);
}

/** Logs a {@link Level#INFO} message (created with {@link String#format(String, Object...)}). */
public static void Log(TaskListener listener, String format, Object... args) {
InternalLog(null, null, listener, null, format, args);
InternalLog(null, null, listener, null, Level.INFO, format, args);
}

/** Logs a {@link Level#SEVERE} message (created with {@link String#format(String, Object...)}) and a {@link Throwable} (with stacktrace). */
public static void Log(TaskListener listener, Throwable ex, String format, Object... args) {
InternalLog(null, null, listener, ex, format, args);
InternalLog(null, null, listener, ex, Level.SEVERE, format, args);
}

/** Logs a {@link Level#INFO} message (created with {@link String#format(String, Object...)}), prefixed by the {@link Slave} name. */
public static void Log(Slave slave, String format, Object... args) {
InternalLog(slave, null, null, null, format, args);
InternalLog(slave, null, null, null, Level.INFO, format, args);
}

/** Logs a {@link Level#SEVERE} message (created with {@link String#format(String, Object...)}) and a {@link Throwable} (with stacktrace), prefixed by the {@link Slave} name. */
public static void Log(Slave slave, Throwable ex, String format, Object... args) {
InternalLog(slave, null, null, ex, format, args);
InternalLog(slave, null, null, ex, Level.SEVERE, format, args);
}

/** Logs a {@link Level#INFO} message (created with {@link String#format(String, Object...)}), prefixed by the {@link Slave} name. */
public static void Log(Slave slave, TaskListener listener, String format, Object... args) {
InternalLog(slave, null, listener, null, format, args);
InternalLog(slave, null, listener, null, Level.INFO, format, args);
}

/** Logs a {@link Level#SEVERE} message (created with {@link String#format(String, Object...)}) and a {@link Throwable} (with stacktrace), prefixed by the {@link Slave} name. */
public static void Log(Slave slave, TaskListener listener, Throwable ex, String format, Object... args) {
InternalLog(slave, null, listener, ex, format, args);
InternalLog(slave, null, listener, ex, Level.SEVERE, format, args);
}

/** Logs a {@link Level#INFO} message (created with {@link String#format(String, Object...)}), prefixed by the {@link SlaveComputer} name. */
public static void Log(SlaveComputer slave, TaskListener listener, String format, Object... args) {
InternalLog(null, slave, listener, null, format, args);
InternalLog(null, slave, listener, null, Level.INFO, format, args);
}

/** Logs a {@link Level#SEVERE} message (created with {@link String#format(String, Object...)}) and a {@link Throwable} (with stacktrace), prefixed by the {@link SlaveComputer} name. */
public static void Log(SlaveComputer slave, TaskListener listener, Throwable ex, String format, Object... args) {
InternalLog(null, slave, listener, ex, format, args);
InternalLog(null, slave, listener, ex, Level.SEVERE, format, args);
}

@Deprecated
Expand Down
9 changes: 9 additions & 0 deletions src/main/java/org/jenkinsci/plugins/vSphereCloudLauncher.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,15 @@ private Object readResolve() throws ObjectStreamException {
return this;
}

/**
* Find the {@link vSphereCloud} for this {@link vSphereCloudLauncher}, or
* dies trying.
*
* @return The {@link vSphereCloud}. It will not return null.
* @throws RuntimeException
* if it cannot find the {@link vSphereCloud} - e.g. if it's
* been deleted or the description has changed.
*/
public vSphereCloud findOurVsInstance() throws RuntimeException {
if (vsDescription != null && vmName != null) {
for (vSphereCloud cloud : vSphereCloud.findAllVsphereClouds(null)) {
Expand Down
15 changes: 11 additions & 4 deletions src/main/java/org/jenkinsci/plugins/vSphereCloudProvisionedSlave.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,13 +57,20 @@ public vSphereCloudProvisionedSlave(String name, String nodeDescription,
@Override
protected void _terminate(TaskListener listener) throws IOException, InterruptedException {
super._terminate(listener);
final vSphereCloudLauncher launcher = (vSphereCloudLauncher) getLauncher();
if (launcher != null) {
final vSphereCloud cloud = launcher.findOurVsInstance();
if (cloud != null) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a reason this cloud != null check was removed? Is an NPE possible here?

Copy link
Member Author

Choose a reason for hiding this comment

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

findOurVsInstance() will throw an exception - it won't return null.

Copy link
Member Author

Choose a reason for hiding this comment

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

I managed to miss off the extra Javadoc I'd added to make this clear from my PR.
I've added another commit that adds this.
(my local code had this javadoc already, but I'm in the process of un-picking my changes into a series of discrete PRs, and this comment got missed out. Sorry for the confusion)

Copy link
Contributor

Choose a reason for hiding this comment

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

np. thanks!

try {
final ComputerLauncher l = getLauncher();
final vSphereCloudLauncher launcher = l instanceof vSphereCloudLauncher ? (vSphereCloudLauncher) l : null;
if (launcher != null) {
final vSphereCloud cloud = launcher.findOurVsInstance();
final String cloneName = this.getComputer().getName();
cloud.provisionedSlaveHasTerminated(cloneName);
} else {
vSphereCloud.Log(listener, "%1s._terminate for vmName %2s failed as getLauncher() returned %3s",
getClass().getSimpleName(), getVmName(), l);
}
} catch (RuntimeException ex) {
vSphereCloud.Log(listener, ex, "%1s._terminate for vmName %2s failed",
getClass().getSimpleName(), getVmName());
}
}

Expand Down
6 changes: 4 additions & 2 deletions src/main/java/org/jenkinsci/plugins/vSphereCloudSlave.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,10 +142,12 @@ public String toString() {
return "Shutting down VSphere Cloud Slave";
}
});
vSphereCloud.Log(this, listener, "Disconnected computer");
vSphereCloud.Log(this, listener, "Disconnected computer %s", vmName);
} else {
vSphereCloud.Log(this, listener, "Can't disconnect computer for %s as there was no Computer node for it.", vmName);
}
} catch(Exception e) {
vSphereCloud.Log(this, listener, e, "Can't disconnect");
vSphereCloud.Log(this, listener, e, "Can't disconnect %s", vmName);
}
}

Expand Down