diff --git a/pkg/reconciler/taskrun/taskrun.go b/pkg/reconciler/taskrun/taskrun.go index 6da61b6f515..7c9e7dd573a 100644 --- a/pkg/reconciler/taskrun/taskrun.go +++ b/pkg/reconciler/taskrun/taskrun.go @@ -302,10 +302,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) + if _, usesAssistant := tr.Annotations[workspace.AnnotationAffinityAssistantName]; usesAssistant { + if err := workspace.ValidateOnlyOnePVCIsUsed(tr.Spec.Workspaces); err != nil { + logger.Errorf("TaskRun %q workspaces incompatible with Affinity Assistant: %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 @@ -775,26 +777,3 @@ 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 -} diff --git a/pkg/workspace/validate.go b/pkg/workspace/validate.go index 2738b270b7d..18ba1535451 100644 --- a/pkg/workspace/validate.go +++ b/pkg/workspace/validate.go @@ -48,3 +48,25 @@ func ValidateBindings(w []v1beta1.WorkspaceDeclaration, wb []v1beta1.WorkspaceBi } return nil } + +// ValidateOnlyOnePVCIsUsed checks that a list of WorkspaceBinding uses only one +// persistent volume claim. +// +// This is only useful to validate that WorkspaceBindings in TaskRuns are compatible +// with affinity rules enforced by the AffinityAssistant. +func ValidateOnlyOnePVCIsUsed(wb []v1beta1.WorkspaceBinding) error { + workspaceVolumes := make(map[string]bool) + for _, w := range wb { + if w.PersistentVolumeClaim != nil { + workspaceVolumes[w.PersistentVolumeClaim.ClaimName] = true + } + if w.VolumeClaimTemplate != nil { + workspaceVolumes[w.Name] = true + } + } + + if len(workspaceVolumes) > 1 { + return fmt.Errorf("more than one PersistentVolumeClaim is bound") + } + return nil +} diff --git a/pkg/workspace/validate_test.go b/pkg/workspace/validate_test.go index e79d104a509..7d115a93136 100644 --- a/pkg/workspace/validate_test.go +++ b/pkg/workspace/validate_test.go @@ -17,9 +17,11 @@ limitations under the License. package workspace import ( + "errors" "testing" "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1" + "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1" corev1 "k8s.io/api/core/v1" ) @@ -131,3 +133,89 @@ func TestValidateBindingsInvalid(t *testing.T) { }) } } + +func TestValidateOnlyOnePVCIsUsed_Valid(t *testing.T) { + for _, tc := range []struct { + name string + bindings []v1beta1.WorkspaceBinding + }{{ + name: "an error is not returned when no bindings are given", + bindings: []v1beta1.WorkspaceBinding{}, + }, { + name: "an error is not returned when volume claims are not used", + bindings: []v1beta1.WorkspaceBinding{{ + EmptyDir: &corev1.EmptyDirVolumeSource{}, + }, { + Secret: &corev1.SecretVolumeSource{}, + }}, + }, { + name: "an error is not returned when one PV claim is used in two bindings", + bindings: []v1beta1.WorkspaceBinding{{ + PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ + ClaimName: "foo", + }, + }, { + PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ + ClaimName: "foo", + }, + }}, + }, { + name: "an error is not returned when one PV claim is used in two bindings with different subpaths", + bindings: []v1beta1.WorkspaceBinding{{ + SubPath: "/pathA", + PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ + ClaimName: "foo", + }, + }, { + SubPath: "/pathB", + PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ + ClaimName: "foo", + }, + }}, + }} { + t.Run(tc.name, func(t *testing.T) { + if err := ValidateOnlyOnePVCIsUsed(tc.bindings); err != nil { + t.Errorf("unexpected error: %v", err) + } + }) + } +} + +func TestValidateOnlyOnePVCIsUsed_Invalid(t *testing.T) { + validationError := errors.New("more than one PersistentVolumeClaim is bound") + for _, tc := range []struct { + name string + bindings []v1beta1.WorkspaceBinding + wantErr error + }{{ + name: "an error is returned when two different PV claims are used", + bindings: []v1beta1.WorkspaceBinding{{ + PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ + ClaimName: "foo", + }, + }, { + PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ + ClaimName: "bar", + }, + }}, + wantErr: validationError, + }, { + name: "an error is returned when a PVC and volume claim template are mixed", + bindings: []v1beta1.WorkspaceBinding{{ + PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ + ClaimName: "foo", + }, + }, { + Name: "bar", + VolumeClaimTemplate: &corev1.PersistentVolumeClaim{}, + }}, + wantErr: validationError, + }} { + t.Run(tc.name, func(t *testing.T) { + err := ValidateOnlyOnePVCIsUsed(tc.bindings) + if err == nil || (tc.wantErr.Error() != err.Error()) { + t.Errorf("expected %v received %v", tc.wantErr, err) + } + }) + } +}