Skip to content

Commit

Permalink
THREESCALE-10090 address comments
Browse files Browse the repository at this point in the history
  • Loading branch information
@MStokluska @valerymo
MStokluska authored and valerymo committed Jan 14, 2024
1 parent 55125da commit 893262b
Show file tree
Hide file tree
Showing 4 changed files with 30 additions and 38 deletions.
6 changes: 1 addition & 5 deletions apis/capabilities/v1alpha1/tenant_types.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,6 @@ const (
// TenantReadyConditionType indicates the tenant has been successfully created.
// Steady state
TenantReadyConditionType common.ConditionType = "Ready"

// TenantFailedConditionType indicates that an error occurred during creation.
// The operator will retry.
TenantFailedConditionType common.ConditionType = "Failed"
)

// TenantSpec defines the desired state of Tenant
Expand Down Expand Up @@ -165,7 +161,7 @@ func (b *TenantStatus) StatusEqual(other *TenantStatus, logger logr.Logger) bool
equal = false
}

equal = conditionsEqual(TenantReadyConditionType, b.Conditions, other.Conditions) && conditionsEqual(TenantFailedConditionType, b.Conditions, other.Conditions)
equal = conditionsEqual(TenantReadyConditionType, b.Conditions, other.Conditions)

return equal
}
Expand Down
12 changes: 5 additions & 7 deletions controllers/capabilities/tenant_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -92,8 +92,6 @@ func (r *TenantReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr
return ctrl.Result{}, err
}

originalTenantCR := tenantCR.DeepCopy()

if reqLogger.V(1).Enabled() {
jsonData, err := json.MarshalIndent(tenantCR, "", " ")
if err != nil {
Expand All @@ -105,7 +103,7 @@ func (r *TenantReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr
// Setup porta client
portaClient, err := r.setupPortaClient(tenantCR, reqLogger)
if err != nil {
_, statusReconcilerError := r.reconcileStatus(tenantCR, originalTenantCR, err)
_, statusReconcilerError := r.reconcileStatus(tenantCR, err)
if statusReconcilerError != nil {
return helper.ReconcileErrorHandler(err, reqLogger), nil
}
Expand Down Expand Up @@ -165,7 +163,7 @@ func (r *TenantReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr
// Validate and update spec if required
internalReconciler := NewTenantThreescaleReconciler(r.BaseReconciler, tenantCR, portaClient, reqLogger)
specReconcileErr := internalReconciler.Run()
statusIsEqual, statusReconcilerError := r.reconcileStatus(tenantCR, originalTenantCR, specReconcileErr)
statusIsEqual, statusReconcilerError := r.reconcileStatus(tenantCR, specReconcileErr)
if statusReconcilerError != nil {
return helper.ReconcileErrorHandler(statusReconcilerError, reqLogger), nil
}
Expand All @@ -183,8 +181,8 @@ func (r *TenantReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr
return ctrl.Result{}, nil
}

func (r *TenantReconciler) reconcileStatus(updatedTenantCR, originalTenantCR *capabilitiesv1alpha1.Tenant, reconcileError error) (bool, error) {
statusReconciler := NewTenantStatusReconciler(r.BaseReconciler, updatedTenantCR, originalTenantCR, reconcileError)
func (r *TenantReconciler) reconcileStatus(tenantCR *capabilitiesv1alpha1.Tenant, reconcileError error) (bool, error) {
statusReconciler := NewTenantStatusReconciler(r.BaseReconciler, tenantCR, reconcileError)
statusEqual, err := statusReconciler.Reconcile()
if err != nil {
return statusEqual, err
Expand All @@ -198,7 +196,7 @@ func (r *TenantReconciler) reconcileMetadata(tenantCR *capabilitiesv1alpha1.Tena
// If the tenant.Status.TenantID is found and the annotation is not found - create
// If the tenant.Status.TenantID is found and the annotation is found but, the value of annotation is different to the status.TenantID - update
tenantId := tenantCR.Status.TenantId
if value, found := tenantCR.ObjectMeta.Annotations[tenantIdAnnotation]; tenantId != 0 && !found || tenantId != 0 && found && value != strconv.FormatInt(tenantCR.Status.TenantId, 10) {
if value, found := tenantCR.ObjectMeta.Annotations[tenantIdAnnotation]; (tenantId != 0 && !found) || (tenantId != 0 && found && value != strconv.FormatInt(tenantCR.Status.TenantId, 10)) {
if tenantCR.ObjectMeta.Annotations == nil {
tenantCR.ObjectMeta.Annotations = make(map[string]string)
}
Expand Down
30 changes: 14 additions & 16 deletions controllers/capabilities/tenant_status_reconciler.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,19 +11,17 @@ import (

type TenantStatusReconciler struct {
*reconcilers.BaseReconciler
resourceUpdated *capabilitiesv1alpha1.Tenant
resourceOriginal *capabilitiesv1alpha1.Tenant
reconcileError error
logger logr.Logger
tenantResource *capabilitiesv1alpha1.Tenant
reconcileError error
logger logr.Logger
}

func NewTenantStatusReconciler(b *reconcilers.BaseReconciler, resourceUpdated, resourceOriginal *capabilitiesv1alpha1.Tenant, reconcileError error) *TenantStatusReconciler {
func NewTenantStatusReconciler(b *reconcilers.BaseReconciler, tenantResource *capabilitiesv1alpha1.Tenant, reconcileError error) *TenantStatusReconciler {
return &TenantStatusReconciler{
BaseReconciler: b,
resourceUpdated: resourceUpdated,
resourceOriginal: resourceOriginal,
reconcileError: reconcileError,
logger: b.Logger().WithValues("Status Reconciler", resourceUpdated.Name),
BaseReconciler: b,
tenantResource: tenantResource,
reconcileError: reconcileError,
logger: b.Logger().WithValues("Status Reconciler", tenantResource.Name),
}
}

Expand All @@ -33,12 +31,12 @@ func (s *TenantStatusReconciler) Reconcile() (bool, error) {
equalStatus := true
// Check for changes to the status
newStatus := s.calculateStatus()
equalStatus = s.resourceOriginal.Status.StatusEqual(&newStatus, s.logger)
equalStatus = s.tenantResource.Status.StatusEqual(&newStatus, s.logger)

if !equalStatus {
s.logger.Info("updating tenant status")
s.resourceUpdated.Status = newStatus
err := s.UpdateResourceStatus(s.resourceUpdated)
s.tenantResource.Status = newStatus
err := s.UpdateResourceStatus(s.tenantResource)
if err != nil {
s.logger.Info("ERROR", "error in tenant status reconciler when updating the resource", err)
return equalStatus, err
Expand All @@ -49,8 +47,8 @@ func (s *TenantStatusReconciler) Reconcile() (bool, error) {
}

func (s *TenantStatusReconciler) calculateStatus() capabilitiesv1alpha1.TenantStatus {
status := s.resourceUpdated.Status
status.Conditions = s.resourceUpdated.Status.Conditions.Copy()
status := s.tenantResource.Status
status.Conditions = s.tenantResource.Status.Conditions.Copy()
status.Conditions.SetCondition(s.readyCondition())

return status
Expand All @@ -62,7 +60,7 @@ func (s *TenantStatusReconciler) readyCondition() common.Condition {
Status: corev1.ConditionFalse,
}

if s.reconcileError == nil && s.resourceUpdated.Status.TenantId != 0 && s.resourceUpdated.Status.AdminId != 0 {
if s.reconcileError == nil && s.tenantResource.Status.TenantId != 0 && s.tenantResource.Status.AdminId != 0 {
condition.Status = corev1.ConditionTrue
}

Expand Down
20 changes: 10 additions & 10 deletions controllers/capabilities/tenant_threescale_reconciler.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,10 @@ import (
"context"
"errors"
"fmt"
"reflect"
"strconv"

porta_client_pkg "github.com/3scale/3scale-porta-go-client/client"
"github.com/go-logr/logr"
"github.com/google/go-cmp/cmp"
v1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"

Expand Down Expand Up @@ -98,7 +96,7 @@ func (r *TenantThreescaleReconciler) reconcileTenant() (bool, error) {
TenantId: tenantDef.Signup.Account.ID,
}

updated, err := r.reconcileStatus(newStatus)
updated, err := r.reconcileStatusIDs(newStatus)
if err != nil {
return false, err
}
Expand Down Expand Up @@ -157,7 +155,7 @@ func (r *TenantThreescaleReconciler) reconcileAdminUser() error {
TenantId: tenantID,
}

updated, err := r.reconcileStatus(newStatus)
updated, err := r.reconcileStatusIDs(newStatus)
if err != nil {
return err
}
Expand Down Expand Up @@ -277,14 +275,16 @@ func (r *TenantThreescaleReconciler) syncAdminUser(tenantID int64, adminUser *po
}

// Returns whether the status should be updated or not and the error
func (r *TenantThreescaleReconciler) reconcileStatus(desiredStatus *apiv1alpha1.TenantStatus) (bool, error) {
if !reflect.DeepEqual(r.tenantR.Status, *desiredStatus) {
diff := cmp.Diff(r.tenantR.Status, *desiredStatus)
r.logger.V(1).Info(fmt.Sprintf("status has changed: %s", diff))
r.tenantR.Status = *desiredStatus
r.logger.Info("Update tenant status with tenantID", "tenantID", r.tenantR.Status.TenantId)
func (r *TenantThreescaleReconciler) reconcileStatusIDs(desiredStatus *apiv1alpha1.TenantStatus) (bool, error) {
if desiredStatus.TenantId != r.tenantR.Status.TenantId {
r.tenantR.Status.TenantId = desiredStatus.TenantId
return true, nil
}
if desiredStatus.AdminId != r.tenantR.Status.AdminId {
r.tenantR.Status.AdminId = desiredStatus.AdminId
return true, nil
}

return false, nil
}

Expand Down

0 comments on commit 893262b

Please sign in to comment.