From 844d3f9215c1834e487f265cb451c0aaeccc5325 Mon Sep 17 00:00:00 2001 From: Joe Lanford Date: Wed, 13 Feb 2019 16:48:44 -0500 Subject: [PATCH 1/7] pkg/helm: migrate to secret storage backend in namespace of CR --- Gopkg.lock | 1 + pkg/helm/controller/reconcile.go | 7 +++- pkg/helm/release/manager.go | 4 ++ pkg/helm/release/manager_factory.go | 59 +++++++++++++++++++---------- pkg/helm/run.go | 13 +------ 5 files changed, 52 insertions(+), 32 deletions(-) diff --git a/Gopkg.lock b/Gopkg.lock index b8a9359723..1893151b1b 100644 --- a/Gopkg.lock +++ b/Gopkg.lock @@ -1608,6 +1608,7 @@ "k8s.io/client-go/discovery/cached", "k8s.io/client-go/kubernetes", "k8s.io/client-go/kubernetes/scheme", + "k8s.io/client-go/kubernetes/typed/core/v1", "k8s.io/client-go/plugin/pkg/client/auth/gcp", "k8s.io/client-go/rest", "k8s.io/client-go/restmapper", diff --git a/pkg/helm/controller/reconcile.go b/pkg/helm/controller/reconcile.go index b71337b680..a13592813c 100644 --- a/pkg/helm/controller/reconcile.go +++ b/pkg/helm/controller/reconcile.go @@ -75,7 +75,12 @@ func (r HelmOperatorReconciler) Reconcile(request reconcile.Request) (reconcile. return reconcile.Result{}, err } - manager := r.ManagerFactory.NewManager(o) + manager, err := r.ManagerFactory.NewManager(o) + if err != nil { + log.Error(err, "Failed to get release manager") + return reconcile.Result{}, err + } + status := types.StatusFor(o) log = log.WithValues("release", manager.ReleaseName()) diff --git a/pkg/helm/release/manager.go b/pkg/helm/release/manager.go index ff68ea5f39..0ebc364f9b 100644 --- a/pkg/helm/release/manager.go +++ b/pkg/helm/release/manager.go @@ -94,6 +94,10 @@ func (m manager) IsUpdateRequired() bool { // Sync ensures the Helm storage backend is in sync with the status of the // custom resource. func (m *manager) Sync(ctx context.Context) error { + // We're now persisting releases as secrets. To support seamless upgrades, we + // need to sync the release status from the CR to the persistent storage backend. + // Once we release the storage backend migration, this function (and comment) + // can be removed. if err := m.syncReleaseStatus(*m.status); err != nil { return fmt.Errorf("failed to sync release status to storage backend: %s", err) } diff --git a/pkg/helm/release/manager_factory.go b/pkg/helm/release/manager_factory.go index 6f777aa6e1..e6a56390de 100644 --- a/pkg/helm/release/manager_factory.go +++ b/pkg/helm/release/manager_factory.go @@ -20,17 +20,20 @@ import ( "github.com/martinlindhe/base36" "github.com/pborman/uuid" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" apitypes "k8s.io/apimachinery/pkg/types" clientset "k8s.io/client-go/kubernetes" + v1 "k8s.io/client-go/kubernetes/typed/core/v1" helmengine "k8s.io/helm/pkg/engine" "k8s.io/helm/pkg/kube" "k8s.io/helm/pkg/storage" + "k8s.io/helm/pkg/storage/driver" "k8s.io/helm/pkg/tiller" "k8s.io/helm/pkg/tiller/environment" + crmanager "sigs.k8s.io/controller-runtime/pkg/manager" + "github.com/operator-framework/operator-sdk/pkg/helm/client" "github.com/operator-framework/operator-sdk/pkg/helm/engine" "github.com/operator-framework/operator-sdk/pkg/helm/internal/types" ) @@ -40,42 +43,54 @@ import ( // improves decoupling between reconciliation logic and the Helm backend // components used to manage releases. type ManagerFactory interface { - NewManager(r *unstructured.Unstructured) Manager + NewManager(r *unstructured.Unstructured) (Manager, error) } type managerFactory struct { - storageBackend *storage.Storage - tillerKubeClient *kube.Client - chartDir string + mgr crmanager.Manager + chartDir string } // NewManagerFactory returns a new Helm manager factory capable of installing and uninstalling releases. -func NewManagerFactory(storageBackend *storage.Storage, tillerKubeClient *kube.Client, chartDir string) ManagerFactory { - return &managerFactory{storageBackend, tillerKubeClient, chartDir} +func NewManagerFactory(mgr crmanager.Manager, chartDir string) ManagerFactory { + return &managerFactory{mgr, chartDir} } -func (f managerFactory) NewManager(r *unstructured.Unstructured) Manager { +func (f managerFactory) NewManager(r *unstructured.Unstructured) (Manager, error) { return f.newManagerForCR(r) } -func (f managerFactory) newManagerForCR(r *unstructured.Unstructured) Manager { +func (f managerFactory) newManagerForCR(r *unstructured.Unstructured) (Manager, error) { + clientv1, err := v1.NewForConfig(f.mgr.GetConfig()) + if err != nil { + return nil, err + } + storageBackend := storage.Init(driver.NewSecrets(clientv1.Secrets(r.GetNamespace()))) + tillerKubeClient, err := client.NewFromManager(f.mgr) + if err != nil { + return nil, err + } + releaseServer, err := getReleaseServer(r, storageBackend, tillerKubeClient) + if err != nil { + return nil, err + } return &manager{ - storageBackend: f.storageBackend, - tillerKubeClient: f.tillerKubeClient, + storageBackend: storageBackend, + tillerKubeClient: tillerKubeClient, chartDir: f.chartDir, - tiller: f.tillerRendererForCR(r), + tiller: releaseServer, releaseName: getReleaseName(r), namespace: r.GetNamespace(), spec: r.Object["spec"], status: types.StatusFor(r), - } + }, nil } // tillerRendererForCR creates a ReleaseServer configured with a rendering engine that adds ownerrefs to rendered assets // based on the CR. -func (f managerFactory) tillerRendererForCR(r *unstructured.Unstructured) *tiller.ReleaseServer { +func getReleaseServer(r *unstructured.Unstructured, storageBackend *storage.Storage, tillerKubeClient *kube.Client) (*tiller.ReleaseServer, error) { controllerRef := metav1.NewControllerRef(r, r.GroupVersionKind()) ownerRefs := []metav1.OwnerReference{ *controllerRef, @@ -87,13 +102,19 @@ func (f managerFactory) tillerRendererForCR(r *unstructured.Unstructured) *tille } env := &environment.Environment{ EngineYard: ey, - Releases: f.storageBackend, - KubeClient: f.tillerKubeClient, + Releases: storageBackend, + KubeClient: tillerKubeClient, + } + kubeconfig, err := tillerKubeClient.ToRESTConfig() + if err != nil { + return nil, err + } + cs, err := clientset.NewForConfig(kubeconfig) + if err != nil { + return nil, err } - kubeconfig, _ := f.tillerKubeClient.ToRESTConfig() - cs := clientset.NewForConfigOrDie(kubeconfig) - return tiller.NewReleaseServer(env, cs, false) + return tiller.NewReleaseServer(env, cs, false), nil } func getReleaseName(r *unstructured.Unstructured) string { diff --git a/pkg/helm/run.go b/pkg/helm/run.go index d9cf3fefe9..2f2f36deda 100644 --- a/pkg/helm/run.go +++ b/pkg/helm/run.go @@ -20,7 +20,6 @@ import ( "os" "runtime" - "github.com/operator-framework/operator-sdk/pkg/helm/client" "github.com/operator-framework/operator-sdk/pkg/helm/controller" hoflags "github.com/operator-framework/operator-sdk/pkg/helm/flags" "github.com/operator-framework/operator-sdk/pkg/helm/release" @@ -30,8 +29,6 @@ import ( sdkVersion "github.com/operator-framework/operator-sdk/version" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/helm/pkg/storage" - "k8s.io/helm/pkg/storage/driver" "sigs.k8s.io/controller-runtime/pkg/client/config" "sigs.k8s.io/controller-runtime/pkg/manager" logf "sigs.k8s.io/controller-runtime/pkg/runtime/log" @@ -73,14 +70,6 @@ func Run(flags *hoflags.HelmOperatorFlags) error { return err } - // Create Tiller's storage backend and kubernetes client - storageBackend := storage.Init(driver.NewMemory()) - tillerKubeClient, err := client.NewFromManager(mgr) - if err != nil { - log.Error(err, "Failed to create new Tiller client.") - return err - } - watches, err := watches.Load(flags.WatchesFile) if err != nil { log.Error(err, "Failed to create new manager factories.") @@ -92,7 +81,7 @@ func Run(flags *hoflags.HelmOperatorFlags) error { err := controller.Add(mgr, controller.WatchOptions{ Namespace: namespace, GVK: w.GroupVersionKind, - ManagerFactory: release.NewManagerFactory(storageBackend, tillerKubeClient, w.ChartDir), + ManagerFactory: release.NewManagerFactory(mgr, w.ChartDir), ReconcilePeriod: flags.ReconcilePeriod, WatchDependentResources: w.WatchDependentResources, }) From 9abd4c54f61f51a925c1ec5e121266da6b96e1c3 Mon Sep 17 00:00:00 2001 From: Joe Lanford Date: Thu, 14 Feb 2019 10:56:22 -0500 Subject: [PATCH 2/7] CHANGELOG.md: added change for PR #1102 --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 89790c2a48..f5d03bcebd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,8 @@ ### Changed +- Updated the helm-operator to store release state in kubernetes secrets in the same namespace of the custom resource that defines the release. ([#1102](https://github.com/operator-framework/operator-sdk/pull/1102)) + ### Deprecated ### Removed From 5843854ea48ff18b571ff96caf43195d8087a9eb Mon Sep 17 00:00:00 2001 From: Joe Lanford Date: Fri, 15 Feb 2019 11:03:45 -0500 Subject: [PATCH 3/7] CHANGELOG.md: add warning about skipping this release --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index f5d03bcebd..ba245b6a40 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ ### Changed - Updated the helm-operator to store release state in kubernetes secrets in the same namespace of the custom resource that defines the release. ([#1102](https://github.com/operator-framework/operator-sdk/pull/1102)) + - **NOTE**: Users with active CRs and releases who are upgrading their helm-based operator should not skip this version. Future versions will not seamlessly transition release state to the persistent backend, and will instead uninstall and reinstall all managed releases. ### Deprecated From 3d3dc86f74a06179e409b54ada02b7a633da6c60 Mon Sep 17 00:00:00 2001 From: Joe Lanford Date: Mon, 4 Mar 2019 14:34:37 -0500 Subject: [PATCH 4/7] pkg/helm,CHANGELOG.md: addressing PR comments --- CHANGELOG.md | 2 +- pkg/helm/release/manager.go | 2 +- pkg/helm/release/manager_factory.go | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a0f816710b..de989e1aa7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,7 +11,7 @@ - Changed the Go, Helm, and Scorecard base images to `registry.access.redhat.com/ubi7-dev-preview/ubi-minimal:7.6` ([#1142](https://github.com/operator-framework/operator-sdk/pull/1142)) - CSV manifest are now versioned according to the `operator-registry` [manifest format](https://github.com/operator-framework/operator-registry#manifest-format). See issue [#900](https://github.com/operator-framework/operator-sdk/issues/900) for more details. ([#1016](https://github.com/operator-framework/operator-sdk/pull/1016)) - Updated the helm-operator to store release state in kubernetes secrets in the same namespace of the custom resource that defines the release. ([#1102](https://github.com/operator-framework/operator-sdk/pull/1102)) - - **NOTE**: Users with active CRs and releases who are upgrading their helm-based operator should not skip this version. Future versions will not seamlessly transition release state to the persistent backend, and will instead uninstall and reinstall all managed releases. + - **WARNING**: Users with active CRs and releases who are upgrading their helm-based operator should not skip this version. Future versions will not seamlessly transition release state to the persistent backend, and will instead uninstall and reinstall all managed releases. ### Deprecated diff --git a/pkg/helm/release/manager.go b/pkg/helm/release/manager.go index 0ebc364f9b..bf383ddede 100644 --- a/pkg/helm/release/manager.go +++ b/pkg/helm/release/manager.go @@ -94,7 +94,7 @@ func (m manager) IsUpdateRequired() bool { // Sync ensures the Helm storage backend is in sync with the status of the // custom resource. func (m *manager) Sync(ctx context.Context) error { - // We're now persisting releases as secrets. To support seamless upgrades, we + // TODO: We're now persisting releases as secrets. To support seamless upgrades, we // need to sync the release status from the CR to the persistent storage backend. // Once we release the storage backend migration, this function (and comment) // can be removed. diff --git a/pkg/helm/release/manager_factory.go b/pkg/helm/release/manager_factory.go index e6a56390de..7154604914 100644 --- a/pkg/helm/release/manager_factory.go +++ b/pkg/helm/release/manager_factory.go @@ -88,7 +88,7 @@ func (f managerFactory) newManagerForCR(r *unstructured.Unstructured) (Manager, }, nil } -// tillerRendererForCR creates a ReleaseServer configured with a rendering engine that adds ownerrefs to rendered assets +// getReleaseServer creates a ReleaseServer configured with a rendering engine that adds ownerrefs to rendered assets // based on the CR. func getReleaseServer(r *unstructured.Unstructured, storageBackend *storage.Storage, tillerKubeClient *kube.Client) (*tiller.ReleaseServer, error) { controllerRef := metav1.NewControllerRef(r, r.GroupVersionKind()) From 573a049cd5e19f0bd71b36a54f47ee6b41b42e63 Mon Sep 17 00:00:00 2001 From: Joe Lanford Date: Wed, 13 Mar 2019 19:31:42 -0400 Subject: [PATCH 5/7] pkg/helm/release/manager_factory.go: address PR comments --- pkg/helm/release/manager_factory.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/helm/release/manager_factory.go b/pkg/helm/release/manager_factory.go index 7154604914..c3e9763e32 100644 --- a/pkg/helm/release/manager_factory.go +++ b/pkg/helm/release/manager_factory.go @@ -24,7 +24,7 @@ import ( "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" apitypes "k8s.io/apimachinery/pkg/types" clientset "k8s.io/client-go/kubernetes" - v1 "k8s.io/client-go/kubernetes/typed/core/v1" + "k8s.io/client-go/kubernetes/typed/core/v1" helmengine "k8s.io/helm/pkg/engine" "k8s.io/helm/pkg/kube" "k8s.io/helm/pkg/storage" @@ -63,16 +63,16 @@ func (f managerFactory) NewManager(r *unstructured.Unstructured) (Manager, error func (f managerFactory) newManagerForCR(r *unstructured.Unstructured) (Manager, error) { clientv1, err := v1.NewForConfig(f.mgr.GetConfig()) if err != nil { - return nil, err + return nil, fmt.Errorf("failed to get core/v1 client: %s", err) } storageBackend := storage.Init(driver.NewSecrets(clientv1.Secrets(r.GetNamespace()))) tillerKubeClient, err := client.NewFromManager(f.mgr) if err != nil { - return nil, err + return nil, fmt.Errorf("failed to get client from manager: %s", err) } releaseServer, err := getReleaseServer(r, storageBackend, tillerKubeClient) if err != nil { - return nil, err + return nil, fmt.Errorf("failed to get helm release server: %s", err) } return &manager{ storageBackend: storageBackend, From 95edbd5aa3f7041d0d85accb5bcbd80a0386a47b Mon Sep 17 00:00:00 2001 From: Joe Lanford Date: Mon, 18 Mar 2019 10:54:02 -0400 Subject: [PATCH 6/7] pkg/helm/release/manager_factory.go: address pr comments --- pkg/helm/release/manager_factory.go | 26 +++++++++++--------------- 1 file changed, 11 insertions(+), 15 deletions(-) diff --git a/pkg/helm/release/manager_factory.go b/pkg/helm/release/manager_factory.go index c3e9763e32..d897554b7b 100644 --- a/pkg/helm/release/manager_factory.go +++ b/pkg/helm/release/manager_factory.go @@ -56,21 +56,17 @@ func NewManagerFactory(mgr crmanager.Manager, chartDir string) ManagerFactory { return &managerFactory{mgr, chartDir} } -func (f managerFactory) NewManager(r *unstructured.Unstructured) (Manager, error) { - return f.newManagerForCR(r) -} - -func (f managerFactory) newManagerForCR(r *unstructured.Unstructured) (Manager, error) { +func (f managerFactory) NewManager(cr *unstructured.Unstructured) (Manager, error) { clientv1, err := v1.NewForConfig(f.mgr.GetConfig()) if err != nil { return nil, fmt.Errorf("failed to get core/v1 client: %s", err) } - storageBackend := storage.Init(driver.NewSecrets(clientv1.Secrets(r.GetNamespace()))) + storageBackend := storage.Init(driver.NewSecrets(clientv1.Secrets(cr.GetNamespace()))) tillerKubeClient, err := client.NewFromManager(f.mgr) if err != nil { return nil, fmt.Errorf("failed to get client from manager: %s", err) } - releaseServer, err := getReleaseServer(r, storageBackend, tillerKubeClient) + releaseServer, err := getReleaseServer(cr, storageBackend, tillerKubeClient) if err != nil { return nil, fmt.Errorf("failed to get helm release server: %s", err) } @@ -80,18 +76,18 @@ func (f managerFactory) newManagerForCR(r *unstructured.Unstructured) (Manager, chartDir: f.chartDir, tiller: releaseServer, - releaseName: getReleaseName(r), - namespace: r.GetNamespace(), + releaseName: getReleaseName(cr), + namespace: cr.GetNamespace(), - spec: r.Object["spec"], - status: types.StatusFor(r), + spec: cr.Object["spec"], + status: types.StatusFor(cr), }, nil } // getReleaseServer creates a ReleaseServer configured with a rendering engine that adds ownerrefs to rendered assets // based on the CR. -func getReleaseServer(r *unstructured.Unstructured, storageBackend *storage.Storage, tillerKubeClient *kube.Client) (*tiller.ReleaseServer, error) { - controllerRef := metav1.NewControllerRef(r, r.GroupVersionKind()) +func getReleaseServer(cr *unstructured.Unstructured, storageBackend *storage.Storage, tillerKubeClient *kube.Client) (*tiller.ReleaseServer, error) { + controllerRef := metav1.NewControllerRef(cr, cr.GroupVersionKind()) ownerRefs := []metav1.OwnerReference{ *controllerRef, } @@ -117,8 +113,8 @@ func getReleaseServer(r *unstructured.Unstructured, storageBackend *storage.Stor return tiller.NewReleaseServer(env, cs, false), nil } -func getReleaseName(r *unstructured.Unstructured) string { - return fmt.Sprintf("%s-%s", r.GetName(), shortenUID(r.GetUID())) +func getReleaseName(cr *unstructured.Unstructured) string { + return fmt.Sprintf("%s-%s", cr.GetName(), shortenUID(cr.GetUID())) } func shortenUID(uid apitypes.UID) string { From 651df0d5606cd143c8cbe136ed8a8dbb5c9f0a48 Mon Sep 17 00:00:00 2001 From: Joe Lanford Date: Mon, 18 Mar 2019 11:26:21 -0400 Subject: [PATCH 7/7] CHANGELOG.md: moved 1102 change to Unreleased --- CHANGELOG.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ed3f74a141..29920de55c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,9 @@ ### Changed +- Updated the helm-operator to store release state in kubernetes secrets in the same namespace of the custom resource that defines the release. ([#1102](https://github.com/operator-framework/operator-sdk/pull/1102)) + - **WARNING**: Users with active CRs and releases who are upgrading their helm-based operator should not skip this version. Future versions will not seamlessly transition release state to the persistent backend, and will instead uninstall and reinstall all managed releases. + ### Deprecated ### Removed @@ -23,8 +26,6 @@ - Changed the Go, Helm, and Scorecard base images to `registry.access.redhat.com/ubi7-dev-preview/ubi-minimal:7.6` ([#1142](https://github.com/operator-framework/operator-sdk/pull/1142)) - CSV manifest are now versioned according to the `operator-registry` [manifest format](https://github.com/operator-framework/operator-registry#manifest-format). See issue [#900](https://github.com/operator-framework/operator-sdk/issues/900) for more details. ([#1016](https://github.com/operator-framework/operator-sdk/pull/1016)) - Unexported `CleanupNoT` function from `pkg/test`, as it is only intended to be used internally ([#1167](https://github.com/operator-framework/operator-sdk/pull/1167)) -- Updated the helm-operator to store release state in kubernetes secrets in the same namespace of the custom resource that defines the release. ([#1102](https://github.com/operator-framework/operator-sdk/pull/1102)) - - **WARNING**: Users with active CRs and releases who are upgrading their helm-based operator should not skip this version. Future versions will not seamlessly transition release state to the persistent backend, and will instead uninstall and reinstall all managed releases. ### Bug Fixes