Skip to content

Commit

Permalink
Merge branch 'main' into pv-errorhandling
Browse files Browse the repository at this point in the history
  • Loading branch information
kispaljr authored Jun 24, 2024
2 parents aa7c673 + b399b8b commit 8218912
Show file tree
Hide file tree
Showing 4 changed files with 65 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ import (
"fmt"
"strconv"
"strings"
"time"

porchapi "github.com/nephio-project/porch/api/porch/v1alpha1"
configapi "github.com/nephio-project/porch/api/porchconfig/v1alpha1"
Expand Down Expand Up @@ -59,8 +58,6 @@ const (

ConditionTypeStalled = "Stalled" // whether or not the packagevariant object is making progress or not
ConditionTypeReady = "Ready" // whether or not the reconciliation succeeded

requeueDuration = 30 * time.Second
)

//go:generate go run sigs.k8s.io/controller-tools/cmd/[email protected] rbac:headerFile=../../../../../scripts/boilerplate.yaml.txt,roleName=porch-controllers-packagevariants webhook paths="." output:rbac:artifacts:config=../../../config/rbac
Expand Down Expand Up @@ -129,7 +126,7 @@ func (r *PackageVariantReconciler) Reconcile(ctx context.Context, req ctrl.Reque
if err != nil {
setStalledConditionsToTrue(pv, err.Error())
// requeue, as the upstream may appear
return ctrl.Result{RequeueAfter: requeueDuration}, err
return ctrl.Result{}, err
}
meta.SetStatusCondition(&pv.Status.Conditions, metav1.Condition{
Type: ConditionTypeStalled,
Expand All @@ -147,7 +144,7 @@ func (r *PackageVariantReconciler) Reconcile(ctx context.Context, req ctrl.Reque
Message: err.Error(),
})
// requeue; it may be an intermittent error
return ctrl.Result{RequeueAfter: requeueDuration}, nil
return ctrl.Result{}, err
}

setTargetStatusConditions(pv, targets)
Expand Down
16 changes: 16 additions & 0 deletions pkg/registry/porch/fieldselector.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ package porch
import (
"fmt"

"github.com/nephio-project/porch/api/porch/v1alpha1"
"github.com/nephio-project/porch/pkg/repository"
apierrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/fields"
Expand Down Expand Up @@ -46,6 +47,8 @@ func convertPackageRevisionFieldSelector(label, value string) (internalLabel, in
return label, value, nil
case "spec.revision", "spec.packageName", "spec.repository":
return label, value, nil
case "spec.workspaceName", "spec.lifecycle":
return label, value, nil
default:
return "", "", fmt.Errorf("%q is not a known field selector", label)
}
Expand Down Expand Up @@ -151,6 +154,19 @@ func parsePackageRevisionFieldSelector(fieldSelector fields.Selector) (packageRe
filter.Package = requirement.Value
case "spec.repository":
filter.Repository = requirement.Value
case "spec.workspaceName":
filter.WorkspaceName = v1alpha1.WorkspaceName(requirement.Value)
case "spec.lifecycle":
v := v1alpha1.PackageRevisionLifecycle(requirement.Value)
switch v {
case v1alpha1.PackageRevisionLifecycleDraft,
v1alpha1.PackageRevisionLifecycleProposed,
v1alpha1.PackageRevisionLifecyclePublished,
v1alpha1.PackageRevisionLifecycleDeletionProposed:
filter.Lifecycle = v
default:
return filter, apierrors.NewBadRequest(fmt.Sprintf("unsupported fieldSelector value %q for field %q", requirement.Value, requirement.Field))
}

default:
return filter, apierrors.NewBadRequest(fmt.Sprintf("unknown fieldSelector field %q", requirement.Field))
Expand Down
6 changes: 6 additions & 0 deletions pkg/repository/repository.go
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,9 @@ type ListPackageRevisionFilter struct {

// Revision matches the revision of the package (spec.revision)
Revision string

// Lifecycle matches the spec.lifecycle of the package
Lifecycle v1alpha1.PackageRevisionLifecycle
}

// Matches returns true if the provided PackageRevision satisfies the conditions in the filter.
Expand All @@ -158,6 +161,9 @@ func (f *ListPackageRevisionFilter) Matches(p PackageRevision) bool {
if f.KubeObjectName != "" && f.KubeObjectName != p.KubeObjectName() {
return false
}
if f.Lifecycle != "" && f.Lifecycle != p.Lifecycle() {
return false
}
return true
}

Expand Down
41 changes: 41 additions & 0 deletions test/e2e/e2e_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import (
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/meta"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/fields"
"k8s.io/apimachinery/pkg/types"
_ "k8s.io/client-go/plugin/pkg/client/auth/gcp"
"sigs.k8s.io/controller-runtime/pkg/client"
Expand Down Expand Up @@ -2371,5 +2372,45 @@ func (t *PorchSuite) TestUniquenessOfUIDs(ctx context.Context) {
}
uids[pr.UID] = &pr
}
}

func (t *PorchSuite) TestPackageRevisionFieldselectors(ctx context.Context) {
t.registerGitRepositoryF(ctx, testBlueprintsRepo, "test-blueprints", "")

prList := porchapi.PackageRevisionList{}

wsName := "v1"
wsSelector := client.MatchingFields(fields.Set{"spec.workspaceName": wsName})
t.ListE(ctx, &prList, client.InNamespace(t.namespace), wsSelector)
if len(prList.Items) == 0 {
t.Errorf("Expected at least one PackageRevision with workspaceName=%q, but got none", wsName)
}
for _, pr := range prList.Items {
if pr.Spec.WorkspaceName != porchapi.WorkspaceName(wsName) {
t.Errorf("PackageRevision %s workspaceName: want %q, but got %q", pr.Name, wsName, pr.Spec.WorkspaceName)
}
}

publishedSelector := client.MatchingFields(fields.Set{"spec.lifecycle": string(porchapi.PackageRevisionLifecyclePublished)})
t.ListE(ctx, &prList, client.InNamespace(t.namespace), publishedSelector)
if len(prList.Items) == 0 {
t.Errorf("Expected at least one PackageRevision with lifecycle=%q, but got none", porchapi.PackageRevisionLifecyclePublished)
}
for _, pr := range prList.Items {
if pr.Spec.Lifecycle != porchapi.PackageRevisionLifecyclePublished {
t.Errorf("PackageRevision %s lifecycle: want %q, but got %q", pr.Name, porchapi.PackageRevisionLifecyclePublished, pr.Spec.Lifecycle)
}
}

draftSelector := client.MatchingFields(fields.Set{"spec.lifecycle": string(porchapi.PackageRevisionLifecycleDraft)})
t.ListE(ctx, &prList, client.InNamespace(t.namespace), draftSelector)
// TODO: add draft packages to the test repo
// if len(prList.Items) == 0 {
// t.Errorf("Expected at least one PackageRevision with lifecycle=%q, but got none", porchapi.PackageRevisionLifecycleDraft)
// }
for _, pr := range prList.Items {
if pr.Spec.Lifecycle != porchapi.PackageRevisionLifecycleDraft {
t.Errorf("PackageRevision %s lifecycle: want %q, but got %q", pr.Name, porchapi.PackageRevisionLifecycleDraft, pr.Spec.Lifecycle)
}
}
}

0 comments on commit 8218912

Please sign in to comment.