Update machine labels job #14
Conversation
VermaSh
force-pushed
the
UpdateMachineLabelsJob
branch
from
0b5c76d
to
1a74c64
Compare
README.md
Outdated
| * ```boolean initialRun``` | ||
| * This is used to add the project or any other required labels on the machine | ||
| * ```String labels``` | ||
| * Labels you would like to be added to the machine. Seperate the labels for a single machine with spaces. Use "," if doing batch update. Or if they will be same across the machines supply only one set |
There was a problem hiding this comment.
This is slightly confusing, but I think I understand it as, a comma-separated list of whitespace separated labels.
I.e.
machine1Label1 machine1Label2, machine2Label1 machine2Label2
Is this correct? Can we make the wording easier to understand?
There was a problem hiding this comment.
yes!
How's this?
String labels- Labels you would like to be added to the machine.
- Each label must be separated by spaces and labels for different machines must be separated by
, - If identical labels need to be applied to all the machines, only one set of labels need to be supplied
- Use Cases:
- Multiple machines, unique labels example:
machine1Label1 machine1Label2, machine2Label1 machine2Label2 - Multiple machines, identical labels example:
Label1 Label2 - Single machine, single set of labels :
Label1 Label2
- Multiple machines, unique labels example:
README.md
Outdated
| * The computers it iterates over can be limited by input parameter, ```projectLabel``` | ||
| * The job expects 5 input parameters | ||
| * ```boolean initialRun``` | ||
| * This is used to add the project or any other required labels on the machine |
There was a problem hiding this comment.
This description doesn't pair well with the boolean name.
README.md
Outdated
| * ```String labels``` | ||
| * Labels you would like to be added to the machine. Seperate the labels for a single machine with spaces. Use "," if doing batch update. Or if they will be same across the machines supply only one set | ||
| * ```String machineNames``` | ||
| * Can either enter a list of names or a single name. For list seperate them with "," |
There was a problem hiding this comment.
Can simply say "a comma-separated list of machine names".
README.md
Outdated
| * ```String machineNames``` | ||
| * Can either enter a list of names or a single name. For list seperate them with "," | ||
| * ```boolean updateDescription``` | ||
| * If this is set true, the job will update labels and update description |
There was a problem hiding this comment.
update labels and update description
This is confusing, is the bool controlling the update labels, update description, or both?
README.md
Outdated
| * ```boolean updateDescription``` | ||
| * If this is set true, the job will update labels and update description | ||
| * ```String projectlabel``` | ||
| * This limits which machines will be touched |
There was a problem hiding this comment.
I assume this parameter cannot be used at the same time as the machineNames parameter?
There was a problem hiding this comment.
They can be. However, if there is a mismatch between the machine name and the project label, the job won't touch it
README.md
Outdated
| * This limits which machines will be touched | ||
| * Use Cases: | ||
| * Update labels: | ||
| * Objective: add default, os, arch, and kernel lables |
There was a problem hiding this comment.
what is default?
typo lables -> labels
README.md
Outdated
| * Procedure: supply labels and machine names | ||
| * Update description: | ||
| * It adds CPU count, Disk space and installed RAM to the description | ||
| * Side note: this is implemented here instead of in CreateNewNode job because sometimes the machine may not have the needed information available in time for the script to udpate the description |
There was a problem hiding this comment.
Don't think this side note is needed here. Is there a better spot to document this?
VermaSh
force-pushed
the
UpdateMachineLabelsJob
branch
2 times, most recently
from
178e359
to
c75359f
Compare
| for (int index = 0; index < machineNames.length; index++) { | ||
| println "Pre-update description of ${machineNames[index]}: ${nodeHelper.getDescription(machineNames[index])}" | ||
| if (!params.projectLabel.equals("all") | ||
| && nodeHelper.getLabels(machineNames[index]).contains(params.projectLabel)) { |
There was a problem hiding this comment.
Shouldn't this be if (all || contains(label))?
There was a problem hiding this comment.
that's what I meant to write, but I think auto correct negated the condition.
| } | ||
|
|
||
| if (params.overWriteLabels && labels == null) { | ||
| error("No labels supplied for overWriteLabels option") |
There was a problem hiding this comment.
why is this case an error? How to I overWrite with just preconfigured labels?
There was a problem hiding this comment.
it's to prevent erasing old labels if nothing is supplied. In order to over write preconfigured labels, you'll have to do a over write with the non preconfigured labels and then run the job with no labels and overWriteLabels option not set.
Initially the motivation behind having it this way was to prevent non preconfigured labels from being overwritten. But I see your point, I'll update the code
- This is part of what NodeMaintaince job will be doing - This job only updates labels and desription - Updated readme for UpdateMachineIdentifiers - Replaced initial_run with overWriteLabels - Added functionality to allow user to overwrite pre-configured labels issue adoptium#10 Signed-off-by: Shubham Verma <shubhamv.sv@gmail.com>
VermaSh
force-pushed
the
UpdateMachineLabelsJob
branch
from
755fe26
to
a83d74f
Compare
I have added the update label and description part of NodeMaintaince job. This is a standalone job that calls the API to update labels and description.
This PR depends on my previous PR, Added storage monitoring Jenkins job