Repository: m3db/m3db-operator
Pull request #290 (2021-04-30 01:15:44)

[m3admin-client] Incorporate zone into client cache
Without this change, we could reuse clients without the proper zone header set, meaning etcd writes could be misdirected when cluster specs change during the duration of the operator's lifetime.

codecov[bot]: # [Codecov](https://codecov.io/gh/m3db/m3db-operator/pull/290?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=m3db) Report
> Merging [#290](https://codecov.io/gh/m3db/m3db-operator/pull/290?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=m3db) (db20c70) into [master](https://codecov.io/gh/m3db/m3db-operator/commit/29e99f399920fa3804d2a0881ad643df3c916596?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=m3db) (29e99f3) will **not change** coverage.
> The diff coverage is `100.00%`.


[![Impacted file tree graph](https://codecov.io/gh/m3db/m3db-operator/pull/290/graphs/tree.svg?width=650&height=150&src=pr&token=jtHDyWhsto&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=m3db)](https://codecov.io/gh/m3db/m3db-operator/pull/290?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=m3db)

```diff
@@           Coverage Diff           @@
##           master     #290   +/-   ##
=======================================
  Coverage   76.09%   76.09%           
=======================================
  Files          32       32           
  Lines        2548     2548           
=======================================
  Hits         1939     1939           
  Misses        430      430           
  Partials      179      179           
```


------

[Continue to review full report at Codecov](https://codecov.io/gh/m3db/m3db-operator/pull/290?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=m3db).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=m3db)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/m3db/m3db-operator/pull/290?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=m3db). Last update [29e99f3...db20c70](https://codecov.io/gh/m3db/m3db-operator/pull/290?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=m3db). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=m3db).


Patch:
diff --git a/pkg/controller/m3admin_client.go b/pkg/controller/m3admin_client.go
index 425f43f..e532553 100644
--- a/pkg/controller/m3admin_client.go
+++ b/pkg/controller/m3admin_client.go
@@ -63,7 +63,8 @@ type multiAdminClient struct {
 
 // clusterKey returns a map key for a given cluster.
 func clusterKey(cluster *myspec.M3DBCluster, url string) string {
-	return cluster.Namespace + "/" + cluster.Name + "/" + url
+	// nb: zone can be empty, but we still want to include it in the cache key in case it is set.
+	return cluster.Namespace + "/" + cluster.Name + "/" + cluster.Spec.Zone + "/" + url
 }
 
 // clusterURL returns the URL to hit


Test patch:
diff --git a/pkg/controller/m3admin_client_test.go b/pkg/controller/m3admin_client_test.go
index fb95aac..ef71602 100644
--- a/pkg/controller/m3admin_client_test.go
+++ b/pkg/controller/m3admin_client_test.go
@@ -55,7 +55,11 @@ func newTestAdminClient(cl m3admin.Client, url string) *multiAdminClient {
 func TestClusterKey(t *testing.T) {
 	cluster := newM3DBCluster("ns", "a")
 	key := clusterKey(cluster, "clustera.local")
-	assert.Equal(t, "ns/a/clustera.local", key)
+	assert.Equal(t, "ns/a//clustera.local", key)
+
+	cluster.Spec.Zone = "m3db"
+	keyWithZone := clusterKey(cluster, "clustera.local")
+	assert.Equal(t, "ns/a/m3db/clustera.local", keyWithZone)
 }
 
 func TestClusterURL(t *testing.T) {