feat: Add support the loadbalancer role in OpenStackSDConfig
#7356
Conversation
Signed-off-by: dongjiang <dongjiang1989@126.com>
|
Lets add e2e test as well https://github.com/prometheus-operator/prometheus-operator/blob/main/test/e2e/scrapeconfig_test.go#L2679 as we have been doing refactoring ScrapeConfig CRD |
|
It would be good to add e2e test for "Valid |
Signed-off-by: dongjiang <dongjiang1989@126.com>
Thanks @slashpai |
| // | ||
| // Note: The `loadbalancer` or `Loadbalancer` role requires Prometheus >= v3.2.0. | ||
| // | ||
| // +kubebuilder:validation:Enum=Instance;instance;Hypervisor;hypervisor;Loadbalancer;loadbalancer |
There was a problem hiding this comment.
(nit)
| // +kubebuilder:validation:Enum=Instance;instance;Hypervisor;hypervisor;Loadbalancer;loadbalancer | |
| // +kubebuilder:validation:Enum=Instance;Hypervisor;Loadbalancer |
There was a problem hiding this comment.
(suggestion) It might make sense here to define a OpenStackRole type. Something like:
// +kubebuilder:validation:Enum=Instance;Hypervisor;Loadbalancer
type OpenStackRole string
const (
OpenStackRoleInstance OpenStackRole = "Instance"
OpenStackRoleHypervisor OpenStackRole = "Hypervisor"
OpenStackRoleLoadBalancer OpenStackRole = "LoadBalancer"
)
We will also need to update the rest of the code base to use these constants.
There was a problem hiding this comment.
Hmmm.... 🤔 Removing a historically supported field is a breaking change
There was a problem hiding this comment.
we're ok breaking the API here because it's v1alpha1. In fact we plan on making all breaking changes before v1beta1. I would only advise to remove the lower-case enums in a separate PR. For this PR, I'd add only LoadBalancer.
There was a problem hiding this comment.
Thanks @simonpasquier @mviswanathsai
Add only LoadBalancer in this PR.
Will remove the lower-case enums in next PR.
|
Why are the AM Config Reloader tests failing? |
Co-authored-by: M Viswanath Sai <110663831+mviswanathsai@users.noreply.github.com>
Maybe other problem |
Co-authored-by: Simon Pasquier <spasquie@redhat.com>
Signed-off-by: dongjiang <dongjiang1989@126.com>
Signed-off-by: dongjiang <dongjiang1989@126.com>
…theus-operator#7356) --------- Signed-off-by: dongjiang <dongjiang1989@126.com> Co-authored-by: M Viswanath Sai <110663831+mviswanathsai@users.noreply.github.com> Co-authored-by: Simon Pasquier <spasquie@redhat.com>
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.
Add support the
loadbalancerrole in OpenStackSDConfigType 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.