diff --git a/controller/health.go b/controller/health.go index 81a33753a0946..6b68cfc054cca 100644 --- a/controller/health.go +++ b/controller/health.go @@ -60,30 +60,54 @@ func (ctrl *kubeAppHealthManager) getDeploymentHealth(config *rest.Config, names if err != nil { return nil, err } - deploy, err := clientSet.AppsV1().Deployments(namespace).Get(name, metav1.GetOptions{}) + deployment, err := clientSet.AppsV1().Deployments(namespace).Get(name, metav1.GetOptions{}) if err != nil { return nil, err } - health := appv1.HealthStatus{ - Status: appv1.HealthStatusUnknown, - } - for _, condition := range deploy.Status.Conditions { - // deployment is healthy is it successfully progressed - if condition.Type == v1.DeploymentProgressing && condition.Status == "True" { - health.Status = appv1.HealthStatusHealthy - } else if condition.Type == v1.DeploymentReplicaFailure && condition.Status == "True" { - health.Status = appv1.HealthStatusDegraded - } else if condition.Type == v1.DeploymentProgressing && condition.Status == "False" { - health.Status = appv1.HealthStatusDegraded - } else if condition.Type == v1.DeploymentAvailable && condition.Status == "False" { - health.Status = appv1.HealthStatusDegraded + + if deployment.Generation <= deployment.Status.ObservedGeneration { + cond := getDeploymentCondition(deployment.Status, v1.DeploymentProgressing) + if cond != nil && cond.Reason == "ProgressDeadlineExceeded" { + return &appv1.HealthStatus{ + Status: appv1.HealthStatusDegraded, + StatusDetails: fmt.Sprintf("Deployment %q exceeded its progress deadline", name), + }, nil + } else if deployment.Spec.Replicas != nil && deployment.Status.UpdatedReplicas < *deployment.Spec.Replicas { + return &appv1.HealthStatus{ + Status: appv1.HealthStatusProgressing, + StatusDetails: fmt.Sprintf("Waiting for rollout to finish: %d out of %d new replicas have been updated...\n", deployment.Status.UpdatedReplicas, *deployment.Spec.Replicas), + }, nil + } else if deployment.Status.Replicas > deployment.Status.UpdatedReplicas { + return &appv1.HealthStatus{ + Status: appv1.HealthStatusProgressing, + StatusDetails: fmt.Sprintf("Waiting for rollout to finish: %d old replicas are pending termination...\n", deployment.Status.Replicas-deployment.Status.UpdatedReplicas), + }, nil + } else if deployment.Status.AvailableReplicas < deployment.Status.UpdatedReplicas { + return &appv1.HealthStatus{ + Status: appv1.HealthStatusProgressing, + StatusDetails: fmt.Sprintf("Waiting for rollout to finish: %d of %d updated replicas are available...\n", deployment.Status.AvailableReplicas, deployment.Status.UpdatedReplicas), + }, nil } - if health.Status != appv1.HealthStatusUnknown { - health.StatusDetails = fmt.Sprintf("%s:%s", condition.Reason, condition.Message) - break + } else { + return &appv1.HealthStatus{ + Status: appv1.HealthStatusProgressing, + StatusDetails: "Waiting for rollout to finish: observed deployment generation less then desired generation", + }, nil + } + + return &appv1.HealthStatus{ + Status: appv1.HealthStatusHealthy, + }, nil +} + +func getDeploymentCondition(status v1.DeploymentStatus, condType v1.DeploymentConditionType) *v1.DeploymentCondition { + for i := range status.Conditions { + c := status.Conditions[i] + if c.Type == condType { + return &c } } - return &health, nil + return nil } func (ctrl *kubeAppHealthManager) GetAppHealth(server string, namespace string, comparisonResult *appv1.ComparisonResult) (*appv1.HealthStatus, error) { diff --git a/examples/guestbook/components/guestbook-ui.jsonnet b/examples/guestbook/components/guestbook-ui.jsonnet index 5e1c81978e397..17c3e3a1039f7 100644 --- a/examples/guestbook/components/guestbook-ui.jsonnet +++ b/examples/guestbook/components/guestbook-ui.jsonnet @@ -23,7 +23,7 @@ local appDeployment = deployment params.replicas, container .new(params.name, params.image) - .withPorts(containerPort.new(targetPort)), + .withPorts(containerPort.new(targetPort)) + if params.command != null then { command: [ params.command ] } else {}, labels); k.core.v1.list.new([appService, appDeployment]) \ No newline at end of file diff --git a/examples/guestbook/components/params.libsonnet b/examples/guestbook/components/params.libsonnet index e571709dc80df..de15be98ebcc0 100644 --- a/examples/guestbook/components/params.libsonnet +++ b/examples/guestbook/components/params.libsonnet @@ -12,7 +12,8 @@ name: "guestbook-ui", replicas: 1, servicePort: 80, - type: "LoadBalancer", + type: "ClusterIP", + command: null, }, }, } diff --git a/server/application/application.go b/server/application/application.go index 8390bb736ab8b..66c200d0b6fdb 100644 --- a/server/application/application.go +++ b/server/application/application.go @@ -561,7 +561,7 @@ func (s *Server) setAppOperation(ctx context.Context, appName string, operationC } a.Operation = op a.Status.OperationState = nil - _, err = s.Update(ctx, &ApplicationUpdateRequest{Application: a}) + _, err = s.appclientset.ArgoprojV1alpha1().Applications(s.ns).Update(a) if err != nil && apierr.IsConflict(err) { log.Warnf("Failed to set operation for app '%s' due to update conflict. Retrying again...", appName) } else { diff --git a/test/e2e/app_management_test.go b/test/e2e/app_management_test.go index 87af237f5c42a..483754ea19705 100644 --- a/test/e2e/app_management_test.go +++ b/test/e2e/app_management_test.go @@ -1,7 +1,6 @@ package e2e import ( - "context" "strconv" "testing" "time" @@ -18,9 +17,7 @@ import ( ) func TestAppManagement(t *testing.T) { - testApp := &v1alpha1.Application{ - ObjectMeta: metav1.ObjectMeta{GenerateName: "e2e-test"}, Spec: v1alpha1.ApplicationSpec{ Source: v1alpha1.ApplicationSource{ RepoURL: "https://github.com/argoproj/argo-cd.git", Path: ".", Environment: "minikube", @@ -72,20 +69,12 @@ func TestAppManagement(t *testing.T) { }) t.Run("TestTrackAppStateAndSyncApp", func(t *testing.T) { - ctrl := fixture.CreateController() - ctx, cancel := context.WithCancel(context.Background()) - go ctrl.Run(ctx, 1, 1) - defer cancel() - - // create app and ensure it reaches OutOfSync state app := fixture.CreateApp(t, testApp) WaitUntil(t, func() (done bool, err error) { app, err = fixture.AppClient.ArgoprojV1alpha1().Applications(fixture.Namespace).Get(app.ObjectMeta.Name, metav1.GetOptions{}) return err == nil && app.Status.ComparisonResult.Status != v1alpha1.ComparisonStatusUnknown, err }) - assert.Equal(t, v1alpha1.ComparisonStatusOutOfSync, app.Status.ComparisonResult.Status) - // sync app and make sure it reaches InSync state _, err := fixture.RunCli("app", "sync", app.Name) if err != nil { @@ -103,30 +92,30 @@ func TestAppManagement(t *testing.T) { t.Run("TestAppRollbackSuccessful", func(t *testing.T) { appWithHistory := testApp.DeepCopy() - appWithHistory.Status.History = []v1alpha1.DeploymentInfo{{ - ID: 1, - Revision: "abc", + + // create app and ensure it's comparion status is not ComparisonStatusUnknown + app := fixture.CreateApp(t, appWithHistory) + app.Status.History = []v1alpha1.DeploymentInfo{{ + ID: 1, + Revision: "abc", + ComponentParameterOverrides: app.Spec.Source.ComponentParameterOverrides, }, { - ID: 2, - Revision: "cdb", + ID: 2, + Revision: "cdb", + ComponentParameterOverrides: app.Spec.Source.ComponentParameterOverrides, }} + app, err := fixture.AppClient.ArgoprojV1alpha1().Applications(fixture.Namespace).Update(app) + if err != nil { + t.Fatalf("Unable to update app %v", err) + } - ctrl := fixture.CreateController() - ctx, cancel := context.WithCancel(context.Background()) - go ctrl.Run(ctx, 1, 1) - defer cancel() - - // create app and ensure it reaches OutOfSync state - app := fixture.CreateApp(t, appWithHistory) WaitUntil(t, func() (done bool, err error) { app, err = fixture.AppClient.ArgoprojV1alpha1().Applications(fixture.Namespace).Get(app.ObjectMeta.Name, metav1.GetOptions{}) return err == nil && app.Status.ComparisonResult.Status != v1alpha1.ComparisonStatusUnknown, err }) - assert.Equal(t, v1alpha1.ComparisonStatusOutOfSync, app.Status.ComparisonResult.Status) - // sync app and make sure it reaches InSync state - _, err := fixture.RunCli("app", "rollback", app.Name, "1") + _, err = fixture.RunCli("app", "rollback", app.Name, "1") if err != nil { t.Fatalf("Unable to sync app %v", err) } @@ -146,11 +135,6 @@ func TestAppManagement(t *testing.T) { invalidApp := testApp.DeepCopy() invalidApp.Spec.Destination.Server = "https://not-registered-cluster/api" - ctrl := fixture.CreateController() - ctx, cancel := context.WithCancel(context.Background()) - go ctrl.Run(ctx, 1, 1) - defer cancel() - app := fixture.CreateApp(t, invalidApp) WaitUntil(t, func() (done bool, err error) { @@ -165,4 +149,39 @@ func TestAppManagement(t *testing.T) { assert.Equal(t, v1alpha1.ComparisonStatusError, app.Status.ComparisonResult.Status) }) + + t.Run("TestArgoCDWaitEnsureAppIsNotCrashing", func(t *testing.T) { + updatedApp := testApp.DeepCopy() + + // deploy app and make sure it is healthy + app := fixture.CreateApp(t, updatedApp) + _, err := fixture.RunCli("app", "sync", app.Name) + if err != nil { + t.Fatalf("Unable to sync app %v", err) + } + + WaitUntil(t, func() (done bool, err error) { + app, err = fixture.AppClient.ArgoprojV1alpha1().Applications(fixture.Namespace).Get(app.ObjectMeta.Name, metav1.GetOptions{}) + return err == nil && app.Status.ComparisonResult.Status == v1alpha1.ComparisonStatusSynced && app.Status.Health.Status == v1alpha1.HealthStatusHealthy, err + }) + + // deploy app which fails and make sure it became unhealthy + app.Spec.Source.ComponentParameterOverrides = append( + app.Spec.Source.ComponentParameterOverrides, + v1alpha1.ComponentParameter{Name: "command", Value: "wrong-command", Component: "guestbook-ui"}) + _, err = fixture.AppClient.ArgoprojV1alpha1().Applications(fixture.Namespace).Update(app) + if err != nil { + t.Fatalf("Unable to set app parameter %v", err) + } + + _, err = fixture.RunCli("app", "sync", app.Name) + if err != nil { + t.Fatalf("Unable to sync app %v", err) + } + + WaitUntil(t, func() (done bool, err error) { + app, err = fixture.AppClient.ArgoprojV1alpha1().Applications(fixture.Namespace).Get(app.ObjectMeta.Name, metav1.GetOptions{}) + return err == nil && app.Status.ComparisonResult.Status == v1alpha1.ComparisonStatusSynced && app.Status.Health.Status == v1alpha1.HealthStatusDegraded, err + }) + }) } diff --git a/test/e2e/fixture.go b/test/e2e/fixture.go index 721ef9d94a2aa..6bce48fb939b7 100644 --- a/test/e2e/fixture.go +++ b/test/e2e/fixture.go @@ -1,7 +1,6 @@ package e2e import ( - "bytes" "context" "encoding/json" "fmt" @@ -21,6 +20,8 @@ import ( "k8s.io/client-go/rest" "k8s.io/client-go/tools/clientcmd" + "strings" + "github.com/argoproj/argo-cd/cmd/argocd/commands" "github.com/argoproj/argo-cd/common" "github.com/argoproj/argo-cd/controller" @@ -143,22 +144,11 @@ func (f *Fixture) setup() error { }) ctx, cancel := context.WithCancel(context.Background()) - go func() { - err = repoServerGRPC.Serve(repoServerListener) - }() go func() { apiServer.Run(ctx, apiServerPort) }() - f.tearDownCallback = func() { - cancel() - repoServerGRPC.Stop() - } - if err != nil { - return err - } - - return waitUntilE(func() (done bool, err error) { + err = waitUntilE(func() (done bool, err error) { clientset, err := f.NewApiClientset() if err != nil { return false, nil @@ -171,6 +161,22 @@ func (f *Fixture) setup() error { _, err = appClient.List(context.Background(), &application.ApplicationQuery{}) return err == nil, nil }) + + ctrl := f.createController() + ctrlCtx, cancelCtrl := context.WithCancel(context.Background()) + go ctrl.Run(ctrlCtx, 1, 1) + + go func() { + err = repoServerGRPC.Serve(repoServerListener) + }() + + f.tearDownCallback = func() { + cancel() + cancelCtrl() + repoServerGRPC.Stop() + } + + return err } func (f *Fixture) ensureClusterRegistered() error { @@ -270,6 +276,8 @@ func NewFixture() (*Fixture, error) { // CreateApp creates application with appropriate controller instance id. func (f *Fixture) CreateApp(t *testing.T, application *v1alpha1.Application) *v1alpha1.Application { + application = application.DeepCopy() + application.Name = fmt.Sprintf("e2e-test-%v", time.Now().Unix()) labels := application.ObjectMeta.Labels if labels == nil { labels = make(map[string]string) @@ -277,6 +285,10 @@ func (f *Fixture) CreateApp(t *testing.T, application *v1alpha1.Application) *v1 } labels[common.LabelKeyApplicationControllerInstanceID] = f.InstanceID + application.Spec.Source.ComponentParameterOverrides = append( + application.Spec.Source.ComponentParameterOverrides, + v1alpha1.ComponentParameter{Name: "name", Value: application.Name, Component: "guestbook-ui"}) + app, err := f.AppClient.ArgoprojV1alpha1().Applications(f.Namespace).Create(application) if err != nil { t.Fatal(fmt.Sprintf("Unable to create app %v", err)) @@ -284,8 +296,8 @@ func (f *Fixture) CreateApp(t *testing.T, application *v1alpha1.Application) *v1 return app } -// CreateController creates new controller instance -func (f *Fixture) CreateController() *controller.ApplicationController { +// createController creates new controller instance +func (f *Fixture) createController() *controller.ApplicationController { appStateManager := controller.NewAppStateManager( f.DB, f.AppClient, reposerver.NewRepositoryServerClientset(f.RepoServerAddress), f.Namespace) @@ -311,12 +323,21 @@ func (f *Fixture) NewApiClientset() (argocdclient.Client, error) { } func (f *Fixture) RunCli(args ...string) (string, error) { - cmd := commands.NewCommand() - cmd.SetArgs(append(args, "--server", f.ApiServerAddress, "--plaintext")) - output := new(bytes.Buffer) - cmd.SetOutput(output) - err := cmd.Execute() - return output.String(), err + args = append([]string{"run", "../../cmd/argocd/main.go"}, args...) + cmd := exec.Command("go", append(args, "--server", f.ApiServerAddress, "--plaintext")...) + outBytes, err := cmd.Output() + if err != nil { + exErr, ok := err.(*exec.ExitError) + if !ok { + return "", err + } + errOutput := string(exErr.Stderr) + if outBytes != nil { + errOutput = string(outBytes) + "\n" + errOutput + } + return "", fmt.Errorf(strings.TrimSpace(errOutput)) + } + return string(outBytes), nil } func waitUntilE(condition wait.ConditionFunc) error { diff --git a/util/git/git.go b/util/git/git.go index 8b2c08f55ac1a..73019fef5d1b4 100644 --- a/util/git/git.go +++ b/util/git/git.go @@ -98,10 +98,12 @@ func TestRepo(repo, username, password string, sshPrivateKey string) error { cmd.Env = env _, err = cmd.Output() if err != nil { - exErr := err.(*exec.ExitError) - errOutput := strings.Split(string(exErr.Stderr), "\n")[0] - errOutput = redactPassword(errOutput, password) - return fmt.Errorf("%s: %s", repo, errOutput) + if exErr, ok := err.(*exec.ExitError); ok { + errOutput := strings.Split(string(exErr.Stderr), "\n")[0] + errOutput = redactPassword(errOutput, password) + return fmt.Errorf("%s: %s", repo, errOutput) + } + return err } return nil }