Skip to content

Commit

Permalink
Merge pull request #156 from fmount/conditions
Browse files Browse the repository at this point in the history 
Re-init conditions each reconcile
  • Loading branch information
@openshift-merge-bot
openshift-merge-bot[bot] authored Apr 16, 2024
2 parents 7e783e8 + a971ed6 commit 7b9ec5a
Show file tree
Hide file tree
Showing 7 changed files with 125 additions and 88 deletions.
8 changes: 8 additions & 0 deletions api/bases/baremetal.openstack.org_openstackbaremetalsets.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -399,6 +399,14 @@ spec:
type: string
description: Map of hashes to track e.g. job status
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
type: object
type: object
served: true
Expand Down
5 changes: 5 additions & 0 deletions api/v1beta1/openstackbaremetalset_types.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,11 @@ type OpenStackBaremetalSetStatus struct {
Hash map[string]string `json:"hash,omitempty"`
// BaremetalHosts that are being processed or have been processed for this OpenStackBaremetalSet
BaremetalHosts map[string]HostStatus `json:"baremetalHosts,omitempty" optional:"true"`
// 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/baremetal.openstack.org_openstackbaremetalsets.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -399,6 +399,14 @@ spec:
type: string
description: Map of hashes to track e.g. job status
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
type: object
type: object
served: true
Expand Down
59 changes: 34 additions & 25 deletions controllers/openstackbaremetalset_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -95,17 +95,23 @@ func (r *OpenStackBaremetalSetReconciler) Reconcile(ctx context.Context, req ctr
return ctrl.Result{}, err
}

// Always patch the instance status when exiting this function so we can persist any changes.
// 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
condition.RestoreLastTransitionTimes(
&instance.Status.Conditions, savedConditions)
if instance.Status.Conditions.IsUnknown(condition.ReadyCondition) {
instance.Status.Conditions.Set(
instance.Status.Conditions.Mirror(condition.ReadyCondition))
}
Expand All @@ -116,28 +122,25 @@ func (r *OpenStackBaremetalSetReconciler) Reconcile(ctx context.Context, req ctr
}
}()

// 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.InputReadyCondition, condition.InitReason, condition.InputReadyInitMessage),
condition.UnknownCondition(baremetalv1.OpenStackBaremetalSetProvServerReadyCondition, condition.InitReason, baremetalv1.OpenStackBaremetalSetProvServerReadyInitMessage),
condition.UnknownCondition(baremetalv1.OpenStackBaremetalSetBmhProvisioningReadyCondition, condition.InitReason, baremetalv1.OpenStackBaremetalSetBmhProvisioningReadyInitMessage),
)
// initialize conditions used later as Status=Unknown
cl := condition.CreateList(
condition.UnknownCondition(condition.ReadyCondition, condition.InitReason, condition.ReadyInitMessage),
condition.UnknownCondition(condition.InputReadyCondition, condition.InitReason, condition.InputReadyInitMessage),
condition.UnknownCondition(baremetalv1.OpenStackBaremetalSetProvServerReadyCondition, condition.InitReason, baremetalv1.OpenStackBaremetalSetProvServerReadyInitMessage),
condition.UnknownCondition(baremetalv1.OpenStackBaremetalSetBmhProvisioningReadyCondition, condition.InitReason, baremetalv1.OpenStackBaremetalSetBmhProvisioningReadyInitMessage),
)

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

// Register overall status immediately to have an early feedback e.g. in the cli
// 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 {
return ctrl.Result{}, nil
}

if instance.Status.Hash == nil {
instance.Status.Hash = map[string]string{}
}
Expand Down Expand Up @@ -446,6 +449,12 @@ func (r *OpenStackBaremetalSetReconciler) reconcileNormal(ctx context.Context, i
instance.Status.Conditions.MarkTrue(baremetalv1.OpenStackBaremetalSetBmhProvisioningReadyCondition, baremetalv1.OpenStackBaremetalSetBmhProvisioningReadyMessage)
// provision BMHs - 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 OpenStackBaremetalSet '%s' successfully", instance.Name))
return ctrl.Result{}, nil
}
Expand Down
127 changes: 67 additions & 60 deletions controllers/openstackprovisionserver_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -119,17 +119,23 @@ func (r *OpenStackProvisionServerReconciler) Reconcile(ctx context.Context, req
return ctrl.Result{}, err
}

// Always patch the instance status when exiting this function so we can persist any changes.
// 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
condition.RestoreLastTransitionTimes(
&instance.Status.Conditions, savedConditions)
if instance.Status.Conditions.IsUnknown(condition.ReadyCondition) {
instance.Status.Conditions.Set(
instance.Status.Conditions.Mirror(condition.ReadyCondition))
}
Expand All @@ -140,61 +146,56 @@ func (r *OpenStackProvisionServerReconciler) Reconcile(ctx context.Context, req
}
}()

// 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.DeploymentReadyCondition,
condition.InitReason,
condition.DeploymentReadyInitMessage,
),
condition.UnknownCondition(
condition.ServiceConfigReadyCondition,
condition.InitReason,
condition.ServiceConfigReadyInitMessage,
),
condition.UnknownCondition(
baremetalv1.OpenStackProvisionServerProvIntfReadyCondition,
condition.InitReason,
baremetalv1.OpenStackProvisionServerProvIntfReadyInitMessage,
),
condition.UnknownCondition(
baremetalv1.OpenStackProvisionServerLocalImageURLReadyCondition,
condition.InitReason,
baremetalv1.OpenStackProvisionServerLocalImageURLReadyInitMessage,
),

// 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,
),
)
instance.Status.Conditions.Init(&cl)

// Register overall status immediately to have an early feedback e.g. in the cli
// initialize conditions used later as Status=Unknown
cl := condition.CreateList(
condition.UnknownCondition(
condition.DeploymentReadyCondition,
condition.InitReason,
condition.DeploymentReadyInitMessage,
),
condition.UnknownCondition(
condition.ServiceConfigReadyCondition,
condition.InitReason,
condition.ServiceConfigReadyInitMessage,
),
condition.UnknownCondition(
baremetalv1.OpenStackProvisionServerProvIntfReadyCondition,
condition.InitReason,
baremetalv1.OpenStackProvisionServerProvIntfReadyInitMessage,
),
condition.UnknownCondition(
baremetalv1.OpenStackProvisionServerLocalImageURLReadyCondition,
condition.InitReason,
baremetalv1.OpenStackProvisionServerLocalImageURLReadyInitMessage,
),

// 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,
),
)
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 {
return ctrl.Result{}, nil
}

if instance.Status.Hash == nil {
instance.Status.Hash = map[string]string{}
}
Expand Down Expand Up @@ -455,6 +456,12 @@ func (r *OpenStackProvisionServerReconciler) reconcileNormal(ctx context.Context
}
// check ProvisionIp/LocalImageURL - 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 OpenStackProvisionServer '%s' successfully", instance.Name))
return ctrl.Result{}, nil
}
Expand Down
2 changes: 1 addition & 1 deletion go.mod
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ require (
github.com/metal3-io/baremetal-operator/apis v0.5.1
github.com/onsi/ginkgo/v2 v2.17.1
github.com/onsi/gomega v1.32.0
github.com/openstack-k8s-operators/lib-common/modules/common v0.3.1-0.20240214144842-5dcac51e5b36
github.com/openstack-k8s-operators/lib-common/modules/common v0.3.1-0.20240412091425-bb628ded5eb8
github.com/openstack-k8s-operators/openstack-baremetal-operator/api v0.0.0-00010101000000-000000000000
github.com/spf13/cobra v1.8.0
k8s.io/api v0.28.8
Expand Down
4 changes: 2 additions & 2 deletions go.sum
View file
Original file line number Diff line number Diff line change
Expand Up @@ -94,8 +94,8 @@ github.com/onsi/ginkgo/v2 v2.17.1 h1:V++EzdbhI4ZV4ev0UTIj0PzhzOcReJFyJaLjtSF55M8
github.com/onsi/ginkgo/v2 v2.17.1/go.mod h1:llBI3WDLL9Z6taip6f33H76YcWtJv+7R3HigUjbIBOs=
github.com/onsi/gomega v1.32.0 h1:JRYU78fJ1LPxlckP6Txi/EYqJvjtMrDC04/MM5XRHPk=
github.com/onsi/gomega v1.32.0/go.mod h1:a4x4gW6Pz2yK1MAmvluYme5lvYTn61afQ2ETw/8n4Lg=
github.com/openstack-k8s-operators/lib-common/modules/common v0.3.1-0.20240214144842-5dcac51e5b36 h1:ZSVQYuevQyYZ+bD/x3NLzZx/oVcrsT2tG5agqFzs8fQ=
github.com/openstack-k8s-operators/lib-common/modules/common v0.3.1-0.20240214144842-5dcac51e5b36/go.mod h1:bQwzyQtWCR9F0+IvWZ30J9d1lB6tcX3CNJ0Ten1smDw=
github.com/openstack-k8s-operators/lib-common/modules/common v0.3.1-0.20240412091425-bb628ded5eb8 h1:5ywsORdn4y7vNO732ODwYlmfRXnEphfoqB6PCYWt9r8=
github.com/openstack-k8s-operators/lib-common/modules/common v0.3.1-0.20240412091425-bb628ded5eb8/go.mod h1:gqByVGUdKQB/NkhKV4eD+8NWYkHq961nC96rTCB3ywE=
github.com/pkg/errors v0.8.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0=
github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4=
github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0=
Expand Down

0 comments on commit 7b9ec5a

Please sign in to comment.