Skip to content

RING-50689 // Create Scality UI Status - #31

Closed
hervedombya wants to merge 4 commits into
mainfrom
feature/RING-50689-create-scality-ui-status
Closed

RING-50689 // Create Scality UI Status#31
hervedombya wants to merge 4 commits into
mainfrom
feature/RING-50689-create-scality-ui-status

Conversation

@hervedombya

Copy link
Copy Markdown
Contributor

No description provided.

Copilot AI review requested due to automatic review settings June 12, 2025 15:50

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR introduces a new status subsystem for the ScalityUI custom resource, adding conditions and phases to track reconciliation state and errors.

  • Adds status update logic with conditions and phases in a new helper (scalityui_status.go)
  • Integrates status initialization and updates into the main reconcile loop (scalityui_controller.go) and extends tests
  • Updates CRD and API types to include phase and conditions fields, and ensures deep copies for the new status fields

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
internal/controller/scalityui_status.go Implements condition- and phase-based status updates
internal/controller/scalityui_controller.go Hooks status init, progressing, and error reporting into Reconcile
internal/controller/scalityui_controller_test.go Adds a test block for status conditions and phases
config/crd/bases/ui.scality.com_scalityuis.yaml Extends CRD schema to define .status.phase and .status.conditions
api/v1alpha1/scalityui_types.go Adds Phase and Conditions fields to ScalityUIStatus
api/v1alpha1/zz_generated.deepcopy.go Ensures deep copy logic includes the new Conditions slice
Comments suppressed due to low confidence (3)

internal/controller/scalityui_status.go:87

  • The progressing condition treats a nil deployment as 'Deployment stable', which could misrepresent missing resources. Add an explicit check for missing resources before marking progression as false.
setScalityUICondition(scalityui, ConditionTypeProgressing, metav1.ConditionFalse, ReasonProgressing, "Deployment stable", now)

internal/controller/scalityui_controller_test.go:716

  • The new status conditions and phase logic are not validated in tests; consider adding assertions that verify the CR's status.Conditions and status.Phase reflect expected states after reconciliation.
Describe("Status Conditions and Phases", func() {

internal/controller/scalityui_status.go:197

  • [nitpick] The default message 'Operation successful' in setScalityUIConditionWithError may be misleading when marking a progressing condition; consider deriving the message from the reason or accepting a custom message.
message := "Operation successful"

Comment thread internal/controller/scalityui_controller.go Outdated
Comment thread internal/controller/scalityui_status.go Outdated
Comment thread internal/controller/scalityui_controller.go Outdated
)

// updateScalityUIStatus updates the ScalityUI status
func (r *ScalityUIReconciler) updateScalityUIStatus(ctx context.Context, scalityui *uiscalitycomv1alpha1.ScalityUI) error {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This status reconciler could also be used for others CR, maybe we can extract the Phase and Status to their own struct, make ScalityUI extending that struct and attach this reconciller to that speicifc Status sub CR

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.

3 participants