diff --git a/api/go.mod b/api/go.mod index feb95712..c29cc516 100644 --- a/api/go.mod +++ b/api/go.mod @@ -5,7 +5,7 @@ go 1.20 require ( github.com/gophercloud/gophercloud v1.11.0 github.com/openstack-k8s-operators/keystone-operator/api v0.3.1-0.20240313143432-9108b7f7290a - github.com/openstack-k8s-operators/lib-common/modules/common v0.3.1-0.20240314165949-fec16b14c33b + github.com/openstack-k8s-operators/lib-common/modules/common v0.3.1-0.20240326081751-56015b1ae3f6 github.com/openstack-k8s-operators/lib-common/modules/openstack v0.3.1-0.20240314165949-fec16b14c33b github.com/openstack-k8s-operators/lib-common/modules/storage v0.3.1-0.20240314165949-fec16b14c33b k8s.io/api v0.28.7 diff --git a/api/go.sum b/api/go.sum index 56850d29..57586834 100644 --- a/api/go.sum +++ b/api/go.sum @@ -69,8 +69,8 @@ github.com/openshift/api v0.0.0-20230414143018-3367bc7e6ac7 h1:rncLxJBpFGqBztyxC github.com/openshift/api v0.0.0-20230414143018-3367bc7e6ac7/go.mod h1:ctXNyWanKEjGj8sss1KjjHQ3ENKFm33FFnS5BKaIPh4= github.com/openstack-k8s-operators/keystone-operator/api v0.3.1-0.20240313143432-9108b7f7290a h1:XcUHh0j65hm8/4orLTH6aRTv3Ah4rGP1rA4yu7G0fR0= github.com/openstack-k8s-operators/keystone-operator/api v0.3.1-0.20240313143432-9108b7f7290a/go.mod h1:8C7VPKXAxiwB5Z4Kwn12VL0guW6onIG0Ayxiio5Vyu0= -github.com/openstack-k8s-operators/lib-common/modules/common v0.3.1-0.20240314165949-fec16b14c33b h1:5EzrrjcGziV69MsEgoBwPdsggY56M6jUxGBP9pp+hwo= -github.com/openstack-k8s-operators/lib-common/modules/common v0.3.1-0.20240314165949-fec16b14c33b/go.mod h1:DL+Ts0k+fzgZmx0XxWArIeAmdKuTkPa1I5DThdybfmE= +github.com/openstack-k8s-operators/lib-common/modules/common v0.3.1-0.20240326081751-56015b1ae3f6 h1:4Z7LjnjEF82XiusXJTI/4TqgwnJas3cdvg/qEgkrW8Q= +github.com/openstack-k8s-operators/lib-common/modules/common v0.3.1-0.20240326081751-56015b1ae3f6/go.mod h1:DL+Ts0k+fzgZmx0XxWArIeAmdKuTkPa1I5DThdybfmE= github.com/openstack-k8s-operators/lib-common/modules/openstack v0.3.1-0.20240314165949-fec16b14c33b h1:FEbadtLx4+ktxf79ZJoKZmfMNsQyqqgL5T9NXWc3i/k= github.com/openstack-k8s-operators/lib-common/modules/openstack v0.3.1-0.20240314165949-fec16b14c33b/go.mod h1:ghnFgNIzj4amS897wEto+L+jYzDSg2cJ6y32RNfFGhk= github.com/openstack-k8s-operators/lib-common/modules/storage v0.3.1-0.20240314165949-fec16b14c33b h1:lygG1KiF5d9HpKpGAl5fa8JVlC9j5VFvC4iKvJkJslA= diff --git a/api/v1beta1/conditions.go b/api/v1beta1/conditions.go index 854a00b3..f5552c55 100644 --- a/api/v1beta1/conditions.go +++ b/api/v1beta1/conditions.go @@ -23,6 +23,8 @@ import ( const ( // GlanceAPIReadyCondition Status=True condition which indicates if the GlanceAPI is configured and operational GlanceAPIReadyCondition condition.Type = "GlanceAPIReady" + // CinderCondition + CinderCondition= "CinderReady" ) // Glance Reasons used by API objects. @@ -35,7 +37,10 @@ const ( // // GlanceAPIReadyInitMessage GlanceAPIReadyInitMessage = "GlanceAPI not started" - // GlanceAPIReadyErrorMessage GlanceAPIReadyErrorMessage = "GlanceAPI error occured %s" + // CinderInitMessage + CinderInitMessage = "Waiting for Cinder resources" + // CinderReadyMessage + CinderReadyMessage = "Cinder resources exist" ) diff --git a/api/v1beta1/glance_types.go b/api/v1beta1/glance_types.go index 59535099..2424831f 100644 --- a/api/v1beta1/glance_types.go +++ b/api/v1beta1/glance_types.go @@ -206,7 +206,11 @@ func init() { SchemeBuilder.Register(&Glance{}, &GlanceList{}) } -// IsReady - returns true if Glance is reconciled successfully +// IsReady - returns true if the underlying GlanceAPI is reconciled successfully +// and set the ReadyCondition to True: it is mirrored to the top-level CR, so +// the purpose of this function is to let the openstack-operator to gather the +// Status of the entire Glance Deployment (intended as a single entity) from a +// single place func (instance Glance) IsReady() bool { return instance.Status.Conditions.IsTrue(condition.ReadyCondition) } diff --git a/controllers/glance_controller.go b/controllers/glance_controller.go index 5a74567c..ec254df8 100644 --- a/controllers/glance_controller.go +++ b/controllers/glance_controller.go @@ -123,6 +123,17 @@ func (r *GlanceReconciler) Reconcile(ctx context.Context, req ctrl.Request) (res return ctrl.Result{}, err } + // initialize status if Conditions is nil, but do not reset if it already + // exists + isNewInstance := instance.Status.Conditions == nil + if isNewInstance { + instance.Status.Conditions = condition.Conditions{} + } + + // Save a copy of the condtions so that we can restore the LastTransitionTime + // when a condition's state doesn't change. + savedConditions := instance.Status.Conditions.DeepCopy() + // Always patch the instance status when exiting this function so we can persist any changes. defer func() { // update the Ready condition based on the sub conditions @@ -137,6 +148,7 @@ func (r *GlanceReconciler) Reconcile(ctx context.Context, req ctrl.Request) (res instance.Status.Conditions.Set( instance.Status.Conditions.Mirror(condition.ReadyCondition)) } + condition.RestoreLastTransitionTimes(&instance.Status.Conditions, savedConditions) err := helper.PatchInstance(ctx, instance) if err != nil { _err = err @@ -144,39 +156,32 @@ func (r *GlanceReconciler) Reconcile(ctx context.Context, req ctrl.Request) (res } }() - // If we're not deleting this and the service object doesn't have our finalizer, add it. - if instance.DeletionTimestamp.IsZero() && controllerutil.AddFinalizer(instance, helper.GetFinalizer()) { - return ctrl.Result{}, nil - } - - // - // initialize status - // - if instance.Status.Conditions == nil { - instance.Status.Conditions = condition.Conditions{} - // initialize conditions used later as Status=Unknown - cl := condition.CreateList( - condition.UnknownCondition(condition.DBReadyCondition, condition.InitReason, condition.DBReadyInitMessage), - condition.UnknownCondition(condition.DBSyncReadyCondition, condition.InitReason, condition.DBSyncReadyInitMessage), - condition.UnknownCondition(condition.MemcachedReadyCondition, condition.InitReason, condition.MemcachedReadyInitMessage), - condition.UnknownCondition(condition.InputReadyCondition, condition.InitReason, condition.InputReadyInitMessage), - condition.UnknownCondition(condition.ServiceConfigReadyCondition, condition.InitReason, condition.ServiceConfigReadyInitMessage), - condition.UnknownCondition(glancev1.GlanceAPIReadyCondition, condition.InitReason, glancev1.GlanceAPIReadyInitMessage), - // right now we have no dedicated KeystoneServiceReadyInitMessage - condition.UnknownCondition(condition.KeystoneServiceReadyCondition, condition.InitReason, ""), - condition.UnknownCondition(condition.NetworkAttachmentsReadyCondition, condition.InitReason, condition.NetworkAttachmentsReadyInitMessage), - // service account, role, rolebinding conditions - condition.UnknownCondition(condition.ServiceAccountReadyCondition, condition.InitReason, condition.ServiceAccountReadyInitMessage), - condition.UnknownCondition(condition.RoleReadyCondition, condition.InitReason, condition.RoleReadyInitMessage), - condition.UnknownCondition(condition.RoleBindingReadyCondition, condition.InitReason, condition.RoleBindingReadyInitMessage), - condition.UnknownCondition(condition.CronJobReadyCondition, condition.InitReason, condition.CronJobReadyInitMessage), - ) - - instance.Status.Conditions.Init(&cl) + // initialize conditions used later as Status=Unknown, except the ReadyCondition + // that should be False when we start + cl := condition.CreateList( + // Mark ReadyCondition as False from the beginning + condition.FalseCondition(condition.ReadyCondition, condition.InitReason, condition.SeverityInfo, condition.ReadyInitMessage), + condition.UnknownCondition(condition.DBReadyCondition, condition.InitReason, condition.DBReadyInitMessage), + condition.UnknownCondition(condition.DBSyncReadyCondition, condition.InitReason, condition.DBSyncReadyInitMessage), + condition.UnknownCondition(condition.MemcachedReadyCondition, condition.InitReason, condition.MemcachedReadyInitMessage), + condition.UnknownCondition(condition.InputReadyCondition, condition.InitReason, condition.InputReadyInitMessage), + condition.UnknownCondition(condition.ServiceConfigReadyCondition, condition.InitReason, condition.ServiceConfigReadyInitMessage), + condition.UnknownCondition(glancev1.GlanceAPIReadyCondition, condition.InitReason, glancev1.GlanceAPIReadyInitMessage), + condition.UnknownCondition(condition.KeystoneServiceReadyCondition, condition.InitReason, ""), + // service account, role, rolebinding conditions + condition.UnknownCondition(condition.ServiceAccountReadyCondition, condition.InitReason, condition.ServiceAccountReadyInitMessage), + condition.UnknownCondition(condition.RoleReadyCondition, condition.InitReason, condition.RoleReadyInitMessage), + condition.UnknownCondition(condition.RoleBindingReadyCondition, condition.InitReason, condition.RoleBindingReadyInitMessage), + condition.UnknownCondition(condition.CronJobReadyCondition, condition.InitReason, condition.CronJobReadyInitMessage), + ) + instance.Status.Conditions.Init(&cl) + // If we're not deleting this and the service object doesn't have our finalizer, add it. + if instance.DeletionTimestamp.IsZero() && controllerutil.AddFinalizer(instance, helper.GetFinalizer()) || isNewInstance { // Register overall status immediately to have an early feedback e.g. in the cli return ctrl.Result{}, nil } + if instance.Status.Hash == nil { instance.Status.Hash = map[string]string{} } @@ -479,12 +484,7 @@ func (r *GlanceReconciler) reconcileInit( r.Log.Info(fmt.Sprintf("Service '%s' - Job %s hash added - %s", instance.Name, jobDef.Name, instance.Status.Hash[glancev1.DbSyncHash])) } instance.Status.Conditions.MarkTrue(condition.DBSyncReadyCondition, condition.DBSyncReadyMessage) - - // when job passed, mark NetworkAttachmentsReadyCondition ready - instance.Status.Conditions.MarkTrue(condition.NetworkAttachmentsReadyCondition, condition.NetworkAttachmentsReadyMessage) - // run Glance db sync - end - r.Log.Info(fmt.Sprintf("Reconciled Service '%s' init successfully", instance.Name)) return ctrl.Result{}, nil } @@ -699,8 +699,14 @@ func (r *GlanceReconciler) reconcileNormal(ctx context.Context, instance *glance return ctrlResult, err } instance.Status.Conditions.MarkTrue(condition.CronJobReadyCondition, condition.CronJobReadyMessage) - // create CronJob - end + + // We reached the end of the Reconcile, update the Ready condition based on + // the sub conditions + if instance.Status.Conditions.AllSubConditionIsTrue() { + instance.Status.Conditions.MarkTrue( + condition.ReadyCondition, condition.ReadyMessage) + } return ctrl.Result{}, nil } @@ -873,6 +879,10 @@ func (r *GlanceReconciler) apiDeploymentCreateOrUpdate( apiSpec.GlanceAPITemplate.StorageRequest = instance.Spec.StorageRequest apiSpec.GlanceAPITemplate.StorageClass = instance.Spec.StorageClass apiSpec.MemcachedInstance = memcached.Name + // Make sure to inject the ContainerImage passed by the OpenStackVersions + // resource to all the underlying instances and rollout a new StatefulSet + // if it has been changed + apiSpec.GlanceAPITemplate.ContainerImage = instance.Spec.ContainerImage // We select which glanceAPI should register the keystoneEndpoint by using // an API selector defined in the main glance CR; if it matches with the diff --git a/controllers/glanceapi_controller.go b/controllers/glanceapi_controller.go index 4f829790..044f234a 100644 --- a/controllers/glanceapi_controller.go +++ b/controllers/glanceapi_controller.go @@ -23,10 +23,8 @@ import ( "time" batchv1 "k8s.io/api/batch/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - - "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/resource" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/fields" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" @@ -120,20 +118,25 @@ func (r *GlanceAPIReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( return ctrl.Result{}, err } + // + // initialize status + // + // initialize status if Conditions is nil, but do not reset if it + // already exists + isNewInstance := instance.Status.Conditions == nil + if isNewInstance { + instance.Status.Conditions = condition.Conditions{} + } + + // Save a copy of the condtions so that we can restore the LastTransitionTime + // when a condition's state doesn't change. + savedConditions := instance.Status.Conditions.DeepCopy() + // Always patch the instance status when exiting this function so we can persist any changes. defer func() { - // update the Ready condition based on the sub conditions - if instance.Status.Conditions.AllSubConditionIsTrue() { - instance.Status.Conditions.MarkTrue( - condition.ReadyCondition, condition.ReadyMessage) - } else { - // something is not ready so reset the Ready condition - instance.Status.Conditions.MarkUnknown( - condition.ReadyCondition, condition.InitReason, condition.ReadyInitMessage) - // and recalculate it based on the state of the rest of the conditions - instance.Status.Conditions.Set( - instance.Status.Conditions.Mirror(condition.ReadyCondition)) - } + // Always mirror the Condition from the sub level CRs + instance.Status.Conditions.Set(instance.Status.Conditions.Mirror(condition.ReadyCondition)) + condition.RestoreLastTransitionTimes(&instance.Status.Conditions, savedConditions) err := helper.PatchInstance(ctx, instance) if err != nil { _err = err @@ -141,34 +144,28 @@ func (r *GlanceAPIReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( } }() - // If we're not deleting this and the service object doesn't have our finalizer, add it. - if instance.DeletionTimestamp.IsZero() && controllerutil.AddFinalizer(instance, helper.GetFinalizer()) { - return ctrl.Result{}, nil - } - - // - // initialize status - // - if instance.Status.Conditions == nil { - instance.Status.Conditions = condition.Conditions{} - // initialize conditions used later as Status=Unknown - cl := condition.CreateList( - condition.UnknownCondition(condition.ExposeServiceReadyCondition, condition.InitReason, condition.ExposeServiceReadyInitMessage), - condition.UnknownCondition(condition.InputReadyCondition, condition.InitReason, condition.InputReadyInitMessage), - condition.UnknownCondition(condition.ServiceConfigReadyCondition, condition.InitReason, condition.ServiceConfigReadyInitMessage), - condition.UnknownCondition(condition.DeploymentReadyCondition, condition.InitReason, condition.DeploymentReadyInitMessage), - condition.UnknownCondition(condition.TLSInputReadyCondition, condition.InitReason, condition.InputReadyInitMessage), - // right now we have no dedicated KeystoneEndpointReadyInitMessage - condition.UnknownCondition(condition.KeystoneEndpointReadyCondition, condition.InitReason, ""), - condition.UnknownCondition(condition.NetworkAttachmentsReadyCondition, condition.InitReason, condition.NetworkAttachmentsReadyInitMessage), - condition.UnknownCondition(condition.CronJobReadyCondition, condition.InitReason, condition.CronJobReadyInitMessage), - ) + // initialize conditions used later as Status=Unknown + cl := condition.CreateList( + condition.UnknownCondition(glancev1.CinderCondition, condition.InitReason, glancev1.CinderInitMessage), + condition.UnknownCondition(condition.ExposeServiceReadyCondition, condition.InitReason, condition.ExposeServiceReadyInitMessage), + condition.UnknownCondition(condition.InputReadyCondition, condition.InitReason, condition.InputReadyInitMessage), + condition.UnknownCondition(condition.ServiceConfigReadyCondition, condition.InitReason, condition.ServiceConfigReadyInitMessage), + condition.UnknownCondition(condition.DeploymentReadyCondition, condition.InitReason, condition.DeploymentReadyInitMessage), + condition.UnknownCondition(condition.TLSInputReadyCondition, condition.InitReason, condition.InputReadyInitMessage), + // right now we have no dedicated KeystoneEndpointReadyInitMessage + condition.UnknownCondition(condition.KeystoneEndpointReadyCondition, condition.InitReason, ""), + condition.UnknownCondition(condition.NetworkAttachmentsReadyCondition, condition.InitReason, condition.NetworkAttachmentsReadyInitMessage), + condition.UnknownCondition(condition.CronJobReadyCondition, condition.InitReason, condition.CronJobReadyInitMessage), + ) - instance.Status.Conditions.Init(&cl) + instance.Status.Conditions.Init(&cl) + // If we're not deleting this and the service object doesn't have our finalizer, add it. + if instance.DeletionTimestamp.IsZero() && controllerutil.AddFinalizer(instance, helper.GetFinalizer()) || isNewInstance { // Register overall status immediately to have an early feedback e.g. in the cli return ctrl.Result{}, nil } + if instance.Status.Hash == nil { instance.Status.Hash = map[string]string{} } @@ -476,6 +473,8 @@ func (r *GlanceAPIReconciler) reconcileInit( serviceLabels, ) if err != nil { + // The ExposeServiceReadyCondition is already marked as False + // within the GetHeadlessService function: we can return return ctrlResult, err } @@ -490,6 +489,12 @@ func (r *GlanceAPIReconciler) reconcileInit( apiEndpoints[string(endpointType)], err = svc.GetAPIEndpoint( svcOverride.EndpointURL, data.Protocol, data.Path) if err != nil { + instance.Status.Conditions.MarkFalse( + condition.ExposeServiceReadyCondition, + condition.ErrorReason, + condition.SeverityWarning, + condition.ExposeServiceReadyErrorMessage, + err.Error()) return ctrl.Result{}, err } } @@ -508,9 +513,14 @@ func (r *GlanceAPIReconciler) reconcileInit( // Create/Patch the KeystoneEndpoints ctrlResult, err := r.ensureKeystoneEndpoints(ctx, helper, instance, serviceLabels) if err != nil { + instance.Status.Conditions.MarkFalse( + condition.KeystoneEndpointReadyCondition, + condition.ErrorReason, + condition.SeverityWarning, + "Creating KeyStoneEndpoint CR %s", + err.Error()) return ctrlResult, err } - r.Log.Info(fmt.Sprintf("Reconciled Service '%s' init successfully", instance.Name)) return ctrl.Result{}, nil } @@ -571,6 +581,12 @@ func (r *GlanceAPIReconciler) reconcileNormal(ctx context.Context, instance *gla availableBackends := glancev1.GetEnabledBackends(instance.Spec.CustomServiceConfig) _, hashChanged, err := r.createHashOfBackendConfig(instance, availableBackends) if err != nil { + instance.Status.Conditions.Set(condition.FalseCondition( + condition.InputReadyCondition, + condition.ErrorReason, + condition.SeverityWarning, + condition.InputReadyErrorMessage, + err.Error())) return ctrl.Result{}, err } // Update the current StateFulSet (by recreating it) only when a backend is @@ -587,12 +603,16 @@ func (r *GlanceAPIReconciler) reconcileNormal(ctx context.Context, instance *gla case backendToken[1] == "cinder": cinder := &cinderv1.Cinder{} err := r.Get(ctx, types.NamespacedName{Namespace: instance.Namespace, Name: glance.CinderName}, cinder) - if err != nil { - if errors.IsNotFound(err) { - // Request object not found, can't run GlanceAPI with this config - r.Log.Info("Cinder resource not found. Waiting for it to be deployed") - return ctrl.Result{RequeueAfter: time.Duration(10) * time.Second}, nil - } + if err != nil && !k8s_errors.IsNotFound(err) { + // Request object not found, GlanceAPI can't be executed with this config + r.Log.Info("Cinder resource not found. Waiting for it to be deployed") + instance.Status.Conditions.Set(condition.FalseCondition( + glancev1.CinderCondition, + condition.RequestedReason, + condition.SeverityInfo, + glancev1.CinderInitMessage), + ) + return ctrl.Result{RequeueAfter: time.Duration(10) * time.Second}, nil } // Cinder CR is found, we can unblock glance deployment because // it represents a valid backend. @@ -603,6 +623,10 @@ func (r *GlanceAPIReconciler) reconcileNormal(ctx context.Context, instance *gla imageConv = true } } + // If we reach this point, it means that either Cinder is not a backend for Glance + // or, in case Cinder is a backend for the current GlanceAPI, the associated resources + // are present in the control plane + instance.Status.Conditions.MarkTrue(glancev1.CinderCondition, glancev1.CinderReadyMessage) // Generate service config err = r.generateServiceConfig(ctx, helper, instance, &configVars, imageConv) @@ -639,9 +663,15 @@ func (r *GlanceAPIReconciler) reconcileNormal(ctx context.Context, instance *gla err.Error())) return ctrlResult, err } else if (ctrlResult != ctrl.Result{}) { + // Marking the condition as Unknown because we are not returining + // an err, but comparing the ctrlResult: this represents an in + // progress operation rather than something that failed + instance.Status.Conditions.MarkUnknown( + condition.TLSInputReadyCondition, + condition.InitReason, + condition.InputReadyInitMessage) return ctrlResult, nil } - if hash != "" { configVars[tls.CABundleKey] = env.SetValue(hash) } @@ -658,6 +688,13 @@ func (r *GlanceAPIReconciler) reconcileNormal(ctx context.Context, instance *gla err.Error())) return ctrlResult, err } else if (ctrlResult != ctrl.Result{}) { + // Marking the condition as Unknown because we are not returining + // an err, but comparing the ctrlResult: this represents an in + // progress operation rather than something that failed + instance.Status.Conditions.MarkUnknown( + condition.TLSInputReadyCondition, + condition.InitReason, + condition.InputReadyInitMessage) return ctrlResult, nil } configVars[tls.TLSHashName] = env.SetValue(certsHash) @@ -677,10 +714,6 @@ func (r *GlanceAPIReconciler) reconcileNormal(ctx context.Context, instance *gla condition.ServiceConfigReadyErrorMessage, err.Error())) return ctrl.Result{}, err - } else if hashChanged { - // Hash changed and instance status should be updated (which will be done by main defer func), - // so we need to return and reconcile again - return ctrl.Result{}, nil } instance.Status.Conditions.MarkTrue(condition.ServiceConfigReadyCondition, condition.ServiceConfigReadyMessage) // Create Secrets - end @@ -688,11 +721,17 @@ func (r *GlanceAPIReconciler) reconcileNormal(ctx context.Context, instance *gla // // TODO check when/if Init, Update, or Upgrade should/could be skipped // - var serviceAnnotations map[string]string // networks to attach to serviceAnnotations, ctrlResult, err = ensureNAD(ctx, &instance.Status.Conditions, instance.Spec.NetworkAttachments, helper) if err != nil { + instance.Status.Conditions.MarkFalse( + condition.NetworkAttachmentsReadyCondition, + condition.ErrorReason, + condition.SeverityWarning, + condition.NetworkAttachmentsReadyErrorMessage, + err, + ) return ctrlResult, err } @@ -762,6 +801,13 @@ func (r *GlanceAPIReconciler) reconcileNormal(ctx context.Context, instance *gla // verify if network attachment matches expectations networkReady, networkAttachmentStatus, err := nad.VerifyNetworkStatusFromAnnotation(ctx, helper, instance.Spec.NetworkAttachments, GetServiceLabels(instance), instance.Status.ReadyCount) if err != nil { + err = fmt.Errorf("verifying API NetworkAttachments (%s) %w", instance.Spec.NetworkAttachments, err) + instance.Status.Conditions.MarkFalse( + condition.NetworkAttachmentsReadyCondition, + condition.ErrorReason, + condition.SeverityWarning, + condition.NetworkAttachmentsReadyErrorMessage, + err) return ctrl.Result{}, err } @@ -779,8 +825,10 @@ func (r *GlanceAPIReconciler) reconcileNormal(ctx context.Context, instance *gla return ctrl.Result{}, err } - - if instance.Status.ReadyCount > 0 || *instance.Spec.Replicas == 0 { + // Mark the Deployment as Ready only if the number of Replicas is equals + // to the Deployed instances (ReadyCount), but mark it as True is Replicas + // is zero + if instance.Status.ReadyCount == *instance.Spec.Replicas || *instance.Spec.Replicas == 0 { instance.Status.Conditions.MarkTrue(condition.DeploymentReadyCondition, condition.DeploymentReadyMessage) } else { instance.Status.Conditions.Set(condition.FalseCondition( @@ -828,10 +876,19 @@ func (r *GlanceAPIReconciler) reconcileNormal(ctx context.Context, instance *gla } } - // Mark the CronJobReadyCondition as True because there's nothing to do here - instance.Status.Conditions.MarkTrue(condition.CronJobReadyCondition, condition.CronJobReadyMessage) - + // If we reach this point, we can mark the CronJobReadyCondition as True + instance.Status.Conditions.MarkTrue( + condition.CronJobReadyCondition, + condition.CronJobReadyMessage, + ) // create ImageCache cronJobs - end + + // We reached the end of the Reconcile, update the Ready condition based on + // the sub conditions + if instance.Status.Conditions.AllSubConditionIsTrue() { + instance.Status.Conditions.MarkTrue( + condition.ReadyCondition, condition.ReadyMessage) + } r.Log.Info(fmt.Sprintf("Reconciled Service '%s' successfully", instance.Name)) return ctrl.Result{}, nil } @@ -1042,13 +1099,11 @@ func (r *GlanceAPIReconciler) ensureKeystoneEndpoints( } return ctrlResult, nil } - // Build the keystoneEndpoints Spec ksEndpointSpec := keystonev1.KeystoneEndpointSpec{ ServiceName: glance.ServiceName, Endpoints: instance.Status.APIEndpoints, } - ksSvc := keystonev1.NewKeystoneEndpoint( instance.Name, instance.Namespace, @@ -1060,14 +1115,12 @@ func (r *GlanceAPIReconciler) ensureKeystoneEndpoints( if err != nil { return ctrlResult, err } - // mirror the Status, Reason, Severity and Message of the latest keystoneendpoint condition // into a local condition with the type condition.KeystoneEndpointReadyCondition c := ksSvc.GetConditions().Mirror(condition.KeystoneEndpointReadyCondition) if c != nil { instance.Status.Conditions.Set(c) } - return ctrlResult, nil } @@ -1232,7 +1285,7 @@ func (r *GlanceAPIReconciler) glanceAPIRefresh( ) error { sts, err := statefulset.GetStatefulSetWithName(ctx, h, fmt.Sprintf("%s-api", instance.Name), instance.Namespace) if err != nil { - if errors.IsNotFound(err) { + if k8s_errors.IsNotFound(err) { // Request object not found r.Log.Info(fmt.Sprintf("GlanceAPI %s-api: Statefulset not found.", instance.Name)) return nil diff --git a/docs/dev/design-decisions.md b/docs/dev/design-decisions.md index 8e2923c8..81b3a6f8 100644 --- a/docs/dev/design-decisions.md +++ b/docs/dev/design-decisions.md @@ -416,3 +416,41 @@ instance that should be registered in Keystone. An update to the `keystoneEndpoint` parameter results in a reconciliation execution that switches the two affected instances (the one registered in keystone and the one proposed by the new update). + +## How Conditions are managed + +Conditions represent a critical aspect for reconciliation because they allow: + +1. to evaluate the status of a particular component validating a set of conditions +2. give a feedback to the end user about the current state of the Deployment +3. identify the status of the underlying GlanceAPIs and report their healhy to + the upper level CR + +The document +[docs/conditions](https://github.com/openstack-k8s-operators/docs/blob/main/conditions.md) +describes the meaning of the k8s-operators shared `Conditions`. +Conditions are re-evaluated when a new `Reconcile` loop starts, and if one of +them is different from what was previously registered, an update is performed, +and it allows to give immediate feedback to the user. +Conditions are initially marked as `Unknown`, while the general `ReadyCondition` +is marked as `False` because we assume that the `Deployment` is in progress but +not `Ready` at the same time, until the entire set of conditions are evaluated. +`Conditions` can be seen as a checklist or steps within the reconcile loop, and +while the loop proceed to the end, each of them is evaluated. +If a particular component with an associated condition is not `Ready`, an `error` +is returned from the operator, and its failing condition is marked as `False` +with an appropriate error message. +If the end of the loop is reached, it means we passed through all the steps and +the `Conditions` are marked to `True`: it is possible, at this point, to mark the +overall `ReadyCondition` to `Status=True` as well and `Mirror` the result to the +top-level CR. +`Conditions` are always Mirrored using the defer function, so that we always +have an information to the `ReadyCondition` that reflects the point where the +loop is exiting/failing. +The general `IsReady` function is used as a wrapper for the `ReadyCondition` +boolean, and it can be used to get the status of the `instance`. In general +in the glance-operator we make the following assumptions: +- A Glance/GlanceAPI is considered `Ready` if all the subconditions are verified +- A Deployment (intended as the `StatefulSet` that represents the GlanceAPIs) is + considered `Ready` if the number of `Replicas` specified in the `Glance` CR + spec is **equal** to the number of available instances (`ReadyCount`). diff --git a/go.mod b/go.mod index 9ff54a9e..e2ce2ad9 100644 --- a/go.mod +++ b/go.mod @@ -12,7 +12,7 @@ require ( github.com/openstack-k8s-operators/glance-operator/api v0.0.0-00010101000000-000000000000 github.com/openstack-k8s-operators/infra-operator/apis v0.3.1-0.20240313161042-88282483a04f github.com/openstack-k8s-operators/keystone-operator/api v0.3.1-0.20240313143432-9108b7f7290a - github.com/openstack-k8s-operators/lib-common/modules/common v0.3.1-0.20240314165949-fec16b14c33b + github.com/openstack-k8s-operators/lib-common/modules/common v0.3.1-0.20240326081751-56015b1ae3f6 github.com/openstack-k8s-operators/lib-common/modules/openstack v0.3.1-0.20240314165949-fec16b14c33b github.com/openstack-k8s-operators/lib-common/modules/storage v0.3.1-0.20240314165949-fec16b14c33b github.com/openstack-k8s-operators/lib-common/modules/test v0.3.1-0.20240314165949-fec16b14c33b diff --git a/go.sum b/go.sum index 098119b0..7f1a2f33 100644 --- a/go.sum +++ b/go.sum @@ -84,8 +84,8 @@ github.com/openstack-k8s-operators/infra-operator/apis v0.3.1-0.20240313161042-8 github.com/openstack-k8s-operators/infra-operator/apis v0.3.1-0.20240313161042-88282483a04f/go.mod h1:qKuzDDDMlAmJn4JWPoUeBEzpAia7J17++hhzR0oPv88= github.com/openstack-k8s-operators/keystone-operator/api v0.3.1-0.20240313143432-9108b7f7290a h1:XcUHh0j65hm8/4orLTH6aRTv3Ah4rGP1rA4yu7G0fR0= github.com/openstack-k8s-operators/keystone-operator/api v0.3.1-0.20240313143432-9108b7f7290a/go.mod h1:8C7VPKXAxiwB5Z4Kwn12VL0guW6onIG0Ayxiio5Vyu0= -github.com/openstack-k8s-operators/lib-common/modules/common v0.3.1-0.20240314165949-fec16b14c33b h1:5EzrrjcGziV69MsEgoBwPdsggY56M6jUxGBP9pp+hwo= -github.com/openstack-k8s-operators/lib-common/modules/common v0.3.1-0.20240314165949-fec16b14c33b/go.mod h1:DL+Ts0k+fzgZmx0XxWArIeAmdKuTkPa1I5DThdybfmE= +github.com/openstack-k8s-operators/lib-common/modules/common v0.3.1-0.20240326081751-56015b1ae3f6 h1:4Z7LjnjEF82XiusXJTI/4TqgwnJas3cdvg/qEgkrW8Q= +github.com/openstack-k8s-operators/lib-common/modules/common v0.3.1-0.20240326081751-56015b1ae3f6/go.mod h1:DL+Ts0k+fzgZmx0XxWArIeAmdKuTkPa1I5DThdybfmE= github.com/openstack-k8s-operators/lib-common/modules/openstack v0.3.1-0.20240314165949-fec16b14c33b h1:FEbadtLx4+ktxf79ZJoKZmfMNsQyqqgL5T9NXWc3i/k= github.com/openstack-k8s-operators/lib-common/modules/openstack v0.3.1-0.20240314165949-fec16b14c33b/go.mod h1:ghnFgNIzj4amS897wEto+L+jYzDSg2cJ6y32RNfFGhk= github.com/openstack-k8s-operators/lib-common/modules/storage v0.3.1-0.20240314165949-fec16b14c33b h1:lygG1KiF5d9HpKpGAl5fa8JVlC9j5VFvC4iKvJkJslA= diff --git a/test/functional/glance_controller_test.go b/test/functional/glance_controller_test.go index c778ae79..eb849176 100644 --- a/test/functional/glance_controller_test.go +++ b/test/functional/glance_controller_test.go @@ -53,7 +53,7 @@ var _ = Describe("Glance controller", func() { It("initializes the status fields", func() { Eventually(func(g Gomega) { glance := GetGlance(glanceName) - g.Expect(glance.Status.Conditions).To(HaveLen(13)) + g.Expect(glance.Status.Conditions).To(HaveLen(12)) g.Expect(glance.Status.DatabaseHostname).To(Equal("")) g.Expect(glance.Status.APIEndpoints).To(BeEmpty()) }, timeout, interval).Should(Succeed()) diff --git a/test/functional/sample_test.go b/test/functional/sample_test.go index 6e454422..f0dabeb0 100644 --- a/test/functional/sample_test.go +++ b/test/functional/sample_test.go @@ -61,7 +61,7 @@ var _ = Describe("Samples", func() { Eventually(func(g Gomega) { name := CreateGlanceFromSample("glance_v1beta1_glance.yaml", glanceTest.Instance) glance := GetGlance(name) - g.Expect(glance.Status.Conditions).To(HaveLen(13)) + g.Expect(glance.Status.Conditions).To(HaveLen(12)) g.Expect(glance.Status.DatabaseHostname).To(Equal("")) g.Expect(glance.Status.APIEndpoints).To(BeEmpty()) }, timeout, interval).Should(Succeed())