Skip to content

Commit

Permalink
Filter out bundle versions lower than installed
Browse files Browse the repository at this point in the history 
Adds an annotation to the App created for each extension indicating the bundle version, which can then be used on future reconciles to filter out older versions. This can be overridden by setting the UpgradeConstraintPolicy to 'Ignore'.

Co-authored-by: Varsha Prasad Narsing <varshaprasad96@gmail.com>
Signed-off-by: dtfranz <dfranz@redhat.com>
  • Loading branch information
@dtfranz @varshaprasad96
dtfranz and varshaprasad96 committed Apr 8, 2024
1 parent 8127270 commit 92ad69a
Show file tree
Hide file tree
Showing 7 changed files with 995 additions and 59 deletions.
35 changes: 0 additions & 35 deletions cmd/manager/main.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ package main

import (
"flag"
"fmt"
"net/http"
"os"
"time"
Expand All @@ -28,13 +27,10 @@ import (
rukpakv1alpha2 "github.com/operator-framework/rukpak/api/v1alpha2"
"github.com/spf13/pflag"
"go.uber.org/zap/zapcore"
"k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/runtime"
utilruntime "k8s.io/apimachinery/pkg/util/runtime"
"k8s.io/client-go/discovery"
clientgoscheme "k8s.io/client-go/kubernetes/scheme"
_ "k8s.io/client-go/plugin/pkg/client/auth"
"k8s.io/client-go/rest"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/healthz"
"sigs.k8s.io/controller-runtime/pkg/log/zap"
Expand Down Expand Up @@ -131,16 +127,9 @@ func main() {
os.Exit(1)
}

hasKappApis, err := hasKappApis(mgr.GetConfig())
if err != nil {
setupLog.Error(err, "unable to evaluate if App needs to be created")
os.Exit(1)
}

if err = (&controllers.ExtensionReconciler{
Client: cl,
BundleProvider: catalogClient,
HasKappApis: hasKappApis,
}).SetupWithManager(mgr); err != nil {
setupLog.Error(err, "unable to create controller", "controller", "Extension")
os.Exit(1)
Expand All @@ -162,27 +151,3 @@ func main() {
os.Exit(1)
}
}

// hasKappApis checks whether the cluster has Kapp APIs installed in the cluster.
// This does not guarantee that the controller is present to reconcile the App CRs.
func hasKappApis(config *rest.Config) (bool, error) {
discoveryClient, err := discovery.NewDiscoveryClientForConfig(config)
if err != nil {
return false, fmt.Errorf("creating discovery client: %v", err)
}
apiResourceList, err := discoveryClient.ServerResourcesForGroupVersion(carvelv1alpha1.SchemeGroupVersion.String())
if err != nil && !errors.IsNotFound(err) {
return false, fmt.Errorf("listing resource APIs: %v", err)
}

if apiResourceList == nil {
return false, nil
}

for _, resource := range apiResourceList.APIResources {
if resource.Kind == "App" {
return true, nil
}
}
return false, nil
}
13 changes: 13 additions & 0 deletions internal/catalogmetadata/filter/bundle_predicates.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,19 @@ func InMastermindsSemverRange(semverRange *mmsemver.Constraints) Predicate[catal
}
}

func HigherBundleVersion(currentVersion *bsemver.Version) Predicate[catalogmetadata.Bundle] {
return func(bundle *catalogmetadata.Bundle) bool {
if currentVersion == nil {
return false
}
bundleVersion, err := bundle.Version()
if err != nil {
return false
}
return bundleVersion.GTE(*currentVersion)
}
}

func InBlangSemverRange(semverRange bsemver.Range) Predicate[catalogmetadata.Bundle] {
return func(bundle *catalogmetadata.Bundle) bool {
bundleVersion, err := bundle.Version()
Expand Down
62 changes: 62 additions & 0 deletions internal/catalogmetadata/filter/bundle_predicates_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package filter_test

import (
"encoding/json"
"fmt"
"testing"

mmsemver "github.com/Masterminds/semver/v3"
Expand Down Expand Up @@ -62,6 +63,67 @@ func TestInMastermindsSemverRange(t *testing.T) {
assert.False(t, f(b3))
}

func TestHigherBundleVersion(t *testing.T) {
testCases := []struct {
name string
requestedVersion string
comparedVersion string
wantResult bool
}{
{
name: "includes equal version",
requestedVersion: "1.0.0",
comparedVersion: "1.0.0",
wantResult: true,
},
{
name: "filters out older version",
requestedVersion: "1.0.0",
comparedVersion: "0.0.1",
wantResult: false,
},
{
name: "includes higher version",
requestedVersion: "1.0.0",
comparedVersion: "2.0.0",
wantResult: true,
},
{
name: "filters out broken version",
requestedVersion: "1.0.0",
comparedVersion: "broken",
wantResult: false,
},
{
name: "filter returns false with nil version",
requestedVersion: "",
comparedVersion: "1.0.0",
wantResult: false,
},
}

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
bundle := &catalogmetadata.Bundle{Bundle: declcfg.Bundle{
Properties: []property.Property{
{
Type: property.TypePackage,
Value: json.RawMessage(fmt.Sprintf(`{"packageName": "package1", "version": "%s"}`, tc.comparedVersion)),
},
},
}}
var version *bsemver.Version
if tc.requestedVersion != "" {
parsedVersion := bsemver.MustParse(tc.requestedVersion)
// to test with nil requested version
version = &parsedVersion
}
f := filter.HigherBundleVersion(version)
assert.Equal(t, tc.wantResult, f(bundle))
})
}
}

func TestInBlangSemverRange(t *testing.T) {
b1 := &catalogmetadata.Bundle{Bundle: declcfg.Bundle{
Properties: []property.Property{
Expand Down
82 changes: 59 additions & 23 deletions internal/controllers/extension_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ package controllers

import (
"context"
"errors"
"fmt"
"sort"
"strings"
Expand All @@ -31,6 +30,7 @@ import (
kappctrlv1alpha1 "github.com/vmware-tanzu/carvel-kapp-controller/pkg/apis/kappctrl/v1alpha1"
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/equality"
apierrors "k8s.io/apimachinery/pkg/api/errors"
apimeta "k8s.io/apimachinery/pkg/api/meta"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
Expand All @@ -55,10 +55,11 @@ import (
type ExtensionReconciler struct {
client.Client
BundleProvider BundleProvider
HasKappApis bool
}

var errkappAPIUnavailable = errors.New("kapp-controller apis unavailable on cluster")
var (
bundleVersionKey = "olm.operatorframework.io/bundleVersion"
)

//+kubebuilder:rbac:groups=olm.operatorframework.io,resources=extensions,verbs=get;list;watch;create;update;patch;delete
//+kubebuilder:rbac:groups=olm.operatorframework.io,resources=extensions/status,verbs=update;patch
Expand Down Expand Up @@ -145,17 +146,6 @@ func (r *ExtensionReconciler) reconcile(ctx context.Context, ext *ocv1alpha1.Ext
return ctrl.Result{}, nil
}

if !r.HasKappApis {
ext.Status.InstalledBundle = nil
setInstalledStatusConditionFailed(&ext.Status.Conditions, errkappAPIUnavailable.Error(), ext.GetGeneration())

ext.Status.ResolvedBundle = nil
setResolvedStatusConditionUnknown(&ext.Status.Conditions, "kapp apis are unavailable", ext.GetGeneration())

setDeprecationStatusesUnknown(&ext.Status.Conditions, "kapp apis are unavailable", ext.GetGeneration())
return ctrl.Result{}, errkappAPIUnavailable
}

// TODO: Improve the resolution logic.
bundle, err := r.resolve(ctx, *ext)
if err != nil {
Expand Down Expand Up @@ -189,7 +179,13 @@ func (r *ExtensionReconciler) reconcile(ctx context.Context, ext *ocv1alpha1.Ext
return ctrl.Result{}, nil
}

app := r.GenerateExpectedApp(*ext, bundle.Image)
app, err := r.GenerateExpectedApp(*ext, bundle)
if err != nil {
setInstalledStatusConditionUnknown(&ext.Status.Conditions, err.Error(), ext.GetGeneration())
setDeprecationStatusesUnknown(&ext.Status.Conditions, "deprecation checks have not been attempted as installation has failed", ext.GetGeneration())
return ctrl.Result{}, err
}

if err := r.ensureApp(ctx, app); err != nil {
// originally Reason: ocv1alpha1.ReasonInstallationFailed
ext.Status.InstalledBundle = nil
Expand Down Expand Up @@ -402,7 +398,13 @@ func extensionRequestsForCatalog(c client.Reader, logger logr.Logger) handler.Ma
}
}

func (r *ExtensionReconciler) GenerateExpectedApp(o ocv1alpha1.Extension, bundlePath string) *unstructured.Unstructured {
func (r *ExtensionReconciler) GenerateExpectedApp(o ocv1alpha1.Extension, bundle *catalogmetadata.Bundle) (*unstructured.Unstructured, error) {
bundleVersion, err := bundle.Version()
if err != nil {
return nil, fmt.Errorf("failed to generate App from Extension %q with bundle %q: %w", o.GetName(), bundle.Name, err)
}
bundlePath := bundle.Image

// We use unstructured here to avoid problems of serializing default values when sending patches to the apiserver.
// If you use a typed object, any default values from that struct get serialized into the JSON patch, which could
// cause unrelated fields to be patched back to the default value even though that isn't the intention. Using an
Expand Down Expand Up @@ -436,6 +438,9 @@ func (r *ExtensionReconciler) GenerateExpectedApp(o ocv1alpha1.Extension, bundle
"metadata": map[string]interface{}{
"name": o.GetName(),
"namespace": o.GetNamespace(),
"annotations": map[string]string{
bundleVersionKey: bundleVersion.String(),
},
},
"spec": spec,
},
Expand All @@ -451,7 +456,24 @@ func (r *ExtensionReconciler) GenerateExpectedApp(o ocv1alpha1.Extension, bundle
BlockOwnerDeletion: ptr.To(true),
},
})
return app
return app, nil
}

func (r *ExtensionReconciler) getInstalledVersion(ctx context.Context, namespacedName types.NamespacedName) (*bsemver.Version, error) {
existingApp, err := r.existingAppUnstructured(ctx, namespacedName.Name, namespacedName.Namespace)
if err != nil {
return nil, err
}
existingVersion, ok := existingApp.GetAnnotations()[bundleVersionKey]
if !ok {
return nil, fmt.Errorf("existing App %q in Namespace %q missing bundle version", namespacedName.Name, namespacedName.Namespace)
}

existingVersionSemver, err := bsemver.New(existingVersion)
if err != nil {
return nil, fmt.Errorf("could not determine bundle version of existing App %q in Namespace %q: %w", namespacedName.Name, namespacedName.Namespace, err)
}
return existingVersionSemver, nil
}

func (r *ExtensionReconciler) resolve(ctx context.Context, extension ocv1alpha1.Extension) (*catalogmetadata.Bundle, error) {
Expand Down Expand Up @@ -480,19 +502,33 @@ func (r *ExtensionReconciler) resolve(ctx context.Context, extension ocv1alpha1.
predicates = append(predicates, catalogfilter.InMastermindsSemverRange(vr))
}

var installedVersion string
// Do not include bundle versions older than currently installed unless UpgradeConstraintPolicy = 'Ignore'
if extension.Spec.Source.Package.UpgradeConstraintPolicy != ocv1alpha1.UpgradeConstraintPolicyIgnore {
installedVersionSemver, err := r.getInstalledVersion(ctx, types.NamespacedName{Name: extension.GetName(), Namespace: extension.GetNamespace()})
if err != nil && !apierrors.IsNotFound(err) {
return nil, err
}
if installedVersionSemver != nil {
installedVersion = installedVersionSemver.String()
predicates = append(predicates, catalogfilter.HigherBundleVersion(installedVersionSemver))
}
}

resultSet := catalogfilter.Filter(allBundles, catalogfilter.And(predicates...))

if len(resultSet) == 0 {
if versionRange != "" && channelName != "" {
return nil, fmt.Errorf("no package %q matching version %q found in channel %q", packageName, versionRange, channelName)
}
var versionError, channelError, existingVersionError string
if versionRange != "" {
return nil, fmt.Errorf("no package %q matching version %q found", packageName, versionRange)
versionError = fmt.Sprintf(" matching version %q", versionRange)
}
if channelName != "" {
return nil, fmt.Errorf("no package %q found in channel %q", packageName, channelName)
channelError = fmt.Sprintf(" in channel %q", channelName)
}
if installedVersion != "" {
existingVersionError = fmt.Sprintf(" which upgrades currently installed version %q", installedVersion)
}
return nil, fmt.Errorf("no package %q found", packageName)
return nil, fmt.Errorf("no package %q%s%s%s found", packageName, versionError, channelError, existingVersionError)
}

sort.SliceStable(resultSet, func(i, j int) bool {
Expand Down
Loading

0 comments on commit 92ad69a

Please sign in to comment.