-
Notifications
You must be signed in to change notification settings - Fork 56
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
fix: test case multi-namespace_support #312
Conversation
d9c07d0
to
a3862f7
Compare
a3862f7
to
d7400d5
Compare
Name: "prometheus", | ||
Image: "quay.io/prometheus/prometheus:v2.39.1", | ||
Name: "prometheus-example-app", | ||
Image: "quay.io/openshifttest/prometheus-example-app@sha256:382dc349f82d730b834515e402b48a9c7e2965d0efbc42388bd254f424f6193e", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if we should instead rely on what upstream Prometheus Operator is using ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would prefer to use this one, for I run the upstream test cases on OCP with all kinds of profiles share between QE team during ObO release, the image will be mirrored by default and the test cases can get the image and run on disconnected or private OCP clusters, other wise, the case will failed on private and disconnected OCP cluster.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@lihongyan1 would you mind leaving a comment that describes this and a link to the upstream that is mirrored? Just for future us when someone asks why we don't use the upstream version :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added comments
Name: "prometheus-example-app", | ||
Image: "quay.io/openshifttest/prometheus-example-app@sha256:382dc349f82d730b834515e402b48a9c7e2965d0efbc42388bd254f424f6193e", | ||
SecurityContext: &corev1.SecurityContext{ | ||
AllowPrivilegeEscalation: &bF, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: consider pointer.Bool
- https://pkg.go.dev/k8s.io/utils/pointer#Bool
Name: "prometheus-example-app", | ||
Image: "quay.io/openshifttest/prometheus-example-app@sha256:382dc349f82d730b834515e402b48a9c7e2965d0efbc42388bd254f424f6193e", | ||
SecurityContext: &corev1.SecurityContext{ | ||
AllowPrivilegeEscalation: &bF, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: consider pointer.Bool
- https://pkg.go.dev/k8s.io/utils/pointer#Bool
@@ -861,7 +861,7 @@ func namespaceSelectorTest(t *testing.T) { | |||
|
|||
promClient := framework.NewPrometheusClient("http://localhost:9090") | |||
if pollErr := wait.Poll(5*time.Second, 5*time.Minute, func() (bool, error) { | |||
query := `prometheus_build_info{namespace=~"test-ns-.*"}` | |||
query := `version{namespace=~"test-ns-.*"}` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is there a different metric that we can reply on that is specific to prometheus_example
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, there are the following metrics, for detail you can refer web site, none of them are specific
version - of type gauge - containing the app version - as a constant metric value 1 and label version, representing this app version
http_requests_total - of type counter - representing the total numbere of incoming HTTP requests
http_request_duration_seconds - of type histogram, representing duration of all HTTP requests
http_request_duration_seconds_count- total count of all incoming HTTP requeests
http_request_duration_seconds_sum - total duration in seconds of all incoming HTTP requests
http_request_duration_seconds_bucket
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know what you mean, I changed the metric as version{pod="prometheus-example-app",namespace=~"test-ns-.*"}
, thanks for the comments
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the patch @lihongyan1. The approach looks good to me. Some comments that needs addressing.
39e0e60
to
c5cbc06
Compare
2131315
to
569c89e
Compare
569c89e
to
db37e9b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
Fix issue #311
Prometheus pod need more resources and has complicated configuration, replace it with prometheus example app pod