Only enqueue installed packages eligible for upgrade on package updates (carvel-dev#136)

Author: ewrenn8

cmd/controller/handlers/installed_pkg_version_handler.go

@@ -5,8 +5,12 @@ package handlers
 
 import (
 	"github.com/go-logr/logr"
+	ipkgv1alpha1 "github.com/vmware-tanzu/carvel-kapp-controller/pkg/apis/installpackage/v1alpha1"
+	pkgv1alpha1 "github.com/vmware-tanzu/carvel-kapp-controller/pkg/apis/package/v1alpha1"
 	kcclient "github.com/vmware-tanzu/carvel-kapp-controller/pkg/client/clientset/versioned"
+	versions "github.com/vmware-tanzu/carvel-vendir/pkg/vendir/versions/v1alpha1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/client-go/util/workqueue"
 	"sigs.k8s.io/controller-runtime/pkg/event"
@@ -31,48 +35,69 @@ func NewInstalledPkgVersionHandler(c kcclient.Interface, log logr.Logger) *Insta
 
 func (ipvh *InstalledPkgVersionHandler) Create(evt event.CreateEvent, q workqueue.RateLimitingInterface) {
 	ipvh.log.Info("enqueueing installedPkgList")
-	err := ipvh.enqueueAllPackages(q)
+	err := ipvh.enqueueEligibleInstalledPackages(q, evt.Object)
 	if err != nil {
 		ipvh.log.Error(err, "enqueueing all installed pakcages")
 	}
 }
 
 func (ipvh *InstalledPkgVersionHandler) Update(evt event.UpdateEvent, q workqueue.RateLimitingInterface) {
 	ipvh.log.Info("enqueueing installedPkgList")
-	err := ipvh.enqueueAllPackages(q)
+	err := ipvh.enqueueEligibleInstalledPackages(q, evt.ObjectNew)
 	if err != nil {
 		ipvh.log.Error(err, "enqueueing all installed pakcages")
 	}
 }
 
 func (ipvh *InstalledPkgVersionHandler) Delete(evt event.DeleteEvent, q workqueue.RateLimitingInterface) {
 	ipvh.log.Info("enqueueing installedPkgList")
-	err := ipvh.enqueueAllPackages(q)
+	err := ipvh.enqueueEligibleInstalledPackages(q, evt.Object)
 	if err != nil {
 		ipvh.log.Error(err, "enqueueing all installed pakcages")
 	}
 }
 
 func (ipvh *InstalledPkgVersionHandler) Generic(evt event.GenericEvent, q workqueue.RateLimitingInterface) {
 	ipvh.log.Info("enqueueing installedPkgList")
-	err := ipvh.enqueueAllPackages(q)
+	err := ipvh.enqueueEligibleInstalledPackages(q, evt.Object)
 	if err != nil {
 		ipvh.log.Error(err, "enqueueing all installed pakcages")
 	}
 }
 
-func (ipvh *InstalledPkgVersionHandler) enqueueAllPackages(q workqueue.RateLimitingInterface) error {
+func (ipvh *InstalledPkgVersionHandler) enqueueEligibleInstalledPackages(q workqueue.RateLimitingInterface, obj runtime.Object) error {
+	pkg := obj.(*pkgv1alpha1.Package)
 	installedPkgList, err := ipvh.client.InstallV1alpha1().InstalledPackages("").List(metav1.ListOptions{})
 	if err != nil {
 		return err
 	}
 
 	for _, ip := range installedPkgList.Items {
-		q.Add(reconcile.Request{NamespacedName: types.NamespacedName{
-			Name:      ip.Name,
-			Namespace: ip.Namespace,
-		}})
+		if ip.Spec.PkgRef.PublicName == pkg.Spec.PublicName && ipvh.isEligibleForVersionUpgrade(pkg.Spec.Version, ip) {
+			q.Add(reconcile.Request{NamespacedName: types.NamespacedName{
+				Name:      ip.Name,
+				Namespace: ip.Namespace,
+			}})
+		}
 	}
 
 	return nil
 }
+
+func (ipvh *InstalledPkgVersionHandler) isEligibleForVersionUpgrade(version string, installedPkg ipkgv1alpha1.InstalledPackage) bool {
+	semverConfig := installedPkg.Spec.PkgRef.VersionSelection
+	if installedPkg.Spec.PkgRef.Version != "" {
+		semverConfig = &versions.VersionSelectionSemver{
+			Constraints: installedPkg.Spec.PkgRef.Version,
+			// Prereleases must be non nil to be included
+			Prereleases: &versions.VersionSelectionSemverPrereleases{},
+		}
+	}
+
+	selectedVersion, err := versions.HighestConstrainedVersion([]string{version}, versions.VersionSelection{Semver: semverConfig})
+	if selectedVersion == "" || err != nil {
+		return false
+	}
+
+	return true
+}

cmd/controller/handlers/installed_pkg_version_handler_test.go

@@ -0,0 +1,94 @@
+// Copyright 2021 VMware, Inc.
+// SPDX-License-Identifier: Apache-2.0
+
+package handlers_test
+
+import (
+	"reflect"
+	"testing"
+
+	"github.com/go-logr/logr"
+
+	instpackagev1 "github.com/vmware-tanzu/carvel-kapp-controller/pkg/apis/installpackage/v1alpha1"
+	packagev1 "github.com/vmware-tanzu/carvel-kapp-controller/pkg/apis/package/v1alpha1"
+	"github.com/vmware-tanzu/carvel-vendir/pkg/vendir/versions/v1alpha1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/types"
+
+	"github.com/vmware-tanzu/carvel-kapp-controller/cmd/controller/handlers"
+	"github.com/vmware-tanzu/carvel-kapp-controller/pkg/client/clientset/versioned/fake"
+	"k8s.io/client-go/util/workqueue"
+	"sigs.k8s.io/controller-runtime/pkg/event"
+	"sigs.k8s.io/controller-runtime/pkg/reconcile"
+)
+
+func TestOnlyEligiblePackagesAreEnqueued(t *testing.T) {
+	q := workqueue.NewRateLimitingQueue(workqueue.DefaultControllerRateLimiter())
+
+	eligibleInstalledPkg := instpackagev1.InstalledPackage{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: "expected-pkg",
+		},
+		Spec: instpackagev1.InstalledPackageSpec{
+			PkgRef: &instpackagev1.PackageRef{
+				PublicName: "expec-pkg",
+				VersionSelection: &v1alpha1.VersionSelectionSemver{
+					Constraints: ">=1.0.0",
+				},
+			},
+		},
+	}
+
+	ineligibleInstalledPkg := instpackagev1.InstalledPackage{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: "expected-pkg-ineligible",
+		},
+		Spec: instpackagev1.InstalledPackageSpec{
+			PkgRef: &instpackagev1.PackageRef{
+				PublicName: "expec-pkg",
+				VersionSelection: &v1alpha1.VersionSelectionSemver{
+					Constraints: "<1.0.0",
+				},
+			},
+		},
+	}
+
+	// Load installed package into fake client
+	kappcs := fake.NewSimpleClientset(&eligibleInstalledPkg, &ineligibleInstalledPkg)
+	ipvh := handlers.NewInstalledPkgVersionHandler(kappcs, &EmptyLog{})
+
+	event := event.GenericEvent{
+		Object: &packagev1.Package{
+			Spec: packagev1.PackageSpec{
+				PublicName: "expec-pkg",
+				Version:    "1.5.0",
+			},
+		},
+	}
+
+	ipvh.Generic(event, q)
+
+	if q.Len() != 1 {
+		t.Fatalf("Expected queue to have length of 1, got %d", q.Len())
+	}
+
+	expectedRequest := reconcile.Request{NamespacedName: types.NamespacedName{Namespace: "", Name: "expected-pkg"}}
+	if obj, _ := q.Get(); !reflect.DeepEqual(obj, expectedRequest) {
+		t.Fatalf("Expected queue to contain the installed package eligible for upgrade, but contained:\n\n%#v\n", obj)
+	}
+
+}
+
+type EmptyLog struct{}
+
+func (l *EmptyLog) Info(msg string, keysAndValues ...interface{}) {}
+
+func (l *EmptyLog) Enabled() bool { return false }
+
+func (l *EmptyLog) Error(err error, msg string, keysAndValues ...interface{}) {}
+
+func (l *EmptyLog) V(level int) logr.InfoLogger { return l }
+
+func (l *EmptyLog) WithValues(keysAndValues ...interface{}) logr.Logger { return l }
+
+func (l *EmptyLog) WithName(name string) logr.Logger { return l }

hack/test.sh

@@ -8,6 +8,6 @@ fi
 
 set -u
 
-go test ./pkg/... -test.v $@
+go test ./pkg/... ./cmd/... -test.v $@
 
 echo UNIT SUCCESS