Skip to content

Commit 67352d1

Browse files
joelanfordjoelanford
committed
controller: remove unnecessary abstraction; improve registry+v1 to helm chart conversion
Signed-off-by: Joe Lanford <[email protected]>
1 parent ee7c35a commit 67352d1

File tree

13 files changed

+78
-558
lines changed

13 files changed

+78
-558
lines changed

Diff for: ‎cmd/manager/main.go

+7-25
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@ import (
2020
"context"
2121
"flag"
2222
"fmt"
23-
"net/url"
2423
"os"
2524
"path/filepath"
2625

@@ -47,11 +46,8 @@ import (
4746
"github.com/operator-framework/operator-controller/internal/controllers"
4847
"github.com/operator-framework/operator-controller/internal/httputil"
4948
"github.com/operator-framework/operator-controller/internal/labels"
50-
registryv1handler "github.com/operator-framework/operator-controller/internal/rukpak/handler"
5149
crdupgradesafety "github.com/operator-framework/operator-controller/internal/rukpak/preflights/crdupgradesafety"
52-
"github.com/operator-framework/operator-controller/internal/rukpak/provisioner/registry"
5350
"github.com/operator-framework/operator-controller/internal/rukpak/source"
54-
"github.com/operator-framework/operator-controller/internal/rukpak/storage"
5551
"github.com/operator-framework/operator-controller/internal/version"
5652
"github.com/operator-framework/operator-controller/pkg/features"
5753
"github.com/operator-framework/operator-controller/pkg/scheme"
@@ -95,6 +91,7 @@ func main() {
9591
flag.StringVar(&systemNamespace, "system-namespace", "", "Configures the namespace that gets used to deploy system resources.")
9692
opts := zap.Options{
9793
Development: true,
94+
TimeEncoder: zapcore.RFC3339NanoTimeEncoder,
9895
}
9996
opts.BindFlags(flag.CommandLine)
10097

@@ -161,7 +158,12 @@ func main() {
161158
}
162159

163160
cl := mgr.GetClient()
164-
catalogClient := catalogclient.New(cl, cache.NewFilesystemCache(cachePath, httpClient))
161+
catalogsCachePath := filepath.Join(cachePath, "catalogs")
162+
if err := os.MkdirAll(catalogsCachePath, 0700); err != nil {
163+
setupLog.Error(err, "unable to create catalogs cache directory")
164+
os.Exit(1)
165+
}
166+
catalogClient := catalogclient.New(cl, cache.NewFilesystemCache(catalogsCachePath, httpClient))
165167

166168
installNamespaceMapper := helmclient.ObjectToStringMapper(func(obj client.Object) (string, error) {
167169
ext := obj.(*ocv1alpha1.ClusterExtension)
@@ -193,7 +195,6 @@ func main() {
193195

194196
domain := ocv1alpha1.GroupVersion.Group
195197
cleanupUnpackCacheKey := fmt.Sprintf("%s/cleanup-unpack-cache", domain)
196-
deleteCachedBundleKey := fmt.Sprintf("%s/delete-cached-bundle", domain)
197198
if err := clusterExtensionFinalizers.Register(cleanupUnpackCacheKey, finalizerFunc(func(ctx context.Context, obj client.Object) (crfinalizer.Result, error) {
198199
ext := obj.(*ocv1alpha1.ClusterExtension)
199200
return crfinalizer.Result{}, os.RemoveAll(filepath.Join(unpacker.BaseCachePath, ext.GetName()))
@@ -202,23 +203,6 @@ func main() {
202203
os.Exit(1)
203204
}
204205

205-
localStorageRoot := filepath.Join(cachePath, "bundles")
206-
if err := os.MkdirAll(localStorageRoot, 0755); err != nil {
207-
setupLog.Error(err, "unable to create local storage root directory", "root", localStorageRoot)
208-
os.Exit(1)
209-
}
210-
localStorage := &storage.LocalDirectory{
211-
RootDirectory: localStorageRoot,
212-
URL: url.URL{},
213-
}
214-
if err := clusterExtensionFinalizers.Register(deleteCachedBundleKey, finalizerFunc(func(ctx context.Context, obj client.Object) (crfinalizer.Result, error) {
215-
ext := obj.(*ocv1alpha1.ClusterExtension)
216-
return crfinalizer.Result{}, localStorage.Delete(ctx, ext.GetName())
217-
})); err != nil {
218-
setupLog.Error(err, "unable to register finalizer", "finalizerKey", deleteCachedBundleKey)
219-
os.Exit(1)
220-
}
221-
222206
aeClient, err := apiextensionsv1client.NewForConfig(mgr.GetConfig())
223207
if err != nil {
224208
setupLog.Error(err, "unable to create apiextensions client")
@@ -234,9 +218,7 @@ func main() {
234218
BundleProvider: catalogClient,
235219
ActionClientGetter: acg,
236220
Unpacker: unpacker,
237-
Storage: localStorage,
238221
InstalledBundleGetter: &controllers.DefaultInstalledBundleGetter{ActionClientGetter: acg},
239-
Handler: registryv1handler.HandlerFunc(registry.HandleBundleDeployment),
240222
Finalizers: clusterExtensionFinalizers,
241223
CaCertDir: caCertDir,
242224
Preflights: preflights,

Diff for: ‎go.mod

-1
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@ require (
1414
github.com/google/go-containerregistry v0.20.0
1515
github.com/google/go-containerregistry/pkg/authn/k8schain v0.0.0-20240505154900-ff385a972813
1616
github.com/google/go-containerregistry/pkg/authn/kubernetes v0.0.0-20240505154900-ff385a972813
17-
github.com/nlepage/go-tarfs v1.2.1
1817
github.com/onsi/ginkgo/v2 v2.19.0
1918
github.com/onsi/gomega v1.33.1
2019
github.com/operator-framework/api v0.26.0

Diff for: ‎go.sum

-2
Original file line numberDiff line numberDiff line change
@@ -560,8 +560,6 @@ github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822/go.mod h1:+n7T8m
560560
github.com/mwitkow/go-conntrack v0.0.0-20161129095857-cc309e4a2223/go.mod h1:qRWi+5nqEBWmkhHvq77mSJWrCKwh8bxhgT7d/eI7P4U=
561561
github.com/mxk/go-flowrate v0.0.0-20140419014527-cca7078d478f h1:y5//uYreIhSUg3J1GEMiLbxo1LJaP8RfCpH6pymGZus=
562562
github.com/mxk/go-flowrate v0.0.0-20140419014527-cca7078d478f/go.mod h1:ZdcZmHo+o7JKHSa8/e818NopupXU1YMK5fe1lsApnBw=
563-
github.com/nlepage/go-tarfs v1.2.1 h1:o37+JPA+ajllGKSPfy5+YpsNHDjZnAoyfvf5GsUa+Ks=
564-
github.com/nlepage/go-tarfs v1.2.1/go.mod h1:rno18mpMy9aEH1IiJVftFsqPyIpwqSUiAOpJYjlV2NA=
565563
github.com/nxadm/tail v1.4.4/go.mod h1:kenIhsEOeOJmVchQTgglprH7qJGnHDVpk1VPCcaMI8A=
566564
github.com/nxadm/tail v1.4.8 h1:nPr65rt6Y5JFSKQO7qToXr7pePgD6Gwiw05lkbyAQTE=
567565
github.com/nxadm/tail v1.4.8/go.mod h1:+ncqLTQzXmGhMZNUePPaPqPvBxHAIsmXswZKocGu+AU=

Diff for: ‎internal/controllers/clusterextension_controller.go

+18-17
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ import (
3636
"helm.sh/helm/v3/pkg/postrender"
3737
"helm.sh/helm/v3/pkg/release"
3838
"helm.sh/helm/v3/pkg/storage/driver"
39+
corev1 "k8s.io/api/core/v1"
3940
"k8s.io/apimachinery/pkg/api/equality"
4041
apimeta "k8s.io/apimachinery/pkg/api/meta"
4142
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -71,10 +72,9 @@ import (
7172
"github.com/operator-framework/operator-controller/internal/httputil"
7273
"github.com/operator-framework/operator-controller/internal/labels"
7374
"github.com/operator-framework/operator-controller/internal/rukpak/bundledeployment"
74-
registryv1handler "github.com/operator-framework/operator-controller/internal/rukpak/handler"
75+
"github.com/operator-framework/operator-controller/internal/rukpak/convert"
7576
crdupgradesafety "github.com/operator-framework/operator-controller/internal/rukpak/preflights/crdupgradesafety"
7677
rukpaksource "github.com/operator-framework/operator-controller/internal/rukpak/source"
77-
"github.com/operator-framework/operator-controller/internal/rukpak/storage"
7878
"github.com/operator-framework/operator-controller/internal/rukpak/util"
7979
)
8080

@@ -88,8 +88,6 @@ type ClusterExtensionReconciler struct {
8888
BundleProvider BundleProvider
8989
Unpacker rukpaksource.Unpacker
9090
ActionClientGetter helmclient.ActionClientGetter
91-
Storage storage.Storage
92-
Handler registryv1handler.Handler
9391
dynamicWatchMutex sync.RWMutex
9492
dynamicWatchGVKs sets.Set[schema.GroupVersionKind]
9593
controller crcontroller.Controller
@@ -132,6 +130,8 @@ type Preflight interface {
132130
// This has been taken from rukpak, and an issue was created before to discuss it: https://github.com/operator-framework/rukpak/issues/800.
133131
func (r *ClusterExtensionReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
134132
l := log.FromContext(ctx).WithName("operator-controller")
133+
ctx = log.IntoContext(ctx, l)
134+
135135
l.V(1).Info("reconcile starting")
136136
defer l.V(1).Info("reconcile ending")
137137

@@ -212,6 +212,9 @@ func checkForUnexpectedFieldChange(a, b ocv1alpha1.ClusterExtension) bool {
212212
*/
213213
//nolint:unparam
214214
func (r *ClusterExtensionReconciler) reconcile(ctx context.Context, ext *ocv1alpha1.ClusterExtension) (ctrl.Result, error) {
215+
l := log.FromContext(ctx)
216+
217+
l.V(1).Info("handling finalizers")
215218
finalizeResult, err := r.Finalizers.Finalize(ctx, ext)
216219
if err != nil {
217220
// TODO: For now, this error handling follows the pattern of other error handling.
@@ -232,6 +235,7 @@ func (r *ClusterExtensionReconciler) reconcile(ctx context.Context, ext *ocv1alp
232235
}
233236

234237
// run resolution
238+
l.V(1).Info("resolving bundle")
235239
bundle, err := r.resolve(ctx, *ext)
236240
if err != nil {
237241
// Note: We don't distinguish between resolution-specific errors and generic errors
@@ -242,6 +246,7 @@ func (r *ClusterExtensionReconciler) reconcile(ctx context.Context, ext *ocv1alp
242246
return ctrl.Result{}, err
243247
}
244248

249+
l.V(1).Info("validating bundle")
245250
if err := r.validateBundle(bundle); err != nil {
246251
ext.Status.ResolvedBundle = nil
247252
ext.Status.InstalledBundle = nil
@@ -269,6 +274,7 @@ func (r *ClusterExtensionReconciler) reconcile(ctx context.Context, ext *ocv1alp
269274
// Note: The BundleDeployment here is not a k8s API, its a simple Go struct which
270275
// necessary embedded values.
271276
bd := r.generateBundleDeploymentForUnpack(ctx, bundle.Image, ext)
277+
l.V(1).Info("unpacking resolved bundle")
272278
unpackResult, err := r.Unpacker.Unpack(ctx, bd)
273279
if err != nil {
274280
setStatusUnpackFailed(ext, err.Error())
@@ -282,31 +288,22 @@ func (r *ClusterExtensionReconciler) reconcile(ctx context.Context, ext *ocv1alp
282288

283289
return ctrl.Result{}, nil
284290
case rukpaksource.StateUnpacked:
285-
// TODO: Add finalizer to clean the stored bundles, after https://github.com/operator-framework/rukpak/pull/897
286-
// merges.
287-
if err := r.Storage.Store(ctx, ext.GetName(), unpackResult.Bundle); err != nil {
288-
setStatusUnpackFailed(ext, err.Error())
289-
return ctrl.Result{}, err
290-
}
291291
setStatusUnpacked(ext, fmt.Sprintf("unpack successful: %v", unpackResult.Message))
292292
default:
293293
setStatusUnpackFailed(ext, "unexpected unpack status")
294294
// We previously exit with a failed status if error is not nil.
295295
return ctrl.Result{}, fmt.Errorf("unexpected unpack status: %v", unpackResult.Message)
296296
}
297297

298-
bundleFS, err := r.Storage.Load(ctx, ext.GetName())
299-
if err != nil {
300-
setInstalledStatusConditionFailed(ext, err.Error())
301-
return ctrl.Result{}, err
302-
}
303-
304-
chrt, values, err := r.Handler.Handle(ctx, bundleFS, bd)
298+
l.V(1).Info("converting bundle to helm chart")
299+
chrt, err := convert.RegistryV1ToHelmChart(ctx, unpackResult.Bundle, ext.Spec.InstallNamespace, []string{corev1.NamespaceAll})
305300
if err != nil {
306301
setInstalledStatusConditionFailed(ext, err.Error())
307302
return ctrl.Result{}, err
308303
}
304+
values := chartutil.Values{}
309305

306+
l.V(1).Info("getting helm client")
310307
ac, err := r.ActionClientGetter.ActionClientFor(ctx, ext)
311308
if err != nil {
312309
ext.Status.InstalledBundle = nil
@@ -324,12 +321,14 @@ func (r *ClusterExtensionReconciler) reconcile(ctx context.Context, ext *ocv1alp
324321
},
325322
}
326323

324+
l.V(1).Info("getting current state of helm release")
327325
rel, desiredRel, state, err := r.getReleaseState(ac, ext, chrt, values, post)
328326
if err != nil {
329327
setInstalledStatusConditionFailed(ext, fmt.Sprintf("%s:%v", ocv1alpha1.ReasonErrorGettingReleaseState, err))
330328
return ctrl.Result{}, err
331329
}
332330

331+
l.V(1).Info("running preflight checks")
333332
for _, preflight := range r.Preflights {
334333
if ext.Spec.Preflight != nil && ext.Spec.Preflight.CRDUpgradeSafety != nil {
335334
if _, ok := preflight.(*crdupgradesafety.Preflight); ok && ext.Spec.Preflight.CRDUpgradeSafety.Disabled {
@@ -354,6 +353,7 @@ func (r *ClusterExtensionReconciler) reconcile(ctx context.Context, ext *ocv1alp
354353
}
355354
}
356355

356+
l.V(1).Info("reconciling helm release changes")
357357
switch state {
358358
case stateNeedsInstall:
359359
rel, err = ac.Install(ext.GetName(), ext.Spec.InstallNamespace, chrt, values, func(install *action.Install) error {
@@ -384,6 +384,7 @@ func (r *ClusterExtensionReconciler) reconcile(ctx context.Context, ext *ocv1alp
384384
return ctrl.Result{}, fmt.Errorf("unexpected release state %q", state)
385385
}
386386

387+
l.V(1).Info("configuring watches for release objects")
387388
relObjects, err := util.ManifestObjects(strings.NewReader(rel.Manifest), fmt.Sprintf("%s-release-manifest", rel.Name))
388389
if err != nil {
389390
setInstalledStatusConditionFailed(ext, fmt.Sprintf("%s:%v", ocv1alpha1.ReasonCreateDynamicWatchFailed, err))

Diff for: ‎internal/controllers/suite_test.go

-39
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,7 @@ package controllers_test
1818

1919
import (
2020
"context"
21-
"io/fs"
2221
"log"
23-
"net/http"
2422
"os"
2523
"path/filepath"
2624
"testing"
@@ -40,7 +38,6 @@ import (
4038
"github.com/operator-framework/operator-controller/internal/controllers"
4139
bd "github.com/operator-framework/operator-controller/internal/rukpak/bundledeployment"
4240
"github.com/operator-framework/operator-controller/internal/rukpak/source"
43-
"github.com/operator-framework/operator-controller/internal/rukpak/storage"
4441
"github.com/operator-framework/operator-controller/internal/testutil"
4542
"github.com/operator-framework/operator-controller/pkg/scheme"
4643
)
@@ -61,39 +58,6 @@ func (m *MockUnpacker) Cleanup(ctx context.Context, bundle *bd.BundleDeployment)
6158
panic("implement me")
6259
}
6360

64-
// MockStorage is a mock of Storage interface
65-
type MockStorage struct {
66-
mock.Mock
67-
}
68-
69-
func (m *MockStorage) Load(ctx context.Context, owner string) (fs.FS, error) {
70-
args := m.Called(ctx, owner)
71-
if args.Get(0) == nil {
72-
return nil, args.Error(1)
73-
}
74-
return args.Get(0).(fs.FS), args.Error(1)
75-
}
76-
77-
func (m *MockStorage) Delete(ctx context.Context, owner string) error {
78-
//TODO implement me
79-
panic("implement me")
80-
}
81-
82-
func (m *MockStorage) ServeHTTP(writer http.ResponseWriter, request *http.Request) {
83-
//TODO implement me
84-
panic("implement me")
85-
}
86-
87-
func (m *MockStorage) URLFor(ctx context.Context, owner string) (string, error) {
88-
//TODO implement me
89-
panic("implement me")
90-
}
91-
92-
func (m *MockStorage) Store(ctx context.Context, owner string, bundle fs.FS) error {
93-
args := m.Called(ctx, owner, bundle)
94-
return args.Error(0)
95-
}
96-
9761
func newClient(t *testing.T) client.Client {
9862
cl, err := client.New(config, client.Options{Scheme: scheme.Scheme})
9963
require.NoError(t, err)
@@ -125,7 +89,6 @@ func newClientAndReconciler(t *testing.T, bundle *ocv1alpha1.BundleMetadata) (cl
12589
BundleProvider: &fakeCatalogClient,
12690
ActionClientGetter: helmClientGetter,
12791
Unpacker: unpacker,
128-
Storage: store,
12992
InstalledBundleGetter: mockInstalledBundleGetter,
13093
Finalizers: crfinalizer.NewFinalizers(),
13194
}
@@ -136,7 +99,6 @@ var (
13699
config *rest.Config
137100
helmClientGetter helmclient.ActionClientGetter
138101
unpacker source.Unpacker // Interface, will be initialized as a mock in TestMain
139-
store storage.Storage
140102
)
141103

142104
func TestMain(m *testing.M) {
@@ -160,7 +122,6 @@ func TestMain(m *testing.M) {
160122
utilruntime.Must(err)
161123

162124
unpacker = new(MockUnpacker)
163-
store = new(MockStorage)
164125

165126
code := m.Run()
166127
utilruntime.Must(testEnv.Stop())

0 commit comments

Comments
 (0)