Merge pull request #1615 from tnozicka/limit-manager-calls

Limit scylla manager API calls duration

pkg/cmd/operator/manager_controller.go

@@ -123,7 +123,12 @@ func (o *ManagerControllerOptions) Complete() error {
 
 	// TODO: Use https and wire certs.
 	url := fmt.Sprintf("http://%s/api/v1", naming.ScyllaManagerServiceName)
-	managerClient, err := mermaidclient.NewClient(url, &http.Transport{})
+	managerClient, err := mermaidclient.NewClient(url, &http.Client{
+		// Limit manager calls by default to a higher bound.
+		// Individual calls can still be further limited using context.
+		// Manager is prone to extremely long calls because it (unfortunately) retries errors internally.
+		Timeout: 15 * time.Second,
+	})
 	if err != nil {
 		return fmt.Errorf("can't build manager client: %w", err)
 	}

pkg/controller/manager/controller.go

@@ -34,7 +34,11 @@ const (
 	ControllerName = "ScyllaManagerController"
 	// maxSyncDuration enforces preemption. Do not raise the value! Controllers shouldn't actively wait,
 	// but rather use the queue.
-	maxSyncDuration = 30 * time.Second
+	// Unfortunately, Scylla Manager calls are synchronous, internally retried and can take ages.
+	// Contrary to what it should be, this needs to be quite high.
+	// Given we reconcile tasks individually with an API call,
+	// this may not be enough for N tasks but should eventually make it.
+	maxSyncDuration = 2 * time.Minute
 )
 
 var (

pkg/mermaidclient/client.go

@@ -4,7 +4,6 @@ package mermaidclient
 
 import (
 	"context"
-	"crypto/tls"
 	"net/http"
 	"net/url"
 	"os"
@@ -30,15 +29,7 @@ type Client struct {
 	operations *operations.Client
 }
 
-// DefaultTLSConfig specifies default TLS configuration used when creating a new
-// client.
-var DefaultTLSConfig = func() *tls.Config {
-	return &tls.Config{
-		InsecureSkipVerify: true,
-	}
-}
-
-func NewClient(rawURL string, transport http.RoundTripper) (Client, error) {
+func NewClient(rawURL string, client *http.Client) (Client, error) {
 	u, err := url.Parse(rawURL)
 	if err != nil {
 		return Client{}, err
@@ -48,17 +39,7 @@ func NewClient(rawURL string, transport http.RoundTripper) (Client, error) {
 		middleware.Debug = false
 	})
 
-	if transport == nil {
-		transport = &http.Transport{
-			TLSClientConfig: DefaultTLSConfig(),
-		}
-	}
-
-	httpClient := &http.Client{
-		Transport: transport,
-	}
-
-	r := api.NewWithClient(u.Host, u.Path, []string{u.Scheme}, httpClient)
+	r := api.NewWithClient(u.Host, u.Path, []string{u.Scheme}, client)
 	// debug can be turned on by SWAGGER_DEBUG or DEBUG env variable
 	// we change that to SCTOOL_DUMP_HTTP
 	r.Debug, _ = strconv.ParseBool(os.Getenv("SCTOOL_DUMP_HTTP"))

test/e2e/utils/helpers.go

@@ -551,7 +551,7 @@ func GetManagerClient(ctx context.Context, client corev1client.CoreV1Interface)
 		Path:   "/api/v1",
 	}).String()
 
-	manager, err := mermaidclient.NewClient(apiAddress, &http.Transport{})
+	manager, err := mermaidclient.NewClient(apiAddress, &http.Client{})
 	if err != nil {
 		return nil, fmt.Errorf("create manager client, %w", err)
 	}