Skip to content

Commit ad97b33

Browse files
authored
Send an async update notification to the next latest revision if the existing latest package revision is deleted (#380)
* Send an update notification after the latest package revision is deleted * Send async notification in crcache when latest pkg revision is deleted * Add tracing to crcache sendLatestPkgUpdateNotification
1 parent 0430e61 commit ad97b33

4 files changed

Lines changed: 265 additions & 0 deletions

File tree

pkg/cache/crcache/repository.go

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -405,6 +405,9 @@ func (r *cachedRepository) DeletePackageRevision(ctx context.Context, prToDelete
405405
}
406406
klog.Infof("PackageRevision %s deleted for real since no finalizers", prToDelete.KubeObjectName())
407407

408+
// Check if this is the latest revision before deletion
409+
isLatest := prToDelete.(*cachedPackageRevision).IsLatestRevision()
410+
408411
// Unwrap
409412
unwrapped := prToDelete.(*cachedPackageRevision).PackageRevision
410413
if err := r.repo.DeletePackageRevision(ctx, unwrapped); err != nil {
@@ -421,6 +424,12 @@ func (r *cachedRepository) DeletePackageRevision(ctx context.Context, prToDelete
421424
identifyLatestRevisions(ctx, r.cachedPackageRevisions)
422425
}
423426

427+
// Check if we need to send async notification for new latest revision
428+
if isLatest {
429+
klog.Infof("crcache: %+v: latest PackageRevision deleted. Sending notification.", prToDelete.Key().PkgKey)
430+
go r.sendLatestPkgUpdateNotification(prToDelete.Key().PkgKey)
431+
}
432+
424433
r.mutex.Unlock()
425434

426435
if _, err := r.metadataStore.Delete(ctx, namespacedName, true); err != nil {
@@ -436,6 +445,31 @@ func (r *cachedRepository) DeletePackageRevision(ctx context.Context, prToDelete
436445
return nil
437446
}
438447

448+
// sendLatestPkgUpdateNotification sends async notification when a new latest package revision is identified
449+
func (r *cachedRepository) sendLatestPkgUpdateNotification(pkgKey repository.PackageKey) {
450+
ctx, cancel := context.WithTimeout(context.Background(), 3*time.Minute)
451+
defer cancel()
452+
_, span := tracer.Start(ctx, "cachedRepository::sendLatestPkgUpdateNotification", trace.WithAttributes())
453+
defer span.End()
454+
// Find the new latest revision for this package
455+
r.mutex.RLock()
456+
var newLatest repository.PackageRevision
457+
for _, pr := range r.cachedPackageRevisions {
458+
if pr.Key().PkgKey == pkgKey && pr.IsLatestRevision() {
459+
newLatest = pr
460+
break
461+
}
462+
}
463+
r.mutex.RUnlock()
464+
465+
if newLatest != nil {
466+
sent := r.repoPRChangeNotifier.NotifyPackageRevisionChange(watch.Modified, newLatest)
467+
klog.Infof("crcache: async notification sent %d for new latest PackageRevision %s/%s", sent, newLatest.KubeObjectNamespace(), newLatest.KubeObjectName())
468+
} else {
469+
klog.Infof("crcache: no new latest revision found for package %s after deletion. Notification not sent.", pkgKey.Package)
470+
}
471+
}
472+
439473
func (r *cachedRepository) ListPackages(ctx context.Context, filter repository.ListPackageFilter) ([]repository.Package, error) {
440474
packages, err := r.getPackages(ctx, filter, false)
441475
if err != nil {

pkg/cache/crcache/repository_test.go

Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ import (
3434
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
3535
"k8s.io/apimachinery/pkg/runtime"
3636
"k8s.io/apimachinery/pkg/types"
37+
"k8s.io/apimachinery/pkg/watch"
3738
)
3839

3940
func TestCachedRepoRefresh(t *testing.T) {
@@ -250,3 +251,111 @@ func TestHandleRunOnceAt(t *testing.T) {
250251
assert.NotNil(t, status, "Expected repository status to be updated")
251252
assert.Contains(t, []string{"Ready", "Error", "Reconciling"}, status.Conditions[0].Reason)
252253
}
254+
255+
func TestCRDeleteLatestRevision(t *testing.T) {
256+
mockRepo := mockrepo.NewMockRepository(t)
257+
mockMeta := mockmeta.NewMockMetadataStore(t)
258+
mockNotifier := mockcachetypes.NewMockRepoPRChangeNotifier(t)
259+
260+
repoSpec := configapi.Repository{
261+
ObjectMeta: metav1.ObjectMeta{
262+
Name: repoName,
263+
Namespace: namespace,
264+
},
265+
}
266+
267+
repoKey := repository.RepositoryKey{
268+
Namespace: namespace,
269+
Name: repoName,
270+
}
271+
272+
// Create two package revisions for the same package
273+
prKey1 := repository.PackageRevisionKey{
274+
PkgKey: repository.PackageKey{
275+
RepoKey: repoKey,
276+
Path: "",
277+
Package: "test-package",
278+
},
279+
WorkspaceName: "v1",
280+
Revision: 1,
281+
}
282+
283+
prKey2 := repository.PackageRevisionKey{
284+
PkgKey: repository.PackageKey{
285+
RepoKey: repoKey,
286+
Path: "",
287+
Package: "test-package",
288+
},
289+
WorkspaceName: "v2",
290+
Revision: 2,
291+
}
292+
293+
fpr1 := &fake.FakePackageRevision{
294+
PrKey: prKey1,
295+
PackageLifecycle: porchapi.PackageRevisionLifecyclePublished,
296+
}
297+
298+
fpr2 := &fake.FakePackageRevision{
299+
PrKey: prKey2,
300+
PackageLifecycle: porchapi.PackageRevisionLifecyclePublished,
301+
}
302+
303+
// Setup mocks
304+
mockRepo.On("Key").Return(repoKey).Maybe()
305+
mockRepo.EXPECT().DeletePackageRevision(mock.Anything, mock.Anything).Return(nil).Maybe()
306+
mockMeta.EXPECT().Delete(mock.Anything, mock.Anything, mock.Anything).Return(metav1.ObjectMeta{}, nil).Maybe()
307+
308+
// Expect exactly 2 notifications: one for deletion, one for async notification
309+
mockNotifier.EXPECT().NotifyPackageRevisionChange(watch.Deleted, mock.Anything).Return(1).Once()
310+
mockNotifier.EXPECT().NotifyPackageRevisionChange(watch.Modified, mock.Anything).Return(1).Once()
311+
312+
// Create repository manually to avoid sync manager
313+
cr := &cachedRepository{
314+
key: repoKey,
315+
repoSpec: &repoSpec,
316+
repo: mockRepo,
317+
metadataStore: mockMeta,
318+
repoPRChangeNotifier: mockNotifier,
319+
cachedPackages: make(map[repository.PackageKey]*cachedPackage), // Initialize to enable deletion
320+
}
321+
322+
// Initialize cache with two revisions
323+
cr.cachedPackageRevisions = make(map[repository.PackageRevisionKey]*cachedPackageRevision)
324+
cr.cachedPackageRevisions[prKey1] = &cachedPackageRevision{
325+
PackageRevision: fpr1,
326+
metadataStore: mockMeta,
327+
isLatestRevision: false,
328+
}
329+
cr.cachedPackageRevisions[prKey2] = &cachedPackageRevision{
330+
PackageRevision: fpr2,
331+
metadataStore: mockMeta,
332+
isLatestRevision: false,
333+
}
334+
335+
// Identify latest revisions (revision 2 should be latest)
336+
identifyLatestRevisions(context.TODO(), cr.cachedPackageRevisions)
337+
338+
// Verify revision 2 is marked as latest before deletion
339+
assert.True(t, cr.cachedPackageRevisions[prKey2].IsLatestRevision())
340+
assert.False(t, cr.cachedPackageRevisions[prKey1].IsLatestRevision())
341+
342+
// Delete the latest revision (revision 2)
343+
err := cr.DeletePackageRevision(context.TODO(), cr.cachedPackageRevisions[prKey2])
344+
assert.NoError(t, err)
345+
346+
// Give time for async notification to complete
347+
time.Sleep(100 * time.Millisecond)
348+
349+
// Now delete pkg1 - should only send 1 notification (deletion) since no more revisions remain
350+
mockNotifier.EXPECT().NotifyPackageRevisionChange(watch.Deleted, mock.Anything).Return(1).Once()
351+
// No async notification expected since no revisions remain
352+
353+
err = cr.DeletePackageRevision(context.TODO(), cr.cachedPackageRevisions[prKey1])
354+
assert.NoError(t, err)
355+
356+
// Give time for any potential async notification
357+
time.Sleep(100 * time.Millisecond)
358+
359+
// Verify all expected mock calls were made
360+
mockNotifier.AssertExpectations(t)
361+
}

pkg/cache/dbcache/dbpackage.go

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import (
2525
"go.opentelemetry.io/otel/trace"
2626
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2727
"k8s.io/apimachinery/pkg/types"
28+
"k8s.io/apimachinery/pkg/watch"
2829
"k8s.io/klog/v2"
2930
)
3031

@@ -134,9 +135,35 @@ func (p *dbPackage) DeletePackageRevision(ctx context.Context, old repository.Pa
134135
return pkgDeleteFromDB(ctx, p.Key())
135136
}
136137

138+
if dbPR.IsLatestRevision() {
139+
klog.Infof("dbPackage %+v: latest PackageRevision deleted. Sending notification.", p.Key())
140+
go p.sendLatestPkgUpdateNotification()
141+
}
142+
137143
return nil
138144
}
139145

146+
// sendLatestPkgUpdateNotification sends async notification when a new latest package revision is identified
147+
func (p *dbPackage) sendLatestPkgUpdateNotification() {
148+
ctx, cancel := context.WithTimeout(context.Background(), 3*time.Minute)
149+
defer cancel()
150+
_, span := tracer.Start(ctx, "dbPackage::sendLatestPkgUpdateNotification", trace.WithAttributes())
151+
defer span.End()
152+
153+
latestRevision, err := pkgRevReadLatestPRFromDB(ctx, p.Key())
154+
if err != nil {
155+
klog.Error(err)
156+
return
157+
} else if latestRevision == nil {
158+
klog.Infof("dbPackage %+v: no new latest PackageRevision found. Notification not sent.", p.Key())
159+
return
160+
}
161+
162+
sent := p.repo.repoPRChangeNotifier.NotifyPackageRevisionChange(watch.Modified, latestRevision)
163+
klog.Infof("dbcache: sent %d for latest PackageRevision %s/%s", sent, latestRevision.KubeObjectNamespace(), latestRevision.KubeObjectName())
164+
165+
}
166+
140167
func (p *dbPackage) GetLatestRevision(ctx context.Context) int {
141168
_, span := tracer.Start(ctx, "dbPackage:GetLatestRevision", trace.WithAttributes())
142169
defer span.End()

pkg/cache/dbcache/dbpackagerevision_test.go

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ package dbcache
1717
import (
1818
"context"
1919
"errors"
20+
"time"
2021

2122
porchapi "github.com/nephio-project/porch/api/porch/v1alpha1"
2223
configapi "github.com/nephio-project/porch/api/porchconfig/v1alpha1"
@@ -310,3 +311,97 @@ type fakeRepoWithDeleteError struct {
310311
func (r *fakeRepoWithDeleteError) DeletePackageRevision(context.Context, repository.PackageRevision) error {
311312
return errors.New("package not found")
312313
}
314+
315+
func (t *DbTestSuite) TestDBDeleteLatestRevision() {
316+
// Test that the async notification logic is triggered
317+
ctx := t.Context()
318+
319+
mockCache := mockcachetypes.NewMockCache(t.T())
320+
cachetypes.CacheInstance = mockCache
321+
322+
dbRepo := t.createTestRepo("test-ns", "test-repo")
323+
mockCache.EXPECT().GetRepository(mock.Anything).Return(dbRepo)
324+
dbPkg := t.createTestPkg(dbRepo.Key(), "test-package")
325+
dbPkg.repo = dbRepo
326+
327+
// Create and write first package revision to database
328+
dbPR1 := dbPackageRevision{
329+
repo: dbRepo,
330+
pkgRevKey: repository.PackageRevisionKey{
331+
PkgKey: dbPkg.Key(),
332+
WorkspaceName: "workspace-1",
333+
Revision: 1,
334+
},
335+
meta: metav1.ObjectMeta{},
336+
spec: &porchapi.PackageRevisionSpec{},
337+
updated: time.Now().UTC(),
338+
updatedBy: "testuser",
339+
lifecycle: porchapi.PackageRevisionLifecyclePublished,
340+
latest: false,
341+
resources: map[string]string{},
342+
}
343+
344+
// Create and write main package revision to make sure len(prSlice) > 0
345+
dbPRMain := dbPackageRevision{
346+
repo: dbRepo,
347+
pkgRevKey: repository.PackageRevisionKey{
348+
PkgKey: dbPkg.Key(),
349+
WorkspaceName: "main",
350+
Revision: -1,
351+
},
352+
meta: metav1.ObjectMeta{},
353+
spec: &porchapi.PackageRevisionSpec{},
354+
updated: time.Now().UTC(),
355+
updatedBy: "testuser",
356+
lifecycle: porchapi.PackageRevisionLifecyclePublished,
357+
resources: map[string]string{},
358+
}
359+
360+
err := pkgRevWriteToDB(ctx, &dbPR1)
361+
t.Require().NoError(err)
362+
363+
err = pkgRevWriteToDB(ctx, &dbPRMain)
364+
t.Require().NoError(err)
365+
366+
// Create and write second package revision to database (this will be latest)
367+
dbPR2 := dbPackageRevision{
368+
repo: dbRepo,
369+
pkgRevKey: repository.PackageRevisionKey{
370+
PkgKey: dbPkg.Key(),
371+
WorkspaceName: "workspace-2",
372+
Revision: 2,
373+
},
374+
meta: metav1.ObjectMeta{},
375+
spec: &porchapi.PackageRevisionSpec{},
376+
updated: time.Now().UTC(),
377+
updatedBy: "testuser",
378+
lifecycle: porchapi.PackageRevisionLifecyclePublished,
379+
latest: true,
380+
resources: map[string]string{},
381+
}
382+
383+
err = pkgRevWriteToDB(ctx, &dbPR2)
384+
t.Require().NoError(err)
385+
386+
// Delete the latest revision - should trigger async notification
387+
err = dbPkg.DeletePackageRevision(ctx, &dbPR2, false)
388+
t.Require().NoError(err)
389+
390+
// wait for async call to finish
391+
time.Sleep(100 * time.Millisecond)
392+
393+
dbPR1.latest = true
394+
err = dbPkg.DeletePackageRevision(ctx, &dbPR1, false)
395+
t.Require().NoError(err)
396+
397+
// wait for async call to finish
398+
time.Sleep(100 * time.Millisecond)
399+
400+
// After deletion, dbPRMain should remain in database so len(prSlice) > 0
401+
err = dbPkg.DeletePackageRevision(ctx, &dbPRMain, false)
402+
t.Require().NoError(err)
403+
404+
// Clean up
405+
err = repoDeleteFromDB(ctx, dbRepo.Key())
406+
t.Require().NoError(err)
407+
}

0 commit comments

Comments
 (0)