Skip to content

Commit 9fee28c

Browse files
JamesMcDermottCatalin-Stratulat-Ericssonliamfallonefiacor
authored
Refactor field-selector filtering for PackageRevision list operation (kptdev#273)
* Issue kptdev#677 - refactor package revision filtering for list flow - first cut at making it simpler to add new selectable fields - reworked the 2 levels of PackageRevision filtering to use Kubernetes SelectionPredicates, so we can more easily feed in a set of field names and mappings to their counterparts in the data structures https://github.com/nephio-project/nephio/issues/677 * refactor ListPackageRevisionFilter parse method * rework selectable-field refactoring to pull everything together in fieldselector.go - results in a performance hit for list flow - work in progress https://github.com/nephio-project/nephio/issues/677 * rework selectable-field refactoring to pull everything together in fieldselector.go - results in a performance hit for list flow - work in progress https://github.com/nephio-project/nephio/issues/677 * fix for manually filtering by namespace with no existing filter fields * fix filtering on spec.repository * Updating Github Action E2E Tests & fixing TestPodEvaluator & TestEditPackageRevision Flakyness (kptdev#271) * updating github action porch e2e test to use latest versions & procedure * changed kind context to 'porch-test' * testing potential delete timing issue * removed time.sleep with awaiting deletion with 20 sec timeout * added generous timeout value for tests * removed await pod deletion and just switching to different namespace to avoid clash in the first place * added accurate comment on timeout * Issue kptdev#677 - final refactor to package revision filtering - selectable fields now all bound to elements of internal repository.PackageRevision type - this allows all filtering to be done at level of the ListPackageRevisionFilter's matching - upper-level packageRevisionFilter now only extends ListPackageRevisionFilter upward into the registry/porch package - list of selectable field names added to API-level PackageRevision type in v1alpha1 types.go - new fieldselector.go file stores mappings of field names to repository.PackageRevision object values https://github.com/nephio-project/nephio/issues/677 * take now-unnecessary deletionPropoesdCacheMutex back out - also create dedicated function type for PackageRevision field mappings * increase fieldselector.go coverage * Implement a Database cache in Porch (kptdev#234) * Implement a database cache in Porch. * Add postgres and db cache config etc * Add unit tests to SQL files * Added unit tests * More unit tests on SQL * Package SQL unit tests done Signed-off-by: liamfallon <liam.fallon@est.tech> * Added unit tests for PackageRevision SQL * Unit tests for resources sql table Signed-off-by: liamfallon <liam.fallon@est.tech> * Added sync and refactored around sync * Update to fix external to cache repo key linking Signed-off-by: liamfallon <liam.fallon@est.tech> * Updated keying to fix some small bugs * Sync and query filtering added * Update tests and fix embedded postgres Signed-off-by: liamfallon <liam.fallon@est.tech> * Remove time check from unit tests Signed-off-by: liamfallon <liam.fallon@est.tech> * Added unit tests for DB Repository Signed-off-by: liamfallon <liam.fallon@est.tech> * More unit tests Signed-off-by: liamfallon <liam.fallon@est.tech> * More unit tests Signed-off-by: liamfallon <liam.fallon@est.tech> * Unit tests Signed-off-by: liamfallon <liam.fallon@est.tech> * Address review comments * Remove reference to nonexistantant fake cache Signed-off-by: liamfallon <liam.fallon@est.tech> * Restore fake cache deleted in error Signed-off-by: liamfallon <liam.fallon@est.tech> * More unit test * More unit test Signed-off-by: liamfallon <liam.fallon@est.tech> * Updated push of PR to external repo to be external repo independent Signed-off-by: liamfallon <liam.fallon@est.tech> * Added more unit test Signed-off-by: liamfallon <liam.fallon@est.tech> * Fixed copyright notices Signed-off-by: liamfallon <liam.fallon@est.tech> * update query and rebase Signed-off-by: liamfallon <liam.fallon@est.tech> * Added context to error messages Signed-off-by: liamfallon <liam.fallon@est.tech> * Add mock for repository Key() function Signed-off-by: liamfallon <liam.fallon@est.tech> * Add check on git version into sync Signed-off-by: liamfallon <liam.fallon@est.tech> * Add owner references to API metadata for PackageRevision and PackageRevisionResource Signed-off-by: liamfallon <liam.fallon@est.tech> * Update metadata on SetMeta() function Signed-off-by: liamfallon <liam.fallon@est.tech> * Tasks and deployment in status now working Signed-off-by: liamfallon <liam.fallon@est.tech> * Sort query results from DB and handle latest revision Signed-off-by: liamfallon <liam.fallon@est.tech> * Update latest PR on deletes Signed-off-by: liamfallon <liam.fallon@est.tech> * Updated code to cope with Latest revision calculated in DB Signed-off-by: liamfallon <liam.fallon@est.tech> * Updated to address review comments Signed-off-by: liamfallon <liam.fallon@est.tech> * Fix typo in query Signed-off-by: liamfallon <liam.fallon@est.tech> * Fix code smells fpound by sonar Signed-off-by: liamfallon <liam.fallon@est.tech> * Update tests for text canges Signed-off-by: liamfallon <liam.fallon@est.tech> * Increase cde coverage Signed-off-by: liamfallon <liam.fallon@est.tech> * Updated with responses to review comments Signed-off-by: liamfallon <liam.fallon@est.tech> * Updated with responses to review comments Signed-off-by: liamfallon <liam.fallon@est.tech> * Remove load balancer type from function runner deployment Signed-off-by: liamfallon <liam.fallon@est.tech> --------- Signed-off-by: liamfallon <liam.fallon@est.tech> Co-authored-by: Fiachra Corcoran <fiachra.corcoran@est.tech> * Issue kptdev#677 - refactor package revision filtering for list flow - first cut at making it simpler to add new selectable fields - reworked the 2 levels of PackageRevision filtering to use Kubernetes SelectionPredicates, so we can more easily feed in a set of field names and mappings to their counterparts in the data structures https://github.com/nephio-project/nephio/issues/677 * refactor ListPackageRevisionFilter parse method * rework selectable-field refactoring to pull everything together in fieldselector.go - results in a performance hit for list flow - work in progress https://github.com/nephio-project/nephio/issues/677 * Issue kptdev#677 - final refactor to package revision filtering - selectable fields now all bound to elements of internal repository.PackageRevision type - this allows all filtering to be done at level of the ListPackageRevisionFilter's matching - upper-level packageRevisionFilter now only extends ListPackageRevisionFilter upward into the registry/porch package - list of selectable field names added to API-level PackageRevision type in v1alpha1 types.go - new fieldselector.go file stores mappings of field names to repository.PackageRevision object values https://github.com/nephio-project/nephio/issues/677 * increase fieldselector.go coverage * resolve rebase conflicts from DB cache introduction - and fold in changes to field-selector mechanism * test fix - and attempt to keep 'make test' from messing up global Git config * fix tests again and try doing Git config change on local level * fix Git config addition in go-test Makefile - lock the config addition behind a check so it doesn't interfere with developers' local Git configuration * bump coverage: fieldselector.go, repository.go * bump coverage: repository/fieldselector.go * fix test * Issue kptdev#677 - fix incidental break in label-selector filtering - this involves moving label selecting to ListPackageRevisionFilter's new SelectionPredicate mechanism - completing bundling of label/field selector handling into ListPackageRevisionFilter - and allows removal of the separate labels.Selector parameter from the list-package-revisions and watch-package-revisions flows - also add a new e2e test to cover label-selector filtering https://github.com/nephio-project/nephio/issues/677 * formatting nitpicks in launch.json to retrigger E2E tests * comment nitpick in Makefile to retrigger E2E tests * Issue kptdev#677 - fix filtering on special Porch-managed latest-revision label - with refactoring to SelectionPredicate filtering, list flow only adds this label to a PackageRevision after the final check for a match is performed - added bit to filter handling (ParseAttrFunc method) to add the label in specially if PackageRevision being parsed is the latest revision - also special-case coverage in e2e test * comment nitpick in launch.json to retrigger E2E tests * comment nitpick to retrigger e2e tests * minor e2e test refactor in attempt to de-flake TestProposeDeleteAndUndo against DB cache * compile/test fixes after rebase * Issue kptdev#677 - cut out SelectionPredicate-based filtering - revert everything to PackageRevisionKey filtering - removing redundant re-filter above DB Cache's SQL filtering, for maximum efficiency - results in no support for not-equals (!=) field selectors - so remove them from scope of testing https://github.com/nephio-project/nephio/issues/677 * coverage improvements - also move field-selector/filter-field mappings into porch package's fieldselector.go - since it's now associated purely with that level * coverage improvements, minor relocation refactors * fix merge detail that slipped through and broke compile * lint fix * review comments - reworked PackageRevisionSelectableFields to an array of constants * move PackageRevisionLifecycle util methods as per review comment * delete unnecessary .bak file --------- Signed-off-by: liamfallon <liam.fallon@est.tech> Co-authored-by: Catalin Stratulat <159934629+Catalin-Stratulat-Ericsson@users.noreply.github.com> Co-authored-by: Liam Fallon <35595825+liamfallon@users.noreply.github.com> Co-authored-by: Fiachra Corcoran <fiachra.corcoran@est.tech>
1 parent 9f5af4f commit 9fee28c

31 files changed

Lines changed: 1317 additions & 279 deletions

.mockery.yaml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,3 +44,4 @@ packages:
4444
github.com/nephio-project/porch/pkg/engine:
4545
interfaces:
4646
CaDEngine: {}
47+
WatcherManager: {}

.mockery.yaml.bak

Lines changed: 0 additions & 62 deletions
This file was deleted.

.vscode/launch.json

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5,15 +5,17 @@
55
"version": "0.2.0",
66

77
"configurations": [
8-
// A configuration for running the porch server through vscode for the use of debugging.
8+
// A configuration for running the porch server through VS Code for the use of debugging.
99
// Assumes a cluster is set up alongside Git (./scripts/setup-dev-env.sh) and porch components (make ../porch/run-in-kind-no-server) are pre-configured
10-
// With the configuration above one can use Vscode (Run & Debug) and launch server.
11-
// This launches the porch server through vscode outside the cluster and the logs can be viewed in the debug console.
10+
// With the configuration above one can use VS Code (Run & Debug) and launch server.
11+
// This launches the porch server through VS Code outside the cluster and the logs can be viewed in the debug console.
1212
// Breakpoints can be added throughout the porch server code to debug.
1313

1414
// CGO_ENABLED & buildFlags
15-
// Enables race condition checking (Slows down porch-server processing) NOTE: The Go race detector uses C code under the hood (via cgo),
16-
// and to build it, your system needs a C toolchain—i.e., a compiler like gcc and other basic build tools.
15+
// Enables race condition checking (Slows down porch-server processing)
16+
//
17+
// NOTE: The Go race detector uses C code under the hood (via cgo),
18+
// and to build it, your system needs a C toolchain - i.e., a compiler like gcc and other basic build tools.
1719
// on a linux distro e.g. ubuntu these can be installed with (sudo apt install build-essential),
1820
// this will install the (gcc = GNU C Compiler, g++ = GNU C++ Compiler, make, libc6-dev etc...)
1921
{
@@ -77,9 +79,9 @@
7779
"ENABLE_PACKAGEVARIANTSETS": "true"
7880
}
7981
},
80-
// A configuration for running a porchctl command using the vscode debugger.
82+
// A configuration for running a porchctl command using the VS Code debugger.
8183
// Assumes a cluster is set up alongside Git (scripts/setup-dev-env.sh) and porch components (make run-in-kind) in the /porch directory are pre-configured
82-
// This allows for the running of porchctl commands through vscode outside the cluster and the logs can be viewed in the debug console.
84+
// This allows for the running of porchctl commands through VS Code outside the cluster and the logs can be viewed in the debug console.
8385
// Breakpoints can be added throughout the porch server code to debug.
8486
{
8587
"name": "Run Porchctl command",

Makefile

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -282,7 +282,7 @@ run-in-kind-no-controller: load-images-to-kind deployment-config-no-controller d
282282
run-in-kind-db-cache-no-controller: IMAGE_REPO=porch-kind
283283
run-in-kind-db-cache-no-controller: IMAGE_TAG=test
284284
run-in-kind-db-cache-no-controller: SKIP_CONTROLLER_BUILD=true
285-
run-in-kind-db-cache-no-controller: load-images-to-kind deployment-config-db-cache deployment-config-no-controller deploy-current-config ## Build and deploy porch without the controllers into a kind cluster
285+
run-in-kind-db-cache-no-controller: load-images-to-kind deployment-config-db-cache deployment-config-no-controller deploy-current-config ## Build and deploy porch without the controllers into a kind cluster with postgres backend
286286

287287
.PHONY: destroy
288288
destroy: ## Deletes all porch resources installed by the last run-in-kind-* command

api/porch/v1alpha1/types.go

Lines changed: 22 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -31,17 +31,35 @@ type PackageRevision struct {
3131
Status PackageRevisionStatus `json:"status,omitempty"`
3232
}
3333

34-
func (pr *PackageRevision) IsPublished() bool {
35-
return LifecycleIsPublished(pr.Spec.Lifecycle)
36-
}
37-
3834
// Key and value of the latest package revision label:
3935

4036
const (
4137
LatestPackageRevisionKey = "kpt.dev/latest-revision"
4238
LatestPackageRevisionValue = "true"
4339
)
4440

41+
type PkgRevFieldSelector string
42+
43+
const (
44+
PkgRevSelectorName PkgRevFieldSelector = "metadata.name"
45+
PkgRevSelectorNamespace PkgRevFieldSelector = "metadata.namespace"
46+
PkgRevSelectorRevision PkgRevFieldSelector = "spec.revision"
47+
PkgRevSelectorPackageName PkgRevFieldSelector = "spec.packageName"
48+
PkgRevSelectorRepository PkgRevFieldSelector = "spec.repository"
49+
PkgRevSelectorWorkspaceName PkgRevFieldSelector = "spec.workspaceName"
50+
PkgRevSelectorLifecycle PkgRevFieldSelector = "spec.lifecycle"
51+
)
52+
53+
var PackageRevisionSelectableFields = []PkgRevFieldSelector{
54+
PkgRevSelectorName,
55+
PkgRevSelectorNamespace,
56+
PkgRevSelectorRevision,
57+
PkgRevSelectorPackageName,
58+
PkgRevSelectorRepository,
59+
PkgRevSelectorWorkspaceName,
60+
PkgRevSelectorLifecycle,
61+
}
62+
4563
// PackageRevisionList
4664
// +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object
4765
type PackageRevisionList struct {

api/porch/v1alpha1/util.go

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,10 +16,26 @@ package v1alpha1
1616

1717
import "slices"
1818

19+
func (pr *PackageRevision) IsPublished() bool {
20+
return LifecycleIsPublished(pr.Spec.Lifecycle)
21+
}
22+
1923
func LifecycleIsPublished(lifecycle PackageRevisionLifecycle) bool {
2024
return lifecycle == PackageRevisionLifecyclePublished || lifecycle == PackageRevisionLifecycleDeletionProposed
2125
}
2226

27+
func (l *PackageRevisionLifecycle) IsValid() bool {
28+
switch *l {
29+
case PackageRevisionLifecycleDraft,
30+
PackageRevisionLifecycleProposed,
31+
PackageRevisionLifecyclePublished,
32+
PackageRevisionLifecycleDeletionProposed:
33+
return true
34+
default:
35+
return false
36+
}
37+
}
38+
2339
// Check ReadinessGates checks if the package has met all readiness gates
2440
func PackageRevisionIsReady(readinessGates []ReadinessGate, conditions []Condition) bool {
2541
// Index our conditions

pkg/cache/crcache/packagerevision.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,3 +101,7 @@ func (c *cachedPackageRevision) SetMeta(ctx context.Context, pkgRevMeta metav1.O
101101

102102
return c.PackageRevision.SetMeta(ctx, storedMeta)
103103
}
104+
105+
func (c *cachedPackageRevision) IsLatestRevision() bool {
106+
return c.isLatestRevision
107+
}

pkg/cache/crcache/repository_test.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,7 @@ func TestCachedRepoRefresh(t *testing.T) {
8484
metadataStore: mockMeta,
8585
isLatestRevision: false,
8686
}
87+
assert.False(t, cr.cachedPackageRevisions[prKey].IsLatestRevision())
8788

8889
err := cr.Refresh(context.TODO())
8990
assert.True(t, err == nil)

pkg/cache/crcache/util.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -70,16 +70,16 @@ func toPackageRevisionSlice(
7070
}
7171
}
7272
sort.Slice(result, func(i, j int) bool {
73-
ki, kl := result[i].Key(), result[j].Key()
74-
switch res := strings.Compare(ki.PkgKey.Package, kl.PkgKey.Package); {
73+
ki, kj := result[i].Key(), result[j].Key()
74+
switch res := strings.Compare(ki.PkgKey.Package, kj.PkgKey.Package); {
7575
case res < 0:
7676
return true
7777
case res > 0:
7878
return false
7979
default:
8080
// Equal. Compare next element
8181
}
82-
res := ki.Revision - kl.Revision
82+
res := ki.Revision - kj.Revision
8383
if res != 0 {
8484
return res < 0
8585
}

pkg/cache/dbcache/dbpackagerevision.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -305,6 +305,10 @@ func (pr *dbPackageRevision) SetMeta(ctx context.Context, meta metav1.ObjectMeta
305305
return pkgRevUpdateDB(ctx, pr, false)
306306
}
307307

308+
func (pr *dbPackageRevision) IsLatestRevision() bool {
309+
return pr.latest
310+
}
311+
308312
func (pr *dbPackageRevision) GetKptfile(ctx context.Context) (kptfile.KptFile, error) {
309313
_, span := tracer.Start(ctx, "dbPackageRevision::GetKptfile", trace.WithAttributes())
310314
defer span.End()

0 commit comments

Comments
 (0)