jenkins
jenkins copied to clipboard
Colored node labels
See JENKINS-XXXXX.
New page with all known 'node labels'
todo: I know it the font is for some reason broken, but I does not know why. Will check it little later.
Labels in node view:
Label view:
Label config page:
Proposed changelog entries
- Colored node labels
- Show all existing labels
Proposed upgrade guidelines
N/A
Submitter checklist
- [ ] (If applicable) Jira issue is well described
- [ ] Changelog entries and upgrade guidelines are appropriate for the audience affected by the change (users or developer, depending on the change) and are in the imperative mood. Examples
- Fill-in the
Proposed changelog entries
section only if there are breaking changes or other changes which may require extra steps from users during the upgrade
- Fill-in the
- [ ] Appropriate autotests or explanation to why this change has no tests
- [ ] New public classes, fields, and methods are annotated with
@Restricted
or have@since TODO
Javadoc, as appropriate. - [ ] For dependency updates: links to external changelogs and, if possible, full diffs
Desired reviewers
@mention
Maintainer checklist
Before the changes are marked as ready-for-merge
:
- [ ] There are at least 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 does not block the change
- [ ] Changelog entries in the PR 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,
upgrade-guide-needed
label is set and there is aProposed upgrade guidelines
section in the PR title. (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).
When this gets merged, #6498 is probably obsolete
Currently there is no other way of providing a new link in the ComputerSet page other than directly in core. So maybe it makes sense to have some kind of ComputerSetAction as extension point. This would allow to provide this functionality in a plugin. I would also be interested in such an extension point.
When this gets merged, #6498 is probably obsolete
not rely. I think this can be good combinate together.
Or make something like this:
and configure page:
But I think it will be much more easy to make a plugin for that, that allow more features in node monitoring (I am currently working on extend-node-monitoring plugin and need this PR for that)
Testing this PR does not work for me, on any agent page after adding a label:
org.apache.commons.jelly.JellyTagException: jar:file:/root/.jenkins/war/WEB-INF/lib/jenkins-core-2.359-SNAPSHOT.jar!/hudson/model/Computer/index.jelly:83:81: <st:include> No page found '../Label/Badge/index.jelly' for class hudson.model.labels.LabelAtom
Testing this PR does not work for me, on any agent page after adding a label:
org.apache.commons.jelly.JellyTagException: jar:file:/root/.jenkins/war/WEB-INF/lib/jenkins-core-2.359-SNAPSHOT.jar!/hudson/model/Computer/index.jelly:83:81: <st:include> No page found '../Label/Badge/index.jelly' for class hudson.model.labels.LabelAtom
strange, checked on my side now, and it works. I will debug it later
Solely from a UI/accessibility perspective, I think it'd be better to limit the potential colors to only those supported by the Jenkins Palette - https://weekly.ci.jenkins.io/design-library/Colors/
This way the colors can change depending on theme and it'd be harder for a user to pick a color that isn't readable etc.
It works for GitHub, why wouldn't it work for Jenkins?
All (well, way more than ~20) colors are readable if the label color is generated with enough contrast to the chosen background color.
About themes and limited count of colors. That was my first try. But like @daniel-beck mentioned in github it works as well, so I has changed it to simple color-picker. So the administrator needs select useful colors. And we can set little border over label depend on theme.
Currently there is no other way of providing a new link in the ComputerSet page other than directly in core. So maybe it makes sense to have some kind of ComputerSetAction as extension point. This would allow to provide this functionality in a plugin. I would also be interested in such an extension point.
I can try that. Bur not sure if I can do that 🤔. We will see.
Testing this PR does not work for me, on any agent page after adding a label:
org.apache.commons.jelly.JellyTagException: jar:file:/root/.jenkins/war/WEB-INF/lib/jenkins-core-2.359-SNAPSHOT.jar!/hudson/model/Computer/index.jelly:83:81: <st:include> No page found '../Label/Badge/index.jelly' for class hudson.model.labels.LabelAtom
Same here, I'm not able to visit the page either. Full stacktrace
I think it'd be better to limit the potential colors to only those supported by the Jenkins Palette - https://weekly.ci.jenkins.io/design-library/Colors/
This way the colors can change depending on theme and it'd be harder for a user to pick a color that isn't readable etc.
A middle way could be used to recommend and advertise the usage of predefined "native Jenkins" colors you can click to select, which have theming support, but keeping up the flexibility to define your own colors, if desired.
Testing this PR does not work for me, on any agent page after adding a label:
org.apache.commons.jelly.JellyTagException: jar:file:/root/.jenkins/war/WEB-INF/lib/jenkins-core-2.359-SNAPSHOT.jar!/hudson/model/Computer/index.jelly:83:81: <st:include> No page found '../Label/Badge/index.jelly' for class hudson.model.labels.LabelAtom
Same here, I'm not able to visit the page either. Full stacktrace
I do not understand it. I have deleted whole war/target directory and rebuild it again and it works (on my side :-) ). So when I understand the stack-trace correctly:
org.apache.commons.jelly.JellyTagException: jar:file:/Users/alex/.jenkins/war/WEB-INF/lib/jenkins-core-2.360-SNAPSHOT.jar!/hudson/model/Computer/index.jelly:83:81: <st:include> No page found '../Label/Badge/index.jelly' for class hudson.model.labels.LabelAtom
It tries to find Label/Badge/index.jelly but it didn find. The file is committed and exist also in war file:
maybe are my build options 'wrong' mvn -am -pl war,bom -DskipTests -Dspotbugs.skip -Dspotless.check.skip -Dcheckstyle.failOnViolation=false clean install && mvn -pl war jetty:run
Developed && tested on Windows. But I also do not see caseSensitive failure for unix paths.
Can somebody help me please?
I think it'd be better to limit the potential colors to only those supported by the Jenkins Palette - https://weekly.ci.jenkins.io/design-library/Colors/
This way the colors can change depending on theme and it'd be harder for a user to pick a color that isn't readable etc.
A middle way could be used to recommend and advertise the usage of predefined "native Jenkins" colors you can click to select, which have theming support, but keeping up the flexibility to define your own colors, if desired.
This is very good idea. @janfaracik is there any picker for jenkins-colors? Before I start with the implementation? Or a programmatic way to get all supported colors (css variables)
Given recent discussions in this PR it seems like there are nontrivial changes pending, so I'll hold off on another round of security review for now.
This is very good idea. @janfaracik is there any picker for jenkins-colors? Before I start with the implementation? Or a programmatic way to get all supported colors (css variables)
There isn't a picker sadly/nor a programatic way to get the colours. You'd have to hardcode the list sadly, the list of available colours in Jenkins (with vars + classes) is here https://weekly.ci.jenkins.io/design-library/Colors/
Testing this PR does not work for me, on any agent page after adding a label:
org.apache.commons.jelly.JellyTagException: jar:file:/root/.jenkins/war/WEB-INF/lib/jenkins-core-2.359-SNAPSHOT.jar!/hudson/model/Computer/index.jelly:83:81: <st:include> No page found '../Label/Badge/index.jelly' for class hudson.model.labels.LabelAtom
After making sure the PR builds if you skip tests (triaging SpotBugs is still up to do), I run into the issue still. There's no difference if I use a fresh or regular setup, the core PR tester or launching via the command line.
I recommend to pause further reviews until this is resolved, given core functionality cannot be warranted in the current state, hence I'll mark the PR as draft.
Feel free to change that once the PR is ready to be reviewed and tested again.
Please take a moment and address the merge conflicts of your pull request. Thanks!
Interesting conflict. @janfaracik How do you suggest the merge conflict be resolved here? Seems awkward to have one "configuration" option in the app bar, and the other in the side panel. But how many items can the app bar realistically hold before it looks silly?
Sorry guys, I really forgot to continue here. Hopefully will found time in next feature to continue here.
OK, I recheckt it again, After a "little" pause I recognized, that there are few things to do. I will probably close this PR, and provide few smaller PRs