Fix: application conditions confusion. (#2834)

* fix condition confusion

* fix test cases
This commit is contained in:
Jian.Li
2021-11-30 16:43:27 +08:00
committed by GitHub
parent f8e1ddc52c
commit ec105cbb02
11 changed files with 127 additions and 79 deletions

View File

@@ -18,6 +18,7 @@ package common
import (
"encoding/json"
"errors"
"github.com/oam-dev/terraform-controller/api/v1beta1"
@@ -163,7 +164,7 @@ type Status struct {
HealthPolicy string `json:"healthPolicy,omitempty"`
}
// ApplicationPhase is a label for the condition of a application at the current time
// ApplicationPhase is a label for the condition of an application at the current time
type ApplicationPhase string
const (
@@ -509,3 +510,48 @@ func (re RawExtensionPointer) MarshalJSON() ([]byte, error) {
// TODO: Check whether ContentType is actually JSON before returning it.
return re.RawExtension.Raw, nil
}
// ApplicationConditionType is a valid value for ApplicationCondition.Type
type ApplicationConditionType int
const (
// ParsedCondition indicates whether the parsing is successful.
ParsedCondition ApplicationConditionType = iota
// RevisionCondition indicates whether the generated revision is successful.
RevisionCondition
// PolicyCondition indicates whether policy processing is successful.
PolicyCondition
// RenderCondition indicates whether render processing is successful.
RenderCondition
// WorkflowCondition indicates whether workflow processing is successful.
WorkflowCondition
// RolloutCondition indicates whether rollout processing is successful.
RolloutCondition
// ReadyCondition indicates whether whole application processing is successful.
ReadyCondition
)
var conditions = map[ApplicationConditionType]string{
ParsedCondition: "Parsed",
RevisionCondition: "Revision",
PolicyCondition: "Policy",
RenderCondition: "Render",
WorkflowCondition: "Workflow",
RolloutCondition: "Rollout",
ReadyCondition: "Ready",
}
// String returns the string corresponding to the condition type.
func (ct ApplicationConditionType) String() string {
return conditions[ct]
}
// ParseApplicationConditionType parse ApplicationCondition Type.
func ParseApplicationConditionType(s string) (ApplicationConditionType, error) {
for k, v := range conditions {
if v == s {
return k, nil
}
}
return -1, errors.New("unknown condition type")
}

View File

@@ -909,7 +909,7 @@ spec:
type: array
status:
description: ApplicationPhase is a label for the condition
of a application at the current time
of an application at the current time
type: string
workflow:
description: Workflow record the status of workflow
@@ -3115,7 +3115,7 @@ spec:
type: array
status:
description: ApplicationPhase is a label for the condition
of a application at the current time
of an application at the current time
type: string
workflow:
description: Workflow record the status of workflow

View File

@@ -633,7 +633,7 @@ spec:
type: object
type: array
status:
description: ApplicationPhase is a label for the condition of a application at the current time
description: ApplicationPhase is a label for the condition of an application at the current time
type: string
workflow:
description: Workflow record the status of workflow
@@ -1457,7 +1457,7 @@ spec:
type: object
type: array
status:
description: ApplicationPhase is a label for the condition of a application at the current time
description: ApplicationPhase is a label for the condition of an application at the current time
type: string
workflow:
description: Workflow record the status of workflow

View File

@@ -909,7 +909,7 @@ spec:
type: array
status:
description: ApplicationPhase is a label for the condition
of a application at the current time
of an application at the current time
type: string
workflow:
description: Workflow record the status of workflow
@@ -3115,7 +3115,7 @@ spec:
type: array
status:
description: ApplicationPhase is a label for the condition
of a application at the current time
of an application at the current time
type: string
workflow:
description: Workflow record the status of workflow

View File

@@ -633,7 +633,7 @@ spec:
type: object
type: array
status:
description: ApplicationPhase is a label for the condition of a application at the current time
description: ApplicationPhase is a label for the condition of an application at the current time
type: string
workflow:
description: Workflow record the status of workflow
@@ -1457,7 +1457,7 @@ spec:
type: object
type: array
status:
description: ApplicationPhase is a label for the condition of a application at the current time
description: ApplicationPhase is a label for the condition of an application at the current time
type: string
workflow:
description: Workflow record the status of workflow

View File

@@ -909,7 +909,7 @@ spec:
type: array
status:
description: ApplicationPhase is a label for the condition
of a application at the current time
of an application at the current time
type: string
workflow:
description: Workflow record the status of workflow
@@ -3115,7 +3115,7 @@ spec:
type: array
status:
description: ApplicationPhase is a label for the condition
of a application at the current time
of an application at the current time
type: string
workflow:
description: Workflow record the status of workflow

View File

@@ -831,7 +831,7 @@ spec:
type: object
type: array
status:
description: ApplicationPhase is a label for the condition of a application
description: ApplicationPhase is a label for the condition of an application
at the current time
type: string
workflow:
@@ -1922,7 +1922,7 @@ spec:
type: object
type: array
status:
description: ApplicationPhase is a label for the condition of a application
description: ApplicationPhase is a label for the condition of an application
at the current time
type: string
workflow:

View File

@@ -71,7 +71,7 @@ const (
legacyOnlyRevisionFinalizer = "app.oam.dev/only-revision-finalizer"
)
// Reconciler reconciles a Application object
// Reconciler reconciles an Application object
type Reconciler struct {
client.Client
dm discoverymapper.DiscoveryMapper
@@ -160,25 +160,27 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu
if err != nil {
logCtx.Error(err, "[Handle PrepareWorkflowAndPolicy]")
r.Recorder.Event(app, event.Warning(velatypes.ReasonFailedRender, err))
return r.endWithNegativeCondition(logCtx, app, condition.ErrorCondition("PrepareWorkflowAndPolicy", err), common.ApplicationPolicyGenerating)
return r.endWithNegativeCondition(logCtx, app, condition.ErrorCondition(common.PolicyCondition.String(), errors.WithMessage(err, "PrepareWorkflowAndPolicy")), common.ApplicationPolicyGenerating)
}
if err := handler.HandleBuiltInPolicies(builtInPolicies); err != nil {
klog.Error(err, "[Handle BuiltIn Policies]")
r.Recorder.Event(app, event.Warning(velatypes.ReasonFailedRender, err))
return r.endWithNegativeCondition(ctx, app, condition.ErrorCondition("HandleBuiltInPolicies", err), common.ApplicationPolicyGenerating)
return r.endWithNegativeCondition(ctx, app, condition.ErrorCondition(common.PolicyCondition.String(), errors.WithMessage(err, "HandleBuiltInPolicies")), common.ApplicationPolicyGenerating)
}
if len(externalPolicies) > 0 {
if err := handler.Dispatch(ctx, "", common.PolicyResourceCreator, externalPolicies...); err != nil {
logCtx.Error(err, "[Handle ApplyPolicyResources]")
r.Recorder.Event(app, event.Warning(velatypes.ReasonFailedApply, err))
return r.endWithNegativeCondition(logCtx, app, condition.ErrorCondition("ApplyPolices", err), common.ApplicationPolicyGenerating)
return r.endWithNegativeCondition(logCtx, app, condition.ErrorCondition(common.PolicyCondition.String(), errors.WithMessage(err, "ApplyPolices")), common.ApplicationPolicyGenerating)
}
logCtx.Info("Successfully generated application policies")
}
app.Status.SetConditions(condition.ReadyCondition("Render"))
app.Status.SetConditions(condition.ReadyCondition(common.PolicyCondition.String()))
app.Status.SetConditions(condition.ReadyCondition(common.RenderCondition.String()))
r.Recorder.Event(app, event.Normal(velatypes.ReasonRendered, velatypes.MessageRendered))
if !appWillRollout(app) {
@@ -186,14 +188,14 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu
if err != nil {
logCtx.Error(err, "[handle workflow]")
r.Recorder.Event(app, event.Warning(velatypes.ReasonFailedWorkflow, err))
return r.endWithNegativeCondition(logCtx, app, condition.ErrorCondition("Workflow", err), common.ApplicationRunningWorkflow)
return r.endWithNegativeCondition(logCtx, app, condition.ErrorCondition(common.WorkflowCondition.String(), err), common.ApplicationRunningWorkflow)
}
wf := workflow.NewWorkflow(app, r.Client, appFile.WorkflowMode)
workflowState, err := wf.ExecuteSteps(logCtx.Fork("workflow"), handler.currentAppRev, steps)
if err != nil {
logCtx.Error(err, "[handle workflow]")
r.Recorder.Event(app, event.Warning(velatypes.ReasonFailedWorkflow, err))
return r.endWithNegativeCondition(logCtx, app, condition.ErrorCondition("Workflow", err), common.ApplicationRunningWorkflow)
return r.endWithNegativeCondition(logCtx, app, condition.ErrorCondition(common.WorkflowCondition.String(), err), common.ApplicationRunningWorkflow)
}
handler.addServiceStatus(false, app.Status.Services...)
@@ -207,7 +209,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu
case common.WorkflowStateTerminated:
logCtx.Info("Workflow return state=Terminated")
if err := r.doWorkflowFinish(app, wf); err != nil {
return r.endWithNegativeConditionWithRetry(ctx, app, condition.ErrorCondition("DoWorkflowFinish", err), common.ApplicationRunningWorkflow)
return r.endWithNegativeConditionWithRetry(ctx, app, condition.ErrorCondition(common.WorkflowCondition.String(), errors.WithMessage(err, "DoWorkflowFinish")), common.ApplicationRunningWorkflow)
}
return ctrl.Result{}, r.patchStatusWithRetryOnConflict(logCtx, app, common.ApplicationWorkflowTerminated)
case common.WorkflowStateExecuting:
@@ -227,14 +229,14 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu
if err != nil {
logCtx.Error(err, "Failed to gc after workflow")
r.Recorder.Event(app, event.Warning(velatypes.ReasonFailedGC, err))
return r.endWithNegativeConditionWithRetry(logCtx, app, condition.ErrorCondition("GCAfterWorkflow", err), common.ApplicationRunningWorkflow)
return r.endWithNegativeConditionWithRetry(logCtx, app, condition.ErrorCondition(common.WorkflowCondition.String(), errors.WithMessage(err, "GCAfterWorkflow")), common.ApplicationRunningWorkflow)
}
app.Status.ResourceTracker = ref
}
if err := r.doWorkflowFinish(app, wf); err != nil {
return r.endWithNegativeConditionWithRetry(logCtx, app, condition.ErrorCondition("DoWorkflowFinish", err), common.ApplicationRunningWorkflow)
return r.endWithNegativeConditionWithRetry(logCtx, app, condition.ErrorCondition(common.WorkflowCondition.String(), errors.WithMessage(err, "DoWorkflowFinish")), common.ApplicationRunningWorkflow)
}
app.Status.SetConditions(condition.ReadyCondition("WorkflowFinished"))
app.Status.SetConditions(condition.ReadyCondition(common.WorkflowCondition.String()))
r.Recorder.Event(app, event.Normal(velatypes.ReasonApplied, velatypes.MessageWorkflowFinished))
logCtx.Info("Application manifests has applied by workflow successfully")
return ctrl.Result{}, r.patchStatusWithRetryOnConflict(logCtx, app, common.ApplicationWorkflowFinished)
@@ -250,7 +252,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu
if err != nil {
logCtx.Error(err, "Failed to render components")
r.Recorder.Event(app, event.Warning(velatypes.ReasonFailedRender, err))
return r.endWithNegativeConditionWithRetry(logCtx, app, condition.ErrorCondition("Render", err), common.ApplicationRendering)
return r.endWithNegativeConditionWithRetry(logCtx, app, condition.ErrorCondition(common.RenderCondition.String(), err), common.ApplicationRendering)
}
assemble.HandleCheckManageWorkloadTrait(*handler.currentAppRev, comps)
@@ -258,7 +260,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu
if err := handler.HandleComponentsRevision(logCtx, comps); err != nil {
logCtx.Error(err, "Failed to handle components revision")
r.Recorder.Event(app, event.Warning(velatypes.ReasonFailedRevision, err))
return r.endWithNegativeConditionWithRetry(logCtx, app, condition.ErrorCondition("Render", err), common.ApplicationRendering)
return r.endWithNegativeConditionWithRetry(logCtx, app, condition.ErrorCondition(common.RenderCondition.String(), err), common.ApplicationRendering)
}
klog.Info("Application manifests has prepared and ready for appRollout to handle", "application", klog.KObj(app))
}
@@ -268,7 +270,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu
if err != nil {
logCtx.Error(err, "Failed to handle rollout")
r.Recorder.Event(app, event.Warning(velatypes.ReasonFailedRollout, err))
return r.endWithNegativeCondition(logCtx, app, condition.ErrorCondition("Rollout", err), common.ApplicationRollingOut)
return r.endWithNegativeCondition(logCtx, app, condition.ErrorCondition(common.RolloutCondition.String(), err), common.ApplicationRollingOut)
}
// skip health check and garbage collection if rollout have not finished
// start next reconcile immediately
@@ -281,7 +283,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu
// there is no need reconcile immediately, that means the rollout operation have finished
r.Recorder.Event(app, event.Normal(velatypes.ReasonRollout, velatypes.MessageRollout))
app.Status.SetConditions(condition.ReadyCondition("Rollout"))
app.Status.SetConditions(condition.ReadyCondition(common.RolloutCondition.String()))
logCtx.Info("Finished rollout ")
}
var phase = common.ApplicationRunning
@@ -299,7 +301,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu
}
logCtx.Info("Successfully garbage collect")
app.Status.SetConditions(condition.Condition{
Type: condition.TypeReady,
Type: condition.ConditionType(common.ReadyCondition.String()),
Status: corev1.ConditionTrue,
LastTransitionTime: metav1.Now(),
Reason: condition.ReasonReconcileSuccess,

View File

@@ -2001,12 +2001,12 @@ var _ = Describe("Test Application Controller", func() {
}
Expect(k8sClient.Create(context.Background(), &ns)).Should(BeNil())
healthComponentDef := &v1beta1.ComponentDefinition{}
hCDefJson, _ := yaml.YAMLToJSON([]byte(cdDefWithHealthStatusYaml))
Expect(json.Unmarshal(hCDefJson, healthComponentDef)).Should(BeNil())
healthComponentDef.Name = "worker-with-health"
healthComponentDef.Namespace = "app-applied-resources"
Expect(k8sClient.Create(ctx, healthComponentDef)).Should(BeNil())
webComponentDef := &v1beta1.ComponentDefinition{}
hCDefJson, _ := yaml.YAMLToJSON([]byte(componentDefYaml))
Expect(json.Unmarshal(hCDefJson, webComponentDef)).Should(BeNil())
webComponentDef.Name = "web-worker"
webComponentDef.Namespace = "app-applied-resources"
Expect(k8sClient.Create(ctx, webComponentDef)).Should(BeNil())
app := &v1beta1.Application{
TypeMeta: metav1.TypeMeta{
@@ -2021,13 +2021,13 @@ var _ = Describe("Test Application Controller", func() {
Components: []common.ApplicationComponent{
{
Name: "myweb1",
Type: "worker-with-health",
Properties: &runtime.RawExtension{Raw: []byte(`{"cmd":["sleep","1000"],"image":"busybox","lives": "i am lives","enemies": "empty"}`)},
Type: "web-worker",
Properties: &runtime.RawExtension{Raw: []byte(`{"cmd":["sleep","1000"],"image":"busybox"}`)},
},
{
Name: "myweb2",
Type: "worker-with-health",
Properties: &runtime.RawExtension{Raw: []byte(`{"cmd":["sleep","1000"],"image":"busybox","lives": "i am lives","enemies": "empty"}`)},
Type: "web-worker",
Properties: &runtime.RawExtension{Raw: []byte(`{"cmd":["sleep","1000"],"image":"busybox"}`)},
},
},
},
@@ -2037,7 +2037,7 @@ var _ = Describe("Test Application Controller", func() {
testutil.ReconcileOnceAfterFinalizer(reconciler, reconcile.Request{NamespacedName: appKey})
checkApp := &v1beta1.Application{}
Expect(k8sClient.Get(ctx, appKey, checkApp)).Should(BeNil())
Expect(checkApp.Status.Phase).Should(BeEquivalentTo(common.ApplicationRunningWorkflow))
Expect(checkApp.Status.Phase).Should(BeEquivalentTo(common.ApplicationRunning))
Expect(checkApp.Status.AppliedResources).Should(BeEquivalentTo([]common.ClusterObjectReference{
{
Cluster: "",
@@ -2051,12 +2051,21 @@ var _ = Describe("Test Application Controller", func() {
{
Cluster: "",
Creator: common.WorkflowResourceCreator,
ObjectReference: corev1.ObjectReference{Kind: "ConfigMap",
ObjectReference: corev1.ObjectReference{Kind: "Deployment",
Namespace: "app-applied-resources",
Name: "myweb1game-config",
APIVersion: "v1",
Name: "myweb2",
APIVersion: "apps/v1",
},
},
}))
// make error
checkApp.Spec.Components[0].Properties = nil
Expect(k8sClient.Update(context.Background(), checkApp)).Should(BeNil())
testutil.ReconcileOnceAfterFinalizer(reconciler, reconcile.Request{NamespacedName: appKey})
checkApp = &v1beta1.Application{}
Expect(k8sClient.Get(ctx, appKey, checkApp)).Should(BeNil())
Expect(checkApp.Status.AppliedResources).Should(BeEquivalentTo([]common.ClusterObjectReference{
{
Cluster: "",
Creator: common.WorkflowResourceCreator,
@@ -2066,23 +2075,19 @@ var _ = Describe("Test Application Controller", func() {
APIVersion: "apps/v1",
},
},
{
Cluster: "",
Creator: common.WorkflowResourceCreator,
ObjectReference: corev1.ObjectReference{Kind: "ConfigMap",
Namespace: "app-applied-resources",
Name: "myweb2game-config",
APIVersion: "v1",
},
},
}))
Expect(checkApp.Status.Conditions[len(checkApp.Status.Conditions)-1].Type).Should(BeEquivalentTo("Render"))
checkApp.Spec.Components[0].Name = "myweb-1"
checkApp.Spec.Components[0] = common.ApplicationComponent{
Name: "myweb-1",
Type: "web-worker",
Properties: &runtime.RawExtension{Raw: []byte(`{"cmd":["sleep","1000"],"image":"busybox"}`)},
}
Expect(k8sClient.Update(context.Background(), checkApp)).Should(BeNil())
testutil.ReconcileOnceAfterFinalizer(reconciler, reconcile.Request{NamespacedName: appKey})
checkApp = &v1beta1.Application{}
Expect(k8sClient.Get(ctx, appKey, checkApp)).Should(BeNil())
Expect(checkApp.Status.Phase).Should(BeEquivalentTo(common.ApplicationRunningWorkflow))
Expect(checkApp.Status.Phase).Should(BeEquivalentTo(common.ApplicationRunning))
Expect(checkApp.Status.AppliedResources).Should(BeEquivalentTo([]common.ClusterObjectReference{
{
Cluster: "",
@@ -2093,15 +2098,6 @@ var _ = Describe("Test Application Controller", func() {
APIVersion: "apps/v1",
},
},
{
Cluster: "",
Creator: common.WorkflowResourceCreator,
ObjectReference: corev1.ObjectReference{Kind: "ConfigMap",
Namespace: "app-applied-resources",
Name: "myweb-1game-config",
APIVersion: "v1",
},
},
{
Cluster: "",
Creator: common.WorkflowResourceCreator,
@@ -2111,15 +2107,6 @@ var _ = Describe("Test Application Controller", func() {
APIVersion: "apps/v1",
},
},
{
Cluster: "",
Creator: common.WorkflowResourceCreator,
ObjectReference: corev1.ObjectReference{Kind: "ConfigMap",
Namespace: "app-applied-resources",
Name: "myweb2game-config",
APIVersion: "v1",
},
},
}))
})

View File

@@ -21,6 +21,8 @@ import (
"fmt"
"time"
"github.com/oam-dev/kubevela/apis/core.oam.dev/condition"
"github.com/pkg/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"sigs.k8s.io/controller-runtime/pkg/client"
@@ -59,7 +61,7 @@ func NewWorkflow(app *oamcore.Application, cli client.Client, mode common.Workfl
// ExecuteSteps process workflow step in order.
func (w *workflow) ExecuteSteps(ctx monitorContext.Context, appRev *oamcore.ApplicationRevision, taskRunners []wfTypes.TaskRunner) (common.WorkflowState, error) {
revAndSpecHash, err := computeAppRevisionHash(appRev.Name, w.app)
revAndSpecHash, err := ComputeWorkflowRevisionHash(appRev.Name, w.app)
if err != nil {
return common.WorkflowStateExecuting, err
}
@@ -78,7 +80,7 @@ func (w *workflow) ExecuteSteps(ctx monitorContext.Context, appRev *oamcore.Appl
w.app.Status.Workflow = &common.WorkflowStatus{
AppRevision: revAndSpecHash,
Mode: common.WorkflowModeStep,
StartTime: metav1.NewTime(time.Now()),
StartTime: metav1.Now(),
}
if w.dagMode {
w.app.Status.Workflow.Mode = common.WorkflowModeDAG
@@ -86,6 +88,18 @@ func (w *workflow) ExecuteSteps(ctx monitorContext.Context, appRev *oamcore.Appl
// clean recorded resources info.
w.app.Status.Services = nil
w.app.Status.AppliedResources = nil
// clean conditions after render
var reservedCondtions []condition.Condition
for i, cond := range w.app.Status.Conditions {
condTpy, err := common.ParseApplicationConditionType(string(cond.Type))
if err == nil {
if condTpy < common.RenderCondition {
reservedCondtions = append(reservedCondtions, w.app.Status.Conditions[i])
}
}
}
w.app.Status.Conditions = reservedCondtions
}
wfStatus := w.app.Status.Workflow
@@ -341,7 +355,8 @@ func (e *engine) needStop() bool {
return e.status.Suspend || e.status.Terminated
}
func computeAppRevisionHash(rev string, app *oamcore.Application) (string, error) {
// ComputeWorkflowRevisionHash compute workflow revision.
func ComputeWorkflowRevisionHash(rev string, app *oamcore.Application) (string, error) {
version := ""
if annos := app.Annotations; annos != nil {
version = annos[oam.AnnotationPublishVersion]

View File

@@ -20,10 +20,6 @@ import (
"context"
"encoding/json"
monitorContext "github.com/oam-dev/kubevela/pkg/monitor/context"
"github.com/oam-dev/kubevela/pkg/cue/model/value"
. "github.com/onsi/ginkgo"
. "github.com/onsi/gomega"
@@ -36,6 +32,8 @@ import (
"github.com/oam-dev/kubevela/apis/core.oam.dev/common"
oamcore "github.com/oam-dev/kubevela/apis/core.oam.dev/v1beta1"
"github.com/oam-dev/kubevela/pkg/cue/model/value"
monitorContext "github.com/oam-dev/kubevela/pkg/monitor/context"
wfContext "github.com/oam-dev/kubevela/pkg/workflow/context"
wfTypes "github.com/oam-dev/kubevela/pkg/workflow/types"
)