cybozu-go / moco

MySQL operator on Kubernetes using GTID-based semi-synchronous replication.
https://cybozu-go.github.io/moco/
Apache License 2.0
269 stars 22 forks source link

Allow setting a cluster offline #683

Closed vsliouniaev closed 2 months ago

vsliouniaev commented 3 months ago

Fixes: https://github.com/cybozu-go/moco/issues/659

Hi! I'm looking for guidance on the general approach here.

Overall this functions fine in my in-cluster testing but I'm only using a small sub-set of functionality.

morimoto-cybozu commented 3 months ago

@vsliouniaev Hi! Thank you for committing! Are you wondering if you can move this pull request forward for review? Your commit seems very reasonable and proper to me. But it seems to lack test code for the new property. How about adding a new It() case to controllers/mysqlcluster_controller_test.go? The case of It("should delete all related resources") would be a useful reference. Please let us know if you feel you need another type of help.

vsliouniaev commented 3 months ago

Certainly, was looking for some initial feedback on the overall approach

morimoto-cybozu commented 3 months ago

@vsliouniaev In terms of code modification approach, it seems good to add a new property for an offline cluster. And the motivation for configuring a cluster as "offline" seems reasonable to me. In terms of review process approach, making a pull request is enough.