From ad47e29e29b26a15e322acf5ae91ac53aeeedd36 Mon Sep 17 00:00:00 2001 From: Oded Ben Ozer Date: Wed, 5 Jun 2024 09:06:33 +0200 Subject: [PATCH] fix Telefonistka "hungs" on diff when ArgoCD application-controller is down issue (#7) * Improve error logging * Move context creation and add timeout to propely handle timeouts --- cmd/telefonistka/server.go | 7 ++++--- internal/pkg/argocd/argocd.go | 13 +++++++++++++ 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/cmd/telefonistka/server.go b/cmd/telefonistka/server.go index bec061b2..6f53483e 100644 --- a/cmd/telefonistka/server.go +++ b/cmd/telefonistka/server.go @@ -37,14 +37,15 @@ func init() { //nolint:gochecknoinits rootCmd.AddCommand(serveCmd) } -func handleWebhook(ctx context.Context, githubWebhookSecret []byte, mainGhClientCache *lru.Cache[string, githubapi.GhClientPair], prApproverGhClientCache *lru.Cache[string, githubapi.GhClientPair]) func(http.ResponseWriter, *http.Request) { +func handleWebhook(githubWebhookSecret []byte, mainGhClientCache *lru.Cache[string, githubapi.GhClientPair], prApproverGhClientCache *lru.Cache[string, githubapi.GhClientPair]) func(http.ResponseWriter, *http.Request) { return func(w http.ResponseWriter, r *http.Request) { + ctx, cancel := context.WithTimeout(r.Context(), 30*time.Second) + defer cancel() githubapi.HandleEvent(r, ctx, mainGhClientCache, prApproverGhClientCache, githubWebhookSecret) } } func serve() { - ctx := context.Background() githubWebhookSecret := []byte(getCrucialEnv("GITHUB_WEBHOOK_SECRET")) livenessChecker := health.NewChecker() // No checks for the moment, other then the http server availability readinessChecker := health.NewChecker() @@ -54,7 +55,7 @@ func serve() { prApproverGhClientCache, _ := lru.New[string, githubapi.GhClientPair](128) mux := http.NewServeMux() - mux.HandleFunc("/webhook", handleWebhook(ctx, githubWebhookSecret, mainGhClientCache, prApproverGhClientCache)) + mux.HandleFunc("/webhook", handleWebhook(githubWebhookSecret, mainGhClientCache, prApproverGhClientCache)) mux.Handle("/metrics", promhttp.Handler()) mux.Handle("/live", health.NewHandler(livenessChecker)) mux.Handle("/ready", health.NewHandler(readinessChecker)) diff --git a/internal/pkg/argocd/argocd.go b/internal/pkg/argocd/argocd.go index b45c9207..b4552462 100644 --- a/internal/pkg/argocd/argocd.go +++ b/internal/pkg/argocd/argocd.go @@ -163,12 +163,14 @@ func generateDiffOfAComponent(ctx context.Context, componentPath string, prBranc // Find ArgoCD application by the path SHA1 label selector and repo name // That label is assumed to be pupulated by the ApplicationSet controller(or apps of apps or similar). labelSelector := fmt.Sprintf("telefonistka.io/component-path-sha1=%s", componentPathSha1) + log.Debugf("Using label selector: %s", labelSelector) appLabelQuery := application.ApplicationQuery{ Selector: &labelSelector, Repo: &repo, } foundApps, err := appIf.List(ctx, &appLabelQuery) if err != nil { + log.Errorf("Error listing ArgoCD applications: %v", err) componentDiffResult.DiffError = err return componentDiffResult } @@ -177,6 +179,7 @@ func generateDiffOfAComponent(ctx context.Context, componentPath string, prBranc return componentDiffResult } + log.Debugf("Found ArgoCD application: %s", foundApps.Items[0].Name) // Get the application and its resources, resources are the live state of the application objects. refreshType := string(argoappv1.RefreshTypeHard) appNameQuery := application.ApplicationQuery{ @@ -189,6 +192,7 @@ func generateDiffOfAComponent(ctx context.Context, componentPath string, prBranc log.Errorf("Error getting app %s: %v", foundApps.Items[0].Name, err) return componentDiffResult } + log.Debugf("Got ArgoCD app %s", app.Name) componentDiffResult.ArgoCdAppName = app.Name componentDiffResult.ArgoCdAppURL = fmt.Sprintf("%s/applications/%s", argoSettings.URL, app.Name) resources, err := appIf.ManagedResources(ctx, &application.ResourcesQuery{ApplicationName: &app.Name, AppNamespace: &app.Namespace}) @@ -197,6 +201,7 @@ func generateDiffOfAComponent(ctx context.Context, componentPath string, prBranc log.Errorf("Error getting (live)resources for app %s: %v", app.Name, err) return componentDiffResult } + log.Debugf("Got (live)resources for app %s", app.Name) // Get the application manifests, these are the target state of the application objects, taken from the git repo, specificly from the PR branch. diffOption := &DifferenceOption{} @@ -212,6 +217,7 @@ func generateDiffOfAComponent(ctx context.Context, componentPath string, prBranc log.Errorf("Error getting manifests for app %s, revision %s: %v", app.Name, prBranch, err) return componentDiffResult } + log.Debugf("Got manifests for app %s, revision %s", app.Name, prBranch) diffOption.res = manifests diffOption.revision = prBranch @@ -238,34 +244,41 @@ func GenerateDiffOfChangedComponents(ctx context.Context, componentPathList []st // env var should be centralized client, err := createArgoCdClient() if err != nil { + log.Errorf("Error creating ArgoCD client: %v", err) return false, true, nil, err } conn, appIf, err := client.NewApplicationClient() if err != nil { + log.Errorf("Error creating ArgoCD app client: %v", err) return false, true, nil, err } defer argoio.Close(conn) conn, projIf, err := client.NewProjectClient() if err != nil { + log.Errorf("Error creating ArgoCD project client: %v", err) return false, true, nil, err } defer argoio.Close(conn) conn, settingsIf, err := client.NewSettingsClient() if err != nil { + log.Errorf("Error creating ArgoCD settings client: %v", err) return false, true, nil, err } defer argoio.Close(conn) argoSettings, err := settingsIf.Get(ctx, &settings.SettingsQuery{}) if err != nil { + log.Errorf("Error getting ArgoCD settings: %v", err) return false, true, nil, err } + log.Debugf("Checking ArgoCD diff for components: %v", componentPathList) for _, componentPath := range componentPathList { currentDiffResult := generateDiffOfAComponent(ctx, componentPath, prBranch, repo, appIf, projIf, argoSettings) if currentDiffResult.DiffError != nil { + log.Errorf("Error generating diff for component %s: %v", componentPath, currentDiffResult.DiffError) hasComponentDiffErrors = true err = currentDiffResult.DiffError }