From bffa01fd6c15d2c9e6907da4f60e575c3421bbfb Mon Sep 17 00:00:00 2001 From: Jesse Suen Date: Fri, 31 Aug 2018 01:25:16 -0700 Subject: [PATCH] Fix `argocd app wait` printing incorrect Sync output (resolves #542) Timeout condition was not printing final status. --- cmd/argocd/commands/app.go | 229 +++++++++++++++------------- hack/migrate/migrate0.3.0to0.4.0.go | 168 -------------------- 2 files changed, 122 insertions(+), 275 deletions(-) delete mode 100644 hack/migrate/migrate0.3.0to0.4.0.go diff --git a/cmd/argocd/commands/app.go b/cmd/argocd/commands/app.go index b6110c98c7785..8569243d555fe 100644 --- a/cmd/argocd/commands/app.go +++ b/cmd/argocd/commands/app.go @@ -7,6 +7,7 @@ import ( "io" "net/url" "os" + "reflect" "strconv" "strings" "text/tabwriter" @@ -845,23 +846,22 @@ func NewApplicationSyncCommand(clientOpts *argocdclient.ClientOptions) *cobra.Co // ResourceState tracks the state of a resource when waiting on an application status. type resourceState struct { - Kind string - Name string - PrevState string - Fields map[string]string - Updated bool + Kind string + Name string + Status string + Health string + Hook string + Message string } -func newResourceState(kind, name, status, healthStatus, resType, message string) *resourceState { +func newResourceState(kind, name, status, health, hook, message string) *resourceState { return &resourceState{ - Kind: kind, - Name: name, - Fields: map[string]string{ - "status": status, - "healthStatus": healthStatus, - "type": resType, - "message": message, - }, + Kind: kind, + Name: name, + Status: status, + Health: health, + Hook: hook, + Message: message, } } @@ -870,47 +870,104 @@ func (rs *resourceState) Key() string { return fmt.Sprintf("%s/%s", rs.Kind, rs.Name) } -// Merge merges the new state into the previous state, returning whether the -// new state contains any additional keys or different values from the old state. -func (rs *resourceState) Merge() bool { - if out := rs.String(); out != rs.PrevState { - rs.PrevState = out - return true - } - return false -} - func (rs *resourceState) String() string { - return fmt.Sprintf("%s\t%s\t%s\t%s\t%s\t%s", rs.Kind, rs.Name, rs.Fields["status"], rs.Fields["healthStatus"], rs.Fields["type"], rs.Fields["message"]) + return fmt.Sprintf("%s\t%s\t%s\t%s\t%s\t%s", rs.Kind, rs.Name, rs.Status, rs.Health, rs.Hook, rs.Message) } -// Update a resourceState with any different contents from another resourceState. +// Merge merges the new state with any different contents from another resourceState. // Blank fields in the receiver state will be updated to non-blank. // Non-blank fields in the receiver state will never be updated to blank. -func (rs *resourceState) Update(newState *resourceState) { - for k, v := range newState.Fields { - if v != "" { - rs.Fields[k] = v +// Returns whether or not any keys were updated. +func (rs *resourceState) Merge(newState *resourceState) bool { + updated := false + for _, field := range []string{"Status", "Health", "Hook", "Message"} { + v := reflect.ValueOf(rs).Elem().FieldByName(field) + currVal := v.String() + newVal := reflect.ValueOf(newState).Elem().FieldByName(field).String() + if newVal != "" && currVal != newVal { + v.SetString(newVal) + updated = true } } + return updated } -func waitOnApplicationStatus(appClient application.ApplicationServiceClient, appName string, timeout uint, watchSync, watchHealth, watchOperations bool) (*argoappv1.Application, error) { +func calculateResourceStates(app *argoappv1.Application) map[string]*resourceState { + resStates := make(map[string]*resourceState) + 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) + } + newState := newResourceState(obj.GetKind(), obj.GetName(), string(res.Status), res.Health.Status, "", "") + key := newState.Key() + if prev, ok := resStates[key]; ok { + prev.Merge(newState) + } else { + resStates[key] = newState + } + } + + var opResult *argoappv1.SyncOperationResult + if app.Status.OperationState.SyncResult != nil { + opResult = app.Status.OperationState.SyncResult + } else if app.Status.OperationState.RollbackResult != nil { + opResult = app.Status.OperationState.SyncResult + } + if opResult == nil { + return resStates + } + + for _, hook := range opResult.Hooks { + newState := newResourceState(hook.Kind, hook.Name, string(hook.Status), "", string(hook.Type), hook.Message) + key := newState.Key() + if prev, ok := resStates[key]; ok { + prev.Merge(newState) + } else { + resStates[key] = newState + } + } + + for _, res := range opResult.Resources { + newState := newResourceState(res.Kind, res.Name, "", "", "", res.Message) + key := newState.Key() + if prev, ok := resStates[key]; ok { + prev.Merge(newState) + } else { + resStates[key] = newState + } + } + return resStates +} + +func waitOnApplicationStatus(appClient application.ApplicationServiceClient, appName string, timeout uint, watchSync, watchHealth, watchOperation bool) (*argoappv1.Application, error) { ctx, cancel := context.WithCancel(context.Background()) defer cancel() - printFinalStatus := func() { - // get refreshed app before printing to show accurate sync/health status - app, err := appClient.Get(ctx, &application.ApplicationQuery{Name: &appName, Refresh: true}) - errors.CheckError(err) + // refresh controls whether or not we refresh the app before printing the final status. + // We only want to do this when an operation is in progress, since operations are the only + // time when the sync status lags behind when an operation completes + refresh := false - fmt.Printf(printOpFmtStr, "Application:", appName) - printOperationResult(app.Status.OperationState) + printFinalStatus := func(app *argoappv1.Application) { + var err error + if refresh { + app, err = appClient.Get(context.Background(), &application.ApplicationQuery{Name: &appName, Refresh: true}) + errors.CheckError(err) + } + + fmt.Println() + fmt.Printf(printOpFmtStr, "Application:", app.Name) + if watchOperation { + printOperationResult(app.Status.OperationState) + } if len(app.Status.ComparisonResult.Resources) > 0 { fmt.Println() - w := tabwriter.NewWriter(os.Stdout, 0, 0, 2, ' ', 0) - printAppResources(w, app, true) + w := tabwriter.NewWriter(os.Stdout, 5, 0, 2, ' ', 0) + printAppResources(w, app, watchOperation) _ = w.Flush() } } @@ -918,89 +975,47 @@ func waitOnApplicationStatus(appClient application.ApplicationServiceClient, app if timeout != 0 { time.AfterFunc(time.Duration(timeout)*time.Second, func() { cancel() - printFinalStatus() }) } - // print the initial components to format the tabwriter columns - w := tabwriter.NewWriter(os.Stdout, 0, 0, 2, ' ', 0) + w := tabwriter.NewWriter(os.Stdout, 5, 0, 2, ' ', 0) fmt.Fprintln(w, "KIND\tNAME\tSTATUS\tHEALTH\tHOOK\tOPERATIONMSG") - _ = w.Flush() prevStates := make(map[string]*resourceState) - conditionallyPrintOutput := func(w io.Writer, newState *resourceState) { - stateKey := newState.Key() - if prevState, found := prevStates[stateKey]; found { - prevState.Update(newState) - } else { - prevStates[stateKey] = newState - } - } - - printCompResults := func(compResult *argoappv1.ComparisonResult) { - if compResult != nil { - for _, res := range compResult.Resources { - obj, err := argoappv1.UnmarshalToUnstructured(res.TargetState) - errors.CheckError(err) - if obj == nil { - obj, err = argoappv1.UnmarshalToUnstructured(res.LiveState) - errors.CheckError(err) - } - - newState := newResourceState(obj.GetKind(), obj.GetName(), string(res.Status), res.Health.Status, "", "") - conditionallyPrintOutput(w, newState) - } - } - } - - printOpResults := func(opResult *argoappv1.SyncOperationResult) { - if opResult != nil { - if opResult.Hooks != nil { - for _, hook := range opResult.Hooks { - newState := newResourceState(hook.Kind, hook.Name, string(hook.Status), "", string(hook.Type), hook.Message) - conditionallyPrintOutput(w, newState) - } - } - - if opResult.Resources != nil { - for _, res := range opResult.Resources { - newState := newResourceState(res.Kind, res.Name, string(res.Status), "", "", res.Message) - conditionallyPrintOutput(w, newState) - } - } - } - } - appEventCh := watchApp(ctx, appClient, appName) - for appEvent := range appEventCh { - app := appEvent.Application + var app *argoappv1.Application - printCompResults(&app.Status.ComparisonResult) - - if opState := app.Status.OperationState; opState != nil { - printOpResults(opState.SyncResult) - printOpResults(opState.RollbackResult) - } - - for _, v := range prevStates { - if v.Merge() { - fmt.Fprintln(w, v) - } + for appEvent := range appEventCh { + app = &appEvent.Application + if app.Operation != nil { + refresh = true } - - _ = w.Flush() - // consider skipped checks successful synced := !watchSync || app.Status.ComparisonResult.Status == argoappv1.ComparisonStatusSynced healthy := !watchHealth || app.Status.Health.Status == argoappv1.HealthStatusHealthy - operational := !watchOperations || appEvent.Application.Operation == nil + operational := !watchOperation || appEvent.Application.Operation == nil if len(app.Status.GetErrorConditions()) == 0 && synced && healthy && operational { - log.Printf("App %q matches desired state", appName) - printFinalStatus() - return &app, nil + printFinalStatus(app) + return app, nil } - } + newStates := calculateResourceStates(app) + for _, newState := range newStates { + var doPrint bool + stateKey := newState.Key() + if prevState, found := prevStates[stateKey]; found { + doPrint = prevState.Merge(newState) + } else { + prevStates[stateKey] = newState + doPrint = true + } + if doPrint { + fmt.Fprintln(w, prevStates[stateKey]) + } + } + _ = w.Flush() + } + printFinalStatus(app) return nil, fmt.Errorf("Timed out (%ds) waiting for app %q match desired state", timeout, appName) } diff --git a/hack/migrate/migrate0.3.0to0.4.0.go b/hack/migrate/migrate0.3.0to0.4.0.go deleted file mode 100644 index 72632463d9e11..0000000000000 --- a/hack/migrate/migrate0.3.0to0.4.0.go +++ /dev/null @@ -1,168 +0,0 @@ -package main - -import ( - "context" - "fmt" - "hash/fnv" - "log" - "os" - "strings" - - argocdclient "github.com/argoproj/argo-cd/pkg/apiclient" - "github.com/argoproj/argo-cd/server/repository" - "github.com/argoproj/argo-cd/util" - "github.com/argoproj/argo-cd/util/git" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/client-go/kubernetes" - "k8s.io/client-go/tools/clientcmd" -) - -// origRepoURLToSecretName hashes repo URL to the secret name using a formula. -// Part of the original repo name is incorporated for debugging purposes -func origRepoURLToSecretName(repo string) string { - repo = git.NormalizeGitURL(repo) - h := fnv.New32a() - _, _ = h.Write([]byte(repo)) - parts := strings.Split(strings.TrimSuffix(repo, ".git"), "/") - return fmt.Sprintf("repo-%s-%v", strings.ToLower(parts[len(parts)-1]), h.Sum32()) -} - -// repoURLToSecretName hashes repo URL to the secret name using a formula. -// Part of the original repo name is incorporated for debugging purposes -func repoURLToSecretName(repo string) string { - repo = strings.ToLower(git.NormalizeGitURL(repo)) - 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()) -} - -// RenameSecret renames a Kubernetes secret in a given namespace. -func renameSecret(namespace, oldName, newName string) { - loadingRules := clientcmd.NewDefaultClientConfigLoadingRules() - loadingRules.DefaultClientConfig = &clientcmd.DefaultClientConfig - overrides := clientcmd.ConfigOverrides{} - clientConfig := clientcmd.NewNonInteractiveDeferredLoadingClientConfig(loadingRules, &overrides) - - log.Printf("Renaming secret %q to %q in namespace %q\n", oldName, newName, namespace) - - config, err := clientConfig.ClientConfig() - if err != nil { - log.Println("Could not retrieve client config: ", err) - return - } - - kubeclientset := kubernetes.NewForConfigOrDie(config) - repoSecret, err := kubeclientset.CoreV1().Secrets(namespace).Get(oldName, metav1.GetOptions{}) - if err != nil { - log.Println("Could not retrieve old secret: ", err) - return - } - - repoSecret.ObjectMeta.Name = newName - repoSecret.ObjectMeta.ResourceVersion = "" - - repoSecret, err = kubeclientset.CoreV1().Secrets(namespace).Create(repoSecret) - if err != nil { - log.Println("Could not create new secret: ", err) - return - } - - err = kubeclientset.CoreV1().Secrets(namespace).Delete(oldName, &metav1.DeleteOptions{}) - if err != nil { - log.Println("Could not remove old secret: ", err) - } -} - -// RenameRepositorySecrets ensures that repository secrets use the new naming format. -func renameRepositorySecrets(clientOpts argocdclient.ClientOptions, namespace string) { - conn, repoIf := argocdclient.NewClientOrDie(&clientOpts).NewRepoClientOrDie() - defer util.Close(conn) - repos, err := repoIf.List(context.Background(), &repository.RepoQuery{}) - if err != nil { - log.Println("An error occurred, so skipping secret renaming: ", err) - return - } - - log.Println("Renaming repository secrets...") - for _, repo := range repos.Items { - oldSecretName := origRepoURLToSecretName(repo.Repo) - newSecretName := repoURLToSecretName(repo.Repo) - if oldSecretName != newSecretName { - log.Printf("Repo %q had its secret name change, so updating\n", repo.Repo) - renameSecret(namespace, oldSecretName, newSecretName) - } - } -} - -/* -// PopulateAppDestinations ensures that apps have a Server and Namespace set explicitly. -func populateAppDestinations(clientOpts argocdclient.ClientOptions) { - conn, appIf := argocdclient.NewClientOrDie(&clientOpts).NewApplicationClientOrDie() - defer util.Close(conn) - apps, err := appIf.List(context.Background(), &application.ApplicationQuery{}) - if err != nil { - log.Println("An error occurred, so skipping destination population: ", err) - return - } - - log.Println("Populating app Destination fields") - for _, app := range apps.Items { - changed := false - - log.Printf("Ensuring destination field is populated on app %q\n", app.ObjectMeta.Name) - if app.Spec.Destination.Server == "" { - if app.Status.ComparisonResult.Status == appv1.ComparisonStatusUnknown || app.Status.ComparisonResult.Status == appv1.ComparisonStatusError { - log.Printf("App %q was missing Destination.Server, but could not fill it in: %s", app.ObjectMeta.Name, app.Status.ComparisonResult.Status) - } else { - log.Printf("App %q was missing Destination.Server, so setting to %q\n", app.ObjectMeta.Name, app.Status.ComparisonResult.Server) - app.Spec.Destination.Server = app.Status.ComparisonResult.Server - changed = true - } - } - if app.Spec.Destination.Namespace == "" { - if app.Status.ComparisonResult.Status == appv1.ComparisonStatusUnknown || app.Status.ComparisonResult.Status == appv1.ComparisonStatusError { - log.Printf("App %q was missing Destination.Namespace, but could not fill it in: %s", app.ObjectMeta.Name, app.Status.ComparisonResult.Status) - } else { - log.Printf("App %q was missing Destination.Namespace, so setting to %q\n", app.ObjectMeta.Name, app.Status.ComparisonResult.Namespace) - app.Spec.Destination.Namespace = app.Status.ComparisonResult.Namespace - changed = true - } - } - - if changed { - _, err = appIf.UpdateSpec(context.Background(), &application.ApplicationSpecRequest{ - AppName: app.Name, - Spec: &app.Spec, - }) - if err != nil { - log.Println("An error occurred (but continuing anyway): ", err) - } - } - } -} -*/ - -func main() { - if len(os.Args) < 3 { - log.Fatalf("USAGE: %s SERVER NAMESPACE\n", os.Args[0]) - } - server, namespace := os.Args[1], os.Args[2] - log.Printf("Using argocd server %q and namespace %q\n", server, namespace) - - isLocalhost := false - switch { - case strings.HasPrefix(server, "localhost:"): - isLocalhost = true - case strings.HasPrefix(server, "127.0.0.1:"): - isLocalhost = true - } - - clientOpts := argocdclient.ClientOptions{ - ServerAddr: server, - Insecure: true, - PlainText: isLocalhost, - } - renameRepositorySecrets(clientOpts, namespace) - //populateAppDestinations(clientOpts) -}