feat: update prometheus agent daemonset proposal #7571
feat: update prometheus agent daemonset proposal #7571slashexx wants to merge 1 commit intoprometheus-operator:mainfrom
Conversation
mviswanathsai
left a comment
There was a problem hiding this comment.
Great start! 🥳
I have a few concerns that I have mentioned.
4a349dd to
3db062c
Compare
|
@mviswanathsai I've added updated text about
|
|
As discussed in the call we can explore the servicemonitor option as a separate feature gate so it doesn't block making current feature GA. |
12a6a10 to
643c2c3
Compare
|
@slashpai @mviswanathsai @simonpasquier I added the remaining selectors and fields, requesting review to see if there are any that I'm missing. |
| CEL validation will provide immediate feedback during `kubectl apply` but we will need runtime validation logic in the controller as a fallback mechanism. This fallback will be integrated directly in the `PrometheusAgent` reconciler loop. | ||
|
|
||
| This is mainly because : | ||
| 1. CEL validation will require Kubernetes version 1.25+ and hence not all users might have CEL supported clusters. |
There was a problem hiding this comment.
CEL has been there for quite some time now (1.25 is 3 years-old) so we can also update our min requirements (no cloud provider should be offering 1.25 by now).
There was a problem hiding this comment.
@simonpasquier then should we drop the runtime validations entirely ? We already have a PR in place.
Or should we specify that while runtime validations exist, most validations will be handled by CEL ? Exceptions include bare metal environments.
There was a problem hiding this comment.
Even when running self-managed Kubernetes you should be upgrading to a reasonably recent version :)
I don't have a strong opinion but I wouldn't mind if we need to require at least 1.25. Maybe a discussion item for the next office hours meeting.
|
cc @slashpai @mviswanathsai ptal at the comments once and let's finalize. |
|
gentle ping @slashpai @mviswanathsai |
slashpai
left a comment
There was a problem hiding this comment.
lgtm
@simonpasquier if no objections we can merge this :)
15bc52b to
f874955
Compare
Add comprehensive CEL (Common Expression Language) validation rules to the agent daemonset proposal documentation, including: - x-kubernetes-validations for field validation - Runtime fallback logic for validation handling - Service monitor section refactoring - Endpointslice section documentation - Related field blocks and validation requirements
f874955 to
d1b7a92
Compare
|
@slashpai done ! |
|
@simonpasquier could you please take a look again |
Description
Describe the big picture of your changes here to communicate to the maintainers why we should accept this pull request.
If it fixes a bug or resolves a feature request, be sure to link to that issue.
Draft PR to start off the change of last years DaemonSet proposal.
Type of change
What type of changes does your code introduce to the Prometheus operator? Put an
xin the box that apply.CHANGE(fix or feature that would cause existing functionality to not work as expected)FEATURE(non-breaking change which adds functionality)BUGFIX(non-breaking change which fixes an issue)ENHANCEMENT(non-breaking change which improves existing functionality)NONE(if none of the other choices apply. Example, tooling, build system, CI, docs, etc.)Verification
Please check the Prometheus-Operator testing guidelines for recommendations about automated tests.
Changelog entry
Please put a one-line changelog entry below. This will be copied to the changelog file during the release process.