Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow setting a cluster offline #683

Merged
merged 4 commits into from
Jun 20, 2024
Merged

Conversation

vsliouniaev
Copy link
Contributor

Fixes: #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
Copy link
Contributor

@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
Copy link
Contributor Author

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

@morimoto-cybozu
Copy link
Contributor

@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.

@takara9 takara9 marked this pull request as ready for review June 12, 2024 07:33
@takara9 takara9 marked this pull request as draft June 13, 2024 05:49
@shunki-fujita shunki-fujita merged commit 11adadd into cybozu-go:main Jun 20, 2024
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Scaling cluster down to 0
3 participants