Skip to content

Commit

Permalink
Add passedConstraint checking when creating taskrun
Browse files Browse the repository at this point in the history
- Before a pipeline run creates a taskrun for the next task, it needs to
check the passed constraints to respect dependecnies between tasks
- add implementation of canTaskRun and unit tests
- add a task to delete tiller at the end of the helm task test
  • Loading branch information
nader-ziada committed Oct 18, 2018
1 parent ceae855 commit 323f693
Show file tree
Hide file tree
Showing 4 changed files with 334 additions and 19 deletions.
174 changes: 174 additions & 0 deletions pkg/reconciler/v1alpha1/pipelinerun/resources/passedconstraint_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,174 @@
/*
Copyright 2018 The Knative Authors
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package resources

import (
"testing"

"github.com/google/go-cmp/cmp"

"github.com/knative/build-pipeline/pkg/apis/pipeline/v1alpha1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)

var mytask1 = &v1alpha1.Task{
ObjectMeta: metav1.ObjectMeta{
Namespace: "namespace",
Name: "mytask1",
},
Spec: v1alpha1.TaskSpec{
Inputs: &v1alpha1.Inputs{
Resources: []v1alpha1.TaskResource{
v1alpha1.TaskResource{
Name: "myresource1",
Type: v1alpha1.PipelineResourceTypeGit,
},
},
},
},
}

var mytask2 = &v1alpha1.Task{
ObjectMeta: metav1.ObjectMeta{
Namespace: "namespace",
Name: "mytask2",
},
Spec: v1alpha1.TaskSpec{
Inputs: &v1alpha1.Inputs{
Resources: []v1alpha1.TaskResource{
v1alpha1.TaskResource{
Name: "myresource1",
Type: v1alpha1.PipelineResourceTypeGit,
},
},
},
},
}

var mypipelinetasks = []v1alpha1.PipelineTask{{
Name: "mypipelinetask1",
TaskRef: v1alpha1.TaskRef{Name: "mytask1"},
InputSourceBindings: []v1alpha1.SourceBinding{{
Name: "some-name-1",
Key: "myresource1",
ResourceRef: v1alpha1.PipelineResourceRef{
Name: "myresource1",
},
}},
}, {
Name: "mypipelinetask2",
TaskRef: v1alpha1.TaskRef{Name: "mytask2"},
InputSourceBindings: []v1alpha1.SourceBinding{{
Name: "some-name-2",
Key: "myresource1",
ResourceRef: v1alpha1.PipelineResourceRef{
Name: "myresource1",
},
PassedConstraints: []string{"mytask1"},
}},
}}

var mytaskruns = []v1alpha1.TaskRun{{
ObjectMeta: metav1.ObjectMeta{
Namespace: "namespace",
Name: "pipelinerun-mytask1",
},
Spec: v1alpha1.TaskRunSpec{},
}, {
ObjectMeta: metav1.ObjectMeta{
Namespace: "namespace",
Name: "pipelinerun-mytask2",
},
Spec: v1alpha1.TaskRunSpec{},
}}

func TestCanTaskRun(t *testing.T) {
tcs := []struct {
name string
state []*PipelineRunTaskRun
canSecondTaskRun bool
}{
{
name: "first-task-not-started",
state: []*PipelineRunTaskRun{{
Task: mytask1,
PipelineTask: &mypipelinetasks[0],
TaskRunName: "pipelinerun-mytask1",
TaskRun: nil,
}, {
Task: mytask2,
PipelineTask: &mypipelinetasks[1],
TaskRunName: "pipelinerun-mytask2",
TaskRun: nil,
}},
canSecondTaskRun: false,
},
{
name: "first-task-running",
state: []*PipelineRunTaskRun{{
Task: mytask1,
PipelineTask: &mypipelinetasks[0],
TaskRunName: "pipelinerun-mytask1",
TaskRun: makeStarted(mytaskruns[0]),
}, {
Task: mytask2,
PipelineTask: &mypipelinetasks[1],
TaskRunName: "pipelinerun-mytask2",
TaskRun: nil,
}},
canSecondTaskRun: false,
},
{
name: "first-task-failed",
state: []*PipelineRunTaskRun{{
Task: mytask1,
PipelineTask: &mypipelinetasks[0],
TaskRunName: "pipelinerun-mytask1",
TaskRun: makeFailed(mytaskruns[0]),
}, {
Task: mytask2,
PipelineTask: &mypipelinetasks[1],
TaskRunName: "pipelinerun-mytask2",
TaskRun: nil,
}},
canSecondTaskRun: false,
},
{
name: "first-task-finished",
state: []*PipelineRunTaskRun{{
Task: mytask1,
PipelineTask: &mypipelinetasks[0],
TaskRunName: "pipelinerun-mytask1",
TaskRun: makeSucceeded(mytaskruns[0]),
}, {
Task: mytask2,
PipelineTask: &mypipelinetasks[1],
TaskRunName: "pipelinerun-mytask2",
TaskRun: nil,
}},
canSecondTaskRun: true,
},
}
for _, tc := range tcs {
t.Run(tc.name, func(t *testing.T) {
cantaskrun := canTaskRun(&mypipelinetasks[1], tc.state)
if d := cmp.Diff(cantaskrun, tc.canSecondTaskRun); d != "" {
t.Fatalf("Expected second task availability to run should be %t, but different state returned: %s", tc.canSecondTaskRun, d)
}
})
}
}
28 changes: 26 additions & 2 deletions pkg/reconciler/v1alpha1/pipelinerun/resources/pipelinestate.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ func GetNextTask(prName string, state []*PipelineRunTaskRun, logger *zap.Sugared
logger.Infof("TaskRun %s is still running so we shouldn't start more for PipelineRun %s", prtr.TaskRunName, prName)
return nil
}
} else if canTaskRun(prtr.PipelineTask) {
} else if canTaskRun(prtr.PipelineTask, state) {
logger.Infof("TaskRun %s should be started for PipelineRun %s", prtr.TaskRunName, prName)
return prtr
}
Expand All @@ -54,8 +54,32 @@ func GetNextTask(prName string, state []*PipelineRunTaskRun, logger *zap.Sugared
return nil
}

func canTaskRun(pt *v1alpha1.PipelineTask) bool {
func canTaskRun(pt *v1alpha1.PipelineTask, state []*PipelineRunTaskRun) bool {
// Check if Task can run now. Go through all the input constraints
for _, input := range pt.InputSourceBindings {
if len(input.PassedConstraints) > 0 {
for _, constrainingTaskName := range input.PassedConstraints {
for _, prtr := range state {
// the constraining task must have a successful task run to allow this task to run
if prtr.Task.Name == constrainingTaskName {
if prtr.TaskRun == nil {
return false
}
c := prtr.TaskRun.Status.GetCondition(duckv1alpha1.ConditionSucceeded)
if c == nil {
return false
}
switch c.Status {
case corev1.ConditionFalse:
return false
case corev1.ConditionUnknown:
return false
}
}
}
}
}
}
return true
}

Expand Down
18 changes: 18 additions & 0 deletions test/crd_checks.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,3 +86,21 @@ func WaitForPipelineRunState(c *clients, name string, inState func(r *v1alpha1.P
return inState(r)
})
}

// WaitForServiceExternalIPState polls the status of the a k8s Service called name from client every
// interval until an external ip is assigned indicating it is done, returns an
// error or timeout. desc will be used to name the metric that is emitted to
// track how long it took for name to get into the state checked by inState.
func WaitForServiceExternalIPState(c *clients, namespace, name string, inState func(s *corev1.Service) (bool, error), desc string) error {
metricName := fmt.Sprintf("WaitForServiceExternalIPState/%s/%s", name, desc)
_, span := trace.StartSpan(context.Background(), metricName)
defer span.End()

return wait.PollImmediate(interval, timeout, func() (bool, error) {
r, err := c.KubeClient.Kube.CoreV1().Services(namespace).Get(name, metav1.GetOptions{})
if err != nil {
return true, err
}
return inState(r)
})
}
Loading

0 comments on commit 323f693

Please sign in to comment.