From f06714e86f9bd481e6f319f0ab1fad63ec3a0e9e Mon Sep 17 00:00:00 2001 From: ezmcdja Date: Fri, 3 Jan 2025 16:07:08 +0000 Subject: [PATCH] Issue #615 - added end-to-end test for PackageVariant readiness gate - also broke ground on testing PackageVariants in end-to-end tests at all https://github.com/nephio-project/nephio/issues/615 --- .../packagevariant_controller.go | 20 +-- test/e2e/e2e_test.go | 123 +++++++++++++++++- 2 files changed, 129 insertions(+), 14 deletions(-) diff --git a/controllers/packagevariants/pkg/controllers/packagevariant/packagevariant_controller.go b/controllers/packagevariants/pkg/controllers/packagevariant/packagevariant_controller.go index 6286d076..1af3ea81 100644 --- a/controllers/packagevariants/pkg/controllers/packagevariant/packagevariant_controller.go +++ b/controllers/packagevariants/pkg/controllers/packagevariant/packagevariant_controller.go @@ -63,13 +63,13 @@ const ( ) var ( - conditionPipelineNotPassed = porchapi.Condition{ + ConditionPipelineNotPassed = porchapi.Condition{ Type: ConditionTypePipelinePassed, Status: porchapi.ConditionFalse, Reason: "WaitingOnPipeline", Message: "waiting for package pipeline to pass", } - conditionPipelinePassed = porchapi.Condition{ + ConditionPipelinePassed = porchapi.Condition{ Type: ConditionTypePipelinePassed, Status: porchapi.ConditionTrue, Reason: "PipelinePassed", @@ -353,7 +353,7 @@ func (r *PackageVariantReconciler) ensurePackageVariant(ctx context.Context, } setPrReadinessGate(newPR, ConditionTypePipelinePassed) - setPrStatusCondition(newPR, conditionPipelineNotPassed) + setPrStatusCondition(newPR, ConditionPipelineNotPassed) if err := r.Client.Update(ctx, newPR); err != nil { return nil, err } @@ -375,7 +375,7 @@ func (r *PackageVariantReconciler) ensurePackageVariant(ctx context.Context, return nil, err } - setPrStatusCondition(newPR, conditionPipelinePassed) + setPrStatusCondition(newPR, ConditionPipelinePassed) if err := r.Client.Update(ctx, newPR); err != nil { return nil, err } @@ -404,13 +404,13 @@ func (r *PackageVariantReconciler) findAndUpdateExistingRevisions(ctx context.Co // and we want to make sure the server is in sync with us setPrReadinessGate(downstream, ConditionTypePipelinePassed) - setPrStatusCondition(downstream, conditionPipelineNotPassed) + setPrStatusCondition(downstream, ConditionPipelineNotPassed) if err := r.Client.Update(ctx, downstream); err != nil { klog.Errorf("error updating package revision lifecycle: %v", err) return nil, err } - setPrStatusCondition(downstream, conditionPipelinePassed) + setPrStatusCondition(downstream, ConditionPipelinePassed) if err := r.Client.Update(ctx, downstream); err != nil { return nil, err } @@ -466,7 +466,7 @@ func (r *PackageVariantReconciler) findAndUpdateExistingRevisions(ctx context.Co // Save the updated PackageRevisionResources setPrReadinessGate(downstream, ConditionTypePipelinePassed) - setPrStatusCondition(downstream, conditionPipelineNotPassed) + setPrStatusCondition(downstream, ConditionPipelineNotPassed) if err := r.Client.Update(ctx, downstream); err != nil { return nil, err } @@ -474,7 +474,7 @@ func (r *PackageVariantReconciler) findAndUpdateExistingRevisions(ctx context.Co return nil, err } - setPrStatusCondition(downstream, conditionPipelinePassed) + setPrStatusCondition(downstream, ConditionPipelinePassed) if err := r.Client.Update(ctx, downstream); err != nil { return nil, err } @@ -747,13 +747,13 @@ func (r *PackageVariantReconciler) updateDraft(ctx context.Context, draft.Spec.Tasks = append(tasks, updateTask) setPrReadinessGate(draft, ConditionTypePipelinePassed) - setPrStatusCondition(draft, conditionPipelineNotPassed) + setPrStatusCondition(draft, ConditionPipelineNotPassed) if err := r.Client.Update(ctx, draft); err != nil { return nil, err } - setPrStatusCondition(draft, conditionPipelinePassed) + setPrStatusCondition(draft, ConditionPipelinePassed) if err := r.Client.Update(ctx, draft); err != nil { return nil, err } diff --git a/test/e2e/e2e_test.go b/test/e2e/e2e_test.go index d2f45d22..98511c3d 100644 --- a/test/e2e/e2e_test.go +++ b/test/e2e/e2e_test.go @@ -28,6 +28,9 @@ import ( "github.com/google/go-cmp/cmp" porchapi "github.com/nephio-project/porch/api/porch/v1alpha1" configapi "github.com/nephio-project/porch/api/porchconfig/v1alpha1" + + variantapi "github.com/nephio-project/porch/controllers/packagevariants/api/v1alpha1" + "github.com/nephio-project/porch/controllers/packagevariants/pkg/controllers/packagevariant" kptfilev1 "github.com/nephio-project/porch/pkg/kpt/api/kptfile/v1" "github.com/nephio-project/porch/pkg/repository" "github.com/stretchr/testify/assert" @@ -48,6 +51,7 @@ const ( var ( packageRevisionGVK = porchapi.SchemeGroupVersion.WithKind("PackageRevision") + packageVariantGVK = variantapi.GroupVersion.WithKind("PackageVariant") configMapGVK = corev1.SchemeGroupVersion.WithKind("ConfigMap") ) @@ -59,10 +63,10 @@ var _ TSetter = &PorchSuite{} var _ Initializer = &PorchSuite{} func TestE2E(t *testing.T) { - e2e := os.Getenv("E2E") - if e2e == "" { - t.Skip("set E2E to run this test") - } + // e2e := os.Getenv("E2E") + // if e2e == "" { + // t.Skip("set E2E to run this test") + // } RunSuite(&PorchSuite{}, t) } @@ -2968,3 +2972,114 @@ func (t *PorchSuite) TestPackageRevisionFieldSelectors(ctx context.Context) { } } } + +func (t *PorchSuite) TestPackageVariantReadinessGate(ctx context.Context) { + const ( + repository = "variant" + variantName = "testing-variant" + upstreamName = "variant-package-upstream" + downstreamName = "variant-package-downstream" + workspace = "workspace" + + setAnnoImage = "gcr.io/kpt-fn/set-annotations:v0.1.4" + applySetterImage = "gcr.io/kpt-fn/apply-setters:v0.2.0" + ) + + var ( + setAnnoMap = map[string]string{ + "nephio.org/cluster-name": "example", + } + setAnnoFunction = kptfilev1.Function{ + Image: setAnnoImage, + ConfigMap: setAnnoMap, + } + applySetterMap = map[string]string{ + "name": "updated-bucket-name", + "namespace": "updated-namespace", + "project-id": "updated-project-id", + "storage-class": "updated-storage-class", + } + applySetterFunction = kptfilev1.Function{ + Image: applySetterImage, + ConfigMap: applySetterMap, + } + mutatorFunctions = func() []kptfilev1.Function { + functions := []kptfilev1.Function{ + setAnnoFunction, + } + // add several repetitions of the applySetter function + // to make sure the pipeline takes enough time to detect + // the Condition changing from False to True + for i := 0; i < 80; i++ { + functions = append(functions, applySetterFunction) + } + return functions + }() + ) + t.Namespace = "porch" + + // Set up the repo and create/propose/approve the upstream package + t.RegisterMainGitRepositoryF(ctx, repository) + upstreamPr := t.CreatePackageSkeleton(repository, upstreamName, workspace) + t.CreateF(ctx, upstreamPr) + upstreamPr.Spec.Lifecycle = porchapi.PackageRevisionLifecycleProposed + t.UpdateF(ctx, upstreamPr) + upstreamPr.Spec.Lifecycle = porchapi.PackageRevisionLifecyclePublished + t.UpdateApprovalF(ctx, upstreamPr, metav1.UpdateOptions{}) + + // Create a new package variant (via init) + pv := &variantapi.PackageVariant{ + TypeMeta: metav1.TypeMeta{ + Kind: packageVariantGVK.Kind, + APIVersion: packageVariantGVK.GroupVersion().String(), + }, + ObjectMeta: metav1.ObjectMeta{ + Namespace: t.Namespace, + Name: variantName, + }, + Spec: variantapi.PackageVariantSpec{ + Downstream: &variantapi.Downstream{ + Package: downstreamName, + Repo: repository, + }, + Upstream: &variantapi.Upstream{ + Package: upstreamName, + Repo: repository, + Revision: "v1", + }, + Pipeline: &kptfilev1.Pipeline{ + Mutators: mutatorFunctions, + }, + Injectors: []variantapi.InjectionSelector{ + { + Name: "nrf-overrides-values", + }, + }, + }, + } + t.CreateF(ctx, pv) + + // Wait till the package variant has created the downstream PR and set the condition to False, + // then get the PR + downstreamPr, _ := t.WaitUntilPackageRevisionFulfillingConditionExists(ctx, 1*time.Minute, func(pr porchapi.PackageRevision) bool { + return pr.Spec.PackageName == pv.Spec.Downstream.Package && + slices.Contains(pr.Status.Conditions, packagevariant.ConditionPipelineNotPassed) + }) + assert.Containsf(t, downstreamPr.Spec.ReadinessGates, + porchapi.ReadinessGate{ConditionType: packagevariant.ConditionTypePipelinePassed}, "PackageVariant controller should have set a readiness gate on the downstream package revision") + + // Attempt to propose the PR - should fail + downstreamPr.Spec.Lifecycle = porchapi.PackageRevisionLifecycleProposed + proposeError := t.Client.Update(ctx, downstreamPr) + assert.ErrorContains(t, proposeError, "another request is already in progress") + + // Wait for the pipeline to finish and the condition to be set to True + downstreamPr, _ = t.WaitUntilPackageRevisionFulfillingConditionExists(ctx, 10*time.Second, func(pr porchapi.PackageRevision) bool { + return slices.Contains(pr.Status.Conditions, packagevariant.ConditionPipelinePassed) + }) + + // Propose the PR again - should succeed this time + downstreamPr.Spec.Lifecycle = porchapi.PackageRevisionLifecycleProposed + proposeError = t.Client.Update(ctx, downstreamPr) + assert.NoError(t, proposeError, "propose operation should have succeeded now that the pipeline has passed") +}