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

chore: Do not reconcile CephCluster spec if managed by Helm #172

Conversation

rrpolanco
Copy link
Contributor

When the Ceph Cluster is created via helm.InstallChartArchive() when rookMinimumNodeCount >= 3, no modifications should be made to the CephCluster CR. Namely for two reasons, 1) CephCluster spec in the cluster should not deviate from what was configured in the chart 2) modifying the CR delays the Rook reconciliation process

@rrpolanco rrpolanco requested a review from a team as a code owner August 10, 2023 14:43
@laverya
Copy link
Member

laverya commented Aug 10, 2023

What is being modified here? Is it just labels? Is it more? Does this exclude 'what nodes should I put storage on' updates, and other things of that nature?

If it is more, we need to keep it. If it is just labels, I would prefer to fix the process to not change the labels.

If it's a true noop apply, I would love a way to skip all noops, and not exclude helm-created clusters from management entirely.

The goal of creating a cephcluster with helm via the operator was not to create two paths in perpetuity (script-created and helm-created cephclusters) it was instead to add an additional way to join the script path. This PR seems to split the paths in perpetuity, and will cause problems in the future.

@rrpolanco rrpolanco marked this pull request as draft August 11, 2023 16:04
@rrpolanco
Copy link
Contributor Author

This is change is not needed. Closing.

@rrpolanco rrpolanco closed this Aug 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants