Skip to content

Commit

Permalink
Removal of ability import legacy artifact into Helm.
Browse files Browse the repository at this point in the history
A helm install would make attempts to find manually installed artifacts
and make them managed by Helm by adding the necessary labels and
annotations. This was important when the Helm chart was first introduced
but is far less so today, and this legacy import was therefore removed.
Maintaining it properly would mean adding regression tests and to fix
pending issues

Signed-off-by: Thomas Hallgren <[email protected]>
  • Loading branch information
thallgren committed May 3, 2024
1 parent 5d89dc1 commit 808ad8c
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 210 deletions.
6 changes: 6 additions & 0 deletions CHANGELOG.yml
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,12 @@ items:
- version: 2.19.0
date: (TBD)
notes:
- type: change
title: Removal of ability import legacy artifact into Helm.
body: >-
A helm install would make attempts to find manually installed artifacts and make them managed by
Helm by adding the necessary labels and annotations. This was important when the Helm chart was first
introduced but is far less so today, and this legacy import was therefore removed.
- type: bugfix
title: Docker aliases deprecation caused failure to detect Kind cluster.
body: >-
Expand Down
37 changes: 10 additions & 27 deletions pkg/client/cli/helm/install.go
Original file line number Diff line number Diff line change
Expand Up @@ -305,21 +305,14 @@ func isInstalled(ctx context.Context, clientGetter genericclioptions.RESTClientG
return existing, helmConfig, nil
}

func EnsureTrafficManager(ctx context.Context, clientGetter genericclioptions.RESTClientGetter, namespace string, req *Request) error {
func EnsureTrafficManager(ctx context.Context, clientGetter genericclioptions.RESTClientGetter, namespace string, req *Request) (err error) {
if req.Crds {
dlog.Debug(ctx, "loading build-in helm chart")
err := ensureIsInstalled(ctx, clientGetter, true, crdReleaseName, namespace, req)
if err != nil {
return fmt.Errorf("failed to install traffic manager CRDs: %w", err)
}
return nil
}
err := ensureIsInstalled(ctx, clientGetter, false, trafficManagerReleaseName, namespace, req)
if err != nil {
return fmt.Errorf("failed to install traffic manager: %w", err)
err = ensureIsInstalled(ctx, clientGetter, true, crdReleaseName, namespace, req)
} else {
err = ensureIsInstalled(ctx, clientGetter, false, trafficManagerReleaseName, namespace, req)
}

return nil
return err
}

// EnsureTrafficManager ensures the traffic manager is installed.
Expand All @@ -329,7 +322,7 @@ func ensureIsInstalled(
) error {
existing, helmConfig, err := isInstalled(ctx, clientGetter, releaseName, namespace)
if err != nil {
return fmt.Errorf("err detecting %s: %w", releaseName, err)
return err
}

// Under various conditions, helm can leave the release history hanging around after the release is gone.
Expand All @@ -352,7 +345,7 @@ func ensureIsInstalled(
var providedVals map[string]any
if len(req.ValuesJson) > 0 {
if err := json.Unmarshal(req.ValuesJson, &providedVals); err != nil {
return fmt.Errorf("unable to parse values JSON: %w", err)
return errcat.User.Newf("unable to parse values JSON: %w", err)
}
}

Expand Down Expand Up @@ -381,19 +374,9 @@ func ensureIsInstalled(
}

switch {
case existing == nil: // fresh install
// Only the traffic manager release has a legacy version.
if releaseName == trafficManagerReleaseName {
dlog.Debugf(ctx, "Importing legacy for namespace %s", namespace)
if err := importLegacy(ctx, releaseName, namespace); err != nil {
// Similarly to the error check for getHelmRelease, this could happen because of missing permissions,
// or a different k8s error. We don't want to block on permissions failures, so let's log and hope.
dlog.Errorf(ctx, "ensureIsInstalled(namespace=%q): unable to import existing k8s resources: %v. Assuming traffic-manager is setup and continuing...",
namespace, err)
return nil
}
}

case existing == nil && req.Type == Upgrade: // fresh install
err = errcat.User.Newf("%s is not installed, use 'telepresence helm install' to install it", releaseName)
case existing == nil:
dlog.Infof(ctx, "ensureIsInstalled(namespace=%q): performing fresh install...", namespace)
err = installNew(ctx, chrt, helmConfig, releaseName, namespace, req, vals)
case req.Type == Upgrade: // replace existing install
Expand Down
183 changes: 0 additions & 183 deletions pkg/client/cli/helm/legacy.go

This file was deleted.

0 comments on commit 808ad8c

Please sign in to comment.