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

[JENKINS-63971] prepare for casc support of node monitors #189

Merged
merged 5 commits into from Dec 1, 2023

Conversation

mawinter69
Copy link
Contributor

@mawinter69 mawinter69 commented Oct 24, 2023

to support node monitors in casc, is is essential that they have DataBoundConstructors defined and a proper symbol is used.

Testing done

Tested with jenkinsci/configuration-as-code-plugin#2392 that the monitors are properly exported and can be imported.

export looks like this:

nodeMonitors:
  - diskSpaceMonitor:
      freeSpaceThreshold: "3GB"
  - tmpSpace:
      freeSpaceThreshold: "3GB"
  - jvmVersion:
      comparisonMode: EXACT_MATCH
      disconnect: true
  - inodesMonitor:
      inodesPercentThreshold: "98%"
  - remotingVersion:
      ignored: true

Submitter checklist

Edit tasklist title
Beta Give feedback Tasklist Submitter checklist, more options

Delete tasklist

Delete tasklist block?
Are you sure? All relationships in this tasklist will be removed.
  1. Make sure you are opening from a topic/feature/bugfix branch (right side) and not your main branch!
    Options
  2. Ensure that the pull request title represents the desired changelog entry
    Options
  3. Please describe what you did
    Options
  4. Link to relevant issues in GitHub or Jira
    Options
  5. Link to relevant pull requests, esp. upstream and downstream changes
    Options
  6. Ensure you have provided tests - that demonstrates feature works or fixes the issue
    Options

to support node monitors in casc, is is essential that they have
DataBoundConstructors defined and a proper symbol is used.
@mawinter69 mawinter69 requested a review from a team as a code owner October 24, 2023 15:26
Copy link
Contributor

@MarkEWaite MarkEWaite left a comment

Choose a reason for hiding this comment

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

This looks great to me. Thanks very much. I need to test it before I merge it. That may take a few days because I am currently traveling and won't return until the end of the week.

@MarkEWaite MarkEWaite added enhancement Improvement or new feature major-rfe Major improvement or new feature and removed enhancement Improvement or new feature labels Oct 24, 2023
@jonesbusy
Copy link
Contributor

Thanks as well. Adding JCasC support for for nodeMonitor is a must.

I was testing similar changes when implementing #192 and I confirm changes in the descriptor behave as expected

@MarkEWaite
Copy link
Contributor

I've confirmed that with the configuration as code pull request and this pull request, I see the configuration as code entries. The configuration as code pull request is required before the results of this pull request will be visible to users.

@MarkEWaite MarkEWaite marked this pull request as draft November 17, 2023 19:11
@MarkEWaite
Copy link
Contributor

I've marked this pull request as a draft because the required pull configuration as code pull request is a draft.

@mawinter69
Copy link
Contributor Author

@MarkEWaite can this be merged now?

@MarkEWaite MarkEWaite merged commit db638ec into jenkinsci:master Dec 1, 2023
14 checks passed
@MarkEWaite
Copy link
Contributor

Yes. Thanks for the pull request @mawinter69 !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
major-rfe Major improvement or new feature
Projects
None yet
3 participants