Closed
Conversation
Add Service creation and reconciliation to ScalityUIComponent controller Revert "Add Service creation and reconciliation to ScalityUIComponent controller" This reverts commit 166b7b2.
Implement mock configuration fetching and enhance ScalityUIComponent reconciliation tests Refactor ScalityUIComponent tests to remove mock client and enhance deployment/service verification Enhance ScalityUIComponent reconciliation logic and tests
Enhance ScalityUI API and CRD documentation
This reverts commit b6188f6.
…plify theme handling in ScalityUI controller
There was a problem hiding this comment.
Pull Request Overview
This PR introduces the ScalityUIComponentExposer resource and wiring to expose component configurations at runtime.
- Adds end-to-end tests for the Exposer CR, validating Ingress, ConfigMap creation, volume mounts, and annotations.
- Extends the ScalityUIComponent reconciler to fetch, parse, and mount
.well-known/micro-app-configurationand emit status conditions. - Updates RBAC, CRD schema, and Go types to support the new Exposer API with camelCase JSON tags and the
AppHistoryPathfield.
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/controller/scalityuicomponentexposer_controller_test.go | Expanded setup/teardown and added assertions for Ingress, ConfigMap, volumes, and hash annotation in Exposer tests. |
| internal/controller/scalityuicomponent_controller_test.go | Added cleanup for deployments, services, Exposer; tests for Exposer creation, requeue logic, and status conditions. |
| internal/controller/scalityuicomponent_controller.go | Implemented fetchMicroAppConfig, JSON parsing, status conditions, and Exposer creation logic. |
| internal/controller/scalityui_controller.go | Added DefaultScalityUIName and DefaultScalityUINamespace constants. |
| config/rbac/role.yaml | Granted RBAC for deployments, configmaps, and ingresses. |
| config/crd/bases/ui.scality.com_scalityuicomponentexposers.yaml | Introduced appHistoryPath and renamed properties to camelCase. |
| api/v1alpha1/scalityuicomponentexposer_types.go | Updated JSON tags to scalityUI, scalityUIComponent, and added appHistoryPath. |
| } | ||
|
|
||
| // Exposer | ||
| exposerName := fmt.Sprintf("%s-exposer", scalityUIComponent.Name) |
There was a problem hiding this comment.
The Exposer creation block is never reached because both the success and failure branches above return before this point. Refactor the readiness check to only return when needed and move Exposer logic outside the early returns.
| // This is crucial for determining if pods are ready before attempting to fetch the UI configuration | ||
| if err := r.Get(ctx, client.ObjectKey{Name: deployment.Name, Namespace: deployment.Namespace}, deployment); err != nil { | ||
| logger.Error(err, "Failed to get deployment status") | ||
| return ctrl.Result{RequeueAfter: time.Second * 10}, nil |
There was a problem hiding this comment.
[nitpick] The literal time.Second * 10 is reused for requeue delays. Consider extracting this into a named constant (e.g. requeueDelay) to improve readability and simplify future changes.
Suggested change
| return ctrl.Result{RequeueAfter: time.Second * 10}, nil | |
| return ctrl.Result{RequeueAfter: requeueDelay}, nil |
|
|
||
| // Add a failure condition | ||
| meta.SetStatusCondition(&scalityUIComponent.Status.Conditions, metav1.Condition{ | ||
| Type: "ConfigurationRetrieved", |
There was a problem hiding this comment.
[nitpick] The condition type string ConfigurationRetrieved is repeated in multiple places. Define it as a package-level constant to avoid typos and centralize its definition.
Suggested change
| Type: "ConfigurationRetrieved", | |
| Type: ConditionTypeConfigurationRetrieved, |
Enhance ScalityUI API and CRD documentation Revert "RING-50435 // Create ScalityUI Service" This reverts commit b6188f6. Enhance ScalityUI API and CRD Update config/crd/bases/ui.scality.com_scalityuis.yaml Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Update internal/controller/scalityui_controller.go Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…ty UI image version, and fix target port in service UI configuration
…ize ConfigMap naming and improve configuration handling
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.