fix(application): fix failed and sync failed check (#1773)

Co-authored-by: xdonggao <xdonggao@tencent.com>
tkestack · Feb 24, 2022 · 54d1a51 · 54d1a51
1 parent 9258bbe
commit 54d1a51
Show file tree

Hide file tree

Showing 7 changed files with 77 additions and 35 deletions.
diff --git a/api/application/types.go b/api/application/types.go
@@ -226,21 +226,18 @@ const (
 type AppPhase string
 
 const (
-	// ChartFetchedFailed means the chart to which the App
-	// refers could not be fetched.
-	AppPhaseChartFetchFailed AppPhase = "ChartFetchFailed"
-
 	// Installing means the installation for the App is running.
 	AppPhaseInstalling AppPhase = "Installing"
+	// InstallFailed means the installation for the App failed.
+	AppPhaseInstallFailed AppPhase = "InstallFailed"
 	// Upgrading means the upgrade for the App is running.
 	AppPhaseUpgrading AppPhase = "Upgrading"
 	// Succeeded means the dry-run, installation, or upgrade for the
 	// App succeeded.
 	AppPhaseSucceeded AppPhase = "Succeeded"
-	// Failed means the installation or upgrade for the App
+	// Failed means the upgrade for the App
 	// failed.
-	AppPhaseFailed AppPhase = "Failed"
-
+	AppPhaseUpgradFailed AppPhase = "UpgradFailed"
 	// RollingBack means a rollback for the App is running.
 	AppPhaseRollingBack AppPhase = "RollingBack"
 	// RolledBack means the App has been rolled back.

diff --git a/api/application/v1/types.go b/api/application/v1/types.go
@@ -226,21 +226,18 @@ const (
 type AppPhase string
 
 const (
-	// ChartFetchedFailed means the chart to which the App
-	// refers could not be fetched.
-	AppPhaseChartFetchFailed AppPhase = "ChartFetchFailed"
-
 	// Installing means the installation for the App is running.
 	AppPhaseInstalling AppPhase = "Installing"
+	// InstallFailed means the installation for the App failed.
+	AppPhaseInstallFailed AppPhase = "InstallFailed"
 	// Upgrading means the upgrade for the App is running.
 	AppPhaseUpgrading AppPhase = "Upgrading"
 	// Succeeded means the dry-run, installation, or upgrade for the
 	// App succeeded.
 	AppPhaseSucceeded AppPhase = "Succeeded"
-	// Failed means the installation or upgrade for the App
+	// Failed means the upgrade for the App
 	// failed.
-	AppPhaseFailed AppPhase = "Failed"
-
+	AppPhaseUpgradFailed AppPhase = "UpgradFailed"
 	// RollingBack means a rollback for the App is running.
 	AppPhaseRollingBack AppPhase = "RollingBack"
 	// RolledBack means the App has been rolled back.

diff --git a/pkg/application/controller/app/action/install.go b/pkg/application/controller/app/action/install.go
@@ -20,6 +20,7 @@ package action
 
 import (
 	"context"
+	"fmt"
 
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	applicationv1 "tkestack.io/tke/api/application/v1"
@@ -31,6 +32,7 @@ import (
 	applicationprovider "tkestack.io/tke/pkg/application/provider/application"
 	"tkestack.io/tke/pkg/application/util"
 	chartpath "tkestack.io/tke/pkg/application/util/chartpath/v1"
+	"tkestack.io/tke/pkg/util/log"
 )
 
 // Install installs a chart archive
@@ -51,7 +53,22 @@ func Install(ctx context.Context,
 	}
 	destfile, err := Pull(ctx, applicationClient, platformClient, app, repo, updateStatusFunc)
 	if err != nil {
-		return nil, err
+		newStatus := app.Status.DeepCopy()
+		if updateStatusFunc != nil {
+			if app.Status.Phase == applicationv1.AppPhaseInstallFailed {
+				log.Error(fmt.Sprintf("install app failed, helm pull err: %s", err.Error()))
+				// delayed retry, queue.AddRateLimited does not meet the demand
+				return app, nil
+			}
+			newStatus.Phase = applicationv1.AppPhaseInstallFailed
+			newStatus.Message = "fetch chart failed"
+			newStatus.Reason = err.Error()
+			newStatus.LastTransitionTime = metav1.Now()
+			_, updateStatusErr := updateStatusFunc(ctx, app, &app.Status, newStatus)
+			if updateStatusErr != nil {
+				return nil, updateStatusErr
+			}
+		}
 	}
 
 	newApp, err := applicationClient.Apps(app.Namespace).Get(ctx, app.Name, metav1.GetOptions{})
@@ -82,7 +99,12 @@ func Install(ctx context.Context,
 		newStatus := newApp.Status.DeepCopy()
 		var updateStatusErr error
 		if err != nil {
-			newStatus.Phase = applicationv1.AppPhaseFailed
+			if app.Status.Phase == applicationv1.AppPhaseInstallFailed {
+				log.Error(fmt.Sprintf("install app failed, helm install err: %s", err.Error()))
+				// delayed retry, queue.AddRateLimited does not meet the demand
+				return app, nil
+			}
+			newStatus.Phase = applicationv1.AppPhaseInstallFailed
 			newStatus.Message = "install app failed"
 			newStatus.Reason = err.Error()
 			newStatus.LastTransitionTime = metav1.Now()

diff --git a/pkg/application/controller/app/action/pull.go b/pkg/application/controller/app/action/pull.go
@@ -21,7 +21,6 @@ package action
 import (
 	"context"
 
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	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"
@@ -51,16 +50,5 @@ func Pull(ctx context.Context,
 	destfile, err := client.Pull(&helmaction.PullOptions{
 		ChartPathOptions: chartPathBasicOptions,
 	})
-	if updateStatusFunc != nil {
-		newStatus := app.Status.DeepCopy()
-		if err != nil {
-			newStatus.Phase = applicationv1.AppPhaseChartFetchFailed
-			newStatus.Message = "fetch chart failed"
-			newStatus.Reason = err.Error()
-			newStatus.LastTransitionTime = metav1.Now()
-			updateStatusFunc(ctx, app, &app.Status, newStatus)
-			return destfile, err
-		}
-	}
 	return destfile, err
 }
diff --git a/pkg/application/controller/app/action/upgrade.go b/pkg/application/controller/app/action/upgrade.go
@@ -20,6 +20,7 @@ package action
 
 import (
 	"context"
+	"fmt"
 
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	applicationv1 "tkestack.io/tke/api/application/v1"
@@ -31,6 +32,7 @@ import (
 	applicationprovider "tkestack.io/tke/pkg/application/provider/application"
 	"tkestack.io/tke/pkg/application/util"
 	chartpath "tkestack.io/tke/pkg/application/util/chartpath/v1"
+	"tkestack.io/tke/pkg/util/log"
 )
 
 // Upgrade upgrade a helm release
@@ -52,6 +54,19 @@ func Upgrade(ctx context.Context,
 
 	destfile, err := Pull(ctx, applicationClient, platformClient, app, repo, updateStatusFunc)
 	if err != nil {
+		newStatus := app.Status.DeepCopy()
+		if updateStatusFunc != nil {
+			if app.Status.Phase == applicationv1.AppPhaseUpgradFailed {
+				log.Error(fmt.Sprintf("upgrade app failed, helm pull err: %s", err.Error()))
+				// delayed retry, queue.AddRateLimited does not meet the demand
+				return app, nil
+			}
+			newStatus.Phase = applicationv1.AppPhaseUpgradFailed
+			newStatus.Message = "fetch chart failed"
+			newStatus.Reason = err.Error()
+			newStatus.LastTransitionTime = metav1.Now()
+			updateStatusFunc(ctx, app, &app.Status, newStatus)
+		}
 		return nil, err
 	}
 
@@ -85,7 +100,12 @@ func Upgrade(ctx context.Context,
 		newStatus := newApp.Status.DeepCopy()
 		var updateStatusErr error
 		if err != nil {
-			newStatus.Phase = applicationv1.AppPhaseFailed
+			if app.Status.Phase == applicationv1.AppPhaseUpgradFailed {
+				log.Error(fmt.Sprintf("upgrade app failed, helm upgrade err: %s", err.Error()))
+				// delayed retry, queue.AddRateLimited does not meet the demand
+				return app, nil
+			}
+			newStatus.Phase = applicationv1.AppPhaseUpgradFailed
 			newStatus.Message = "upgrade app failed"
 			newStatus.Reason = err.Error()
 			newStatus.LastTransitionTime = metav1.Now()

diff --git a/pkg/application/controller/app/app_controller.go b/pkg/application/controller/app/app_controller.go
@@ -157,6 +157,14 @@ func (c *Controller) needsUpdate(old *applicationv1.App, new *applicationv1.App)
 		return true
 	}
 
+	if new.Status.Phase == applicationv1.AppPhaseSyncFailed ||
+		new.Status.Phase == applicationv1.AppPhaseInstallFailed ||
+		new.Status.Phase == applicationv1.AppPhaseUpgradFailed ||
+		new.Status.Phase == applicationv1.AppPhaseSucceeded ||
+		new.Status.Phase == applicationv1.AppPhaseTerminating {
+		return true
+	}
+
 	return false
 }
 
@@ -326,11 +334,13 @@ func (c *Controller) handlePhase(ctx context.Context, key string, cachedApp *cac
 			return c.updateStatus(ctx, app, &app.Status, newStatus)
 		}
 		return action.Upgrade(ctx, c.client.ApplicationV1(), c.platformClient, app, c.repo, c.updateStatus)
+	case applicationv1.AppPhaseInstallFailed:
+		return action.Install(ctx, c.client.ApplicationV1(), c.platformClient, app, c.repo, c.updateStatus)
 	case applicationv1.AppPhaseSucceeded:
 		c.startAppHealthCheck(ctx, key)
 		// sync release status
 		return c.syncAppFromRelease(ctx, cachedApp, app)
-	case applicationv1.AppPhaseFailed:
+	case applicationv1.AppPhaseUpgradFailed:
 		return action.Upgrade(ctx, c.client.ApplicationV1(), c.platformClient, app, c.repo, c.updateStatus)
 	case applicationv1.AppPhaseRollingBack:
 		if app.Status.RollbackRevision > 0 {
@@ -341,18 +351,16 @@ func (c *Controller) handlePhase(ctx context.Context, key string, cachedApp *cac
 		return c.syncAppFromRelease(ctx, cachedApp, app)
 	case applicationv1.AppPhaseRollbackFailed:
 		break
-	case applicationv1.AppPhaseChartFetchFailed:
-		break
 	case applicationv1.AppPhaseSyncFailed:
-		break
+		return c.syncAppFromRelease(ctx, cachedApp, app)
 	default:
 		break
 	}
 	return app, nil
 }
 
 func (c *Controller) syncAppFromRelease(ctx context.Context, cachedApp *cachedApp, app *applicationv1.App) (*applicationv1.App, error) {
-	if app.Status.Phase == applicationv1.AppPhaseSucceeded && hasSynced(app) {
+	if hasSynced(app) {
 		return app, nil
 	}
 	defer func() {
@@ -363,6 +371,11 @@ func (c *Controller) syncAppFromRelease(ctx context.Context, cachedApp *cachedAp
 	newStatus := app.Status.DeepCopy()
 	rels, err := action.List(ctx, c.client.ApplicationV1(), c.platformClient, app)
 	if err != nil {
+		if app.Status.Phase == applicationv1.AppPhaseSyncFailed {
+			log.Error(fmt.Sprintf("sync app failed, helm list failed, err: %s", err.Error()))
+			// delayed retry, queue.AddRateLimited does not meet the demand
+			return app, nil
+		}
 		newStatus.Phase = applicationv1.AppPhaseSyncFailed
 		newStatus.Message = "sync app failed"
 		newStatus.Reason = err.Error()
@@ -371,6 +384,11 @@ func (c *Controller) syncAppFromRelease(ctx context.Context, cachedApp *cachedAp
 	}
 	rel, found := helmutil.Filter(rels, app.Spec.TargetNamespace, app.Spec.Name)
 	if !found {
+		if app.Status.Phase == applicationv1.AppPhaseSyncFailed {
+			log.Error(fmt.Sprintf("sync app failed, release not found: %s/%s", app.Spec.TargetNamespace, app.Spec.Name))
+			// delayed retry, queue.AddRateLimited does not meet the demand
+			return app, nil
+		}
 		newStatus.Phase = applicationv1.AppPhaseSyncFailed
 		newStatus.Message = "sync app failed"
 		newStatus.Reason = fmt.Sprintf("release not found: %s/%s", app.Spec.TargetNamespace, app.Spec.Name)

diff --git a/pkg/application/controller/app/deletion/app_resources_deleter.go b/pkg/application/controller/app/deletion/app_resources_deleter.go
@@ -289,7 +289,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") {
+		if strings.Contains(err.Error(), "release: not found") || errors.IsNotFound(err) {
 			log.Warn(err.Error())
 			return nil
 		}