Skip to content

Commit

Permalink
[KOGITO-8792] Review build failures and signal the reasoning in the E…
Browse files Browse the repository at this point in the history 
…vents API

Signed-off-by: desmax74 <[email protected]>
  • Loading branch information
@desmax74
desmax74 committed Jul 26, 2023
1 parent d3b8020 commit f571cc4
Show file tree
Hide file tree
Showing 6 changed files with 81 additions and 60 deletions.
8 changes: 5 additions & 3 deletions controllers/builder/containerbuilder.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,9 +64,11 @@ func (c *containerBuilderManager) Schedule(build *operatorapi.SonataFlowBuild) e
if err = build.Status.SetInnerBuild(containerBuilder); err != nil {
return err
}
build.Status.BuildPhase = operatorapi.BuildPhase(containerBuilder.Status.Phase)
build.Status.Error = containerBuilder.Status.Error
build.Status.ImageTag = imageNameTag
if containerBuilder != nil {
build.Status.BuildPhase = operatorapi.BuildPhase(containerBuilder.Status.Phase)
build.Status.Error = containerBuilder.Status.Error
build.Status.ImageTag = imageNameTag
}
return nil
}

Expand Down
27 changes: 27 additions & 0 deletions controllers/profiles/reconciler.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ import (
"context"
"fmt"

v1 "k8s.io/api/core/v1"

"k8s.io/client-go/tools/record"

"k8s.io/client-go/rest"
Expand Down Expand Up @@ -82,6 +84,31 @@ func (s stateSupport) performStatusUpdate(ctx context.Context, workflow *operato
return true, err
}

// performStatusUpdate updates the SonataFlow Status conditions on the last version available
func (s stateSupport) getAndUpdateStatusWorkFlow(ctx context.Context, workflow *operatorapi.SonataFlow) (bool, error) {
freshWorkFlow := operatorapi.SonataFlow{}
err := s.client.Get(ctx, client.ObjectKeyFromObject(workflow), &freshWorkFlow)
freshWorkFlow.Status = workflow.Status
result, err := s.performStatusUpdate(ctx, &freshWorkFlow)
if !result && err != nil {
s.recorder.Event(&freshWorkFlow, v1.EventTypeWarning, "SonataFlowStatusUpdateError 3", fmt.Sprintf("Error: %v", err))
return false, err
}
return true, nil
}

func getAndUpdateStatusBuild(ctx context.Context, build *operatorapi.SonataFlowBuild, support *stateSupport) error {
freshBuild := operatorapi.SonataFlowBuild{}
err := support.client.Get(ctx, client.ObjectKeyFromObject(build), &freshBuild)
freshBuild.Status = build.Status
if err = support.client.Status().Update(ctx, &freshBuild); err != nil {
support.logger.Error(err, "Failed to update Workflow status")
support.recorder.Event(&freshBuild, v1.EventTypeWarning, "SonataFlowStatusUpdateError 3", fmt.Sprintf("Error: %v", err))
return err
}
return nil
}

// PostReconcile function to perform all the other operations required after the reconciliation - placeholder for null pattern usages
func (s stateSupport) PostReconcile(ctx context.Context, workflow *operatorapi.SonataFlow) error {
//By default, we don't want to perform anything after the reconciliation, and so we will simply return no error
Expand Down
24 changes: 12 additions & 12 deletions controllers/profiles/reconciler_dev.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ func (e *ensureRunningDevWorkflowReconciliationState) Do(ctx context.Context, wo
externalCM, err := workflowdef.FetchExternalResourcesConfigMapsRef(e.client, workflow)
if err != nil {
workflow.Status.Manager().MarkFalse(api.RunningConditionType, api.ExternalResourcesNotFoundReason, "External Resources ConfigMap not found: %s", err.Error())
if _, err = e.performStatusUpdate(ctx, workflow); err != nil {
if _, err = e.getAndUpdateStatusWorkFlow(ctx, workflow); err != nil {
return ctrl.Result{RequeueAfter: requeueAfterFailure}, objs, err
}
return ctrl.Result{RequeueAfter: requeueAfterFailure}, objs, nil
Expand Down Expand Up @@ -218,7 +218,7 @@ func (e *ensureRunningDevWorkflowReconciliationState) Do(ctx context.Context, wo
if workflow.Status.GetTopLevelCondition().IsUnknown() {
e.logger.Info("Workflow is in WaitingForDeployment Condition")
workflow.Status.Manager().MarkFalse(api.RunningConditionType, api.WaitingForDeploymentReason, "")
if _, err = e.performStatusUpdate(ctx, workflow); err != nil {
if _, err = e.getAndUpdateStatusWorkFlow(ctx, workflow); err != nil {
return ctrl.Result{RequeueAfter: requeueAfterFailure}, objs, err
}
return ctrl.Result{RequeueAfter: requeueAfterIsRunning}, objs, nil
Expand All @@ -229,7 +229,7 @@ func (e *ensureRunningDevWorkflowReconciliationState) Do(ctx context.Context, wo
if !kubeutil.IsDeploymentAvailable(convertedDeployment) {
e.logger.Info("Workflow is not running due to a problem in the Deployment. Attempt to recover.")
workflow.Status.Manager().MarkFalse(api.RunningConditionType, api.DeploymentUnavailableReason, getDeploymentFailureMessage(convertedDeployment))
if _, err = e.performStatusUpdate(ctx, workflow); err != nil {
if _, err = e.getAndUpdateStatusWorkFlow(ctx, workflow); err != nil {
return ctrl.Result{RequeueAfter: requeueAfterFailure}, objs, err
}
}
Expand All @@ -251,7 +251,7 @@ func (f *followDeployDevWorkflowReconciliationState) Do(ctx context.Context, wor
if err := f.client.Get(ctx, client.ObjectKeyFromObject(workflow), deployment); err != nil {
// we should have the deployment by this time, so even if the error above is not found, we should halt.
workflow.Status.Manager().MarkFalse(api.RunningConditionType, api.DeploymentUnavailableReason, "Couldn't find deployment anymore while waiting for the deploy")
if _, err := f.performStatusUpdate(ctx, workflow); err != nil {
if _, err := f.getAndUpdateStatusWorkFlow(ctx, workflow); err != nil {
return ctrl.Result{RequeueAfter: requeueAfterFailure}, nil, err
}
return ctrl.Result{RequeueAfter: requeueAfterFailure}, nil, err
Expand All @@ -260,7 +260,7 @@ func (f *followDeployDevWorkflowReconciliationState) Do(ctx context.Context, wor
if kubeutil.IsDeploymentAvailable(deployment) {
workflow.Status.Manager().MarkTrue(api.RunningConditionType)
f.logger.Info("Workflow is in Running Condition")
if _, err := f.performStatusUpdate(ctx, workflow); err != nil {
if _, err := f.getAndUpdateStatusWorkFlow(ctx, workflow); err != nil {
return ctrl.Result{RequeueAfter: requeueAfterFailure}, nil, err

}
Expand All @@ -270,7 +270,7 @@ func (f *followDeployDevWorkflowReconciliationState) Do(ctx context.Context, wor
if kubeutil.IsDeploymentProgressing(deployment) {
workflow.Status.Manager().MarkFalse(api.RunningConditionType, api.WaitingForDeploymentReason, "")
f.logger.Info("Workflow is in WaitingForDeployment Condition")
if _, err := f.performStatusUpdate(ctx, workflow); err != nil {
if _, err := f.getAndUpdateStatusWorkFlow(ctx, workflow); err != nil {
return ctrl.Result{RequeueAfter: requeueAfterFailure}, nil, err
}
return ctrl.Result{RequeueAfter: requeueAfterFollowDeployment}, nil, nil
Expand All @@ -280,7 +280,7 @@ func (f *followDeployDevWorkflowReconciliationState) Do(ctx context.Context, wor
workflow.Status.LastTimeRecoverAttempt = metav1.Now()
workflow.Status.Manager().MarkFalse(api.RunningConditionType, api.DeploymentFailureReason, failedReason)
f.logger.Info("Workflow deployment failed", "Reason Message", failedReason)
_, err := f.performStatusUpdate(ctx, workflow)
_, err := f.getAndUpdateStatusWorkFlow(ctx, workflow)
return ctrl.Result{RequeueAfter: requeueAfterFailure}, nil, err
}

Expand All @@ -294,7 +294,7 @@ func (f *followDeployDevWorkflowReconciliationState) PostReconcile(ctx context.C
if _, err := f.enrichers.networkInfo.Enrich(ctx, workflow); err != nil {
return err
}
if _, err := f.performStatusUpdate(ctx, workflow); err != nil {
if _, err := f.getAndUpdateStatusWorkFlow(ctx, workflow); err != nil {
return err
}
}
Expand All @@ -318,7 +318,7 @@ func (r *recoverFromFailureDevReconciliationState) Do(ctx context.Context, workf
r.logger.Info("Tried to recover from failed state, no deployment found, trying to reset the workflow conditions")
workflow.Status.RecoverFailureAttempts = 0
workflow.Status.Manager().MarkUnknown(api.RunningConditionType, "", "")
if _, updateErr := r.performStatusUpdate(ctx, workflow); updateErr != nil {
if _, updateErr := r.getAndUpdateStatusWorkFlow(ctx, workflow); updateErr != nil {
return ctrl.Result{Requeue: false}, nil, updateErr
}
return ctrl.Result{RequeueAfter: requeueAfterFailure}, nil, nil
Expand All @@ -330,7 +330,7 @@ func (r *recoverFromFailureDevReconciliationState) Do(ctx context.Context, workf
if kubeutil.IsDeploymentAvailable(deployment) {
workflow.Status.RecoverFailureAttempts = 0
workflow.Status.Manager().MarkTrue(api.RunningConditionType)
if _, updateErr := r.performStatusUpdate(ctx, workflow); updateErr != nil {
if _, updateErr := r.getAndUpdateStatusWorkFlow(ctx, workflow); updateErr != nil {
return ctrl.Result{Requeue: false}, nil, updateErr
}
return ctrl.Result{RequeueAfter: requeueAfterFailure}, nil, nil
Expand All @@ -339,7 +339,7 @@ func (r *recoverFromFailureDevReconciliationState) Do(ctx context.Context, workf
if workflow.Status.RecoverFailureAttempts >= recoverDeploymentErrorRetries {
workflow.Status.Manager().MarkFalse(api.RunningConditionType, api.RedeploymentExhaustedReason,
"Can't recover workflow from failure after maximum attempts: %d", workflow.Status.RecoverFailureAttempts)
if _, updateErr := r.performStatusUpdate(ctx, workflow); updateErr != nil {
if _, updateErr := r.getAndUpdateStatusWorkFlow(ctx, workflow); updateErr != nil {
return ctrl.Result{}, nil, updateErr
}
return ctrl.Result{RequeueAfter: requeueRecoverDeploymentErrorInterval}, nil, nil
Expand Down Expand Up @@ -369,7 +369,7 @@ func (r *recoverFromFailureDevReconciliationState) Do(ctx context.Context, workf

workflow.Status.RecoverFailureAttempts += 1
workflow.Status.LastTimeRecoverAttempt = metav1.Now()
if _, err := r.performStatusUpdate(ctx, workflow); err != nil {
if _, err := r.getAndUpdateStatusWorkFlow(ctx, workflow); err != nil {
return ctrl.Result{Requeue: false}, nil, err
}
return ctrl.Result{RequeueAfter: requeueRecoverDeploymentErrorInterval}, nil, nil
Expand Down
16 changes: 13 additions & 3 deletions controllers/profiles/reconciler_dev_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -336,7 +336,10 @@ func Test_newDevProfileWithExternalConfigMaps(t *testing.T) {

// reconcile again
workflow.Status.Manager().MarkTrue(api.RunningConditionType)
err = client.Update(context.TODO(), workflow)
freshWorkFlow := operatorapi.SonataFlow{}
client.Get(context.TODO(), types.NamespacedName{Namespace: workflow.Namespace, Name: workflow.Name}, &freshWorkFlow)
freshWorkFlow.Status = workflow.Status
err = client.Update(context.TODO(), &freshWorkFlow)
assert.NoError(t, err)
result, err = devReconciler.Reconcile(context.TODO(), workflow)
deployment = test.MustGetDeployment(t, client, workflow)
Expand All @@ -353,7 +356,10 @@ func Test_newDevProfileWithExternalConfigMaps(t *testing.T) {
assert.Equal(t, quarkusDevConfigMountPath+"/routes", extCamelRouteOne.MountPath)

workflow.Status.Manager().MarkTrue(api.RunningConditionType)
err = client.Update(context.TODO(), workflow)
freshWorkFlow = operatorapi.SonataFlow{}
client.Get(context.TODO(), types.NamespacedName{Namespace: workflow.Namespace, Name: workflow.Name}, &freshWorkFlow)
freshWorkFlow.Status = workflow.Status
err = client.Update(context.TODO(), &freshWorkFlow)
assert.NoError(t, err)
result, err = devReconciler.Reconcile(context.TODO(), workflow)
deployment = test.MustGetDeployment(t, client, workflow)
Expand All @@ -379,7 +385,11 @@ func Test_newDevProfileWithExternalConfigMaps(t *testing.T) {

// delete the link
workflow.Spec.Resources.ConfigMaps = nil
assert.NoError(t, client.Update(context.TODO(), workflow))
freshWorkFlow = operatorapi.SonataFlow{}
client.Get(context.TODO(), types.NamespacedName{Namespace: workflow.Namespace, Name: workflow.Name}, &freshWorkFlow)
freshWorkFlow.Status = workflow.Status
err = client.Update(context.TODO(), &freshWorkFlow)
assert.NoError(t, err)
result, err = devReconciler.Reconcile(context.TODO(), workflow)
assert.NoError(t, err)
assert.NotNil(t, result)
Expand Down
Loading

0 comments on commit f571cc4

Please sign in to comment.