diff --git a/pkg/artifacts/artifact_storage_test.go b/pkg/artifacts/artifact_storage_test.go index 23a5573e8d5..15e272fc536 100644 --- a/pkg/artifacts/artifact_storage_test.go +++ b/pkg/artifacts/artifact_storage_test.go @@ -33,21 +33,41 @@ import ( ) var ( - pipelinerun = &v1alpha1.PipelineRun{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: "foo", - Name: "pipelineruntest", - }, - } - persistentVolumeClaim = GetPersistentVolumeClaim(DefaultPvcSize) - quantityComparer = cmp.Comparer(func(x, y resource.Quantity) bool { + quantityComparer = cmp.Comparer(func(x, y resource.Quantity) bool { return x.Cmp(y) == 0 }) + tasksWithFrom = []v1alpha1.PipelineTask{ + { + Name: "task1", + TaskRef: v1alpha1.TaskRef{ + Name: "task", + }, + Resources: &v1alpha1.PipelineTaskResources{ + Outputs: []v1alpha1.PipelineTaskOutputResource{{ + Name: "output", + Resource: "resource", + }}, + }, + }, + { + Name: "task2", + TaskRef: v1alpha1.TaskRef{ + Name: "task", + }, + Resources: &v1alpha1.PipelineTaskResources{ + Inputs: []v1alpha1.PipelineTaskInputResource{{ + Name: "input1", + Resource: "resource", + From: []string{"task1"}, + }}, + }, + }, + } ) -func GetPersistentVolumeClaim(size string) *corev1.PersistentVolumeClaim { +func GetPersistentVolumeClaim(pipelinerun *v1alpha1.PipelineRun, size string) *corev1.PersistentVolumeClaim { pvc := &corev1.PersistentVolumeClaim{ - ObjectMeta: metav1.ObjectMeta{Name: "pipelineruntest-pvc", Namespace: "foo", OwnerReferences: pipelinerun.GetOwnerReference()}, + ObjectMeta: metav1.ObjectMeta{Name: "pipelineruntest-pvc", Namespace: pipelinerun.Namespace, OwnerReferences: pipelinerun.GetOwnerReference()}, Spec: corev1.PersistentVolumeClaimSpec{ AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadWriteOnce}, Resources: corev1.ResourceRequirements{Requests: corev1.ResourceList{corev1.ResourceStorage: resource.MustParse(size)}}, @@ -56,7 +76,7 @@ func GetPersistentVolumeClaim(size string) *corev1.PersistentVolumeClaim { return pvc } -func TestNeedsPVC(t *testing.T) { +func TestConfigMapNeedsPVC(t *testing.T) { logger := logtesting.TestLogger(t) for _, c := range []struct { desc string @@ -126,12 +146,12 @@ func TestNeedsPVC(t *testing.T) { pvcNeeded: false, }} { t.Run(c.desc, func(t *testing.T) { - needed, err := NeedsPVC(c.configMap, nil, logger) + needed, err := ConfigMapNeedsPVC(c.configMap, nil, logger) if err != nil { t.Fatalf("Somehow had error checking if PVC was needed run: %s", err) } if needed != c.pvcNeeded { - t.Fatalf("Expected that NeedsPVC would be %t, but was %t", c.pvcNeeded, needed) + t.Fatalf("Expected that ConfigMapNeedsPVC would be %t, but was %t", c.pvcNeeded, needed) } }) } @@ -139,11 +159,20 @@ func TestNeedsPVC(t *testing.T) { } func TestInitializeArtifactStorageWithConfigMap(t *testing.T) { + pipelinerun := &v1alpha1.PipelineRun{ + ObjectMeta: metav1.ObjectMeta{ + Name: "pipelineruntest", + }, + Spec: v1alpha1.PipelineRunSpec{ + PipelineRef: v1alpha1.PipelineRef{ + Name: "pipelineWithFrom", + }, + }, + } logger := logtesting.TestLogger(t) for _, c := range []struct { desc string configMap *corev1.ConfigMap - pipelinerun *v1alpha1.PipelineRun expectedArtifactStorage ArtifactStorageInterface storagetype string }{{ @@ -157,10 +186,9 @@ func TestInitializeArtifactStorageWithConfigMap(t *testing.T) { PvcSizeKey: "10Gi", }, }, - pipelinerun: pipelinerun, expectedArtifactStorage: &v1alpha1.ArtifactPVC{ Name: "pipelineruntest", - PersistentVolumeClaim: GetPersistentVolumeClaim("10Gi"), + PersistentVolumeClaim: GetPersistentVolumeClaim(pipelinerun, "10Gi"), }, storagetype: "pvc", }, { @@ -176,7 +204,6 @@ func TestInitializeArtifactStorageWithConfigMap(t *testing.T) { v1alpha1.BucketServiceAccountSecretKey: "sakey", }, }, - pipelinerun: pipelinerun, expectedArtifactStorage: &v1alpha1.ArtifactBucket{ Location: "gs://fake-bucket", Secrets: []v1alpha1.SecretParam{{ @@ -199,10 +226,9 @@ func TestInitializeArtifactStorageWithConfigMap(t *testing.T) { v1alpha1.BucketServiceAccountSecretKey: "sakey", }, }, - pipelinerun: pipelinerun, expectedArtifactStorage: &v1alpha1.ArtifactPVC{ Name: "pipelineruntest", - PersistentVolumeClaim: persistentVolumeClaim, + PersistentVolumeClaim: GetPersistentVolumeClaim(pipelinerun, DefaultPvcSize), }, storagetype: "pvc", }, { @@ -217,10 +243,9 @@ func TestInitializeArtifactStorageWithConfigMap(t *testing.T) { v1alpha1.BucketServiceAccountSecretKey: "sakey", }, }, - pipelinerun: pipelinerun, expectedArtifactStorage: &v1alpha1.ArtifactPVC{ Name: "pipelineruntest", - PersistentVolumeClaim: persistentVolumeClaim, + PersistentVolumeClaim: GetPersistentVolumeClaim(pipelinerun, DefaultPvcSize), }, storagetype: "pvc", }, { @@ -231,10 +256,9 @@ func TestInitializeArtifactStorageWithConfigMap(t *testing.T) { Name: v1alpha1.BucketConfigName, }, }, - pipelinerun: pipelinerun, expectedArtifactStorage: &v1alpha1.ArtifactPVC{ Name: "pipelineruntest", - PersistentVolumeClaim: persistentVolumeClaim, + PersistentVolumeClaim: GetPersistentVolumeClaim(pipelinerun, DefaultPvcSize), }, storagetype: "pvc", }, { @@ -248,7 +272,6 @@ func TestInitializeArtifactStorageWithConfigMap(t *testing.T) { v1alpha1.BucketLocationKey: "gs://fake-bucket", }, }, - pipelinerun: pipelinerun, expectedArtifactStorage: &v1alpha1.ArtifactBucket{ Location: "gs://fake-bucket", }, @@ -256,129 +279,241 @@ func TestInitializeArtifactStorageWithConfigMap(t *testing.T) { }} { t.Run(c.desc, func(t *testing.T) { fakekubeclient := fakek8s.NewSimpleClientset(c.configMap) - artifactStorage, err := InitializeArtifactStorage(c.pipelinerun, fakekubeclient, logger) + artifactStorage, err := InitializeArtifactStorage(pipelinerun, tasksWithFrom, fakekubeclient, logger) if err != nil { t.Fatalf("Somehow had error initializing artifact storage run out of fake client: %s", err) } + if artifactStorage == nil { + t.Fatal("artifactStorage was nil, expected an actual value") + } // If the expected storage type is PVC, make sure we're actually creating that PVC. if c.storagetype == "pvc" { - _, err := fakekubeclient.CoreV1().PersistentVolumeClaims(c.pipelinerun.Namespace).Get(GetPVCName(c.pipelinerun), metav1.GetOptions{}) + _, err := fakekubeclient.CoreV1().PersistentVolumeClaims(pipelinerun.Namespace).Get(GetPVCName(pipelinerun), metav1.GetOptions{}) if err != nil { - t.Fatalf("Error getting expected PVC %s for PipelineRun %s: %s", GetPVCName(c.pipelinerun), c.pipelinerun.Name, err) + t.Fatalf("Error getting expected PVC %s for PipelineRun %s: %s", GetPVCName(pipelinerun), pipelinerun.Name, err) } } // Make sure we don't get any errors running CleanupArtifactStorage against the resulting storage, whether it's // a bucket or a PVC. - if err := CleanupArtifactStorage(c.pipelinerun, fakekubeclient, logger); err != nil { + if err := CleanupArtifactStorage(pipelinerun, fakekubeclient, logger); err != nil { t.Fatalf("Error cleaning up artifact storage: %s", err) } if diff := cmp.Diff(artifactStorage.GetType(), c.storagetype); diff != "" { - t.Fatalf("want %v, but got %v", c.storagetype, artifactStorage.GetType()) + t.Fatalf(diff) } if diff := cmp.Diff(artifactStorage, c.expectedArtifactStorage, quantityComparer); diff != "" { - t.Fatalf("want %v, but got %v", c.expectedArtifactStorage, artifactStorage) + t.Fatalf(diff) } }) } } -func TestCleanupArtifactStorage(t *testing.T) { +func TestInitializeArtifactStorageNoStorageNeeded(t *testing.T) { logger := logtesting.TestLogger(t) + // This Pipeline has Tasks that use both inputs and outputs, but there is + // no link between the inputs and outputs, so no storage is needed + pipeline := &v1alpha1.Pipeline{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "foo", + Name: "pipelineruntest", + }, + Spec: v1alpha1.PipelineSpec{ + Tasks: []v1alpha1.PipelineTask{ + { + Name: "task1", + TaskRef: v1alpha1.TaskRef{ + Name: "task", + }, + Resources: &v1alpha1.PipelineTaskResources{ + Inputs: []v1alpha1.PipelineTaskInputResource{{ + Name: "input1", + Resource: "resource", + }}, + Outputs: []v1alpha1.PipelineTaskOutputResource{{ + Name: "output", + Resource: "resource", + }}, + }, + }, + { + Name: "task2", + TaskRef: v1alpha1.TaskRef{ + Name: "task", + }, + Resources: &v1alpha1.PipelineTaskResources{ + Inputs: []v1alpha1.PipelineTaskInputResource{{ + Name: "input1", + Resource: "resource", + }}, + Outputs: []v1alpha1.PipelineTaskOutputResource{{ + Name: "output", + Resource: "resource", + }}, + }, + }, + }, + }, + } + pipelinerun := &v1alpha1.PipelineRun{ + ObjectMeta: metav1.ObjectMeta{ + Name: "pipelinerun", + Namespace: "namespace", + }, + Spec: v1alpha1.PipelineRunSpec{ + PipelineRef: v1alpha1.PipelineRef{ + Name: "pipeline", + }, + }, + } for _, c := range []struct { - desc string - configMap *corev1.ConfigMap - pipelinerun *v1alpha1.PipelineRun + desc string + configMap *corev1.ConfigMap }{{ - desc: "location empty", + desc: "has pvc configured", configMap: &corev1.ConfigMap{ ObjectMeta: metav1.ObjectMeta{ Namespace: system.GetNamespace(), - Name: v1alpha1.BucketConfigName, + Name: PvcConfigName, }, Data: map[string]string{ - v1alpha1.BucketLocationKey: "", - v1alpha1.BucketServiceAccountSecretName: "secret1", - v1alpha1.BucketServiceAccountSecretKey: "sakey", + PvcSizeKey: "10Gi", }, }, - pipelinerun: &v1alpha1.PipelineRun{ + }, { + desc: "has bucket configured", + configMap: &corev1.ConfigMap{ ObjectMeta: metav1.ObjectMeta{ - Namespace: "foo", - Name: "pipelineruntest", + Namespace: system.GetNamespace(), + Name: v1alpha1.BucketConfigName, + }, + Data: map[string]string{ + v1alpha1.BucketLocationKey: "gs://fake-bucket", + v1alpha1.BucketServiceAccountSecretName: "secret1", + v1alpha1.BucketServiceAccountSecretKey: "sakey", }, }, }, { - desc: "missing location", + desc: "no configmap", configMap: &corev1.ConfigMap{ ObjectMeta: metav1.ObjectMeta{ Namespace: system.GetNamespace(), Name: v1alpha1.BucketConfigName, }, Data: map[string]string{ + v1alpha1.BucketLocationKey: "", v1alpha1.BucketServiceAccountSecretName: "secret1", v1alpha1.BucketServiceAccountSecretKey: "sakey", }, }, - pipelinerun: &v1alpha1.PipelineRun{ + }} { + t.Run(c.desc, func(t *testing.T) { + fakekubeclient := fakek8s.NewSimpleClientset(c.configMap) + artifactStorage, err := InitializeArtifactStorage(pipelinerun, pipeline.Spec.Tasks, fakekubeclient, logger) + if err != nil { + t.Fatalf("Somehow had error initializing artifact storage run out of fake client: %s", err) + } + if artifactStorage.GetType() != "none" { + t.Errorf("Expected NoneArtifactStorage when none is needed but got %s", artifactStorage.GetType()) + } + }) + } +} + +func TestCleanupArtifactStorage(t *testing.T) { + pipelinerun := &v1alpha1.PipelineRun{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "foo", + Name: "pipelineruntest", + }, + } + logger := logtesting.TestLogger(t) + for _, c := range []struct { + desc string + configMap *corev1.ConfigMap + }{{ + desc: "location empty", + configMap: &corev1.ConfigMap{ ObjectMeta: metav1.ObjectMeta{ - Namespace: "foo", - Name: "pipelineruntest", + Namespace: system.GetNamespace(), + Name: v1alpha1.BucketConfigName, + }, + Data: map[string]string{ + v1alpha1.BucketLocationKey: "", + v1alpha1.BucketServiceAccountSecretName: "secret1", + v1alpha1.BucketServiceAccountSecretKey: "sakey", }, }, }, { - desc: "no config map data", + desc: "missing location", configMap: &corev1.ConfigMap{ ObjectMeta: metav1.ObjectMeta{ Namespace: system.GetNamespace(), Name: v1alpha1.BucketConfigName, }, + Data: map[string]string{ + v1alpha1.BucketServiceAccountSecretName: "secret1", + v1alpha1.BucketServiceAccountSecretKey: "sakey", + }, }, - pipelinerun: &v1alpha1.PipelineRun{ + }, { + desc: "no config map data", + configMap: &corev1.ConfigMap{ ObjectMeta: metav1.ObjectMeta{ - Namespace: "foo", - Name: "pipelineruntest", + Namespace: system.GetNamespace(), + Name: v1alpha1.BucketConfigName, }, }, }} { t.Run(c.desc, func(t *testing.T) { - fakekubeclient := fakek8s.NewSimpleClientset(c.configMap, GetPVCSpec(c.pipelinerun, persistentVolumeClaim.Spec.Resources.Requests["storage"])) - _, err := fakekubeclient.CoreV1().PersistentVolumeClaims(c.pipelinerun.Namespace).Get(GetPVCName(c.pipelinerun), metav1.GetOptions{}) + fakekubeclient := fakek8s.NewSimpleClientset(c.configMap, GetPVCSpec(pipelinerun, GetPersistentVolumeClaim(pipelinerun, DefaultPvcSize).Spec.Resources.Requests["storage"])) + _, err := fakekubeclient.CoreV1().PersistentVolumeClaims(pipelinerun.Namespace).Get(GetPVCName(pipelinerun), metav1.GetOptions{}) if err != nil { - t.Fatalf("Error getting expected PVC %s for PipelineRun %s: %s", GetPVCName(c.pipelinerun), c.pipelinerun.Name, err) + t.Fatalf("Error getting expected PVC %s for PipelineRun %s: %s", GetPVCName(pipelinerun), pipelinerun.Name, err) } - if err := CleanupArtifactStorage(c.pipelinerun, fakekubeclient, logger); err != nil { + if err := CleanupArtifactStorage(pipelinerun, fakekubeclient, logger); err != nil { t.Fatalf("Error cleaning up artifact storage: %s", err) } - _, err = fakekubeclient.CoreV1().PersistentVolumeClaims(c.pipelinerun.Namespace).Get(GetPVCName(c.pipelinerun), metav1.GetOptions{}) + _, err = fakekubeclient.CoreV1().PersistentVolumeClaims(pipelinerun.Namespace).Get(GetPVCName(pipelinerun), metav1.GetOptions{}) if err == nil { - t.Fatalf("Found PVC %s for PipelineRun %s after it should have been cleaned up", GetPVCName(c.pipelinerun), c.pipelinerun.Name) + t.Fatalf("Found PVC %s for PipelineRun %s after it should have been cleaned up", GetPVCName(pipelinerun), pipelinerun.Name) } else if !errors.IsNotFound(err) { - t.Fatalf("Error checking if PVC %s for PipelineRun %s has been cleaned up: %s", GetPVCName(c.pipelinerun), c.pipelinerun.Name, err) + t.Fatalf("Error checking if PVC %s for PipelineRun %s has been cleaned up: %s", GetPVCName(pipelinerun), pipelinerun.Name, err) } }) } } func TestInitializeArtifactStorageWithoutConfigMap(t *testing.T) { + pipelinerun := &v1alpha1.PipelineRun{ + ObjectMeta: metav1.ObjectMeta{ + Name: "pipelineruntest", + }, + } logger := logtesting.TestLogger(t) fakekubeclient := fakek8s.NewSimpleClientset() - pvc, err := InitializeArtifactStorage(pipelinerun, fakekubeclient, logger) + pvc, err := InitializeArtifactStorage(pipelinerun, tasksWithFrom, fakekubeclient, logger) if err != nil { t.Fatalf("Somehow had error initializing artifact storage run out of fake client: %s", err) } expectedArtifactPVC := &v1alpha1.ArtifactPVC{ Name: "pipelineruntest", - PersistentVolumeClaim: persistentVolumeClaim, + PersistentVolumeClaim: GetPersistentVolumeClaim(pipelinerun, DefaultPvcSize), } if diff := cmp.Diff(pvc, expectedArtifactPVC, cmpopts.IgnoreUnexported(resource.Quantity{})); diff != "" { - t.Fatalf("want %v, but got %v", expectedArtifactPVC, pvc) + t.Fatal(diff) } } func TestGetArtifactStorageWithConfigMap(t *testing.T) { + pipelinerun := &v1alpha1.PipelineRun{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "foo", + Name: "pipelineruntest", + }, + } logger := logtesting.TestLogger(t) for _, c := range []struct { desc string diff --git a/pkg/artifacts/artifacts_storage.go b/pkg/artifacts/artifacts_storage.go index f972a19eae0..41f4301c07d 100644 --- a/pkg/artifacts/artifacts_storage.go +++ b/pkg/artifacts/artifacts_storage.go @@ -53,11 +53,55 @@ type ArtifactStorageInterface interface { StorageBasePath(pr *v1alpha1.PipelineRun) string } +// ArtifactStorageNone is used when no storage is needed. +type ArtifactStorageNone struct{} + +// GetCopyToStorageFromContainerSpec returns no containers because none are needed. +func (a *ArtifactStorageNone) GetCopyToStorageFromContainerSpec(name, sourcePath, destinationPath string) []corev1.Container { + return nil +} + +// GetCopyFromStorageToContainerSpec returns no containers because none are needed. +func (a *ArtifactStorageNone) GetCopyFromStorageToContainerSpec(name, sourcePath, destinationPath string) []corev1.Container { + return nil +} + +// GetSecretsVolumes returns no volumes because none are needed. +func (a *ArtifactStorageNone) GetSecretsVolumes() []corev1.Volume { + return nil +} + +// GetType returns the string "none" to indicate this is the None storage type. +func (a *ArtifactStorageNone) GetType() string { + return "none" +} + +// StorageBasePath returns an empty string because no storage is being used and so +// there is no path that resources should be copied from / to. +func (a *ArtifactStorageNone) StorageBasePath(pr *v1alpha1.PipelineRun) string { + return "" +} + // InitializeArtifactStorage will check if there is there is a -// bucket configured or create a PVC -func InitializeArtifactStorage(pr *v1alpha1.PipelineRun, c kubernetes.Interface, logger *zap.SugaredLogger) (ArtifactStorageInterface, error) { +// bucket configured, create a PVC or return nil if no storage is required. +func InitializeArtifactStorage(pr *v1alpha1.PipelineRun, ts []v1alpha1.PipelineTask, c kubernetes.Interface, logger *zap.SugaredLogger) (ArtifactStorageInterface, error) { + // Artifact storage is only required if a pipeline has tasks that take inputs from previous tasks. + needStorage := false + for _, t := range ts { + if t.Resources != nil { + for _, i := range t.Resources.Inputs { + if len(i.From) != 0 { + needStorage = true + } + } + } + } + if !needStorage { + return &ArtifactStorageNone{}, nil + } + configMap, err := c.CoreV1().ConfigMaps(system.GetNamespace()).Get(v1alpha1.BucketConfigName, metav1.GetOptions{}) - shouldCreatePVC, err := NeedsPVC(configMap, err, logger) + shouldCreatePVC, err := ConfigMapNeedsPVC(configMap, err, logger) if err != nil { return nil, err } @@ -76,7 +120,7 @@ func InitializeArtifactStorage(pr *v1alpha1.PipelineRun, c kubernetes.Interface, // an output workspace or artifacts from one Task to another Task. No other PVCs will be impacted by this cleanup. func CleanupArtifactStorage(pr *v1alpha1.PipelineRun, c kubernetes.Interface, logger *zap.SugaredLogger) error { configMap, err := c.CoreV1().ConfigMaps(system.GetNamespace()).Get(v1alpha1.BucketConfigName, metav1.GetOptions{}) - shouldCreatePVC, err := NeedsPVC(configMap, err, logger) + shouldCreatePVC, err := ConfigMapNeedsPVC(configMap, err, logger) if err != nil { return err } @@ -89,9 +133,9 @@ func CleanupArtifactStorage(pr *v1alpha1.PipelineRun, c kubernetes.Interface, lo return nil } -// NeedsPVC checks if the possibly-nil config map passed to it is configured to use a bucket for artifact storage, +// ConfigMapNeedsPVC checks if the possibly-nil config map passed to it is configured to use a bucket for artifact storage, // returning true if instead a PVC is needed. -func NeedsPVC(configMap *corev1.ConfigMap, err error, logger *zap.SugaredLogger) (bool, error) { +func ConfigMapNeedsPVC(configMap *corev1.ConfigMap, err error, logger *zap.SugaredLogger) (bool, error) { if err != nil { if errors.IsNotFound(err) { return true, nil @@ -120,7 +164,7 @@ func NeedsPVC(configMap *corev1.ConfigMap, err error, logger *zap.SugaredLogger) // consumer code to get a container step for copy to/from storage func GetArtifactStorage(prName string, c kubernetes.Interface, logger *zap.SugaredLogger) (ArtifactStorageInterface, error) { configMap, err := c.CoreV1().ConfigMaps(system.GetNamespace()).Get(v1alpha1.BucketConfigName, metav1.GetOptions{}) - pvc, err := NeedsPVC(configMap, err, logger) + pvc, err := ConfigMapNeedsPVC(configMap, err, logger) if err != nil { return nil, xerrors.Errorf("couldn't determine if PVC was needed from config map: %w", err) } diff --git a/pkg/reconciler/v1alpha1/pipelinerun/pipelinerun.go b/pkg/reconciler/v1alpha1/pipelinerun/pipelinerun.go index 23a602fb683..20e02480df9 100644 --- a/pkg/reconciler/v1alpha1/pipelinerun/pipelinerun.go +++ b/pkg/reconciler/v1alpha1/pipelinerun/pipelinerun.go @@ -380,7 +380,7 @@ func (c *Reconciler) reconcile(ctx context.Context, pr *v1alpha1.PipelineRun) er rprts := pipelineState.GetNextTasks(candidateTasks) var as artifacts.ArtifactStorageInterface - if as, err = artifacts.InitializeArtifactStorage(pr, c.KubeClientSet, c.Logger); err != nil { + if as, err = artifacts.InitializeArtifactStorage(pr, p.Spec.Tasks, c.KubeClientSet, c.Logger); err != nil { c.Logger.Infof("PipelineRun failed to initialize artifact storage %s", pr.Name) return err } diff --git a/pkg/reconciler/v1alpha1/pipelinerun/pipelinerun_test.go b/pkg/reconciler/v1alpha1/pipelinerun/pipelinerun_test.go index c5de39d7788..aeb820f17e6 100644 --- a/pkg/reconciler/v1alpha1/pipelinerun/pipelinerun_test.go +++ b/pkg/reconciler/v1alpha1/pipelinerun/pipelinerun_test.go @@ -682,6 +682,56 @@ func TestReconcileWithTimeout(t *testing.T) { t.Errorf("TaskRun timeout %s should be less than or equal to PipelineRun timeout %s", actual.Spec.Timeout.Duration.String(), prs[0].Spec.Timeout.Duration.String()) } } + +func TestReconcileWithoutPVC(t *testing.T) { + ps := []*v1alpha1.Pipeline{tb.Pipeline("test-pipeline", "foo", tb.PipelineSpec( + tb.PipelineTask("hello-world-1", "hello-world"), + tb.PipelineTask("hello-world-2", "hello-world"), + ))} + + prs := []*v1alpha1.PipelineRun{tb.PipelineRun("test-pipeline-run", "foo", + tb.PipelineRunSpec("test-pipeline")), + } + ts := []*v1alpha1.Task{tb.Task("hello-world", "foo")} + + d := test.Data{ + PipelineRuns: prs, + Pipelines: ps, + Tasks: ts, + } + + // create fake recorder for testing + fr := record.NewFakeRecorder(2) + + testAssets := getPipelineRunController(t, d, fr) + c := testAssets.Controller + clients := testAssets.Clients + + err := c.Reconciler.Reconcile(context.Background(), "foo/test-pipeline-run") + if err != nil { + t.Errorf("Did not expect to see error when reconciling PipelineRun but saw %s", err) + } + + // Check that the PipelineRun was reconciled correctly + reconciledRun, err := clients.Pipeline.Tekton().PipelineRuns("foo").Get("test-pipeline-run", metav1.GetOptions{}) + if err != nil { + t.Fatalf("Somehow had error getting reconciled run out of fake client: %s", err) + } + + // Check that the expected TaskRun was created + for _, a := range clients.Kube.Actions() { + if ca, ok := a.(ktesting.CreateAction); ok { + obj := ca.GetObject() + if pvc, ok := obj.(*corev1.PersistentVolumeClaim); ok { + t.Errorf("Did not expect to see a PVC created when no resources are linked. %s was created", pvc) + } + } + } + + if !reconciledRun.Status.GetCondition(apis.ConditionSucceeded).IsUnknown() { + t.Errorf("Expected PipelineRun to be running, but condition status is %s", reconciledRun.Status.GetCondition(apis.ConditionSucceeded)) + } +} func TestReconcileCancelledPipelineRun(t *testing.T) { ps := []*v1alpha1.Pipeline{tb.Pipeline("test-pipeline", "foo", tb.PipelineSpec( tb.PipelineTask("hello-world-1", "hello-world", tb.Retries(1)), @@ -881,7 +931,7 @@ func TestReconcileWithDifferentServiceAccounts(t *testing.T) { ), tb.TaskRunLabel("tekton.dev/pipeline", "test-pipeline"), tb.TaskRunLabel("tekton.dev/pipelineRun", "test-pipeline-run-different-service-accs"), - tb.TaskRunLabel("tekton.dev/pipelineTask", "hello-world-0"), + tb.TaskRunLabel("tekton.dev/pipelineTask", "hello-world-0"), ), tb.TaskRun(taskRunNames[1], "foo", tb.TaskRunOwnerReference("PipelineRun", "test-pipeline-run-different-service-accs", @@ -894,7 +944,7 @@ func TestReconcileWithDifferentServiceAccounts(t *testing.T) { ), tb.TaskRunLabel("tekton.dev/pipeline", "test-pipeline"), tb.TaskRunLabel("tekton.dev/pipelineRun", "test-pipeline-run-different-service-accs"), - tb.TaskRunLabel("tekton.dev/pipelineTask", "hello-world-1"), + tb.TaskRunLabel("tekton.dev/pipelineTask", "hello-world-1"), ), } for i := range ps[0].Spec.Tasks {