Clusters Client #1814
Clusters Client #1814
Conversation
| if len(errs) > 0 { | ||
| return fmt.Errorf("failed to list resources: %s", errs) | ||
| } |
There was a problem hiding this comment.
To clarify: if one of these Lists fails we will return the whole thing as a failure? Is this what we want?
There was a problem hiding this comment.
I made myself the same question, and I don't think the end goal would be this, but we need to coordinate with the UI to settle down the best way to handle this "partial success" response.
core/clustersmngr/client.go
Outdated
| ) | ||
|
|
||
| var ( | ||
| ErrClusterNotFound error = errors.New("cluster not found") |
There was a problem hiding this comment.
can we turn this into an error type that lets us pass in the cluster name? might be useful for debugging
core/logger/logger_test.go
Outdated
| @@ -157,7 +157,7 @@ func TestBuildConfigWithOptions(t *testing.T) { | |||
| } | |||
|
|
|||
| var ( | |||
| rfc3339Regexp = `^((\d{4}-\d{2}-\d{2})T(\d{2}:\d{2}:\d{2}(?:\.\d+)?))(Z|[\+-]\d{2}:\d{2})$` | |||
| rfc3339Regexp = `^((\d{4}-\d{2}-\d{2})T(\d{2}:\d{2}:\d{2}(?:\.\d+)?))(Z|[\+-]\d{2}:?\d{2})$` | |||
There was a problem hiding this comment.
i think balazs fixed this in his last pr, check on main?
There was a problem hiding this comment.
just pulled from main
core/clustersmngr/client.go
Outdated
| } | ||
|
|
||
| func (c *clustersClient) Get(ctx context.Context, cluster string, key client.ObjectKey, obj client.Object) error { | ||
| client := c.pool.Clients()[cluster] |
There was a problem hiding this comment.
that c.pool.Clients()[cluster] hurts my eyes a bit, totally personal taste. I know it does not fail, but my brain constantly says "it should fail if the key is not defined".
func (cp *clientsPool) Client(name string) (client.Client, error) {
if c, found := cp.clients[name]; found && c != nil {
return c, nil
}
return ClusterNotFoundError{Cluster: name}
}
client, err := c.pool.Client(cluster)
if err != nil {
return err
}Not a blocker at all. That's only extra sugar.
There was a problem hiding this comment.
agree, much better, pushing the change
…ave-gitops into 1777-single-cluster-client
Closes: #1777
What changed?
Introduces the Clusters Client, this is an abstraction to help make calls to all the clusters. It is a thin wrapper around the
controller-runtime/clientthat handles the connections to the clusters and when contacting them does it in parallel.Why?
How did you test it?
Release notes
Documentation Changes