Skip to content

Commit

Permalink
✨ refactor ClusterExtensionReconciler.reconcile() (#1068)
Browse files Browse the repository at this point in the history
* update reconciler with logical interface abstractions to simply unit testing

by moving bundle validation logic to the resolver, moving installation
of content behind a new Applier interface, and using the new
contentmanager.Watcher interface for watching managed objects

Signed-off-by: everettraven <[email protected]>

* update testing

Signed-off-by: everettraven <[email protected]>

* update testing

Signed-off-by: everettraven <[email protected]>

* remove bundle validation unit tests from controller package

Signed-off-by: everettraven <[email protected]>

* make linter happy

Signed-off-by: everettraven <[email protected]>

* remove postrenderer, replace with map[string]string for labels

Signed-off-by: everettraven <[email protected]>

* update debugging error string

Signed-off-by: everettraven <[email protected]>

* rebase fixes

Signed-off-by: everettraven <[email protected]>

---------

Signed-off-by: everettraven <[email protected]>
  • Loading branch information
everettraven authored Aug 2, 2024
1 parent 989a3df commit cab9eab
Show file tree
Hide file tree
Showing 11 changed files with 791 additions and 361 deletions.
33 changes: 21 additions & 12 deletions cmd/manager/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ import (

ocv1alpha1 "github.com/operator-framework/operator-controller/api/v1alpha1"
"github.com/operator-framework/operator-controller/internal/action"
"github.com/operator-framework/operator-controller/internal/applier"
"github.com/operator-framework/operator-controller/internal/authentication"
"github.com/operator-framework/operator-controller/internal/catalogmetadata/cache"
catalogclient "github.com/operator-framework/operator-controller/internal/catalogmetadata/client"
Expand Down Expand Up @@ -191,6 +192,7 @@ func main() {
}
return tempConfig, nil
}

cfgGetter, err := helmclient.NewActionConfigGetter(mgr.GetConfig(), mgr.GetRESTMapper(),
helmclient.StorageNamespaceMapper(installNamespaceMapper),
helmclient.ClientNamespaceMapper(installNamespaceMapper),
Expand Down Expand Up @@ -232,16 +234,6 @@ func main() {
os.Exit(1)
}

aeClient, err := apiextensionsv1client.NewForConfig(mgr.GetConfig())
if err != nil {
setupLog.Error(err, "unable to create apiextensions client")
os.Exit(1)
}

preflights := []controllers.Preflight{
crdupgradesafety.NewPreflight(aeClient.CustomResourceDefinitions()),
}

cl := mgr.GetClient()

catalogsCachePath := filepath.Join(cachePath, "catalogs")
Expand All @@ -264,16 +256,33 @@ func main() {
},
catalogClient.GetPackage,
),
Validations: []resolve.ValidationFunc{
resolve.NoDependencyValidation,
},
}

aeClient, err := apiextensionsv1client.NewForConfig(mgr.GetConfig())
if err != nil {
setupLog.Error(err, "unable to create apiextensions client")
os.Exit(1)
}

preflights := []applier.Preflight{
crdupgradesafety.NewPreflight(aeClient.CustomResourceDefinitions()),
}

applier := &applier.Helm{
ActionClientGetter: acg,
Preflights: preflights,
}

if err = (&controllers.ClusterExtensionReconciler{
Client: cl,
Resolver: resolver,
ActionClientGetter: acg,
Unpacker: unpacker,
Applier: applier,
InstalledBundleGetter: &controllers.DefaultInstalledBundleGetter{ActionClientGetter: acg},
Finalizers: clusterExtensionFinalizers,
Preflights: preflights,
Watcher: contentmanager.New(restConfigMapper, mgr.GetConfig(), mgr.GetRESTMapper()),
}).SetupWithManager(mgr); err != nil {
setupLog.Error(err, "unable to create controller", "controller", "ClusterExtension")
Expand Down
166 changes: 166 additions & 0 deletions internal/applier/helm.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,166 @@
package applier

import (
"context"
"errors"
"fmt"
"io/fs"
"strings"

"helm.sh/helm/v3/pkg/action"
"helm.sh/helm/v3/pkg/chart"
"helm.sh/helm/v3/pkg/chartutil"
"helm.sh/helm/v3/pkg/release"
"helm.sh/helm/v3/pkg/storage/driver"
corev1 "k8s.io/api/core/v1"
"sigs.k8s.io/controller-runtime/pkg/client"

helmclient "github.com/operator-framework/helm-operator-plugins/pkg/client"

ocv1alpha1 "github.com/operator-framework/operator-controller/api/v1alpha1"
"github.com/operator-framework/operator-controller/internal/rukpak/convert"
"github.com/operator-framework/operator-controller/internal/rukpak/preflights/crdupgradesafety"
"github.com/operator-framework/operator-controller/internal/rukpak/util"
)

const (
StateNeedsInstall string = "NeedsInstall"
StateNeedsUpgrade string = "NeedsUpgrade"
StateUnchanged string = "Unchanged"
StateError string = "Error"
maxHelmReleaseHistory = 10
)

// Preflight is a check that should be run before making any changes to the cluster
type Preflight interface {
// Install runs checks that should be successful prior
// to installing the Helm release. It is provided
// a Helm release and returns an error if the
// check is unsuccessful
Install(context.Context, *release.Release) error

// Upgrade runs checks that should be successful prior
// to upgrading the Helm release. It is provided
// a Helm release and returns an error if the
// check is unsuccessful
Upgrade(context.Context, *release.Release) error
}

type Helm struct {
ActionClientGetter helmclient.ActionClientGetter
Preflights []Preflight
}

func (h *Helm) Apply(ctx context.Context, contentFS fs.FS, ext *ocv1alpha1.ClusterExtension, labels map[string]string) ([]client.Object, string, error) {
chrt, err := convert.RegistryV1ToHelmChart(ctx, contentFS, ext.Spec.InstallNamespace, []string{corev1.NamespaceAll})
if err != nil {
return nil, "", err
}
values := chartutil.Values{}

ac, err := h.ActionClientGetter.ActionClientFor(ctx, ext)
if err != nil {
return nil, "", err
}

rel, desiredRel, state, err := h.getReleaseState(ac, ext, chrt, values, labels)
if err != nil {
return nil, "", err
}

for _, preflight := range h.Preflights {
if ext.Spec.Preflight != nil && ext.Spec.Preflight.CRDUpgradeSafety != nil {
if _, ok := preflight.(*crdupgradesafety.Preflight); ok && ext.Spec.Preflight.CRDUpgradeSafety.Disabled {
// Skip this preflight check because it is of type *crdupgradesafety.Preflight and the CRD Upgrade Safety
// preflight check has been disabled
continue
}
}
switch state {
case StateNeedsInstall:
err := preflight.Install(ctx, desiredRel)
if err != nil {
return nil, state, err
}
case StateNeedsUpgrade:
err := preflight.Upgrade(ctx, desiredRel)
if err != nil {
return nil, state, err
}
}
}

switch state {
case StateNeedsInstall:
rel, err = ac.Install(ext.GetName(), ext.Spec.InstallNamespace, chrt, values, func(install *action.Install) error {
install.CreateNamespace = false
install.Labels = labels
return nil
})
if err != nil {
return nil, state, err
}
case StateNeedsUpgrade:
rel, err = ac.Upgrade(ext.GetName(), ext.Spec.InstallNamespace, chrt, values, func(upgrade *action.Upgrade) error {
upgrade.MaxHistory = maxHelmReleaseHistory
upgrade.Labels = labels
return nil
})
if err != nil {
return nil, state, err
}
case StateUnchanged:
if err := ac.Reconcile(rel); err != nil {
return nil, state, err
}
default:
return nil, state, fmt.Errorf("unexpected release state %q", state)
}

relObjects, err := util.ManifestObjects(strings.NewReader(rel.Manifest), fmt.Sprintf("%s-release-manifest", rel.Name))
if err != nil {
return nil, state, err
}

return relObjects, state, nil
}

func (h *Helm) getReleaseState(cl helmclient.ActionInterface, ext *ocv1alpha1.ClusterExtension, chrt *chart.Chart, values chartutil.Values, labels map[string]string) (*release.Release, *release.Release, string, error) {
currentRelease, err := cl.Get(ext.GetName())
if err != nil && !errors.Is(err, driver.ErrReleaseNotFound) {
return nil, nil, StateError, err
}
if errors.Is(err, driver.ErrReleaseNotFound) {
return nil, nil, StateNeedsInstall, nil
}

if errors.Is(err, driver.ErrReleaseNotFound) {
desiredRelease, err := cl.Install(ext.GetName(), ext.Spec.InstallNamespace, chrt, values, func(i *action.Install) error {
i.DryRun = true
i.DryRunOption = "server"
i.Labels = labels
return nil
})
if err != nil {
return nil, nil, StateError, err
}
return nil, desiredRelease, StateNeedsInstall, nil
}
desiredRelease, err := cl.Upgrade(ext.GetName(), ext.Spec.InstallNamespace, chrt, values, func(upgrade *action.Upgrade) error {
upgrade.MaxHistory = maxHelmReleaseHistory
upgrade.DryRun = true
upgrade.DryRunOption = "server"
upgrade.Labels = labels
return nil
})
if err != nil {
return currentRelease, nil, StateError, err
}
relState := StateUnchanged
if desiredRelease.Manifest != currentRelease.Manifest ||
currentRelease.Info.Status == release.StatusFailed ||
currentRelease.Info.Status == release.StatusSuperseded {
relState = StateNeedsUpgrade
}
return currentRelease, desiredRelease, relState, nil
}
6 changes: 3 additions & 3 deletions internal/catalogmetadata/filter/successors.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,17 +20,17 @@ func SuccessorsOf(installedBundle *ocv1alpha1.BundleMetadata, channels ...declcf

installedBundleVersion, err := mmsemver.NewVersion(installedBundle.Version)
if err != nil {
return nil, err
return nil, fmt.Errorf("parsing installed bundle %q version %q: %w", installedBundle.Name, installedBundle.Version, err)
}

installedVersionConstraint, err := mmsemver.NewConstraint(installedBundleVersion.String())
if err != nil {
return nil, err
return nil, fmt.Errorf("parsing installed version constraint %q: %w", installedBundleVersion.String(), err)
}

successorsPredicate, err := successors(installedBundle, channels...)
if err != nil {
return nil, err
return nil, fmt.Errorf("getting successorsPredicate: %w", err)
}

// We need either successors or current version (no upgrade)
Expand Down
Loading

0 comments on commit cab9eab

Please sign in to comment.