V1alpha2 Metrics collector (part 1) #484
Conversation
| getreq := &api.GetExperimentRequest{ | ||
| ExperimentName: *experimentName, | ||
| } | ||
| getrep, err := c.GetExperiment(ctx, getreq) |
There was a problem hiding this comment.
Should we consider populating metric names similar to other flags? We can then avoid this API call, which will reduce the API dependency. We can avoid Manager Service flag too
There was a problem hiding this comment.
Manager service is still needed to make the Report call
manifests/v1alpha2/katib-controller/metricsControllerConfigMap.yaml
Outdated
Show resolved
Hide resolved
| metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" | ||
| "k8s.io/apimachinery/pkg/labels" | ||
| "k8s.io/client-go/kubernetes" | ||
| restclient "k8s.io/client-go/rest" |
There was a problem hiding this comment.
Can we use controller runtime client itself instead of directly using client-go?
There was a problem hiding this comment.
This is only used to get the in-cluster config, which needs the REST client. The controller runtime client also calls rest client under the hood: https://github.com/kubernetes-sigs/controller-runtime/blob/master/pkg/client/config/config.go#L78
There was a problem hiding this comment.
Changed to controller-runtime
| apiv1 "k8s.io/api/core/v1" | ||
| metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" | ||
| "k8s.io/apimachinery/pkg/labels" | ||
| "k8s.io/client-go/kubernetes" |
There was a problem hiding this comment.
According to https://stackoverflow.com/questions/53852530/how-to-get-logs-from-kubernetes-using-golang, we cannot get logs without using client-go for now.
|
/lgtm |
richardsliu
changed the title
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: richardsliu 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 |
Part of #442
This change is