Propagate Metric status changes to the API. #7525
Conversation
knative-prow-robot
added
the
size/L
There was a problem hiding this comment.
@markusthoemmes: 0 warnings.
In response to this:
Proposed Changes
The Metric status changes have never been reflected to the API itself, which make them not very useful. This does a few things:
- Move status determination into the reconciler.
- Connect the collector with the reconciler to be able to inform it about changes of status. The mechanism is the same as for the Multiscaler/KPA reconciler.
- Actually set a status that is reflected into the API.
Release Note
Metric status is now reflected in the API./assign @taragu @vagababov
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/test-infra repository.
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: markusthoemmes 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 |
knative-prow-robot
added
approved
| logger = logger.Named("collector").With( | ||
| zap.String(logkey.Key, fmt.Sprintf("%s/%s", metric.Namespace, metric.Name))) | ||
| key := types.NamespacedName{Namespace: metric.Namespace, Name: metric.Name} | ||
| logger = logger.Named("collector").With(zap.String(logkey.Key, key.String())) |
There was a problem hiding this comment.
We do the zap.String thing everywhere we use logkey.Key and a stringified "namespace/name"
| @@ -66,5 +66,7 @@ func NewController( | |||
| }, | |||
| }) | |||
|
|
|||
| collector.Watch(impl.EnqueueKey) | |||
There was a problem hiding this comment.
Here it presumes metrics name == pa name.
Probably fine, but 🤷♂
There was a problem hiding this comment.
I don't think it does? The collector manages metrics and this keys off of metrics. Both are Metric names.
|
The following is the coverage report on the affected files.
|
Proposed Changes
The Metric status changes have never been reflected to the API itself, which make them not very useful. This does a few things:
Release Note
/assign @taragu @vagababov