From d543e4298f52c1fb356c68f045f2f97d9b660325 Mon Sep 17 00:00:00 2001
From: Gabriel Nagy <gabriel.nagy@canonical.com>
Date: Wed, 28 Feb 2024 12:30:18 +0200
Subject: [PATCH] Increase gpolist command timeout for AD tests

As the AD object is initialized multiple times in tests but only once in
production I opted for the less intuitive approach of defaulting to the
longer timeout and overriding with the production one instead of the
other way around.

This should fix the ad tests we've seen time out in armhf autopkgtests.
---
 internal/ad/ad.go                     | 24 ++++++++++++++++++------
 internal/adsysservice/adsysservice.go |  1 +
 internal/consts/consts.go             |  9 ++++++++-
 3 files changed, 27 insertions(+), 7 deletions(-)

diff --git a/internal/ad/ad.go b/internal/ad/ad.go
index 37f84bfa8..6036a20a5 100644
--- a/internal/ad/ad.go
+++ b/internal/ad/ad.go
@@ -79,6 +79,7 @@ type AD struct {
 
 	withoutKerberos bool
 	gpoListCmd      []string
+	gpoListTimeout  time.Duration
 }
 
 type options struct {
@@ -88,6 +89,7 @@ type options struct {
 
 	withoutKerberos bool
 	gpoListCmd      []string
+	gpoListTimeout  time.Duration
 }
 
 // Option reprents an optional function to change AD behavior.
@@ -109,6 +111,14 @@ func WithRunDir(runDir string) Option {
 	}
 }
 
+// WithGpoListTimeout specifies a custom timeout for the adsys-gpolist command.
+func WithGpoListTimeout(timeout time.Duration) Option {
+	return func(o *options) error {
+		o.gpoListTimeout = timeout
+		return nil
+	}
+}
+
 // AdsysGpoListCode is the embedded script which request
 // Samba to get our GPO list for the given object.
 //
@@ -126,10 +136,11 @@ func New(ctx context.Context, configBackend backends.Backend, hostname string, o
 
 	// defaults
 	args := options{
-		runDir:     consts.DefaultRunDir,
-		cacheDir:   consts.DefaultCacheDir,
-		gpoListCmd: []string{"python3", "-c", AdsysGpoListCode},
-		versionID:  versionID,
+		runDir:         consts.DefaultRunDir,
+		cacheDir:       consts.DefaultCacheDir,
+		gpoListCmd:     []string{"python3", "-c", AdsysGpoListCode},
+		versionID:      versionID,
+		gpoListTimeout: 30 * time.Second, // this is used in tests and set to consts.DefaultGpoListTimeout in production
 	}
 	// applied options
 	for _, o := range opts {
@@ -167,8 +178,9 @@ func New(ctx context.Context, configBackend backends.Backend, hostname string, o
 		policiesCacheDir: policiesCacheDir,
 		krb5CacheDir:     krb5CacheDir,
 
-		downloadables: make(map[string]*downloadable),
-		gpoListCmd:    args.gpoListCmd,
+		downloadables:  make(map[string]*downloadable),
+		gpoListCmd:     args.gpoListCmd,
+		gpoListTimeout: args.gpoListTimeout,
 	}, nil
 }
 
diff --git a/internal/adsysservice/adsysservice.go b/internal/adsysservice/adsysservice.go
index 557b9fa57..0ad802ff1 100644
--- a/internal/adsysservice/adsysservice.go
+++ b/internal/adsysservice/adsysservice.go
@@ -240,6 +240,7 @@ func New(ctx context.Context, opts ...option) (s *Service, err error) {
 	if args.runDir != "" {
 		adOptions = append(adOptions, ad.WithRunDir(args.runDir))
 	}
+	adOptions = append(adOptions, ad.WithGpoListTimeout(consts.DefaultGpoListTimeout))
 
 	hostname, err := os.Hostname()
 	if err != nil {
diff --git a/internal/consts/consts.go b/internal/consts/consts.go
index c5b73097c..b0350c7ed 100644
--- a/internal/consts/consts.go
+++ b/internal/consts/consts.go
@@ -1,7 +1,11 @@
 // Package consts defines the constants used by the project
 package consts
 
-import log "github.com/sirupsen/logrus"
+import (
+	"time"
+
+	log "github.com/sirupsen/logrus"
+)
 
 var (
 	// Version is the version of the executable.
@@ -36,6 +40,9 @@ const (
 	// DefaultServiceTimeout is the default time in seconds without any active request before the service exits.
 	DefaultServiceTimeout = 120
 
+	// DefaultGpoListTimeout is the default time to wait for the GPO list subcommand to finish.
+	DefaultGpoListTimeout = 10 * time.Second
+
 	// DistroID is the distro ID which can be overridden at build time.
 	DistroID = "Ubuntu"
 )