Skip to content

Commit

Permalink
Validate TaskRun compatibility with the Affinity Assistant
Browse files Browse the repository at this point in the history
A TaskRun that mount more than one PVC-backed workspace is incompatible
with the Affinity Assistant. But there is no validation if the TaskRun
is compatible - so the TaskRun Pod is stuck with little information on why
to the user.

This commit adds validation of TaskRuns. When a TaskRun is associated with
an Affinity Assistant, it is checked that not more than one PVC workspace
is used - if so, the TaskRun will fail with a TaskRunValidationFailed condition.

Proposed in tektoncd#2829 (comment)
Closes tektoncd#2864
  • Loading branch information
jlpettersson committed Jul 27, 2020
1 parent a81f43d commit 1cd4d8c
Show file tree
Hide file tree
Showing 2 changed files with 97 additions and 0 deletions.
29 changes: 29 additions & 0 deletions pkg/reconciler/taskrun/taskrun.go
Original file line number Diff line number Diff line change
Expand Up @@ -294,6 +294,12 @@ func (c *Reconciler) prepare(ctx context.Context, tr *v1beta1.TaskRun) (*v1beta1
return nil, nil, controller.NewPermanentError(err)
}

if err := validateWorkspaceCompatibilityWithAffinityAssistant(tr); err != nil {
logger.Errorf("TaskRun %q workspaces are invalid: %v", tr.Name, err)
tr.Status.MarkResourceFailed(podconvert.ReasonFailedValidation, err)
return nil, nil, controller.NewPermanentError(err)
}

// Initialize the cloud events if at least a CloudEventResource is defined
// and they have not been initialized yet.
// FIXME(afrittoli) This resource specific logic will have to be replaced
Expand Down Expand Up @@ -702,3 +708,26 @@ func storeTaskSpec(ctx context.Context, tr *v1beta1.TaskRun, ts *v1beta1.TaskSpe
}
return nil
}

// validateWorkspaceCompatibilityWithAffinityAssistant validates the TaskRun's compatibility
// with the Affinity Assistant - if associated with an Affinity Assistant.
// No more than one PVC-backed workspace can be used for a TaskRun that is associated with an
// Affinity Assistant.
func validateWorkspaceCompatibilityWithAffinityAssistant(tr *v1beta1.TaskRun) error {
_, isAssociatedWithAnAffinityAssistant := tr.Annotations[workspace.AnnotationAffinityAssistantName]
if !isAssociatedWithAnAffinityAssistant {
return nil
}

pvcWorkspaces := 0
for _, w := range tr.Spec.Workspaces {
if w.PersistentVolumeClaim != nil || w.VolumeClaimTemplate != nil {
pvcWorkspaces++
}
}

if pvcWorkspaces > 1 {
return fmt.Errorf("TaskRun mounts more than one PersistentVolumeClaim - that is forbidden when the Affinity Assistant is enabled")
}
return nil
}
68 changes: 68 additions & 0 deletions pkg/reconciler/taskrun/taskrun_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ import (
"github.com/tektoncd/pipeline/pkg/reconciler/volumeclaim"
"github.com/tektoncd/pipeline/pkg/system"
"github.com/tektoncd/pipeline/pkg/timeout"
"github.com/tektoncd/pipeline/pkg/workspace"
test "github.com/tektoncd/pipeline/test"
"github.com/tektoncd/pipeline/test/diff"
"github.com/tektoncd/pipeline/test/names"
Expand Down Expand Up @@ -2820,6 +2821,73 @@ func TestReconcileTaskResourceResolutionAndValidation(t *testing.T) {
}
}

// TestReconcileWithWorkspacesIncompatibleWithAffinityAssistant tests that a TaskRun used with an associated
// Affinity Assistant is validated and that the validation fails for a TaskRun that is incompatible with
// Affinity Assistant; e.g. using more than one PVC-backed workspace.
func TestReconcileWithWorkspacesIncompatibleWithAffinityAssistant(t *testing.T) {
taskWithTwoWorkspaces := tb.Task("test-task-two-workspaces", tb.TaskNamespace("foo"),
tb.TaskSpec(
tb.TaskWorkspace("ws1", "task workspace", "", true),
tb.TaskWorkspace("ws2", "another workspace", "", false),
))
taskRun := tb.TaskRun("taskrun-with-two-workspaces", tb.TaskRunNamespace("foo"), tb.TaskRunSpec(
tb.TaskRunTaskRef(taskWithTwoWorkspaces.Name, tb.TaskRefAPIVersion("a1")),
tb.TaskRunWorkspacePVC("ws1", "", "pvc1"),
tb.TaskRunWorkspaceVolumeClaimTemplate("ws2", "", &corev1.PersistentVolumeClaim{
ObjectMeta: metav1.ObjectMeta{
Name: "pvc2",
},
Spec: corev1.PersistentVolumeClaimSpec{},
}),
))

// associate the TaskRun with a dummy Affinity Assistant
taskRun.Annotations[workspace.AnnotationAffinityAssistantName] = "dummy-affinity-assistant"

d := test.Data{
Tasks: []*v1beta1.Task{taskWithTwoWorkspaces},
TaskRuns: []*v1beta1.TaskRun{taskRun},
ClusterTasks: nil,
PipelineResources: nil,
}
names.TestingSeed()
testAssets, cancel := getTaskRunController(t, d)
defer cancel()
clients := testAssets.Clients

t.Logf("Creating SA %s in %s", "default", "foo")
if _, err := clients.Kube.CoreV1().ServiceAccounts("foo").Create(&corev1.ServiceAccount{
ObjectMeta: metav1.ObjectMeta{
Name: "default",
Namespace: "foo",
},
}); err != nil {
t.Fatal(err)
}

_ = testAssets.Controller.Reconciler.Reconcile(context.Background(), getRunName(taskRun))

_, err := clients.Pipeline.TektonV1beta1().Tasks(taskRun.Namespace).Get(taskWithTwoWorkspaces.Name, metav1.GetOptions{})
if err != nil {
t.Fatalf("krux: %v", err)
}

ttt, err := clients.Pipeline.TektonV1beta1().TaskRuns(taskRun.Namespace).Get(taskRun.Name, metav1.GetOptions{})
if err != nil {
t.Fatalf("expected TaskRun %s to exist but instead got error when getting it: %v", taskRun.Name, err)
}

if len(ttt.Status.Conditions) != 1 {
t.Errorf("unexpected number of Conditions, expected 1 Condition")
}

for _, cond := range ttt.Status.Conditions {
if cond.Reason != podconvert.ReasonFailedValidation {
t.Errorf("unexpected Reason on the Condition, expected: %s, got: %s", podconvert.ReasonFailedValidation, cond.Reason)
}
}
}

// TestReconcileWorkspaceWithVolumeClaimTemplate tests a reconcile of a TaskRun that has
// a Workspace with VolumeClaimTemplate and check that it is translated to a created PersistentVolumeClaim.
func TestReconcileWorkspaceWithVolumeClaimTemplate(t *testing.T) {
Expand Down

0 comments on commit 1cd4d8c

Please sign in to comment.