-
Notifications
You must be signed in to change notification settings - Fork 357
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
MON-3866: create separate metrics client cert for metrics server #2329
base: master
Are you sure you want to change the base?
Conversation
slashpai
commented
Apr 23, 2024
- I added CHANGELOG entry for this change.
- No user facing changes, so no entry in CHANGELOG was needed.
@slashpai: This pull request references Jira Issue OCPBUGS-32510, which is invalid:
Comment The bug has been updated to refer to the pull request using the external bug tracker. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
cc @machine424 |
/jira refresh |
@slashpai: This pull request references Jira Issue OCPBUGS-32510, which is valid. The bug has been moved to the POST state. 3 validation(s) were run on this bug
Requesting review from QA contact: In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
data := make(map[string]string) | ||
|
||
for k, v := range tlsSecret.Data { |
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.
This is followup change as in #2315
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: slashpai The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
…cs-server"" Fix is added in openshift/cluster-monitoring-operator#2329 This reverts commit a7a9d56.
…cs-server"" Fix is added in openshift/cluster-monitoring-operator#2329 This reverts commit a7a9d56.
/test e2e-aws-ovn-techpreview |
/payload-job periodic-ci-openshift-hypershift-release-4.16-periodics-e2e-aws-ovn-conformance |
@slashpai: trigger 1 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/d9e6b810-015c-11ef-8aff-ee7e9eefe979-0 |
/payload-job periodic-ci-openshift-hypershift-release-4.16-periodics-e2e-aws-ovn-conformance openshift/api#1865 |
/payload-job-with-prs periodic-ci-openshift-hypershift-release-4.16-periodics-e2e-aws-ovn-conformance openshift/api#1865 Sorry I missed the different command name. |
@jan--f: An error was encountered. No known errors were detected, please see the full error message for details. Full error message.
unable to get additional pr info from string: I: string: I doesn't match expected format: org/repo#number
Please contact an administrator to resolve this issue. |
/payload-job-with-prs periodic-ci-openshift-hypershift-release-4.16-periodics-e2e-aws-ovn-conformance openshift/api#1865 |
@jan--f: trigger 1 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/d8dbc4d0-0169-11ef-80b3-937f07d81a34-0 |
@slashpai: trigger 1 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/e5caf660-0214-11ef-8541-c562082f3eaa-0 |
@machine424 Please take a look again if latest changes are good |
fb321d1
to
11936a2
Compare
pkg/manifests/manifests.go
Outdated
// is rotated. | ||
dep.Spec.Template.Annotations["monitoring.openshift.io/metrics-client-cert-hash"] = hashByteMap(metricsClientCert.Data) | ||
// TODO(slashpai): Remove this line release after metrics server is GA | ||
dep.Spec.Template.Annotations["monitoring.openshift.io/metrics-client-certs-hash-"] = "" |
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.
My bad, we're not using library-go's utils for Deployement resources yet. So no need for this. I'll give this a try locally.
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.
Tested with 4dcd2e4 on the cluster bot, all seem ok.
Got some "dial tcp 172.30.0.1:443: connect: connection refused", but as I explain on the ticket, they're not metrics-server specific.
We just need to check out the payload tests.
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.
For hypershift, we're getting a weird:
"""
2024-04-24T17:21:29.994703270Z W0424 17:21:29.994657 1 tasks.go:73] task 8 of 16: UpdatingMetricsServer failed: waiting for metrics-server-client-certs secret failed: waiting for secret openshift-monitoring/metrics-server-client-certs: context deadline exceeded
"""
payload: https://pr-payload-tests.ci.openshift.org/runs/ci/e77c90a0-0259-11ef-92b5-46f4e3c5ea9b-0
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 just realized that CMO never creates these secrets (the prometheus' one as well), it delegates that to the CSR controller.
Apparently there is an issue there, the metrics-server CSR controller is not able to create the secret.
pkg/operator/operator.go
Outdated
kubeInformersOperatorNS.Core().V1().Secrets(), | ||
o.client.KubernetesInterface().CoreV1(), | ||
o.client.EventRecorder(), | ||
"OpenShiftMonitoringClientCertRequester", |
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.
maybe chose another name to differentiate it from the other one.
4775411
to
74d3a50
Compare
/payload-job-with-prs periodic-ci-openshift-hypershift-release-4.16-periodics-e2e-aws-ovn-conformance openshift/api#1865 |
@slashpai: trigger 1 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/f972a990-02a7-11ef-85c8-8a3d6e2fa727-0 |
f430ce4
to
d6dc8a2
Compare
/payload-job-with-prs periodic-ci-openshift-hypershift-release-4.16-periodics-e2e-aws-ovn-conformance openshift/api#1865 |
@slashpai: trigger 1 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/aaa566b0-02b4-11ef-86e4-14f7e6cef418-0 |
I am working on update for this |
Signed-off-by: Jayapriya Pai <janantha@redhat.com>
/payload-job-with-prs periodic-ci-openshift-release-master-nightly-4.16-e2e-aws-ovn-single-node openshift/api#1865 let's re-test single-noie, I think I saw it fail |
@machine424: trigger 1 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/b85820b0-02ee-11ef-8b3a-9bacb4e24e64-0 |
/hold |
@slashpai: This pull request references MON-3866 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.16.0" version, but no target version was set. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
@slashpai: The following tests failed, say
Full PR test history. Your PR dashboard. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |