From edabff2a8e0eaffb6add5b924f2297a174297d60 Mon Sep 17 00:00:00 2001 From: Mario Manno Date: Fri, 13 Sep 2024 14:13:21 +0200 Subject: [PATCH 1/3] Helmdeployer, rename local variables --- .../controller/bundledeployment_controller.go | 1 + internal/cmd/agent/deployer/deployer.go | 16 ++++++++-------- internal/helmdeployer/deployer.go | 5 ++++- internal/helmdeployer/install.go | 4 ++-- .../v1alpha1/bundledeployment_types.go | 1 + 5 files changed, 16 insertions(+), 11 deletions(-) diff --git a/internal/cmd/agent/controller/bundledeployment_controller.go b/internal/cmd/agent/controller/bundledeployment_controller.go index 75bec7e4fe..f1e54fbbdb 100644 --- a/internal/cmd/agent/controller/bundledeployment_controller.go +++ b/internal/cmd/agent/controller/bundledeployment_controller.go @@ -136,6 +136,7 @@ func (r *BundleDeploymentReconciler) Reconcile(ctx context.Context, req ctrl.Req bd.Status = setCondition(status, nil, condition.Cond(fleetv1.BundleDeploymentConditionDeployed)) } + // retrieve the resources from the helm history. // if we can't retrieve the resources, we don't need to try any of the other operations and requeue now resources, err := r.Deployer.Resources(bd.Name, bd.Status.Release) if err != nil { diff --git a/internal/cmd/agent/deployer/deployer.go b/internal/cmd/agent/deployer/deployer.go index bf4bcbfd80..afcac10a29 100644 --- a/internal/cmd/agent/deployer/deployer.go +++ b/internal/cmd/agent/deployer/deployer.go @@ -43,8 +43,8 @@ func New(localClient client.Client, upstreamClient client.Reader, lookup Lookup, } } -func (d *Deployer) Resources(name string, release string) (*helmdeployer.Resources, error) { - return d.helm.Resources(name, release) +func (d *Deployer) Resources(name string, releaseID string) (*helmdeployer.Resources, error) { + return d.helm.Resources(name, releaseID) } func (d *Deployer) RemoveExternalChanges(ctx context.Context, bd *fleet.BundleDeployment) (string, error) { @@ -62,7 +62,7 @@ func (d *Deployer) DeployBundle(ctx context.Context, bd *fleet.BundleDeployment) return status, err } - release, err := d.helmdeploy(ctx, bd) + releaseID, err := d.helmdeploy(ctx, bd) if err != nil { // When an error from DeployBundle is returned it causes DeployBundle // to requeue and keep trying to deploy on a loop. If there is something @@ -83,11 +83,11 @@ func (d *Deployer) DeployBundle(ctx context.Context, bd *fleet.BundleDeployment) } return status, err } - status.Release = release + status.Release = releaseID status.AppliedDeploymentID = bd.Spec.DeploymentID - logger.Info("Deployed bundle", "release", release, "appliedDeploymentID", status.AppliedDeploymentID) + logger.Info("Deployed bundle", "release", releaseID, "appliedDeploymentID", status.AppliedDeploymentID) - if err := d.setNamespaceLabelsAndAnnotations(ctx, bd, release); err != nil { + if err := d.setNamespaceLabelsAndAnnotations(ctx, bd, releaseID); err != nil { return fleet.BundleDeploymentStatus{}, err } @@ -152,12 +152,12 @@ func (d *Deployer) helmdeploy(ctx context.Context, bd *fleet.BundleDeployment) ( } manifest.Commit = bd.Labels["fleet.cattle.io/commit"] - resource, err := d.helm.Deploy(ctx, bd.Name, manifest, bd.Spec.Options) + resources, err := d.helm.Deploy(ctx, bd.Name, manifest, bd.Spec.Options) if err != nil { return "", err } - return resource.ID, nil + return resources.ID, nil } // setNamespaceLabelsAndAnnotations updates the namespace for the release, applying all labels and annotations to that namespace as configured in the bundle spec. diff --git a/internal/helmdeployer/deployer.go b/internal/helmdeployer/deployer.go index f3f2505ee2..a3fe8a7aeb 100644 --- a/internal/helmdeployer/deployer.go +++ b/internal/helmdeployer/deployer.go @@ -51,8 +51,11 @@ type Helm struct { labelSuffix string } +// Resources contains information from a helm release type Resources struct { - ID string `json:"id,omitempty"` + // ID of the helm release + ID string `json:"id,omitempty"` + // DefaultNamespace is the namespace of the helm release DefaultNamespace string `json:"defaultNamespace,omitempty"` Objects []runtime.Object `json:"objects,omitempty"` } diff --git a/internal/helmdeployer/install.go b/internal/helmdeployer/install.go index 5feb03dfe3..0f8a7578a8 100644 --- a/internal/helmdeployer/install.go +++ b/internal/helmdeployer/install.go @@ -53,10 +53,10 @@ func (h *Helm) Deploy(ctx context.Context, bundleID string, manifest *manifest.M chart.Metadata.Annotations[CommitAnnotation] = manifest.Commit } - if resources, err := h.install(ctx, bundleID, manifest, chart, options, true); err != nil { + if release, err := h.install(ctx, bundleID, manifest, chart, options, true); err != nil { return nil, err } else if h.template { - return releaseToResources(resources) + return releaseToResources(release) } release, err := h.install(ctx, bundleID, manifest, chart, options, false) diff --git a/pkg/apis/fleet.cattle.io/v1alpha1/bundledeployment_types.go b/pkg/apis/fleet.cattle.io/v1alpha1/bundledeployment_types.go index 4062a2c968..08023783e7 100644 --- a/pkg/apis/fleet.cattle.io/v1alpha1/bundledeployment_types.go +++ b/pkg/apis/fleet.cattle.io/v1alpha1/bundledeployment_types.go @@ -336,6 +336,7 @@ type BundleDeploymentStatus struct { Conditions []genericcondition.GenericCondition `json:"conditions,omitempty"` // +nullable AppliedDeploymentID string `json:"appliedDeploymentID,omitempty"` + // Release is the Helm release ID // +nullable Release string `json:"release,omitempty"` Ready bool `json:"ready,omitempty"` From d176b384a61c75114f9e02024573f3f2360bdef1 Mon Sep 17 00:00:00 2001 From: Mario Manno Date: Fri, 13 Sep 2024 14:44:11 +0200 Subject: [PATCH 2/3] Fleet CLI deploy output matches dry-run output --- charts/fleet-crd/templates/crds.yaml | 1 + integrationtests/cli/deploy/deploy_test.go | 8 ++++---- internal/cmd/agent/deployer/deployer.go | 4 ++-- internal/cmd/cli/deploy.go | 9 +++++++-- internal/helmdeployer/history.go | 13 +++++++++++-- internal/helmdeployer/install.go | 11 +++-------- internal/helmdeployer/rollback.go | 2 +- internal/helmdeployer/template.go | 2 +- 8 files changed, 30 insertions(+), 20 deletions(-) diff --git a/charts/fleet-crd/templates/crds.yaml b/charts/fleet-crd/templates/crds.yaml index 2438b825c0..660d11c19e 100644 --- a/charts/fleet-crd/templates/crds.yaml +++ b/charts/fleet-crd/templates/crds.yaml @@ -974,6 +974,7 @@ spec: ready: type: boolean release: + description: Release is the Helm release ID nullable: true type: string resources: diff --git a/integrationtests/cli/deploy/deploy_test.go b/integrationtests/cli/deploy/deploy_test.go index 0096586d09..6216502850 100644 --- a/integrationtests/cli/deploy/deploy_test.go +++ b/integrationtests/cli/deploy/deploy_test.go @@ -115,9 +115,9 @@ var _ = Describe("Fleet CLI Deploy", func() { It("creates resources", func() { buf, err := act(args) Expect(err).NotTo(HaveOccurred()) - Expect(buf).To(gbytes.Say("defaultNamespace: default")) - Expect(buf).To(gbytes.Say("objects")) Expect(buf).To(gbytes.Say("- apiVersion: v1")) + Expect(buf).To(gbytes.Say(" data:")) + Expect(buf).To(gbytes.Say(" name: example-value")) cm := &corev1.ConfigMap{} err = k8sClient.Get(ctx, types.NamespacedName{Namespace: "default", Name: "test-simple-chart-config"}, cm) @@ -136,9 +136,9 @@ var _ = Describe("Fleet CLI Deploy", func() { It("creates resources", func() { buf, err := act(args) Expect(err).NotTo(HaveOccurred()) - Expect(buf).To(gbytes.Say("defaultNamespace: " + namespace)) - Expect(buf).To(gbytes.Say("objects")) Expect(buf).To(gbytes.Say("- apiVersion: v1")) + Expect(buf).To(gbytes.Say(" data:")) + Expect(buf).To(gbytes.Say(" name: example-value")) cm := &corev1.ConfigMap{} err = k8sClient.Get(ctx, types.NamespacedName{Namespace: namespace, Name: "test-simple-chart-config"}, cm) diff --git a/internal/cmd/agent/deployer/deployer.go b/internal/cmd/agent/deployer/deployer.go index afcac10a29..9462b7cf24 100644 --- a/internal/cmd/agent/deployer/deployer.go +++ b/internal/cmd/agent/deployer/deployer.go @@ -152,12 +152,12 @@ func (d *Deployer) helmdeploy(ctx context.Context, bd *fleet.BundleDeployment) ( } manifest.Commit = bd.Labels["fleet.cattle.io/commit"] - resources, err := d.helm.Deploy(ctx, bd.Name, manifest, bd.Spec.Options) + release, err := d.helm.Deploy(ctx, bd.Name, manifest, bd.Spec.Options) if err != nil { return "", err } - return resources.ID, nil + return helmdeployer.ReleaseToResourceID(release), nil } // setNamespaceLabelsAndAnnotations updates the namespace for the release, applying all labels and annotations to that namespace as configured in the bundle spec. diff --git a/internal/cmd/cli/deploy.go b/internal/cmd/cli/deploy.go index 2f21d1ae99..be4334c510 100644 --- a/internal/cmd/cli/deploy.go +++ b/internal/cmd/cli/deploy.go @@ -159,12 +159,17 @@ func (d *Deploy) Run(cmd *cobra.Command, args []string) error { return err } - resources, err := deployer.Deploy(ctx, bd.Name, manifest, bd.Spec.Options) + release, err := deployer.Deploy(ctx, bd.Name, manifest, bd.Spec.Options) if err != nil { return err } - b, err = yaml.Marshal(resources) + objects, err := helmdeployer.ReleaseToObjects(release) + if err != nil { + return err + } + + b, err = yaml.Marshal(objects) if err != nil { return err } diff --git a/internal/helmdeployer/history.go b/internal/helmdeployer/history.go index 00edeaabe2..c16fb7e898 100644 --- a/internal/helmdeployer/history.go +++ b/internal/helmdeployer/history.go @@ -8,6 +8,7 @@ import ( "helm.sh/helm/v3/pkg/action" "helm.sh/helm/v3/pkg/release" "helm.sh/helm/v3/pkg/storage/driver" + "k8s.io/apimachinery/pkg/runtime" "github.com/rancher/wrangler/v3/pkg/kv" "github.com/rancher/wrangler/v3/pkg/yaml" @@ -94,17 +95,25 @@ func (h *Helm) getRelease(releaseName, namespace string, version int) (*release. return nil, ErrNoRelease } -func releaseToResourceID(release *release.Release) string { +func ReleaseToResourceID(release *release.Release) string { return fmt.Sprintf("%s/%s:%d", release.Namespace, release.Name, release.Version) } +func ReleaseToObjects(release *release.Release) ([]runtime.Object, error) { + var ( + err error + ) + objs, err := yaml.ToObjects(bytes.NewBufferString(release.Manifest)) + return objs, err +} + func releaseToResources(release *release.Release) (*Resources, error) { var ( err error ) resources := &Resources{ DefaultNamespace: release.Namespace, - ID: releaseToResourceID(release), + ID: ReleaseToResourceID(release), } resources.Objects, err = yaml.ToObjects(bytes.NewBufferString(release.Manifest)) diff --git a/internal/helmdeployer/install.go b/internal/helmdeployer/install.go index 0f8a7578a8..01e78ff47a 100644 --- a/internal/helmdeployer/install.go +++ b/internal/helmdeployer/install.go @@ -23,7 +23,7 @@ import ( ) // Deploy deploys an unpacked content resource with helm. bundleID is the name of the bundledeployment. -func (h *Helm) Deploy(ctx context.Context, bundleID string, manifest *manifest.Manifest, options fleet.BundleDeploymentOptions) (*Resources, error) { +func (h *Helm) Deploy(ctx context.Context, bundleID string, manifest *manifest.Manifest, options fleet.BundleDeploymentOptions) (*release.Release, error) { if options.Helm == nil { options.Helm = &fleet.HelmOptions{} } @@ -56,15 +56,10 @@ func (h *Helm) Deploy(ctx context.Context, bundleID string, manifest *manifest.M if release, err := h.install(ctx, bundleID, manifest, chart, options, true); err != nil { return nil, err } else if h.template { - return releaseToResources(release) + return release, nil } - release, err := h.install(ctx, bundleID, manifest, chart, options, false) - if err != nil { - return nil, err - } - - return releaseToResources(release) + return h.install(ctx, bundleID, manifest, chart, options, false) } // install runs helm install or upgrade and supports dry running the action. Will run helm rollback in case of a failed upgrade. diff --git a/internal/helmdeployer/rollback.go b/internal/helmdeployer/rollback.go index 6ea00ac934..433461a0cc 100644 --- a/internal/helmdeployer/rollback.go +++ b/internal/helmdeployer/rollback.go @@ -47,7 +47,7 @@ func (h *Helm) RemoveExternalChanges(ctx context.Context, bd *fleet.BundleDeploy if err != nil { return "", err } - return releaseToResourceID(release), nil + return ReleaseToResourceID(release), nil } func removeFailedRollback(cfg action.Configuration, currentRelease *release.Release, err error) error { diff --git a/internal/helmdeployer/template.go b/internal/helmdeployer/template.go index 79c0703a57..3ea3c87121 100644 --- a/internal/helmdeployer/template.go +++ b/internal/helmdeployer/template.go @@ -57,5 +57,5 @@ func Template(ctx context.Context, bundleID string, manifest *manifest.Manifest, return nil, err } - return resources.Objects, nil + return ReleaseToObjects(resources) } From b57e51be048c05c35750730492453df79b621499 Mon Sep 17 00:00:00 2001 From: Mario Manno Date: Fri, 13 Sep 2024 15:27:45 +0200 Subject: [PATCH 3/3] Simplify helmdeployer history --- internal/helmdeployer/deployer.go | 2 -- internal/helmdeployer/history.go | 28 ++++++++++------------------ internal/helmdeployer/template.go | 4 ++-- 3 files changed, 12 insertions(+), 22 deletions(-) diff --git a/internal/helmdeployer/deployer.go b/internal/helmdeployer/deployer.go index a3fe8a7aeb..1ad85ca5b7 100644 --- a/internal/helmdeployer/deployer.go +++ b/internal/helmdeployer/deployer.go @@ -53,8 +53,6 @@ type Helm struct { // Resources contains information from a helm release type Resources struct { - // ID of the helm release - ID string `json:"id,omitempty"` // DefaultNamespace is the namespace of the helm release DefaultNamespace string `json:"defaultNamespace,omitempty"` Objects []runtime.Object `json:"objects,omitempty"` diff --git a/internal/helmdeployer/history.go b/internal/helmdeployer/history.go index c16fb7e898..4030e19e85 100644 --- a/internal/helmdeployer/history.go +++ b/internal/helmdeployer/history.go @@ -41,7 +41,10 @@ func (h *Helm) Resources(bundleID, resourcesID string) (*Resources, error) { } else if err != nil { return nil, err } - return releaseToResources(release) + + resources := &Resources{DefaultNamespace: release.Namespace} + resources.Objects, err = ReleaseToObjects(release) + return resources, err } func (h *Helm) ResourcesFromPreviousReleaseVersion(bundleID, resourcesID string) (*Resources, error) { @@ -56,7 +59,10 @@ func (h *Helm) ResourcesFromPreviousReleaseVersion(bundleID, resourcesID string) } else if err != nil { return nil, err } - return releaseToResources(release) + + resources := &Resources{DefaultNamespace: release.Namespace} + resources.Objects, err = ReleaseToObjects(release) + return resources, err } func getReleaseNameVersionAndNamespace(bundleID, resourcesID string) (string, int, string, error) { @@ -100,22 +106,8 @@ func ReleaseToResourceID(release *release.Release) string { } func ReleaseToObjects(release *release.Release) ([]runtime.Object, error) { - var ( - err error - ) + var err error + objs, err := yaml.ToObjects(bytes.NewBufferString(release.Manifest)) return objs, err } - -func releaseToResources(release *release.Release) (*Resources, error) { - var ( - err error - ) - resources := &Resources{ - DefaultNamespace: release.Namespace, - ID: ReleaseToResourceID(release), - } - - resources.Objects, err = yaml.ToObjects(bytes.NewBufferString(release.Manifest)) - return resources, err -} diff --git a/internal/helmdeployer/template.go b/internal/helmdeployer/template.go index 3ea3c87121..4eef67daef 100644 --- a/internal/helmdeployer/template.go +++ b/internal/helmdeployer/template.go @@ -52,10 +52,10 @@ func Template(ctx context.Context, bundleID string, manifest *manifest.Manifest, h.globalCfg.Log = logrus.Infof h.globalCfg.Releases = storage.Init(mem) - resources, err := h.Deploy(ctx, bundleID, manifest, options) + release, err := h.Deploy(ctx, bundleID, manifest, options) if err != nil { return nil, err } - return ReleaseToObjects(resources) + return ReleaseToObjects(release) }