Skip to content

Commit

Permalink
Add ObservedGeneration to the sub Resources
Browse files Browse the repository at this point in the history 
This patch does a few things:

1. it adds observedGeneration to the sub custom resources

2. it proposes to bump the observedGeneration at the beginning of the
   reconciliation loop (knative/serving#4937)

3. it checks, at the top level, if the ObservedGeneration matches with
   the metadata.generation assigned to the subCR(s)

4. before marking the DeploymentReadyCondition as True, the ObservedGeneration
   is compared with the Generation of the Deployment/DaemonSets

Signed-off-by: Francesco Pantano <[email protected]>
  • Loading branch information
@fmount
fmount committed Apr 9, 2024
1 parent f4d5094 commit 7891792
Show file tree
Hide file tree
Showing 9 changed files with 171 additions and 43 deletions.
8 changes: 8 additions & 0 deletions api/bases/octavia.openstack.org_octaviaamphoracontrollers.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -304,6 +304,14 @@ spec:
type: array
description: NetworkAttachment status of the deployment pods
type: object
observedGeneration:
description: ObservedGeneration - the most recent generation observed
for this service. If the observed generation is less than the spec
generation, then the controller has not processed the latest changes
injected by the opentack-operator in the top-level CR (e.g. the
ContainerImage)
format: int64
type: integer
readyCount:
description: ReadyCount of Octavia Amphora Controllers
format: int32
Expand Down
8 changes: 8 additions & 0 deletions api/bases/octavia.openstack.org_octaviaapis.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -459,6 +459,14 @@ spec:
type: array
description: NetworkAttachment status of the deployment pods
type: object
observedGeneration:
description: ObservedGeneration - the most recent generation observed
for this service. If the observed generation is less than the spec
generation, then the controller has not processed the latest changes
injected by the opentack-operator in the top-level CR (e.g. the
ContainerImage)
format: int64
type: integer
readyCount:
description: ReadyCount of octavia API instances
format: int32
Expand Down
6 changes: 6 additions & 0 deletions api/v1beta1/amphoracontroller_types.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,12 @@ type OctaviaAmphoraControllerStatus struct {

// NetworkAttachment status of the deployment pods
NetworkAttachments map[string][]string `json:"networkAttachments,omitempty"`

// ObservedGeneration - the most recent generation observed for this
// service. If the observed generation is less than the spec generation,
// then the controller has not processed the latest changes injected by
// the opentack-operator in the top-level CR (e.g. the ContainerImage)
ObservedGeneration int64 `json:"observedGeneration,omitempty"`
}

//+kubebuilder:object:root=true
Expand Down
6 changes: 6 additions & 0 deletions api/v1beta1/octaviaapi_types.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,12 @@ type OctaviaAPIStatus struct {

// NetworkAttachment status of the deployment pods
NetworkAttachments map[string][]string `json:"networkAttachments,omitempty"`

// ObservedGeneration - the most recent generation observed for this
// service. If the observed generation is less than the spec generation,
// then the controller has not processed the latest changes injected by
// the opentack-operator in the top-level CR (e.g. the ContainerImage)
ObservedGeneration int64 `json:"observedGeneration,omitempty"`
}

//+kubebuilder:object:root=true
Expand Down
8 changes: 8 additions & 0 deletions config/crd/bases/octavia.openstack.org_octaviaamphoracontrollers.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -304,6 +304,14 @@ spec:
type: array
description: NetworkAttachment status of the deployment pods
type: object
observedGeneration:
description: ObservedGeneration - the most recent generation observed
for this service. If the observed generation is less than the spec
generation, then the controller has not processed the latest changes
injected by the opentack-operator in the top-level CR (e.g. the
ContainerImage)
format: int64
type: integer
readyCount:
description: ReadyCount of Octavia Amphora Controllers
format: int32
Expand Down
8 changes: 8 additions & 0 deletions config/crd/bases/octavia.openstack.org_octaviaapis.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -459,6 +459,14 @@ spec:
type: array
description: NetworkAttachment status of the deployment pods
type: object
observedGeneration:
description: ObservedGeneration - the most recent generation observed
for this service. If the observed generation is less than the spec
generation, then the controller has not processed the latest changes
injected by the opentack-operator in the top-level CR (e.g. the
ContainerImage)
format: int64
type: integer
readyCount:
description: ReadyCount of octavia API instances
format: int32
Expand Down
4 changes: 3 additions & 1 deletion controllers/amphoracontroller_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,7 @@ func (r *OctaviaAmphoraControllerReconciler) Reconcile(ctx context.Context, req
)

instance.Status.Conditions.Init(&cl)
instance.Status.ObservedGeneration = instance.Generation

if isNewInstance {
if err := r.Status().Update(ctx, instance); err != nil {
Expand Down Expand Up @@ -493,7 +494,8 @@ func (r *OctaviaAmphoraControllerReconciler) reconcileNormal(ctx context.Context
instance.Status.DesiredNumberScheduled = dset.GetDaemonSet().Status.DesiredNumberScheduled
// TODO(gthiemonge) change for NumberReady?
instance.Status.ReadyCount = dset.GetDaemonSet().Status.NumberReady
if instance.Status.ReadyCount == instance.Status.DesiredNumberScheduled {
if instance.Status.ReadyCount == instance.Status.DesiredNumberScheduled &&
(dset.GetDaemonSet().Generation == dset.GetDaemonSet().Status.ObservedGeneration) {
instance.Status.Conditions.MarkTrue(condition.DeploymentReadyCondition, condition.DeploymentReadyMessage)
}

Expand Down
162 changes: 121 additions & 41 deletions controllers/octavia_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,7 @@ func (r *OctaviaReconciler) Reconcile(ctx context.Context, req ctrl.Request) (re
)

instance.Status.Conditions.Init(&cl)
instance.Status.ObservedGeneration = instance.Generation

// 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 {
Expand Down Expand Up @@ -609,19 +610,34 @@ func (r *OctaviaReconciler) reconcileNormal(ctx context.Context, instance *octav
err.Error()))
return ctrl.Result{}, err
}
if op != controllerutil.OperationResultNone {
Log.Info(fmt.Sprintf("Deployment %s successfully reconciled - operation: %s", instance.Name, string(op)))
// Check the underlying OctaviaAPI condition according to the
// ObservedGeneration
apiObsGen, err := r.checkOctaviaAPIGeneration(instance)
if err != nil {
instance.Status.Conditions.Set(condition.FalseCondition(
octaviav1.OctaviaAPIReadyCondition,
condition.ErrorReason,
condition.SeverityWarning,
octaviav1.OctaviaAPIReadyErrorMessage,
err.Error()))
return ctrlResult, nil
}

// Mirror OctaviaAPI status' ReadyCount to this parent CR
// TODO(beagles): We need to have a way to aggregate conditions from the other services into this
//
instance.Status.OctaviaAPIReadyCount = octaviaAPI.Status.ReadyCount
conditionStatus := octaviaAPI.Status.Conditions.Mirror(octaviav1.OctaviaAPIReadyCondition)
if conditionStatus != nil {
instance.Status.Conditions.Set(conditionStatus)
if !apiObsGen {
instance.Status.Conditions.Set(condition.UnknownCondition(
octaviav1.OctaviaAPIReadyCondition,
condition.InitReason,
octaviav1.OctaviaAPIReadyInitMessage,
))
} else {
instance.Status.Conditions.MarkTrue(octaviav1.OctaviaAPIReadyCondition, condition.DeploymentReadyMessage)
// Mirror OctaviaAPI status' ReadyCount to this parent CR
instance.Status.OctaviaAPIReadyCount = octaviaAPI.Status.ReadyCount
conditionStatus := octaviaAPI.Status.Conditions.Mirror(octaviav1.OctaviaAPIReadyCondition)
if conditionStatus != nil {
instance.Status.Conditions.Set(conditionStatus)
}
}
if op != controllerutil.OperationResultNone && apiObsGen {
Log.Info(fmt.Sprintf("Deployment %s successfully reconciled - operation: %s", instance.Name, string(op)))
}

// ------------------------------------------------------------------------------------------------------------
Expand Down Expand Up @@ -654,17 +670,34 @@ func (r *OctaviaReconciler) reconcileNormal(ctx context.Context, instance *octav
err.Error()))
return ctrl.Result{}, err
}

if op != controllerutil.OperationResultNone {
Log.Info(fmt.Sprintf("Deployment of OctaviaHealthManager for %s successfully reconciled - operation: %s", instance.Name, string(op)))
// Even if we trigger three deployments, the Amphora subCR is only one, no
// need to call this functions three times in the same reconciliation loop
ampObsGen, err := r.checkAmphoraGeneration(instance)
if err != nil {
instance.Status.Conditions.Set(condition.FalseCondition(
amphoraControllerReadyCondition(octaviav1.HealthManager),
condition.ErrorReason,
condition.SeverityWarning,
amphoraControllerErrorMessage(octaviav1.HealthManager),
err.Error()))
return ctrlResult, nil
}

instance.Status.OctaviaHealthManagerReadyCount = octaviaHealthManager.Status.ReadyCount
conditionStatus = octaviaHealthManager.Status.Conditions.Mirror(amphoraControllerReadyCondition(octaviav1.HealthManager))
if conditionStatus != nil {
instance.Status.Conditions.Set(conditionStatus)
if !ampObsGen {
instance.Status.Conditions.Set(condition.UnknownCondition(
amphoraControllerReadyCondition(octaviav1.HealthManager),
condition.InitReason,
amphoraControllerErrorMessage(octaviav1.HealthManager),
))
} else {
instance.Status.Conditions.MarkTrue(amphoraControllerReadyCondition(octaviav1.HealthManager), condition.DeploymentReadyMessage)
instance.Status.OctaviaHealthManagerReadyCount = octaviaHealthManager.Status.ReadyCount
conditionStatus := octaviaHealthManager.Status.Conditions.Mirror(amphoraControllerReadyCondition(octaviav1.HealthManager))
if conditionStatus != nil {
instance.Status.Conditions.Set(conditionStatus)
}
}

if op != controllerutil.OperationResultNone && ampObsGen {
Log.Info(fmt.Sprintf("Deployment of OctaviaHealthManager for %s successfully reconciled - operation: %s", instance.Name, string(op)))
}

//
Expand All @@ -687,17 +720,22 @@ func (r *OctaviaReconciler) reconcileNormal(ctx context.Context, instance *octav
err.Error()))
return ctrl.Result{}, err
}

if op != controllerutil.OperationResultNone {
Log.Info(fmt.Sprintf("Deployment of OctaviaHousekeeping for %s successfully reconciled - operation: %s", instance.Name, string(op)))
if !ampObsGen {
instance.Status.Conditions.Set(condition.UnknownCondition(
amphoraControllerReadyCondition(octaviav1.Housekeeping),
condition.InitReason,
amphoraControllerErrorMessage(octaviav1.Housekeeping),
))
} else {
instance.Status.OctaviaHousekeepingReadyCount = octaviaHousekeeping.Status.ReadyCount
conditionStatus := octaviaHousekeeping.Status.Conditions.Mirror(amphoraControllerReadyCondition(octaviav1.Housekeeping))
if conditionStatus != nil {
instance.Status.Conditions.Set(conditionStatus)
}
}

instance.Status.OctaviaHousekeepingReadyCount = octaviaHousekeeping.Status.ReadyCount
conditionStatus = octaviaHousekeeping.Status.Conditions.Mirror(amphoraControllerReadyCondition(octaviav1.Housekeeping))
if conditionStatus != nil {
instance.Status.Conditions.Set(conditionStatus)
} else {
instance.Status.Conditions.MarkTrue(amphoraControllerReadyCondition(octaviav1.Housekeeping), condition.DeploymentReadyMessage)
if op != controllerutil.OperationResultNone && ampObsGen {
Log.Info(fmt.Sprintf("Deployment of OctaviaHousekeeping for %s successfully reconciled - operation: %s", instance.Name, string(op)))
}

octaviaWorker, op, err := r.amphoraControllerDaemonSetCreateOrUpdate(instance, networkInfo,
Expand All @@ -711,17 +749,21 @@ func (r *OctaviaReconciler) reconcileNormal(ctx context.Context, instance *octav
err.Error()))
return ctrl.Result{}, err
}

if op != controllerutil.OperationResultNone {
Log.Info(fmt.Sprintf("Deployment of OctaviaWorker for %s successfully reconciled - operation: %s", instance.Name, string(op)))
}

instance.Status.OctaviaWorkerReadyCount = octaviaWorker.Status.ReadyCount
conditionStatus = octaviaWorker.Status.Conditions.Mirror(amphoraControllerReadyCondition(octaviav1.Worker))
if conditionStatus != nil {
instance.Status.Conditions.Set(conditionStatus)
if !ampObsGen {
instance.Status.Conditions.Set(condition.UnknownCondition(
amphoraControllerReadyCondition(octaviav1.Worker),
condition.InitReason,
amphoraControllerErrorMessage(octaviav1.Worker),
))
} else {
instance.Status.Conditions.MarkTrue(amphoraControllerReadyCondition(octaviav1.Worker), condition.DeploymentReadyMessage)
instance.Status.OctaviaWorkerReadyCount = octaviaWorker.Status.ReadyCount
conditionStatus := octaviaWorker.Status.Conditions.Mirror(amphoraControllerReadyCondition(octaviav1.Worker))
if conditionStatus != nil {
instance.Status.Conditions.Set(conditionStatus)
}
}
if op != controllerutil.OperationResultNone && ampObsGen {
Log.Info(fmt.Sprintf("Deployment of OctaviaWorker for %s successfully reconciled - operation: %s", instance.Name, string(op)))
}

// remove finalizers from unused MariaDBAccount records
Expand Down Expand Up @@ -751,8 +793,6 @@ func (r *OctaviaReconciler) reconcileNormal(ctx context.Context, instance *octav

// create Deployment - end

// Update the lastObserved generation before evaluating conditions
instance.Status.ObservedGeneration = instance.Generation
// We reached the end of the Reconcile, update the Ready condition based on
// the sub conditions
if instance.Status.Conditions.AllSubConditionIsTrue() {
Expand Down Expand Up @@ -1355,3 +1395,43 @@ func amphoraControllerErrorMessage(role string) string {
}
return condMap[role]
}

// checkOctaviaAPIGeneration -
func (r *OctaviaReconciler) checkOctaviaAPIGeneration(
instance *octaviav1.Octavia,
) (bool, error) {
api := &octaviav1.OctaviaAPIList{}
listOpts := []client.ListOption{
client.InNamespace(instance.Namespace),
}
if err := r.Client.List(context.Background(), api, listOpts...); err != nil {
r.Log.Error(err, "Unable to retrieve OctaviaAPI %w")
return false, err
}
for _, item := range api.Items {
if item.Generation != item.Status.ObservedGeneration {
return false, nil
}
}
return true, nil
}

// checkAmphoraGeneration -
func (r *OctaviaReconciler) checkAmphoraGeneration(
instance *octaviav1.Octavia,
) (bool, error) {
amph := &octaviav1.OctaviaAmphoraControllerList{}
listOpts := []client.ListOption{
client.InNamespace(instance.Namespace),
}
if err := r.Client.List(context.Background(), amph, listOpts...); err != nil {
r.Log.Error(err, "Unable to retrieve OctaviaAPI %w")
return false, err
}
for _, item := range amph.Items {
if item.Generation != item.Status.ObservedGeneration {
return false, nil
}
}
return true, nil
}
4 changes: 3 additions & 1 deletion controllers/octaviaapi_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,7 @@ func (r *OctaviaAPIReconciler) Reconcile(ctx context.Context, req ctrl.Request)
)

instance.Status.Conditions.Init(&cl)
instance.Status.ObservedGeneration = instance.Generation

// 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 {
Expand Down Expand Up @@ -811,7 +812,8 @@ func (r *OctaviaAPIReconciler) reconcileNormal(ctx context.Context, instance *oc
return ctrl.Result{}, err
}
instance.Status.ReadyCount = depl.GetDeployment().Status.ReadyReplicas
if instance.Status.ReadyCount > 0 {
if (instance.Status.ReadyCount == *instance.Spec.Replicas) &&
(depl.GetDeployment().Generation == depl.GetDeployment().Status.ObservedGeneration) {
instance.Status.Conditions.MarkTrue(condition.DeploymentReadyCondition, condition.DeploymentReadyMessage)
}
// create Deployment - end
Expand Down

0 comments on commit 7891792

Please sign in to comment.