fix(pruneOrphan): convert nomad expirtyTime from ns to ms

[arsiesys]

We are using Nomad 1.4.3 and jenkins 2.361.4.

I noticed that the pruneOrphan feature wasn't working on our side.
After digging, I noticed two anomalies:

• The submitTime returned in the nomad Job Definition by the nomad API is using nanoseconds and the plugins seems to assume it's Milliseconds. Which means the parsed date was incorrect and the prune never happening.
• The pluggin add the timeout value to the parsed expirtyTime but do not attribute the output to a variable, as a consequence, it wasn't used.

I address the previous anomalies by converting the parsed values to milliseconds and update the expirtyTime variable with the added timeout.

I did a small fix that seems to work on our side based on my first tests:

Dec 19, 2022 9:17:45 PM FINE org.jenkinsci.plugins.nomad.NomadCloud
Found Orphaned Node: jenkins-xx-xxx-4c83d5ce4910a6 in namespace default in region global

I am far away of being an expert with Java. If this PR need some "touch up", please help! :)

Thanks !

[j3t]

Thanks for the fix!

You have to fix the "SpotBugs-finding" first in order to get the build green. Suggestion, just remove Math.round like below:

Instant expiryTime = Instant.ofEpochMilli(job.getLong("SubmitTime") / 1000000);
expiryTime = expiryTime.plusSeconds(this.workerTimeout * 60L);

[arsiesys]

I used math.Float because ofEpochMilli is expececting a long and the divide to convert to ns generate a number with decimals result of a BigDecimal type. The Math.Round solved both issues (remove the decimals + return a long).
I pushed a different way to do it (and tested it), hope it's a better way ! :D

groovy.lang.MissingMethodException: No signature of method: static java.time.Instant.toEpochMilli() is applicable for argument types: (java.math.BigDecimal) values: [1671487054166.230409]
Possible solutions: toEpochMilli(), ofEpochMilli(long)

[j3t]

I adjusted the expiry time calculation so that it is more clear. Could you please check whether this is working for you?

[arsiesys]

It's working :D !

[arsiesys]

Also, I think this fix should come with a warning.. As this feature wasn't working before.
In the case of multiple separated jenkins instance using the same nomad configuration and mostly the same "job prefix" name, instance A could remove the job of the instance B if they use the same prefix. Each jenkins instance using the same nomad cluster should use differents prefix in their nomad templates.

[j3t]

Also, I think this fix should come with a warning.. As this feature wasn't working before.

Makes sense, we could add something like this to the release notes I guess.

In the case of multiple separated jenkins instance using the same nomad configuration and mostly the same "job prefix" name, instance A could remove the job of the instance B if they use the same prefix. Each jenkins instance using the same nomad cluster should use differents prefix in their nomad templates.

tbh, I'm not familiar with the prune feature but I guess you are right, that could be the case. We should add a warning which is displayed right to the prune option so that the user is at least aware of the risk. Maybe we could also migrate the cloud configurations so that the prune feature is deactivated for all existing configurations and must be activated actively.

[aarnaud]

Hi,
When this PR can be merge, it's a bugfix we need because nomad jobs aren't clear correctly.
If the there is breaking changes / behaviour changes we can change the version to increment the major version ?

[aarnaud]

I tested and running this patch in our environment its work well