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

Refactor Status update calls #669

Closed
burmanm opened this issue Jun 25, 2024 · 0 comments · Fixed by #678
Closed

Refactor Status update calls #669

burmanm opened this issue Jun 25, 2024 · 0 comments · Fixed by #678
Assignees
Labels
done Issues in the state 'done' enhancement New feature or request refactoring

Comments

@burmanm
Copy link
Contributor

burmanm commented Jun 25, 2024

What is missing?

What we want from the code is to keep the CassandraDatacenter Status as descriptive of the state of the reconciliation. However, that has caused a repetition of the same code all over the codebase:

			        dcPatch := client.MergeFrom(dc.DeepCopy())
				if updated := rc.setCondition(api.NewDatacenterCondition(api.DatacenterValid, corev1.ConditionFalse)); updated {
					if err := rc.Client.Status().Patch(rc.Ctx, dc, dcPatch); err != nil {
						rc.ReqLogger.Error(err, "error patching datacenter status for updating")
						return result.Error(err)
					}
				}

While it works, this takes a lot of lines and makes the code look messy (with some silly structural hacks to make Patch happen only once). Instead, the rc.setCondition() should probably do this directly, or even masquerade the NewDatacenterCondition call at the same time. Or do the Status Patching itself on one centralized place, although that could be a lot larger refactoring, so first step could be do to it in the setCondition and then later make that patch happen somewhere else.

Why is this needed?

For easier maintenance.

@burmanm burmanm added enhancement New feature or request refactoring labels Jun 25, 2024
@burmanm burmanm self-assigned this Jul 12, 2024
@burmanm burmanm moved this to In Progress in K8ssandra Jul 12, 2024
@adejanovski adejanovski added the in-progress Issues in the state 'in-progress' label Jul 12, 2024
burmanm added a commit to burmanm/cass-operator that referenced this issue Jul 12, 2024
@burmanm burmanm moved this from In Progress to Review in K8ssandra Jul 12, 2024
@adejanovski adejanovski added review Issues in the state 'review' and removed in-progress Issues in the state 'in-progress' labels Jul 12, 2024
@burmanm burmanm moved this from Review to Ready For Review in K8ssandra Jul 12, 2024
@adejanovski adejanovski added ready-for-review Issues in the state 'ready-for-review' and removed review Issues in the state 'review' labels Jul 12, 2024
@adejanovski adejanovski moved this from Ready For Review to Review in K8ssandra Jul 12, 2024
@adejanovski adejanovski added review Issues in the state 'review' and removed ready-for-review Issues in the state 'ready-for-review' labels Jul 12, 2024
@adejanovski adejanovski moved this from Review to Ready For Review in K8ssandra Jul 12, 2024
@adejanovski adejanovski added ready-for-review Issues in the state 'ready-for-review' and removed review Issues in the state 'review' labels Jul 12, 2024
@adejanovski adejanovski moved this from Ready For Review to Review in K8ssandra Jul 12, 2024
@adejanovski adejanovski added review Issues in the state 'review' and removed ready-for-review Issues in the state 'ready-for-review' labels Jul 12, 2024
@adejanovski adejanovski moved this from Review to Ready For Review in K8ssandra Jul 12, 2024
@adejanovski adejanovski added ready-for-review Issues in the state 'ready-for-review' and removed review Issues in the state 'review' labels Jul 12, 2024
@adejanovski adejanovski moved this from Ready For Review to Review in K8ssandra Jul 12, 2024
@adejanovski adejanovski added review Issues in the state 'review' and removed ready-for-review Issues in the state 'ready-for-review' labels Jul 12, 2024
@adejanovski adejanovski moved this from Review to Ready For Review in K8ssandra Jul 12, 2024
@adejanovski adejanovski added the ready-for-review Issues in the state 'ready-for-review' label Jul 12, 2024
@adejanovski adejanovski moved this from Ready For Review to Review in K8ssandra Jul 12, 2024
@adejanovski adejanovski added review Issues in the state 'review' and removed ready-for-review Issues in the state 'ready-for-review' labels Jul 12, 2024
@adejanovski adejanovski moved this from Review to Ready For Review in K8ssandra Jul 12, 2024
@adejanovski adejanovski added ready-for-review Issues in the state 'ready-for-review' and removed review Issues in the state 'review' labels Jul 12, 2024
@adejanovski adejanovski moved this from Ready For Review to Review in K8ssandra Jul 12, 2024
@adejanovski adejanovski added review Issues in the state 'review' and removed ready-for-review Issues in the state 'ready-for-review' labels Jul 12, 2024
@adejanovski adejanovski moved this from Review to Ready For Review in K8ssandra Jul 12, 2024
@adejanovski adejanovski moved this from Ready For Review to Review in K8ssandra Jul 12, 2024
@adejanovski adejanovski added ready-for-review Issues in the state 'ready-for-review' review Issues in the state 'review' and removed review Issues in the state 'review' ready-for-review Issues in the state 'ready-for-review' labels Jul 12, 2024
@adejanovski adejanovski moved this from Review to Ready For Review in K8ssandra Jul 12, 2024
@adejanovski adejanovski moved this from Ready For Review to Review in K8ssandra Jul 12, 2024
@burmanm burmanm moved this from Review to Ready For Review in K8ssandra Jul 12, 2024
@adejanovski adejanovski added ready-for-review Issues in the state 'ready-for-review' and removed review Issues in the state 'review' labels Jul 12, 2024
@rzvoncek rzvoncek moved this from Ready For Review to Review in K8ssandra Jul 17, 2024
@adejanovski adejanovski added review Issues in the state 'review' and removed ready-for-review Issues in the state 'ready-for-review' labels Jul 17, 2024
burmanm added a commit to burmanm/cass-operator that referenced this issue Jul 18, 2024
burmanm added a commit that referenced this issue Jul 18, 2024
* Refactor statusConditions updates, Fixes #669

* Add new metrics for all the condition statuses as well as OperatorProgress

* Remove task metrics for now

* Add CHANGELOG
@github-project-automation github-project-automation bot moved this from Review to Done in K8ssandra Jul 18, 2024
@adejanovski adejanovski added done Issues in the state 'done' and removed review Issues in the state 'review' labels Jul 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
done Issues in the state 'done' enhancement New feature or request refactoring
Projects
No open projects
Status: Done
Development

Successfully merging a pull request may close this issue.

2 participants