[CHORE] starting operator deployment #4
Conversation
nicolastakashi
force-pushed
the
chore/adding-k8s-client
branch
from
fade119 to
2586048
Compare
nicolastakashi
requested review from
ArthurSens and
simonpasquier
and removed request for
ArthurSens and
simonpasquier
| "k8s.io/client-go/tools/clientcmd" | ||
| ) | ||
|
|
||
| func NewPrometheusOperatorV1(logger *slog.Logger, kubeConfig string) (*opClientv1.MonitoringV1Client, error) { |
There was a problem hiding this comment.
Do we really need this? Can't we reuse the client code from the operator repository?
Here's a code snippet showing how we use today:
There was a problem hiding this comment.
On a similar note, I'm also not sure if extra code is needed to set up a k8s client 🤔
There was a problem hiding this comment.
I think you were mentining this code snippet to create the k8s clientset and not the po client set right?
For the Prometheus ClientSet it's almost the same thing, the different is only the way we're building the rest.Config object.
The operator is expecting the API Server Address and here I'm reading a kubeconfig file where the user que provide one or we can discovery it automatically.
Regarding the Prometheus Operator ClientSet I don't see other way to do that, since we have two different client, one for v1 and other for alpha1, maybe I'm missing something.
There was a problem hiding this comment.
No, I meant both clients actually.
We already have abstractions to create both Kubernetes and Monitoring clients inside prometheus-operator codebase. I don't think we need this internal package here
There was a problem hiding this comment.
Ok!
I'll review it carefully next week 🫶
There was a problem hiding this comment.
@ArthurSens I just open a PR on the operator doing the changes to allow use the NewClusterConfig function, meanwhile I changed the code to create the rest config on the poctl and use the clientset as you show in the example.
Do you think we can merge it and fix the TODO after the operator merge the changes?
nicolastakashi
force-pushed
the
chore/adding-k8s-client
branch
from
2586048 to
dcc3aa1
Compare
Signed-off-by: Nicolas Takashi <nicolas.tcs@hotmail.com>
nicolastakashi
force-pushed
the
chore/adding-k8s-client
branch
from
dcc3aa1 to
537dbea
Compare
| if err != nil { | ||
| logger.With("error", err.Error()).Error("error while getting kubeconfig") | ||
| os.Exit(1) | ||
| } |
There was a problem hiding this comment.
@nicolastakashi, noticed just now 😬
The err handling here is duplicated
There was a problem hiding this comment.
I'll fix in a following PR
I'm adding the foundations to deploy the operator manifests, it's working but at the moment it requires you manually apply the crds to the cluster.
I'll do the crds installation automatically in a up coming pr.