Skip to content

Commit

Permalink
fix(lifecycle-operator): retrieve KeptnEvaluationDefinition before cr…
Browse files Browse the repository at this point in the history
…eating KeptnEvaluation (#3144)

Signed-off-by: odubajDT <[email protected]>
  • Loading branch information
odubajDT authored Feb 29, 2024
1 parent c996315 commit 54a9b8b
Show file tree
Hide file tree
Showing 16 changed files with 276 additions and 21 deletions.
5 changes: 4 additions & 1 deletion lifecycle-operator/apis/lifecycle/v1beta1/common/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -124,9 +124,12 @@ func GetOverallState(s StatusSummary) KeptnState {
if s.Pending > 0 {
return StatePending
}
if s.Unknown > 0 || s.GetTotalCount() != s.Total {
if s.Unknown > 0 {
return StateUnknown
}
if s.GetTotalCount() != s.Total {
return StatePending
}
return StateSucceeded
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -257,6 +257,11 @@ func Test_GeOverallState(t *testing.T) {
Summary: StatusSummary{1, 0, 0, 1, 0, 0, 0},
Want: StateSucceeded,
},
{
Name: "pending total count",
Summary: StatusSummary{2, 0, 0, 1, 0, 0, 0},
Want: StatePending,
},
}
for _, tt := range tests {
t.Run(tt.Name, func(t *testing.T) {
Expand Down
24 changes: 21 additions & 3 deletions lifecycle-operator/controllers/common/evaluation/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,21 @@ func (r Handler) ReconcileEvaluations(ctx context.Context, phaseCtx context.Cont
&evaluationStatus,
)
if err != nil {
return nil, summary, err
if errors.IsNotFound(err) {
r.Log.Info("EvaluationDefinition for Evaluation not found",
"evaluation", evaluationStatus.Name,
"evaluationDefinition", evaluationName,
"namespace", piWrapper.GetNamespace(),
)
} else {
// log the error, but continue to proceed with other evaluations that may be created
r.Log.Error(err, "Could not create evaluation",
"evaluation", evaluationStatus.Name,
"evaluationDefinition", evaluationName,
"namespace", piWrapper.GetNamespace(),
)
}
continue
}
} else {
r.handleEvaluationExists(
Expand Down Expand Up @@ -181,8 +195,12 @@ func (r Handler) setupEvaluations(evaluationCreateAttributes CreateEvaluationAtt
}

func (r Handler) handleEvaluationNotExists(ctx context.Context, phaseCtx context.Context, evaluationCreateAttributes CreateEvaluationAttributes, evaluationName string, piWrapper *interfaces.PhaseItemWrapper, reconcileObject client.Object, evaluation *klcv1beta1.KeptnEvaluation, evaluationStatus *klcv1beta1.ItemStatus) error {
evaluationCreateAttributes.Definition.Name = evaluationName
evaluationName, err := r.CreateKeptnEvaluation(ctx, reconcileObject, evaluationCreateAttributes)
definition, err := common.GetEvaluationDefinition(r.Client, r.Log, ctx, evaluationName, piWrapper.GetNamespace())
if err != nil {
return controllererrors.ErrCannotGetKeptnEvaluationDefinition
}
evaluationCreateAttributes.Definition = *definition
evaluationName, err = r.CreateKeptnEvaluation(ctx, reconcileObject, evaluationCreateAttributes)
if err != nil {
return err
}
Expand Down
134 changes: 133 additions & 1 deletion lifecycle-operator/controllers/common/evaluation/handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,11 @@ import (

"github.com/keptn/lifecycle-toolkit/lifecycle-operator/apis/lifecycle/v1beta1"
apicommon "github.com/keptn/lifecycle-toolkit/lifecycle-operator/apis/lifecycle/v1beta1/common"
"github.com/keptn/lifecycle-toolkit/lifecycle-operator/controllers/common/config"
"github.com/keptn/lifecycle-toolkit/lifecycle-operator/controllers/common/eventsender"
"github.com/keptn/lifecycle-toolkit/lifecycle-operator/controllers/common/telemetry"
telemetryfake "github.com/keptn/lifecycle-toolkit/lifecycle-operator/controllers/common/telemetry/fake"
"github.com/keptn/lifecycle-toolkit/lifecycle-operator/controllers/common/testcommon"
controllererrors "github.com/keptn/lifecycle-toolkit/lifecycle-operator/controllers/errors"
"github.com/stretchr/testify/require"
"go.opentelemetry.io/otel/trace"
Expand All @@ -32,6 +34,7 @@ func TestEvaluationHandler(t *testing.T) {
wantStatus []v1beta1.ItemStatus
wantSummary apicommon.StatusSummary
evalObj v1beta1.KeptnEvaluation
evalDef *v1beta1.KeptnEvaluationDefinition
wantErr error
getSpanCalls int
unbindSpanCalls int
Expand Down Expand Up @@ -67,9 +70,126 @@ func TestEvaluationHandler(t *testing.T) {
getSpanCalls: 0,
unbindSpanCalls: 0,
},
{
name: "evaluation not started - could not find evaluationDefinition",
object: &v1beta1.KeptnAppVersion{
ObjectMeta: v1.ObjectMeta{
Namespace: "namespace",
},
Spec: v1beta1.KeptnAppVersionSpec{
KeptnAppContextSpec: v1beta1.KeptnAppContextSpec{
DeploymentTaskSpec: v1beta1.DeploymentTaskSpec{
PreDeploymentEvaluations: []string{"eval-def"},
},
},
},
},
evalObj: v1beta1.KeptnEvaluation{},
createAttr: CreateEvaluationAttributes{
SpanName: "",
Definition: v1beta1.KeptnEvaluationDefinition{
ObjectMeta: v1.ObjectMeta{
Name: "eval-def",
},
},
CheckType: apicommon.PreDeploymentEvaluationCheckType,
},
wantStatus: nil,
wantSummary: apicommon.StatusSummary{Total: 1, Pending: 0},
wantErr: nil,
getSpanCalls: 0,
unbindSpanCalls: 0,
},
{
name: "evaluations not started - could not find evaluationDefinition of one evaluation",
object: &v1beta1.KeptnAppVersion{
ObjectMeta: v1.ObjectMeta{
Namespace: "namespace",
},
Spec: v1beta1.KeptnAppVersionSpec{
KeptnAppContextSpec: v1beta1.KeptnAppContextSpec{
DeploymentTaskSpec: v1beta1.DeploymentTaskSpec{
PreDeploymentEvaluations: []string{"eval-def", "other-eval-def"},
},
},
},
},
evalDef: &v1beta1.KeptnEvaluationDefinition{
ObjectMeta: v1.ObjectMeta{
Namespace: testcommon.KeptnNamespace,
Name: "eval-def",
},
},
evalObj: v1beta1.KeptnEvaluation{},
createAttr: CreateEvaluationAttributes{
SpanName: "",
Definition: v1beta1.KeptnEvaluationDefinition{
ObjectMeta: v1.ObjectMeta{
Name: "eval-def",
},
},
CheckType: apicommon.PreDeploymentEvaluationCheckType,
},
wantStatus: []v1beta1.ItemStatus{
{
DefinitionName: "eval-def",
Status: apicommon.StatePending,
Name: "pre-eval-eval-def-",
},
},
wantSummary: apicommon.StatusSummary{Total: 2, Pending: 1},
wantErr: nil,
getSpanCalls: 1,
unbindSpanCalls: 0,
},
{
name: "evaluation not started - evaluationDefinition in default Keptn namespace",
object: &v1beta1.KeptnAppVersion{
ObjectMeta: v1.ObjectMeta{
Namespace: "namespace",
},
Spec: v1beta1.KeptnAppVersionSpec{
KeptnAppContextSpec: v1beta1.KeptnAppContextSpec{
DeploymentTaskSpec: v1beta1.DeploymentTaskSpec{
PreDeploymentEvaluations: []string{"eval-def"},
},
},
},
},
evalDef: &v1beta1.KeptnEvaluationDefinition{
ObjectMeta: v1.ObjectMeta{
Namespace: testcommon.KeptnNamespace,
Name: "eval-def",
},
},
evalObj: v1beta1.KeptnEvaluation{},
createAttr: CreateEvaluationAttributes{
SpanName: "",
Definition: v1beta1.KeptnEvaluationDefinition{
ObjectMeta: v1.ObjectMeta{
Name: "eval-def",
},
},
CheckType: apicommon.PreDeploymentEvaluationCheckType,
},
wantStatus: []v1beta1.ItemStatus{
{
DefinitionName: "eval-def",
Status: apicommon.StatePending,
Name: "pre-eval-eval-def-",
},
},
wantSummary: apicommon.StatusSummary{Total: 1, Pending: 1},
wantErr: nil,
getSpanCalls: 1,
unbindSpanCalls: 0,
},
{
name: "evaluation not started",
object: &v1beta1.KeptnAppVersion{
ObjectMeta: v1.ObjectMeta{
Namespace: "namespace",
},
Spec: v1beta1.KeptnAppVersionSpec{
KeptnAppContextSpec: v1beta1.KeptnAppContextSpec{
DeploymentTaskSpec: v1beta1.DeploymentTaskSpec{
Expand All @@ -78,6 +198,12 @@ func TestEvaluationHandler(t *testing.T) {
},
},
},
evalDef: &v1beta1.KeptnEvaluationDefinition{
ObjectMeta: v1.ObjectMeta{
Namespace: "namespace",
Name: "eval-def",
},
},
evalObj: v1beta1.KeptnEvaluation{},
createAttr: CreateEvaluationAttributes{
SpanName: "",
Expand Down Expand Up @@ -263,6 +389,8 @@ func TestEvaluationHandler(t *testing.T) {
},
}

config.Instance().SetDefaultNamespace(testcommon.KeptnNamespace)

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
err := v1beta1.AddToScheme(scheme.Scheme)
Expand All @@ -275,9 +403,13 @@ func TestEvaluationHandler(t *testing.T) {
return nil
},
}
initObjs := []client.Object{&tt.evalObj}
if tt.evalDef != nil {
initObjs = append(initObjs, tt.evalDef)
}
fakeRecorder := record.NewFakeRecorder(100)
handler := NewHandler(
fake.NewClientBuilder().WithObjects(&tt.evalObj).Build(),
fake.NewClientBuilder().WithObjects(initObjs...).Build(),
eventsender.NewK8sSender(fakeRecorder),
ctrl.Log.WithName("controller"),
noop.NewTracerProvider().Tracer("tracer"),
Expand Down
1 change: 1 addition & 0 deletions lifecycle-operator/controllers/errors/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ var ErrCannotMarshalParams = fmt.Errorf("could not marshal parameters")
var ErrNoTaskDefinitionSpec = fmt.Errorf("the TaskDefinition specs are empty")
var ErrUnsupportedWorkloadVersionResourceReference = fmt.Errorf("unsupported Resource Reference")
var ErrCannotGetKeptnTaskDefinition = fmt.Errorf("cannot retrieve KeptnTaskDefinition")
var ErrCannotGetKeptnEvaluationDefinition = fmt.Errorf("cannot retrieve KeptnEvaluationDefinition")
var ErrNoMatchingAppVersionFound = fmt.Errorf("no matching KeptnAppVersion found")

var ErrCannotRetrieveConfigMsg = "could not retrieve KeptnConfig: %w"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,3 +10,23 @@ status:
preDeploymentStatus: Succeeded
status: Failed
workloadOverallStatus: Deprecated
---
apiVersion: lifecycle.keptn.sh/v1beta1
kind: KeptnEvaluation
spec:
checkType: pre-eval
evaluationDefinition: available-cpus
appName: podtato-head
appVersion: 0.1.0
retries: 10
retryInterval: 5s
---
apiVersion: lifecycle.keptn.sh/v1beta1
kind: KeptnEvaluationDefinition
metadata:
name: available-cpus
spec:
objectives:
- evaluationTarget: ">1000"
keptnMetricRef:
name: available-cpus
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ status:
postDeploymentEvaluationStatus: Pending
postDeploymentStatus: Pending
preDeploymentEvaluationStatus: Pending
preDeploymentStatus: Unknown
preDeploymentStatus: Pending
status: Progressing
workloadOverallStatus: Pending
---
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
apiVersion: chainsaw.kyverno.io/v1alpha1
kind: Test
metadata:
name: workload-instance-failing-pre-task
name: workload-version-failing-pre-task
spec:
namespaceTemplate:
metadata:
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
apiVersion: lifecycle.keptn.sh/v1beta1
kind: KeptnAppVersion
metadata:
name: podtato-head-1.3-6b86b273
name: podtato-head-0.1.0-6b86b273
status:
currentPhase: AppDeploy
postDeploymentEvaluationStatus: Pending
Expand Down
Original file line number Diff line number Diff line change
@@ -1,15 +1,3 @@
apiVersion: lifecycle.keptn.sh/v1beta1
kind: KeptnApp
metadata:
name: podtato-head
spec:
version: "1.3"
workloads:
- name: podtato-head-entry
version: 0.1.0
postDeploymentTasks:
- post-deployment-hello
---
apiVersion: apps/v1
kind: Deployment
metadata:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
apiVersion: chainsaw.kyverno.io/v1alpha1
kind: Test
metadata:
name: workload-instance-missing-evaluation
name: workload-version-missing-evaluation
spec:
namespaceTemplate:
metadata:
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
apiVersion: lifecycle.keptn.sh/v1beta1
kind: KeptnAppVersion
metadata:
name: podtato-head-0.1.0-6b86b273
status:
currentPhase: AppDeploy
postDeploymentEvaluationStatus: Pending
postDeploymentStatus: Pending
preDeploymentEvaluationStatus: Succeeded
preDeploymentStatus: Succeeded
status: Progressing
workloadOverallStatus: Progressing
---
apiVersion: lifecycle.keptn.sh/v1beta1
kind: KeptnWorkloadVersion
metadata:
name: podtato-head-podtato-head-entry-0.1.0
status:
currentPhase: WorkloadPreDeployTasks
deploymentStatus: Pending
preDeploymentEvaluationStatus: Pending
preDeploymentStatus: Pending
status: Progressing
Loading

0 comments on commit 54a9b8b

Please sign in to comment.