Skip to content

Commit

Permalink
fix(application): app install/upgrade failed when release pending (tk…
Browse files Browse the repository at this point in the history
…estack#2287)

Co-authored-by: xdonggao <[email protected]>
  • Loading branch information
GaoXiaodong and xdonggao committed May 18, 2023
1 parent 07a73f8 commit 45c2bf7
Show file tree
Hide file tree
Showing 4 changed files with 43 additions and 15 deletions.
5 changes: 3 additions & 2 deletions pkg/application/controller/app/action/uninstall.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,10 @@ package action

import (
"context"
"strings"
"errors"

"helm.sh/helm/v3/pkg/release"
"helm.sh/helm/v3/pkg/storage/driver"
applicationv1 "tkestack.io/tke/api/application/v1"
applicationversionedclient "tkestack.io/tke/api/client/clientset/versioned/typed/application/v1"
platformversionedclient "tkestack.io/tke/api/client/clientset/versioned/typed/platform/v1"
Expand Down Expand Up @@ -52,7 +53,7 @@ func Uninstall(ctx context.Context,
Timeout: defaultTimeout,
})

if err != nil && !strings.Contains(err.Error(), "release: not found") {
if err != nil && !errors.Is(err, driver.ErrReleaseNotFound) {
return resp, err
}

Expand Down
21 changes: 11 additions & 10 deletions pkg/application/controller/app/deletion/app_resources_deleter.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,12 @@ package deletion

import (
"context"
"errors"
"fmt"
"reflect"
"strings"

"k8s.io/apimachinery/pkg/api/errors"
"helm.sh/helm/v3/pkg/storage/driver"
k8serrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
utilerrors "k8s.io/apimachinery/pkg/util/errors"
"k8s.io/apimachinery/pkg/util/sets"
Expand Down Expand Up @@ -74,7 +75,7 @@ type applicationResourcesDeleter struct {
finalizerToken applicationv1.FinalizerName
// Also delete the app when all resources in the app have been deleted.
deleteAppWhenDone bool
//RepoConfiguration contains options to connect to a chart repo.
// RepoConfiguration contains options to connect to a chart repo.
repo appconfig.RepoConfiguration
}

Expand All @@ -98,7 +99,7 @@ func (d *applicationResourcesDeleter) Delete(ctx context.Context, namespace, app
// if the app was deleted already, don't do anything
app, err := d.applicationClient.Apps(namespace).Get(ctx, appName, metav1.GetOptions{})
if err != nil {
if errors.IsNotFound(err) {
if k8serrors.IsNotFound(err) {
return nil
}
return err
Expand All @@ -113,7 +114,7 @@ func (d *applicationResourcesDeleter) Delete(ctx context.Context, namespace, app
// if we get a not found error, we assume the app is truly gone
app, err = d.retryOnConflictError(ctx, app, d.updateApplicationStatusFunc)
if err != nil {
if errors.IsNotFound(err) {
if k8serrors.IsNotFound(err) {
return nil
}
return err
Expand Down Expand Up @@ -141,7 +142,7 @@ func (d *applicationResourcesDeleter) Delete(ctx context.Context, namespace, app
// in normal practice, this should not be possible, but if a deployment is running
// two controllers to do app deletion that share a common finalizer token it's
// possible that a not found could occur since the other controller would have finished the delete.
if errors.IsNotFound(err) {
if k8serrors.IsNotFound(err) {
return nil
}
return err
Expand All @@ -162,7 +163,7 @@ func (d *applicationResourcesDeleter) deleteApplication(ctx context.Context, app
opts = metav1.DeleteOptions{Preconditions: &metav1.Preconditions{UID: &uid}}
}
err := d.applicationClient.Apps(app.Namespace).Delete(ctx, app.Name, opts)
if err != nil && !errors.IsNotFound(err) {
if err != nil && !k8serrors.IsNotFound(err) {
return err
}
return nil
Expand All @@ -181,7 +182,7 @@ func (d *applicationResourcesDeleter) retryOnConflictError(ctx context.Context,
if err == nil {
return result, nil
}
if !errors.IsConflict(err) {
if !k8serrors.IsConflict(err) {
return nil, err
}
prevApplication := latestApplication
Expand Down Expand Up @@ -244,7 +245,7 @@ func (d *applicationResourcesDeleter) finalizeApplication(ctx context.Context, a

if err != nil {
// it was removed already, so life is good
if errors.IsNotFound(err) {
if k8serrors.IsNotFound(err) {
return app, nil
}
}
Expand Down Expand Up @@ -289,7 +290,7 @@ func deleteApplication(ctx context.Context,
repo appconfig.RepoConfiguration) error {
_, err := action.Uninstall(ctx, deleter.applicationClient, deleter.platformClient, app, repo)
if err != nil {
if strings.Contains(err.Error(), "release: not found") || errors.IsNotFound(err) {
if errors.Is(err, driver.ErrReleaseNotFound) || k8serrors.IsNotFound(err) {
log.Warn(err.Error())
return nil
}
Expand Down
20 changes: 20 additions & 0 deletions pkg/application/helm/action/install.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import (
"helm.sh/helm/v3/pkg/chart/loader"
"helm.sh/helm/v3/pkg/getter"
"helm.sh/helm/v3/pkg/release"
"helm.sh/helm/v3/pkg/storage/driver"
"tkestack.io/tke/pkg/util/file"
"tkestack.io/tke/pkg/util/log"
)
Expand Down Expand Up @@ -99,6 +100,25 @@ func (c *Client) InstallWithLocal(ctx context.Context, options *InstallOptions,
return nil, err
}

histClient := action.NewHistory(actionConfig)
histClient.Max = 1
rels, err := histClient.Run(options.ReleaseName)
if err != nil {
if errors.Is(err, driver.ErrReleaseNotFound) {
return nil, err
}
} else {
for _, rel := range rels {
if rel.Info.Status == release.StatusDeployed {
// if release status is deployed, ignore it
log.Infof("Release %s is already exist. igonre it now.", options.ReleaseName)
return nil, nil
}
// if release status is other, delete it
log.Infof("install release %s is already exist, status is %s. delete it now.", options.ReleaseName, rel.Info.Status)
actionConfig.Releases.Delete(rel.Name, rel.Version)
}
}
client := action.NewInstall(actionConfig)
client.DryRun = options.DryRun
client.DependencyUpdate = options.DependencyUpdate
Expand Down
12 changes: 9 additions & 3 deletions pkg/application/helm/action/upgrade.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ import (
"helm.sh/helm/v3/pkg/getter"
"helm.sh/helm/v3/pkg/release"
"helm.sh/helm/v3/pkg/storage/driver"

"tkestack.io/tke/pkg/util/file"
"tkestack.io/tke/pkg/util/log"
)
Expand Down Expand Up @@ -78,8 +77,8 @@ func (c *Client) Upgrade(ctx context.Context, options *UpgradeOptions) (*release
// If a release does not exist, install it.
histClient := action.NewHistory(actionConfig)
histClient.Max = 1
_, err = histClient.Run(options.ReleaseName)
if err == driver.ErrReleaseNotFound {
rels, err := histClient.Run(options.ReleaseName)
if errors.Is(err, driver.ErrReleaseNotFound) {
log.Infof("Release %d does not exist. Installing it now.", options.ReleaseName)
return c.Install(ctx, &InstallOptions{
DryRun: options.DryRun,
Expand All @@ -97,6 +96,13 @@ func (c *Client) Upgrade(ctx context.Context, options *UpgradeOptions) (*release
} else if err != nil {
return nil, err
}
for _, rel := range rels {
if rel.Info.Status == release.StatusPendingInstall || rel.Info.Status == release.StatusPendingUpgrade || rel.Info.Status == release.StatusPendingRollback {
// if release status is pending, delete it
log.Infof("upgrade release %s is already exist, status is %s. delete it now.", options.ReleaseName, rel.Info.Status)
actionConfig.Releases.Delete(rel.Name, rel.Version)
}
}
}

client := action.NewUpgrade(actionConfig)
Expand Down

0 comments on commit 45c2bf7

Please sign in to comment.