diff --git a/api/v1alpha1/conditions.go b/api/v1alpha1/conditions.go index c1b74b96..f0a0edfe 100644 --- a/api/v1alpha1/conditions.go +++ b/api/v1alpha1/conditions.go @@ -73,3 +73,37 @@ type Condition struct { // +kubebuilder:validation:Minimum=0 ObservedGeneration int64 `json:"observedGeneration,omitempty"` } + +// NewCondition returns a new Condition instance. +func NewCondition(t ConditionType, status metav1.ConditionStatus, reason, message string) Condition { + return Condition{ + Type: t, + Status: status, + LastTransitionTime: &metav1.Time{Time: metav1.Now().Time}, + Reason: &reason, + Message: &message, + } +} + +func GetCondition(conditions []Condition, t ConditionType) *Condition { + for _, c := range conditions { + if c.Type == t { + return &c + } + } + return nil +} + +func SetCondition(conditions []Condition, condition Condition) []Condition { + for i, c := range conditions { + if c.Type == condition.Type { + conditions[i] = condition + return conditions + } + } + return append(conditions, condition) +} + +func HasCondition(conditions []Condition, t ConditionType) bool { + return GetCondition(conditions, t) != nil +} diff --git a/internal/controller/resourcegroup/condition/condition.go b/internal/controller/resourcegroup/condition/condition.go deleted file mode 100644 index f54db00c..00000000 --- a/internal/controller/resourcegroup/condition/condition.go +++ /dev/null @@ -1,54 +0,0 @@ -// Copyright Amazon.com Inc. or its affiliates. All Rights Reserved. -// -// Licensed under the Apache License, Version 2.0 (the "License"). You may -// not use this file except in compliance with the License. A copy of the -// License is located at -// -// http://aws.amazon.com/apache2.0/ -// -// or in the "license" file accompanying this file. This file is distributed -// on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either -// express or implied. See the License for the specific language governing -// permissions and limitations under the License. - -package condition - -import ( - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - - v1alpha1 "github.com/awslabs/kro/api/v1alpha1" -) - -// NewCondition returns a new Condition instance. -func NewCondition(t v1alpha1.ConditionType, status metav1.ConditionStatus, reason, message string) v1alpha1.Condition { - return v1alpha1.Condition{ - Type: t, - Status: status, - LastTransitionTime: &metav1.Time{Time: metav1.Now().Time}, - Reason: &reason, - Message: &message, - } -} - -func GetCondition(conditions []v1alpha1.Condition, t v1alpha1.ConditionType) *v1alpha1.Condition { - for _, c := range conditions { - if c.Type == t { - return &c - } - } - return nil -} - -func SetCondition(conditions []v1alpha1.Condition, condition v1alpha1.Condition) []v1alpha1.Condition { - for i, c := range conditions { - if c.Type == condition.Type { - conditions[i] = condition - return conditions - } - } - return append(conditions, condition) -} - -func HasCondition(conditions []v1alpha1.Condition, t v1alpha1.ConditionType) bool { - return GetCondition(conditions, t) != nil -} diff --git a/internal/controller/resourcegroup/condition/condition_instance.go b/internal/controller/resourcegroup/condition/condition_instance.go deleted file mode 100644 index 7f30f660..00000000 --- a/internal/controller/resourcegroup/condition/condition_instance.go +++ /dev/null @@ -1,35 +0,0 @@ -// Copyright Amazon.com Inc. or its affiliates. All Rights Reserved. -// -// Licensed under the Apache License, Version 2.0 (the "License"). You may -// not use this file except in compliance with the License. A copy of the -// License is located at -// -// http://aws.amazon.com/apache2.0/ -// -// or in the "license" file accompanying this file. This file is distributed -// on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either -// express or implied. See the License for the specific language governing -// permissions and limitations under the License. - -package condition - -import ( - v1alpha1 "github.com/awslabs/kro/api/v1alpha1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" -) - -func NewInstanceConditionReady(status metav1.ConditionStatus, reason, message string) v1alpha1.Condition { - return NewCondition(v1alpha1.InstanceConditionTypeReady, status, reason, message) -} - -func NewInstanceConditionProgressing(status metav1.ConditionStatus, reason, message string) v1alpha1.Condition { - return NewCondition(v1alpha1.InstanceConditionTypeProgressing, status, reason, message) -} - -func NewInstanceConditionDegraded(status metav1.ConditionStatus, reason, message string) v1alpha1.Condition { - return NewCondition(v1alpha1.InstanceConditionTypeDegraded, status, reason, message) -} - -func NewInstanceConditionErrored(status metav1.ConditionStatus, reason, message string) v1alpha1.Condition { - return NewCondition(v1alpha1.InstanceConditionTypeError, status, reason, message) -} diff --git a/internal/controller/resourcegroup/condition/condition_resourcegroup.go b/internal/controller/resourcegroup/condition/condition_resourcegroup.go deleted file mode 100644 index 97b4f396..00000000 --- a/internal/controller/resourcegroup/condition/condition_resourcegroup.go +++ /dev/null @@ -1,32 +0,0 @@ -// Copyright Amazon.com Inc. or its affiliates. All Rights Reserved. -// -// Licensed under the Apache License, Version 2.0 (the "License"). You may -// not use this file except in compliance with the License. A copy of the -// License is located at -// -// http://aws.amazon.com/apache2.0/ -// -// or in the "license" file accompanying this file. This file is distributed -// on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either -// express or implied. See the License for the specific language governing -// permissions and limitations under the License. - -package condition - -import ( - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - - v1alpha1 "github.com/awslabs/kro/api/v1alpha1" -) - -func NewReconcilerReadyCondition(status metav1.ConditionStatus, reason, message string) v1alpha1.Condition { - return NewCondition(v1alpha1.ResourceGroupConditionTypeReconcilerReady, status, reason, message) -} - -func NewGraphVerifiedCondition(status metav1.ConditionStatus, reason, message string) v1alpha1.Condition { - return NewCondition(v1alpha1.ResourceGroupConditionTypeGraphVerified, status, reason, message) -} - -func NewCustomResourceDefinitionSyncedCondition(status metav1.ConditionStatus, reason, message string) v1alpha1.Condition { - return NewCondition(v1alpha1.ResourceGroupConditionTypeCustomResourceDefinitionSynced, status, reason, message) -} diff --git a/internal/controller/resourcegroup/controller.go b/internal/controller/resourcegroup/controller.go index 3e7c2d02..dd59c59c 100644 --- a/internal/controller/resourcegroup/controller.go +++ b/internal/controller/resourcegroup/controller.go @@ -81,28 +81,16 @@ func (r *ResourceGroupReconciler) SetupWithManager(mgr ctrl.Manager) error { Complete(r) } -// Reconcile is part of the main kubernetes reconciliation loop which aims to -// move the current state of the cluster closer to the desired state. -// TODO(user): Modify the Reconcile function to compare the state specified by -// the ResourceGroup object against the actual cluster state, and then -// perform operations to make the cluster state reflect the state specified by -// the user. -// -// For more details, check Reconcile and its Result here: -// - https://pkg.go.dev/sigs.k8s.io/controller-runtime@v0.16.3/pkg/reconcile func (r *ResourceGroupReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { rlog := r.log.WithValues("resourcegroup", req.NamespacedName) ctx = log.IntoContext(ctx, rlog) var resourcegroup v1alpha1.ResourceGroup - err := r.Get(ctx, req.NamespacedName, &resourcegroup) - if err != nil { + if err := r.Get(ctx, req.NamespacedName, &resourcegroup); err != nil { return ctrl.Result{}, client.IgnoreNotFound(err) } - // reconcile resourcegroup fiesta - err = r.reconcile(ctx, &resourcegroup) - if err != nil { + if err := r.reconcile(ctx, &resourcegroup); err != nil { return ctrl.Result{}, err } @@ -111,18 +99,15 @@ func (r *ResourceGroupReconciler) Reconcile(ctx context.Context, req ctrl.Reques func (r *ResourceGroupReconciler) reconcile(ctx context.Context, resourcegroup *v1alpha1.ResourceGroup) error { log, _ := logr.FromContext(ctx) - // if deletion timestamp is set, call cleanupResourceGroup + if !resourcegroup.DeletionTimestamp.IsZero() { log.V(1).Info("ResourceGroup is being deleted") - err := r.cleanupResourceGroup(ctx, resourcegroup) - if err != nil { + if err := r.cleanupResourceGroup(ctx, resourcegroup); err != nil { return err } log.V(1).Info("Setting resourcegroup as unmanaged") - // remove finalizer - err = r.setUnmanaged(ctx, resourcegroup) - if err != nil { + if err := r.setUnmanaged(ctx, resourcegroup); err != nil { return err } @@ -130,9 +115,7 @@ func (r *ResourceGroupReconciler) reconcile(ctx context.Context, resourcegroup * } log.V(1).Info("Setting resource group as managed") - // set finalizer - err := r.setManaged(ctx, resourcegroup) - if err != nil { + if err := r.setManaged(ctx, resourcegroup); err != nil { return err } @@ -140,10 +123,9 @@ func (r *ResourceGroupReconciler) reconcile(ctx context.Context, resourcegroup * topologicalOrder, resourcesInformation, reconcileErr := r.reconcileResourceGroup(ctx, resourcegroup) log.V(1).Info("Setting resourcegroup status") - // set status - err = r.setResourceGroupStatus(ctx, resourcegroup, topologicalOrder, resourcesInformation, reconcileErr) - if err != nil { + if err := r.setResourceGroupStatus(ctx, resourcegroup, topologicalOrder, resourcesInformation, reconcileErr); err != nil { return err } + return nil } diff --git a/internal/controller/resourcegroup/controller_cleanup.go b/internal/controller/resourcegroup/controller_cleanup.go index bd26d434..9515d5ab 100644 --- a/internal/controller/resourcegroup/controller_cleanup.go +++ b/internal/controller/resourcegroup/controller_cleanup.go @@ -26,45 +26,57 @@ import ( "github.com/awslabs/kro/internal/metadata" ) +// cleanupResourceGroup handles the deletion of a ResourceGroup by shutting down its associated +// microcontroller and cleaning up the CRD if enabled. It executes cleanup operations in order: +// 1. Shuts down the microcontroller +// 2. Deletes the associated CRD (if CRD deletion is enabled) func (r *ResourceGroupReconciler) cleanupResourceGroup(ctx context.Context, rg *v1alpha1.ResourceGroup) error { log, _ := logr.FromContext(ctx) + log.V(1).Info("cleaning up resource group", "name", rg.Name) - log.V(1).Info("Cleaning up resource group") + // shutdown microcontroller gvr := metadata.GetResourceGroupInstanceGVR(rg.Spec.Schema.APIVersion, rg.Spec.Schema.Kind) - - log.V(1).Info("Shutting down resource group microcontroller") - err := r.shutdownResourceGroupMicroController(ctx, &gvr) - if err != nil { - return err + if err := r.shutdownResourceGroupMicroController(ctx, &gvr); err != nil { + return fmt.Errorf("failed to shutdown microcontroller: %w", err) } - crdName := r.extractCRDName(rg.Spec.Schema.Kind) - log.V(1).Info("Cleaning up resource group CRD", "crd", crdName) - err = r.cleanupResourceGroupCRD(ctx, crdName) - if err != nil { - return err + // cleanup CRD + crdName := extractCRDName(rg.Spec.Schema.Kind) + if err := r.cleanupResourceGroupCRD(ctx, crdName); err != nil { + return fmt.Errorf("failed to cleanup CRD %s: %w", crdName, err) } return nil } -func (r *ResourceGroupReconciler) extractCRDName(kind string) string { - pluralKind := flect.Pluralize(strings.ToLower(kind)) - return fmt.Sprintf("%s.%s", pluralKind, v1alpha1.KroDomainName) -} - +// shutdownResourceGroupMicroController stops the dynamic controller associated with the given GVR. +// This ensures no new reconciliations occur for this resource type. func (r *ResourceGroupReconciler) shutdownResourceGroupMicroController(ctx context.Context, gvr *schema.GroupVersionResource) error { - return r.dynamicController.StopServiceGVK(ctx, *gvr) + if err := r.dynamicController.StopServiceGVK(ctx, *gvr); err != nil { + return fmt.Errorf("error stopping service: %w", err) + } + return nil } +// cleanupResourceGroupCRD deletes the CRD with the given name if CRD deletion is enabled. +// If CRD deletion is disabled, it logs the skip and returns nil. func (r *ResourceGroupReconciler) cleanupResourceGroupCRD(ctx context.Context, crdName string) error { - if r.allowCRDDeletion { - err := r.crdManager.Delete(ctx, crdName) - if err != nil { - return err - } - } else { - r.log.Info("CRD deletion is disabled, skipping CRD deletion", "crd", crdName) + if !r.allowCRDDeletion { + log, _ := logr.FromContext(ctx) + log.Info("skipping CRD deletion (disabled)", "crd", crdName) + return nil + } + + if err := r.crdManager.Delete(ctx, crdName); err != nil { + return fmt.Errorf("error deleting CRD: %w", err) } return nil } + +// extractCRDName generates the CRD name from a given kind by converting it to plural form +// and appending the Kro domain name. +func extractCRDName(kind string) string { + return fmt.Sprintf("%s.%s", + flect.Pluralize(strings.ToLower(kind)), + v1alpha1.KroDomainName) +} diff --git a/internal/controller/resourcegroup/controller_reconcile.go b/internal/controller/resourcegroup/controller_reconcile.go index 2e59e83e..25fbdaa2 100644 --- a/internal/controller/resourcegroup/controller_reconcile.go +++ b/internal/controller/resourcegroup/controller_reconcile.go @@ -24,40 +24,66 @@ import ( "github.com/awslabs/kro/api/v1alpha1" instancectrl "github.com/awslabs/kro/internal/controller/instance" - "github.com/awslabs/kro/internal/controller/resourcegroup/errors" "github.com/awslabs/kro/internal/dynamiccontroller" "github.com/awslabs/kro/internal/graph" "github.com/awslabs/kro/internal/metadata" ) +// reconcileResourceGroup orchestrates the reconciliation of a ResourceGroup by: +// 1. Processing the resource graph +// 2. Ensuring CRDs are present +// 3. Setting up and starting the microcontroller func (r *ResourceGroupReconciler) reconcileResourceGroup(ctx context.Context, rg *v1alpha1.ResourceGroup) ([]string, []v1alpha1.ResourceInformation, error) { log, _ := logr.FromContext(ctx) - log.V(1).Info("Reconciling resource group graph") - processedRG, resourcesInformation, err := r.reconcileResourceGroupGraph(ctx, rg) + // Process resource group graph first to validate structure + log.V(1).Info("reconciling resource group graph") + processedRG, resourcesInfo, err := r.reconcileResourceGroupGraph(ctx, rg) if err != nil { - return nil, nil, errors.NewReconcileGraphError(err) + return nil, nil, err } - log.V(1).Info("Reconciling resource group CRD") - err = r.reconcileResourceGroupCRD(ctx, processedRG.Instance.GetCRD()) - if err != nil { - return processedRG.TopologicalOrder, resourcesInformation, err + // Ensure CRD exists and is up to date + log.V(1).Info("reconciling resource group CRD") + if err := r.reconcileResourceGroupCRD(ctx, processedRG.Instance.GetCRD()); err != nil { + return processedRG.TopologicalOrder, resourcesInfo, err } - rgLabeler := metadata.NewResourceGroupLabeler(rg) - // Merge the ResourceGroupLabeler with the KroLabeler - graphExecLabeler, err := r.metadataLabeler.Merge(rgLabeler) + // Setup metadata labeling + graphExecLabeler, err := r.setupLabeler(rg) if err != nil { - return nil, nil, fmt.Errorf("failed to merge labelers: %w", err) + return nil, nil, fmt.Errorf("failed to setup labeler: %w", err) } + // Setup and start microcontroller gvr := processedRG.Instance.GetGroupVersionResource() - //id := fmt.Sprintf("%s.%s/%s", gvr.Resource, gvr.Group, gvr.Version) + controller := r.setupMicroController(gvr, processedRG, rg.Spec.DefaultServiceAccounts, graphExecLabeler) + + log.V(1).Info("reconciling resource group micro controller") + if err := r.reconcileResourceGroupMicroController(ctx, &gvr, controller.Reconcile); err != nil { + return processedRG.TopologicalOrder, resourcesInfo, err + } + + return processedRG.TopologicalOrder, resourcesInfo, nil +} + +// setupLabeler creates and merges the required labelers for the resource group +func (r *ResourceGroupReconciler) setupLabeler(rg *v1alpha1.ResourceGroup) (metadata.Labeler, error) { + rgLabeler := metadata.NewResourceGroupLabeler(rg) + return r.metadataLabeler.Merge(rgLabeler) +} + +// setupMicroController creates a new controller instance with the required configuration +func (r *ResourceGroupReconciler) setupMicroController( + gvr schema.GroupVersionResource, + processedRG *graph.Graph, + defaultSVCs map[string]string, + labeler metadata.Labeler, +) *instancectrl.Controller { + instanceLogger := r.rootLogger.WithName("controller." + gvr.Resource) - // instanceLogger = instanceLogger.WithValues("gvr", id) - graphexecController := instancectrl.NewController( + return instancectrl.NewController( instanceLogger, instancectrl.ReconcileConfig{ DefaultRequeueDuration: 3 * time.Second, @@ -67,52 +93,77 @@ func (r *ResourceGroupReconciler) reconcileResourceGroup(ctx context.Context, rg gvr, processedRG, r.clientSet, - rg.Spec.DefaultServiceAccounts, - graphExecLabeler, + defaultSVCs, + labeler, ) - - log.V(1).Info("Reconcile resource group micro controller") - err = r.reconcileResourceGroupMicroController(ctx, &gvr, graphexecController.Reconcile) - if err != nil { - return processedRG.TopologicalOrder, resourcesInformation, err - } - - return processedRG.TopologicalOrder, resourcesInformation, nil } +// reconcileResourceGroupGraph processes the resource group to build a dependency graph +// and extract resource information func (r *ResourceGroupReconciler) reconcileResourceGroupGraph(_ context.Context, rg *v1alpha1.ResourceGroup) (*graph.Graph, []v1alpha1.ResourceInformation, error) { processedRG, err := r.rgBuilder.NewResourceGroup(rg) if err != nil { - return nil, nil, errors.NewReconcileGraphError(err) + return nil, nil, newGraphError(err) } - resourcesInformation := make([]v1alpha1.ResourceInformation, 0, len(processedRG.Resources)) - + resourcesInfo := make([]v1alpha1.ResourceInformation, 0, len(processedRG.Resources)) for name, resource := range processedRG.Resources { - if len(resource.GetDependencies()) > 0 { - d := make([]v1alpha1.Dependency, 0, len(resource.GetDependencies())) - for _, dependency := range resource.GetDependencies() { - d = append(d, v1alpha1.Dependency{Name: dependency}) - } - resourcesInformation = append(resourcesInformation, v1alpha1.ResourceInformation{ - Name: name, - Dependencies: d, - }) + deps := resource.GetDependencies() + if len(deps) > 0 { + resourcesInfo = append(resourcesInfo, buildResourceInfo(name, deps)) } } - return processedRG, resourcesInformation, nil + return processedRG, resourcesInfo, nil } -func (r *ResourceGroupReconciler) reconcileResourceGroupCRD(ctx context.Context, crd *v1.CustomResourceDefinition) error { - err := r.crdManager.Ensure(ctx, *crd) - if err != nil { - return errors.NewReconcileCRDError(err) +// buildResourceInfo creates a ResourceInformation struct from name and dependencies +func buildResourceInfo(name string, deps []string) v1alpha1.ResourceInformation { + dependencies := make([]v1alpha1.Dependency, 0, len(deps)) + for _, dep := range deps { + dependencies = append(dependencies, v1alpha1.Dependency{Name: dep}) + } + return v1alpha1.ResourceInformation{ + Name: name, + Dependencies: dependencies, } +} +// reconcileResourceGroupCRD ensures the CRD is present and up to date in the cluster +func (r *ResourceGroupReconciler) reconcileResourceGroupCRD(ctx context.Context, crd *v1.CustomResourceDefinition) error { + if err := r.crdManager.Ensure(ctx, *crd); err != nil { + return newCRDError(err) + } return nil } +// reconcileResourceGroupMicroController starts the microcontroller for handling the resources func (r *ResourceGroupReconciler) reconcileResourceGroupMicroController(ctx context.Context, gvr *schema.GroupVersionResource, handler dynamiccontroller.Handler) error { - return r.dynamicController.StartServingGVK(ctx, *gvr, handler) + err := r.dynamicController.StartServingGVK(ctx, *gvr, handler) + if err != nil { + return newMicroControllerError(err) + } + return nil } + +// Error types for the resourcegroup controller +type ( + graphError struct{ err error } + crdError struct{ err error } + microControllerError struct{ err error } +) + +// Error interface implementation +func (e *graphError) Error() string { return e.err.Error() } +func (e *crdError) Error() string { return e.err.Error() } +func (e *microControllerError) Error() string { return e.err.Error() } + +// Unwrap interface implementation +func (e *graphError) Unwrap() error { return e.err } +func (e *crdError) Unwrap() error { return e.err } +func (e *microControllerError) Unwrap() error { return e.err } + +// Error constructors +func newGraphError(err error) error { return &graphError{err} } +func newCRDError(err error) error { return &crdError{err} } +func newMicroControllerError(err error) error { return µControllerError{err} } diff --git a/internal/controller/resourcegroup/controller_status.go b/internal/controller/resourcegroup/controller_status.go index bc327d3f..23214fac 100644 --- a/internal/controller/resourcegroup/controller_status.go +++ b/internal/controller/resourcegroup/controller_status.go @@ -16,165 +16,169 @@ package resourcegroup import ( "context" "errors" + "fmt" - "github.com/go-logr/logr" - v1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/runtime/schema" - ctrl "sigs.k8s.io/controller-runtime" + "k8s.io/client-go/util/retry" "sigs.k8s.io/controller-runtime/pkg/client" - "sigs.k8s.io/controller-runtime/pkg/log" "github.com/awslabs/kro/api/v1alpha1" - "github.com/awslabs/kro/internal/controller/resourcegroup/condition" - serr "github.com/awslabs/kro/internal/controller/resourcegroup/errors" - "github.com/awslabs/kro/internal/requeue" + "github.com/awslabs/kro/internal/metadata" + "github.com/go-logr/logr" ) -// handleReconcileError will handle errors from reconcile handlers, which -// respects runtime errors. -func (r *ResourceGroupReconciler) handleReconcileError(ctx context.Context, err error) (ctrl.Result, error) { - log := log.FromContext(ctx) - - var requeueNeededAfter *requeue.RequeueNeededAfter - if errors.As(err, &requeueNeededAfter) { - after := requeueNeededAfter.Duration() - log.Info( - "requeue needed after error", - "error", requeueNeededAfter.Unwrap(), - "after", after, - ) - return ctrl.Result{RequeueAfter: after}, nil +// StatusProcessor handles the processing of ResourceGroup status updates +type StatusProcessor struct { + conditions []v1alpha1.Condition + state v1alpha1.ResourceGroupState +} + +// NewStatusProcessor creates a new StatusProcessor with default active state +func NewStatusProcessor() *StatusProcessor { + return &StatusProcessor{ + conditions: []v1alpha1.Condition{}, + state: v1alpha1.ResourceGroupStateActive, } +} - var requeueNeeded *requeue.RequeueNeeded - if errors.As(err, &requeueNeeded) { - log.Info( - "requeue needed error", - "error", requeueNeeded.Unwrap(), - ) - return ctrl.Result{Requeue: true}, nil +// setDefaultConditions sets the default conditions for an active resource group +func (sp *StatusProcessor) setDefaultConditions() { + sp.conditions = []v1alpha1.Condition{ + newReconcilerReadyCondition(metav1.ConditionTrue, ""), + newGraphVerifiedCondition(metav1.ConditionTrue, ""), + newCustomResourceDefinitionSyncedCondition(metav1.ConditionTrue, ""), } +} - var noRequeue *requeue.NoRequeue - if errors.As(err, &noRequeue) { - return ctrl.Result{}, nil +// processGraphError handles graph-related errors +func (sp *StatusProcessor) processGraphError(err error) { + sp.conditions = []v1alpha1.Condition{ + newGraphVerifiedCondition(metav1.ConditionFalse, err.Error()), + newReconcilerReadyCondition(metav1.ConditionUnknown, "Faulty Graph"), + newCustomResourceDefinitionSyncedCondition(metav1.ConditionUnknown, "Faulty Graph"), } + sp.state = v1alpha1.ResourceGroupStateInactive +} - return ctrl.Result{}, err +// processCRDError handles CRD-related errors +func (sp *StatusProcessor) processCRDError(err error) { + sp.conditions = []v1alpha1.Condition{ + newGraphVerifiedCondition(metav1.ConditionTrue, ""), + newCustomResourceDefinitionSyncedCondition(metav1.ConditionFalse, err.Error()), + newReconcilerReadyCondition(metav1.ConditionUnknown, "CRD not-synced"), + } + sp.state = v1alpha1.ResourceGroupStateInactive } -func (r *ResourceGroupReconciler) setResourceGroupStatus(ctx context.Context, resourcegroup *v1alpha1.ResourceGroup, topologicalOrder []string, resources []v1alpha1.ResourceInformation, reconcileErr error) error { - log, _ := logr.FromContext(ctx) +// processMicroControllerError handles microcontroller-related errors +func (sp *StatusProcessor) processMicroControllerError(err error) { + sp.conditions = []v1alpha1.Condition{ + newGraphVerifiedCondition(metav1.ConditionTrue, ""), + newCustomResourceDefinitionSyncedCondition(metav1.ConditionTrue, ""), + newReconcilerReadyCondition(metav1.ConditionFalse, err.Error()), + } + sp.state = v1alpha1.ResourceGroupStateInactive +} +// setResourceGroupStatus calculates the ResourceGroup status and updates it +// in the API server. +func (r *ResourceGroupReconciler) setResourceGroupStatus( + ctx context.Context, + resourcegroup *v1alpha1.ResourceGroup, + topologicalOrder []string, + resources []v1alpha1.ResourceInformation, + reconcileErr error, +) error { + log, _ := logr.FromContext(ctx) log.V(1).Info("calculating resource group status and conditions") - dc := resourcegroup.DeepCopy() - - // set conditions - dc.Status.Conditions = condition.SetCondition(dc.Status.Conditions, - condition.NewReconcilerReadyCondition(metav1.ConditionTrue, "", "micro controller is ready"), - ) - dc.Status.Conditions = condition.SetCondition(dc.Status.Conditions, - condition.NewGraphVerifiedCondition(metav1.ConditionTrue, "", "Directed Acyclic Graph is synced"), - ) - dc.Status.Conditions = condition.SetCondition(dc.Status.Conditions, - condition.NewCustomResourceDefinitionSyncedCondition(metav1.ConditionTrue, "", "Custom Resource Definition is synced"), - ) - dc.Status.State = v1alpha1.ResourceGroupStateActive - dc.Status.TopologicalOrder = topologicalOrder - dc.Status.Resources = resources - - if reconcileErr != nil { - log.V(1).Info("Error occurred during reconcile", "error", reconcileErr) - - // if the error is graph error, graph condition should be false and the rest should be unknown - var reconcielGraphErr *serr.ReconcileGraphError - if errors.As(reconcileErr, &reconcielGraphErr) { - log.V(1).Info("Processing reconcile graph error", "error", reconcileErr) - dc.Status.Conditions = condition.SetCondition(dc.Status.Conditions, - condition.NewGraphVerifiedCondition(metav1.ConditionFalse, reconcileErr.Error(), "Directed Acyclic Graph is synced"), - ) - - reason := "Faulty Graph" - dc.Status.Conditions = condition.SetCondition(dc.Status.Conditions, - condition.NewReconcilerReadyCondition(metav1.ConditionUnknown, reason, "micro controller is ready"), - ) - dc.Status.Conditions = condition.SetCondition(dc.Status.Conditions, - condition.NewCustomResourceDefinitionSyncedCondition(metav1.ConditionUnknown, reason, "Custom Resource Definition is synced"), - ) + processor := NewStatusProcessor() + + if reconcileErr == nil { + processor.setDefaultConditions() + } else { + log.V(1).Info("processing reconciliation error", "error", reconcileErr) + + var graphErr *graphError + var crdErr *crdError + var microControllerErr *microControllerError + + switch { + case errors.As(reconcileErr, &graphErr): + processor.processGraphError(reconcileErr) + case errors.As(reconcileErr, &crdErr): + processor.processCRDError(reconcileErr) + case errors.As(reconcileErr, µControllerErr): + processor.processMicroControllerError(reconcileErr) + default: + log.Error(reconcileErr, "unhandled reconciliation error type") + return fmt.Errorf("unhandled reconciliation error: %w", reconcileErr) } + } - // if the error is crd error, crd condition should be false, graph condition should be true and the rest should be unknown - var reconcileCRDErr *serr.ReconcileCRDError - if errors.As(reconcileErr, &reconcileCRDErr) { - log.V(1).Info("Processing reconcile crd error", "error", reconcileErr) - dc.Status.Conditions = condition.SetCondition(dc.Status.Conditions, - condition.NewGraphVerifiedCondition(metav1.ConditionTrue, "", "Directed Acyclic Graph is synced"), - ) - dc.Status.Conditions = condition.SetCondition(dc.Status.Conditions, - condition.NewCustomResourceDefinitionSyncedCondition(metav1.ConditionFalse, reconcileErr.Error(), "Custom Resource Definition is synced"), - ) - reason := "CRD not-synced" - dc.Status.Conditions = condition.SetCondition(dc.Status.Conditions, - condition.NewReconcilerReadyCondition(metav1.ConditionUnknown, reason, "micro controller is ready"), - ) + return retry.RetryOnConflict(retry.DefaultRetry, func() error { + // Get fresh copy to avoid conflicts + current := &v1alpha1.ResourceGroup{} + if err := r.Get(ctx, client.ObjectKeyFromObject(resourcegroup), current); err != nil { + return fmt.Errorf("failed to get current resource group: %w", err) } - // if the error is micro controller error, micro controller condition should be false, graph condition should be true and the rest should be unknown - var reconcileMicroController *serr.ReconcileMicroControllerError - if errors.As(reconcileErr, &reconcileMicroController) { - log.V(1).Info("Processing reconcile micro controller error", "error", reconcileErr) - dc.Status.Conditions = condition.SetCondition(dc.Status.Conditions, - condition.NewGraphVerifiedCondition(metav1.ConditionTrue, "", "Directed Acyclic Graph is synced"), - ) - dc.Status.Conditions = condition.SetCondition(dc.Status.Conditions, - condition.NewCustomResourceDefinitionSyncedCondition(metav1.ConditionTrue, "", "Custom Resource Definition is synced"), - ) - dc.Status.Conditions = condition.SetCondition(dc.Status.Conditions, - condition.NewReconcilerReadyCondition(metav1.ConditionFalse, reconcileErr.Error(), "micro controller is ready"), - ) - } + // Update status + dc := current.DeepCopy() + dc.Status.Conditions = processor.conditions + dc.Status.State = processor.state + dc.Status.TopologicalOrder = topologicalOrder + dc.Status.Resources = resources + + log.V(1).Info("updating resource group status", + "state", dc.Status.State, + "conditions", len(dc.Status.Conditions), + ) + + return r.Status().Patch(ctx, dc, client.MergeFrom(current)) + }) +} - log.V(1).Info("Setting resource group status to INACTIVE", "error", reconcileErr) - dc.Status.State = v1alpha1.ResourceGroupStateInactive +// setManaged sets the resourcegroup as managed, by adding the +// default finalizer if it doesn't exist. +func (r *ResourceGroupReconciler) setManaged(ctx context.Context, rg *v1alpha1.ResourceGroup) error { + log, _ := logr.FromContext(ctx) + log.V(1).Info("setting resourcegroup as managed") + + // Skip if finalizer already exists + if metadata.HasResourceGroupFinalizer(rg) { + return nil } - log.V(1).Info("Setting resource group status", "status", dc.Status) - patch := client.MergeFrom(resourcegroup.DeepCopy()) - return r.Status().Patch(ctx, dc.DeepCopy(), patch) + dc := rg.DeepCopy() + metadata.SetResourceGroupFinalizer(dc) + return r.Patch(ctx, dc, client.MergeFrom(rg)) } -func (r *ResourceGroupReconciler) setManaged(ctx context.Context, resourcegroup *v1alpha1.ResourceGroup) error { - log := log.FromContext(ctx) - log.V(1).Info("setting resourcegroup as managed - adding finalizer") +// setUnmanaged sets the resourcegroup as unmanaged, by removing the +// default finalizer if it exists. +func (r *ResourceGroupReconciler) setUnmanaged(ctx context.Context, rg *v1alpha1.ResourceGroup) error { + log, _ := logr.FromContext(ctx) + log.V(1).Info("setting resourcegroup as unmanaged") - newFinalizers := []string{v1alpha1.KroDomainName} - dc := resourcegroup.DeepCopy() - dc.Finalizers = newFinalizers - if len(dc.Finalizers) != len(resourcegroup.Finalizers) { - patch := client.MergeFrom(resourcegroup.DeepCopy()) - return r.Patch(ctx, dc.DeepCopy(), patch) + // Skip if finalizer already removed + if !metadata.HasResourceGroupFinalizer(rg) { + return nil } - return nil + + dc := rg.DeepCopy() + metadata.RemoveResourceGroupFinalizer(dc) + return r.Patch(ctx, dc, client.MergeFrom(rg)) } -func (r *ResourceGroupReconciler) setUnmanaged(ctx context.Context, resourcegroup *v1alpha1.ResourceGroup) error { - log := log.FromContext(ctx) - log.V(1).Info("setting resourcegroup as unmanaged - removing finalizer") +func newReconcilerReadyCondition(status metav1.ConditionStatus, reason string) v1alpha1.Condition { + return v1alpha1.NewCondition(v1alpha1.ResourceGroupConditionTypeReconcilerReady, status, reason, "micro controller is ready") +} - newFinalizers := []string{} - dc := resourcegroup.DeepCopy() - dc.Finalizers = newFinalizers - patch := client.MergeFrom(resourcegroup.DeepCopy()) - return r.Patch(ctx, dc.DeepCopy(), patch) +func newGraphVerifiedCondition(status metav1.ConditionStatus, reason string) v1alpha1.Condition { + return v1alpha1.NewCondition(v1alpha1.ResourceGroupConditionTypeGraphVerified, status, reason, "Directed Acyclic Graph is synced") } -func getGVR(customRD *v1.CustomResourceDefinition) *schema.GroupVersionResource { - return &schema.GroupVersionResource{ - Group: customRD.Spec.Group, - // Deal with complex versioning later on - Version: customRD.Spec.Versions[0].Name, - Resource: customRD.Spec.Names.Plural, - } +func newCustomResourceDefinitionSyncedCondition(status metav1.ConditionStatus, reason string) v1alpha1.Condition { + return v1alpha1.NewCondition(v1alpha1.ResourceGroupConditionTypeCustomResourceDefinitionSynced, status, reason, "Custom Resource Definition is synced") } diff --git a/internal/controller/resourcegroup/errors/errors.go b/internal/controller/resourcegroup/errors/errors.go deleted file mode 100644 index 5e0b35ef..00000000 --- a/internal/controller/resourcegroup/errors/errors.go +++ /dev/null @@ -1,20 +0,0 @@ -// Copyright Amazon.com Inc. or its affiliates. All Rights Reserved. -// -// Licensed under the Apache License, Version 2.0 (the "License"). You may -// not use this file except in compliance with the License. A copy of the -// License is located at -// -// http://aws.amazon.com/apache2.0/ -// -// or in the "license" file accompanying this file. This file is distributed -// on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either -// express or implied. See the License for the specific language governing -// permissions and limitations under the License. - -package errors - -type ResourceGroupReconcileError struct { - CRDReconcileError error - GraphReconcileError error - MicroControllerReconcileError error -} diff --git a/internal/controller/resourcegroup/errors/resourcegroup.go b/internal/controller/resourcegroup/errors/resourcegroup.go deleted file mode 100644 index a875af2e..00000000 --- a/internal/controller/resourcegroup/errors/resourcegroup.go +++ /dev/null @@ -1,63 +0,0 @@ -// Copyright Amazon.com Inc. or its affiliates. All Rights Reserved. -// -// Licensed under the Apache License, Version 2.0 (the "License"). You may -// not use this file except in compliance with the License. A copy of the -// License is located at -// -// http://aws.amazon.com/apache2.0/ -// -// or in the "license" file accompanying this file. This file is distributed -// on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either -// express or implied. See the License for the specific language governing -// permissions and limitations under the License. -package errors - -import "fmt" - -// BaseError is a common structure for all custom errors -type BaseError struct { - ErrType string - Err error -} - -func (e *BaseError) Error() string { - if e == nil { - return "" - } - return fmt.Sprintf("%s: %v", e.ErrType, e.Err) -} - -func (e *BaseError) Unwrap() error { - if e == nil { - return nil - } - return e.Err -} - -// Custom error types -type ( - ReconcileGraphError struct { - BaseError - } - - ReconcileMicroControllerError struct { - BaseError - } - - ReconcileCRDError struct { - BaseError - } -) - -// Constructor functions -func NewReconcileGraphError(err error) *ReconcileGraphError { - return &ReconcileGraphError{BaseError{ErrType: "ReconcileGraphError", Err: err}} -} - -func NewReconcileMicroControllerError(err error) *ReconcileMicroControllerError { - return &ReconcileMicroControllerError{BaseError{ErrType: "ReconcileMicroControllerError", Err: err}} -} - -func NewReconcileCRDError(err error) *ReconcileCRDError { - return &ReconcileCRDError{BaseError{ErrType: "ReconcileCRDError", Err: err}} -} diff --git a/internal/graph/builder.go b/internal/graph/builder.go index 6852dabb..8e90579f 100644 --- a/internal/graph/builder.go +++ b/internal/graph/builder.go @@ -198,7 +198,7 @@ func (b *Builder) NewResourceGroup(originalCR *v1alpha1.ResourceGroup) (*Graph, resources, ) if err != nil { - return nil, fmt.Errorf("failed to build resourcegroup %v: %w", err, nil) + return nil, fmt.Errorf("failed to build resourcegroup '%v': %w", rg.Name, err) } // Before getting into the dependency graph, we need to validate the CEL expressions diff --git a/test/integration/suites/core/status_test.go b/test/integration/suites/core/status_test.go index c242175a..5a443091 100644 --- a/test/integration/suites/core/status_test.go +++ b/test/integration/suites/core/status_test.go @@ -120,7 +120,7 @@ var _ = Describe("Status", func() { g.Expect(crdCondition).ToNot(BeNil()) g.Expect(crdCondition.Status).To(Equal(metav1.ConditionFalse)) - g.Expect(*crdCondition.Reason).To(ContainSubstring("Error")) + g.Expect(*crdCondition.Reason).To(ContainSubstring("failed to build resourcegroup")) }, 10*time.Second, time.Second).Should(Succeed()) }) })