Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Loop ranges should handle from = to, and generate one iteration. #941

Merged
merged 4 commits into from
May 18, 2022
Merged

Loop ranges should handle from = to, and generate one iteration. #941

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
08c3535
Loop ranges should handle start = 0 and end = 0, should generate one…
ScrapCodes May 16, 2022
2c6f576
When from = to, then generate one iteration. emulating how enumerate…
ScrapCodes May 16, 2022
36b2bac
Fixed edge cases.
ScrapCodes May 17, 2022
f146771
if from and to are unset, we should not pick defaults either.
ScrapCodes May 18, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 27 additions & 18 deletions tekton-catalog/pipeline-loops/pkg/reconciler/pipelinelooprun/pipelineloop_range_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -157,10 +157,19 @@ func TestReconcilePipelineLoopRunRange(t *testing.T) {
from: "1",
to: "1",
step: "1",
expectedStatus: corev1.ConditionTrue,
expectedReason: pipelineloopv1alpha1.PipelineLoopRunReasonSucceeded,
expectedPipelineruns: []*v1beta1.PipelineRun{},
expectedEvents: []string{"Normal Started ", "Normal Succeeded All PipelineRuns completed successfully"},
expectedStatus: corev1.ConditionUnknown,
expectedReason: pipelineloopv1alpha1.PipelineLoopRunReasonRunning,
expectedPipelineruns: []*v1beta1.PipelineRun{expectedPipelineRunWithRange2},
expectedEvents: []string{"Normal Started ", "Normal Running Iterations completed: 0"},
}, {
name: "Case from 0 to 0 and non zero step increment",
from: "0",
step: "1",
to: "0",
expectedStatus: corev1.ConditionUnknown,
expectedReason: pipelineloopv1alpha1.PipelineLoopRunReasonRunning,
expectedPipelineruns: []*v1beta1.PipelineRun{expectedPipelineRunWithRange3},
expectedEvents: []string{"Normal Started ", "Normal Running Iterations completed: 0"},
}, {
name: "Case from < to and +ve step increment",
from: "-1",
Expand All @@ -180,32 +189,32 @@ func TestReconcilePipelineLoopRunRange(t *testing.T) {
expectedPipelineruns: []*v1beta1.PipelineRun{},
expectedEvents: []string{"Normal Started ", "Warning Failed Cannot determine number of iterations: invalid values step: 0 found in runs"},
}, {
name: "Case from > to and +ve increment",
name: "Case to - from < step and step > 0",
from: "1",
step: "1",
to: "-1",
expectedStatus: corev1.ConditionFalse,
expectedReason: pipelineloopv1alpha1.PipelineLoopRunReasonFailedValidation,
expectedPipelineruns: []*v1beta1.PipelineRun{},
expectedEvents: []string{"Normal Started ", "Warning Failed Cannot determine number of iterations: invalid values for from:1, to:-1 & step: 1 found in runs"},
expectedStatus: corev1.ConditionUnknown,
expectedReason: pipelineloopv1alpha1.PipelineLoopRunReasonRunning,
expectedPipelineruns: []*v1beta1.PipelineRun{expectedPipelineRunWithRange2},
expectedEvents: []string{"Normal Started ", "Normal Running Iterations completed: 0"},
}, {
name: "Case from > to and +ve step increment",
name: "Case to - from < step and step > 0",
from: "0",
step: "1",
to: "-1",
expectedStatus: corev1.ConditionFalse,
expectedReason: pipelineloopv1alpha1.PipelineLoopRunReasonFailedValidation,
expectedPipelineruns: []*v1beta1.PipelineRun{},
expectedEvents: []string{"Normal Started ", "Warning Failed Cannot determine number of iterations: invalid values for from:0, to:-1 & step: 1 found in runs"},
expectedStatus: corev1.ConditionUnknown,
expectedReason: pipelineloopv1alpha1.PipelineLoopRunReasonRunning,
expectedPipelineruns: []*v1beta1.PipelineRun{expectedPipelineRunWithRange3},
expectedEvents: []string{"Normal Started ", "Normal Running Iterations completed: 0"},
}, {
name: "Case from = to",
from: "-1",
step: "1",
to: "-1",
expectedStatus: corev1.ConditionTrue,
expectedReason: pipelineloopv1alpha1.PipelineLoopRunReasonSucceeded,
expectedPipelineruns: []*v1beta1.PipelineRun{},
expectedEvents: []string{"Normal Started ", "Normal Succeeded All PipelineRuns completed successfully"},
expectedStatus: corev1.ConditionUnknown,
expectedReason: pipelineloopv1alpha1.PipelineLoopRunReasonRunning,
expectedPipelineruns: []*v1beta1.PipelineRun{expectedPipelineRunWithRange1},
expectedEvents: []string{"Normal Started ", "Normal Running Iterations completed: 0"},
}, {
name: "Case from > to and non -ve step increment",
from: "1",
Expand Down
17 changes: 16 additions & 1 deletion tekton-catalog/pipeline-loops/pkg/reconciler/pipelinelooprun/pipelinelooprun.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -790,18 +790,22 @@ func computeIterations(run *v1alpha1.Run, tls *pipelineloopv1alpha1.PipelineLoop
from := 0
step := 1
to := 0
fromProvided := false
toProvided := false
iterationElements := []interface{}{}
iterationParamStr := ""
iterationParamStrSeparator := ""
for _, p := range run.Spec.Params {
if p.Name == "from" {
from, _ = strconv.Atoi(p.Value.StringVal)
fromProvided = true
}
if p.Name == "step" {
step, _ = strconv.Atoi(p.Value.StringVal)
}
if p.Name == "to" {
to, _ = strconv.Atoi(p.Value.StringVal)
toProvided = true
}
if p.Name == tls.IterateParam {
if p.Value.Type == v1beta1.ParamTypeString {
Expand Down Expand Up @@ -872,10 +876,16 @@ func computeIterations(run *v1alpha1.Run, tls *pipelineloopv1alpha1.PipelineLoop
}
}
}
if from != to {
if from != to && fromProvided && toProvided {
if step == 0 {
return 0, iterationElements, fmt.Errorf("invalid values step: %d found in runs", step)
}
if (to-from < step && step > 0) || (to-from > step && step < 0) {
// This is a special case, to emulate "python's enumerate" behaviour see issue #935
numberOfIterations = 1
iterationElements = append(iterationElements, from)
return numberOfIterations, iterationElements, nil
}
if (from > to && step > 0) || (from < to && step < 0) {
return 0, iterationElements, fmt.Errorf("invalid values for from:%d, to:%d & step: %d found in runs", from, to, step)
}
Expand All @@ -892,6 +902,11 @@ func computeIterations(run *v1alpha1.Run, tls *pipelineloopv1alpha1.PipelineLoop
}
}
}
if from == to && step != 0 && fromProvided && toProvided {
// This is a special case, to emulate "python's enumerate" behaviour see issue #935
numberOfIterations = 1
iterationElements = append(iterationElements, from)
}
return numberOfIterations, iterationElements, nil
}

Expand Down