Take into account number of unavailable replicas to decided if deployment is healthy or not (#270)

* Take into account number of unavailable replicas to decided if deployment is healthy or not

* Run one controller for all e2e tests to reduce tests duration

* Apply reviewer notes: use logic from kubectl/rollout_status.go to check deployment health

Gopkg.lock

@@ -747,6 +747,8 @@
     "discovery/fake",
     "dynamic",
     "dynamic/fake",
+    "informers/core/v1",
+    "informers/internalinterfaces",
     "kubernetes",
     "kubernetes/fake",
     "kubernetes/scheme",
@@ -806,6 +808,7 @@
     "kubernetes/typed/storage/v1alpha1/fake",
     "kubernetes/typed/storage/v1beta1",
     "kubernetes/typed/storage/v1beta1/fake",
+    "listers/core/v1",
     "pkg/version",
     "plugin/pkg/client/auth/gcp",
     "plugin/pkg/client/auth/oidc",
@@ -876,6 +879,6 @@
 [solve-meta]
   analyzer-name = "dep"
   analyzer-version = 1
-  inputs-digest = "736e8116bcf49bf0889c2caf8e832a78a43fd5af65c5d10c86a443993de63d3c"
+  inputs-digest = "e336c1acaf22142bbb031deed1513923d09b6fda70d228e48a9556f9d40cb785"
   solver-name = "gps-cdcl"
   solver-version = 1

VERSION

@@ -1 +1 @@
-0.4.5
+0.4.7

cmd/argocd/commands/app.go

@@ -3,11 +3,13 @@ package commands
 import (
 	"context"
 	"fmt"
+	"io"
 	"net/url"
 	"os"
 	"strconv"
 	"strings"
 	"text/tabwriter"
+	"time"
 
 	"github.com/argoproj/argo-cd/errors"
 	argocdclient "github.com/argoproj/argo-cd/pkg/apiclient"
@@ -21,7 +23,9 @@ import (
 	"github.com/spf13/pflag"
 	"github.com/yudai/gojsondiff/formatter"
 	"golang.org/x/crypto/ssh/terminal"
+	"google.golang.org/grpc/codes"
 	"google.golang.org/grpc/metadata"
+	"google.golang.org/grpc/status"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
@@ -156,15 +160,7 @@ func NewApplicationGetCommand(clientOpts *argocdclient.ClientOptions) *cobra.Com
 				fmt.Println()
 				w := tabwriter.NewWriter(os.Stdout, 0, 0, 2, ' ', 0)
 				fmt.Fprintf(w, "KIND\tNAME\tSTATUS\tHEALTH\n")
-				for _, res := range app.Status.ComparisonResult.Resources {
-					obj, err := argoappv1.UnmarshalToUnstructured(res.TargetState)
-					errors.CheckError(err)
-					if obj == nil {
-						obj, err = argoappv1.UnmarshalToUnstructured(res.LiveState)
-						errors.CheckError(err)
-					}
-					fmt.Fprintf(w, "%s\t%s\t%s\t%s\n", obj.GetKind(), obj.GetName(), res.Status, res.Health.Status)
-				}
+				printAppResources(w, app)
 				_ = w.Flush()
 			}
 		},
@@ -470,51 +466,43 @@ func NewApplicationWaitCommand(clientOpts *argocdclient.ClientOptions) *cobra.Co
 			if syncOnly && healthOnly {
 				log.Fatalln("Please specify at most one of --sync-only or --health-only.")
 			}
+			appName := args[0]
 			conn, appIf := argocdclient.NewClientOrDie(clientOpts).NewApplicationClientOrDie()
 			defer util.Close(conn)
+			ctx := context.Background()
+			ctx, cancel := context.WithCancel(ctx)
+			defer cancel()
 
-			appName := args[0]
-			wc, err := appIf.Watch(context.Background(), &application.ApplicationQuery{
-				Name: &appName,
-			})
-			errors.CheckError(err)
-
-			success := util.Wait(timeout, func(done chan<- bool) {
-				for {
-					appEvent, err := wc.Recv()
-					errors.CheckError(err)
-
-					app := appEvent.Application
-					healthStatus := app.Status.Health.Status
-					syncStatus := app.Status.ComparisonResult.Status
-
-					log.Printf("App %q has sync status %q and health status %q", appName, syncStatus, healthStatus)
-					synced := (syncStatus == argoappv1.ComparisonStatusSynced)
-					healthy := (healthStatus == argoappv1.HealthStatusHealthy)
-
-					if (synced && healthy) || (synced && syncOnly) || (healthy && healthOnly) {
-						done <- true
-					}
-				}
-			})
-
-			if success {
-				log.Printf("App %q matches desired state", appName)
-			} else {
-				app, err := appIf.Get(context.Background(), &application.ApplicationQuery{Name: &appName})
-				errors.CheckError(err)
-
-				if len(app.Status.ComparisonResult.Resources) > 0 {
-					for _, res := range app.Status.ComparisonResult.Resources {
-						targetObj, err := argoappv1.UnmarshalToUnstructured(res.TargetState)
-						errors.CheckError(err)
-						if res.Status != argoappv1.ComparisonStatusSynced || res.Health.Status != argoappv1.HealthStatusHealthy {
-							log.Warnf("%s %q has sync status %q and health status %q: %s", targetObj.GetKind(), targetObj.GetName(), res.Status, res.Health.Status, res.Health.StatusDetails)
-						}
-					}
-				}
-				log.Fatalf("Timed out before seeing app %q match desired state", appName)
+			if timeout != 0 {
+				time.AfterFunc(time.Duration(timeout)*time.Second, func() {
+					cancel()
+				})
 			}
+
+			// print the initial components to format the tabwriter columns
+			app, err := appIf.Get(ctx, &application.ApplicationQuery{Name: &appName})
+			errors.CheckError(err)
+			w := tabwriter.NewWriter(os.Stdout, 0, 0, 2, ' ', 0)
+			fmt.Fprintf(w, "KIND\tNAME\tSTATUS\tHEALTH\n")
+			printAppResources(w, app)
+			_ = w.Flush()
+			prevCompRes := &app.Status.ComparisonResult
+
+			appEventCh := watchApp(ctx, appIf, appName)
+			for appEvent := range appEventCh {
+				app := appEvent.Application
+				printAppStateChange(w, prevCompRes, &app)
+				_ = w.Flush()
+				prevCompRes = &app.Status.ComparisonResult
+
+				synced := (app.Status.ComparisonResult.Status == argoappv1.ComparisonStatusSynced)
+				healthy := (app.Status.Health.Status == argoappv1.HealthStatusHealthy)
+				if (synced && healthy) || (synced && syncOnly) || (healthy && healthOnly) {
+					log.Printf("App %q matches desired state", appName)
+					return
+				}
+			}
+			log.Fatalf("Timed out (%ds) waiting for app %q match desired state", timeout, appName)
 		},
 	}
 	command.Flags().BoolVar(&syncOnly, "sync-only", false, "Wait only for sync")
@@ -523,6 +511,104 @@ func NewApplicationWaitCommand(clientOpts *argocdclient.ClientOptions) *cobra.Co
 	return command
 }
 
+func isCanceledContextErr(err error) bool {
+	if err == context.Canceled {
+		return true
+	}
+	if stat, ok := status.FromError(err); ok {
+		if stat.Code() == codes.Canceled {
+			return true
+		}
+	}
+	return false
+}
+
+// watchApp returns a channel of watch events for an app, retrying the watch upon errors. Closes
+// the returned channel when the context is discovered to be canceled.
+func watchApp(ctx context.Context, appIf application.ApplicationServiceClient, appName string) chan *argoappv1.ApplicationWatchEvent {
+	appEventsCh := make(chan *argoappv1.ApplicationWatchEvent)
+	go func() {
+		defer close(appEventsCh)
+		for {
+			wc, err := appIf.Watch(ctx, &application.ApplicationQuery{
+				Name: &appName,
+			})
+			if err != nil {
+				if isCanceledContextErr(err) {
+					return
+				}
+				if err != io.EOF {
+					log.Warnf("watch err: %v", err)
+				}
+				time.Sleep(1 * time.Second)
+				continue
+			}
+			for {
+				appEvent, err := wc.Recv()
+				if err != nil {
+					if isCanceledContextErr(err) {
+						return
+					}
+					if err != io.EOF {
+						log.Warnf("recv err: %v", err)
+					}
+					time.Sleep(1 * time.Second)
+					break
+				} else {
+					appEventsCh <- appEvent
+				}
+			}
+		}
+
+	}()
+	return appEventsCh
+}
+
+// printAppResources prints the resources of an application in a tabwriter table
+func printAppResources(w io.Writer, app *argoappv1.Application) {
+	for _, res := range app.Status.ComparisonResult.Resources {
+		obj, err := argoappv1.UnmarshalToUnstructured(res.TargetState)
+		errors.CheckError(err)
+		if obj == nil {
+			obj, err = argoappv1.UnmarshalToUnstructured(res.LiveState)
+			errors.CheckError(err)
+		}
+		fmt.Fprintf(w, "%s\t%s\t%s\t%s\n", obj.GetKind(), obj.GetName(), res.Status, res.Health.Status)
+	}
+}
+
+// printAppStateChange prints a component state change if it was different from the last time we saw it
+func printAppStateChange(w io.Writer, prevComp *argoappv1.ComparisonResult, app *argoappv1.Application) {
+	getPrevResState := func(kind, name string) (argoappv1.ComparisonStatus, argoappv1.HealthStatusCode) {
+		for _, res := range prevComp.Resources {
+			obj, err := argoappv1.UnmarshalToUnstructured(res.TargetState)
+			errors.CheckError(err)
+			if obj == nil {
+				obj, err = argoappv1.UnmarshalToUnstructured(res.LiveState)
+				errors.CheckError(err)
+			}
+			if obj.GetKind() == kind && obj.GetName() == name {
+				return res.Status, res.Health.Status
+			}
+		}
+		return "", ""
+	}
+	if len(app.Status.ComparisonResult.Resources) > 0 {
+		for _, res := range app.Status.ComparisonResult.Resources {
+			obj, err := argoappv1.UnmarshalToUnstructured(res.TargetState)
+			errors.CheckError(err)
+			if obj == nil {
+				obj, err = argoappv1.UnmarshalToUnstructured(res.LiveState)
+				errors.CheckError(err)
+			}
+			prevSync, prevHealth := getPrevResState(obj.GetKind(), obj.GetName())
+			if prevSync != res.Status || prevHealth != res.Health.Status {
+				fmt.Fprintf(w, "%s\t%s\t%s\t%s\n", obj.GetKind(), obj.GetName(), res.Status, res.Health.Status)
+			}
+		}
+	}
+}
+
 // NewApplicationSyncCommand returns a new instance of an `argocd app sync` command
 func NewApplicationSyncCommand(clientOpts *argocdclient.ClientOptions) *cobra.Command {
 	var (

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) {

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])

examples/guestbook/components/params.libsonnet

@@ -12,7 +12,8 @@
       name: "guestbook-ui",
       replicas: 1,
       servicePort: 80,
-      type: "LoadBalancer",
+      type: "ClusterIP",
+      command: null,
     },
   },
 }

server/application/application.go

@@ -510,7 +510,7 @@ func (s *Server) setAppOperation(ctx context.Context, appName string, operationC
 		}
 		a.Operation = op
 		a.Status.OperationState = nil
-		_, err = s.Update(ctx, 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 {

server/server.go

@@ -162,7 +162,6 @@ func (a *ArgoCDServer) Run(ctx context.Context, port int) {
 		go func() { a.checkServeErr("httpsS", httpsS.Serve(httpsL)) }()
 		go func() { a.checkServeErr("tlsm", tlsm.Serve()) }()
 	}
-	go a.initializeOIDCClientApp()
 	go a.watchSettings(ctx)
 	go func() { a.checkServeErr("tcpm", tcpm.Serve()) }()
 
@@ -234,31 +233,6 @@ func (a *ArgoCDServer) watchSettings(ctx context.Context) {
 	close(updateCh)
 }
 
-// initializeOIDCClientApp initializes the OIDC Client application, querying the well known oidc
-// configuration path. Because ArgoCD is a OIDC client to itself, we have a chicken-and-egg problem
-// of (1) serving dex over HTTP, and (2) querying the OIDC provider (ourselves) to initialize the
-// app (HTTP GET http://example-argocd.com/api/dex/.well-known/openid-configuration)
-// This method is expected to be invoked right after we start listening over HTTP
-func (a *ArgoCDServer) initializeOIDCClientApp() {
-	if !a.settings.IsSSOConfigured() {
-		return
-	}
-	// wait for dex to become ready
-	dexClient, err := dexutil.NewDexClient()
-	errors.CheckError(err)
-	dexClient.WaitUntilReady()
-	var realErr error
-	_ = wait.ExponentialBackoff(backoff, func() (bool, error) {
-		_, realErr = a.sessionMgr.OIDCProvider()
-		if realErr != nil {
-			a.log.Warnf("failed to initialize client app: %v", realErr)
-			return false, nil
-		}
-		return true, nil
-	})
-	errors.CheckError(realErr)
-}
-
 func (a *ArgoCDServer) useTLS() bool {
 	if a.Insecure || a.settings.Certificate == nil {
 		return false

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",
-		}, {
-			ID:       2,
-			Revision: "cdb",
-		}}
 
-		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
+		// 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",
+			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)
+		}
+
 		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
+		})
+	})
 }

test/e2e/fixture.go

@@ -1,7 +1,6 @@
 package e2e
 
 import (
-	"bytes"
 	"context"
 	"fmt"
 	"log"
@@ -13,6 +12,8 @@ import (
 
 	"encoding/json"
 
+	"strings"
+
 	"github.com/argoproj/argo-cd/cmd/argocd/commands"
 	"github.com/argoproj/argo-cd/common"
 	"github.com/argoproj/argo-cd/controller"
@@ -128,22 +129,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
@@ -156,6 +146,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 {
@@ -251,6 +257,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)
@@ -258,6 +266,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))
@@ -265,8 +277,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)
 
@@ -292,12 +304,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 {

util/db/repository.go

@@ -123,7 +123,8 @@ func repoURLToSecretName(repo string) string {
 	h := fnv.New32a()
 	_, _ = h.Write([]byte(repo))
 	parts := strings.Split(strings.TrimSuffix(repo, ".git"), "/")
-	return fmt.Sprintf("repo-%s-%v", parts[len(parts)-1], h.Sum32())
+	shortName := strings.Replace(parts[len(parts)-1], "_", "-", -1)
+	return fmt.Sprintf("repo-%s-%v", shortName, h.Sum32())
 }
 
 // repoToStringData converts a repository object to string data for serialization to a secret

util/db/repository_test.go

@@ -8,6 +8,7 @@ func TestRepoURLToSecretName(t *testing.T) {
 		"https://github.com/argoproj/ARGO-cd":       "repo-argo-cd-821842295",
 		"https://github.com/argoproj/argo-cd":       "repo-argo-cd-821842295",
 		"https://github.com/argoproj/argo-cd.git":   "repo-argo-cd-821842295",
+		"https://github.com/argoproj/argo_cd.git":   "repo-argo-cd-1049844989",
 		"ssh://git@github.com/argoproj/argo-cd.git": "repo-argo-cd-1019298066",
 	}
 

util/dex/dex.go

@@ -12,17 +12,18 @@ import (
 	"os"
 	"time"
 
-	"github.com/argoproj/argo-cd/common"
-	"github.com/argoproj/argo-cd/errors"
-	"github.com/argoproj/argo-cd/util/cache"
-	"github.com/argoproj/argo-cd/util/session"
-	"github.com/argoproj/argo-cd/util/settings"
 	"github.com/coreos/dex/api"
 	oidc "github.com/coreos/go-oidc"
 	jwt "github.com/dgrijalva/jwt-go"
 	log "github.com/sirupsen/logrus"
 	"golang.org/x/oauth2"
 	"google.golang.org/grpc"
+
+	"github.com/argoproj/argo-cd/common"
+	"github.com/argoproj/argo-cd/errors"
+	"github.com/argoproj/argo-cd/util/cache"
+	"github.com/argoproj/argo-cd/util/session"
+	"github.com/argoproj/argo-cd/util/settings"
 )
 
 const (
@@ -141,15 +142,18 @@ func NewClientApp(settings *settings.ArgoCDSettings, sessionMgr *session.Session
 	return &a, nil
 }
 
-func (a *ClientApp) oauth2Config(scopes []string) *oauth2.Config {
-	provider, _ := a.sessionMgr.OIDCProvider()
+func (a *ClientApp) oauth2Config(scopes []string) (*oauth2.Config, error) {
+	provider, err := a.sessionMgr.OIDCProvider()
+	if err != nil {
+		return nil, err
+	}
 	return &oauth2.Config{
 		ClientID:     a.clientID,
 		ClientSecret: a.clientSecret,
 		Endpoint:     provider.Endpoint(),
 		Scopes:       scopes,
 		RedirectURL:  a.redirectURI,
-	}
+	}, nil
 }
 
 var letters = []rune("abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ")
@@ -196,18 +200,23 @@ func (a *ClientApp) verifyAppState(state string) (*appState, error) {
 }
 
 func (a *ClientApp) HandleLogin(w http.ResponseWriter, r *http.Request) {
-	var authCodeURL string
+	var opts []oauth2.AuthCodeOption
 	returnURL := r.FormValue("return_url")
 	scopes := []string{"openid", "profile", "email", "groups"}
 	appState := a.generateAppState(returnURL)
 	if r.FormValue("offline_access") != "yes" {
-		authCodeURL = a.oauth2Config(scopes).AuthCodeURL(appState)
+		// no-op
 	} else if a.sessionMgr.OfflineAsScope() {
 		scopes = append(scopes, "offline_access")
-		authCodeURL = a.oauth2Config(scopes).AuthCodeURL(appState)
 	} else {
-		authCodeURL = a.oauth2Config(scopes).AuthCodeURL(appState, oauth2.AccessTypeOffline)
+		opts = append(opts, oauth2.AccessTypeOffline)
 	}
+	oauth2Config, err := a.oauth2Config(scopes)
+	if err != nil {
+		http.Error(w, err.Error(), http.StatusInternalServerError)
+		return
+	}
+	authCodeURL := oauth2Config.AuthCodeURL(appState, opts...)
 	http.Redirect(w, r, authCodeURL, http.StatusSeeOther)
 }
 
@@ -219,7 +228,11 @@ func (a *ClientApp) HandleCallback(w http.ResponseWriter, r *http.Request) {
 	)
 
 	ctx := oidc.ClientContext(r.Context(), a.client)
-	oauth2Config := a.oauth2Config(nil)
+	oauth2Config, err := a.oauth2Config(nil)
+	if err != nil {
+		http.Error(w, err.Error(), http.StatusInternalServerError)
+		return
+	}
 	switch r.Method {
 	case "GET":
 		// Authorization redirect callback from OAuth2 auth flow.

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
 }

util/session/sessionmanager.go

@@ -197,7 +197,11 @@ func MakeCookieMetadata(key, value string, flags ...string) string {
 	return strings.Join(components, "; ")
 }
 
-// OIDCProvider lazily returns the OIDC provider
+// OIDCProvider lazily initializes and returns the OIDC provider, querying the well known oidc
+// configuration path (http://example-argocd.com/api/dex/.well-known/openid-configuration).
+// We have to initialize the proviver lazily since ArgoCD is an OIDC client to itself, which
+// presents a chicken-and-egg problem of (1) serving dex over HTTP, and (2) querying the OIDC
+// provider (ourselves) to initialize the app.
 func (mgr *SessionManager) OIDCProvider() (*oidc.Provider, error) {
 	if mgr.provider != nil {
 		return mgr.provider, nil