Skip to content

Commit

Permalink
Add CLI flag for aggregation timeout
Browse files Browse the repository at this point in the history
One of the most common config settings that may change between
different environments and test cases is the amount of time
Sonobuoy will wait for results. Currently, this value is configurable
but it is in a nested config structure and not terribly well
documented at this time.

Personally, in the past, I had projects where we had to start
using a sonobuoy config-based flow instead of flags due solely
to this field.

This change adds the flag to make it more easy to set.

Fixes #482

Signed-off-by: John Schnake <jschnake@vmware.com>
  • Loading branch information
johnSchnake committed Apr 26, 2019
1 parent 1b56c31 commit aaa5e13
Show file tree
Hide file tree
Showing 6 changed files with 41 additions and 8 deletions.
9 changes: 9 additions & 0 deletions cmd/sonobuoy/app/args.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ const (
sonobuoyImageFlag = "sonobuoy-image"
imagePullPolicyFlag = "image-pull-policy"
pluginFlag = "plugin"
timeoutFlag = "timeout"
)

// AddNamespaceFlag initialises a namespace flag.
Expand Down Expand Up @@ -253,6 +254,14 @@ func AddRunWaitFlag(flag *int, flags *pflag.FlagSet) {
flags.Lookup("wait").NoOptDefVal = "1440"
}

// AddTimeoutFlag adds an int flag for waiting for the entire run to finish.
func AddTimeoutFlag(flag *int, flags *pflag.FlagSet) {
flags.IntVar(
flag, timeoutFlag, config.DefaultAggregationServerTimeoutSeconds,
"How long (in seconds) Sonobuoy will wait for plugins to complete before exiting. 0 indicates no timeout.",
)
}

// AddImagePullPolicyFlag adds a boolean flag for deleting everything (including E2E tests).
func AddImagePullPolicyFlag(policy *ImagePullPolicy, flags *pflag.FlagSet) {
*policy = ImagePullPolicy(v1.PullAlways) //default
Expand Down
6 changes: 6 additions & 0 deletions cmd/sonobuoy/app/gen.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ type genFlags struct {
kubeConformanceImageVersion imagepkg.ConformanceImageVersion
imagePullPolicy ImagePullPolicy
e2eRepoList string
timeoutSeconds int

// plugins will keep a list of the plugins we want. Custom type for
// flag support.
Expand All @@ -68,6 +69,7 @@ func GenFlagSet(cfg *genFlags, rbac RBACMode) *pflag.FlagSet {
cfg.e2eflags = AddE2EConfigFlags(genset)
AddRBACModeFlags(&cfg.rbacMode, genset, rbac)
AddImagePullPolicyFlag(&cfg.imagePullPolicy, genset)
AddTimeoutFlag(&cfg.timeoutSeconds, genset)

AddNamespaceFlag(&cfg.namespace, genset)
AddSonobuoyImage(&cfg.sonobuoyImage, genset)
Expand Down Expand Up @@ -276,5 +278,9 @@ func (g *genFlags) resolveConfig() *config.Config {
conf.ImagePullPolicy = g.imagePullPolicy.String()
}

if g.genflags.Changed(timeoutFlag) {
conf.Aggregation.TimeoutSeconds = g.timeoutSeconds
}

return conf
}
22 changes: 17 additions & 5 deletions cmd/sonobuoy/app/gen_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ func TestResolveConformanceImage(t *testing.T) {

func TestResolveConfig(t *testing.T) {
defaultPluginSearchPath := config.New().PluginSearchPath
defaultAggr := plugin.AggregationConfig{TimeoutSeconds: 10800}

tcs := []struct {
name string
Expand All @@ -101,6 +102,7 @@ func TestResolveConfig(t *testing.T) {
plugin.Selection{Name: "systemd-logs"},
},
PluginSearchPath: defaultPluginSearchPath,
Aggregation: defaultAggr,
},
}, {
name: "Quick mode and a non-nil supplied config",
Expand All @@ -124,8 +126,9 @@ func TestResolveConfig(t *testing.T) {
plugin.Selection{Name: "e2e"},
},
Aggregation: plugin.AggregationConfig{
BindAddress: "10.0.0.1",
BindPort: config.DefaultAggregationServerBindPort,
BindAddress: "10.0.0.1",
BindPort: config.DefaultAggregationServerBindPort,
TimeoutSeconds: 10800,
},
PluginSearchPath: defaultPluginSearchPath,
},
Expand Down Expand Up @@ -155,8 +158,9 @@ func TestResolveConfig(t *testing.T) {
},
PluginSearchPath: defaultPluginSearchPath,
Aggregation: plugin.AggregationConfig{
BindAddress: config.DefaultAggregationServerBindAddress,
BindPort: config.DefaultAggregationServerBindPort,
BindAddress: config.DefaultAggregationServerBindAddress,
BindPort: config.DefaultAggregationServerBindPort,
TimeoutSeconds: config.DefaultAggregationServerTimeoutSeconds,
},
},
}, {
Expand All @@ -166,7 +170,7 @@ func TestResolveConfig(t *testing.T) {
Config: config.Config{Namespace: "configNS"},
},
},
cliInput: "--namespace=flagNS --sonobuoy-image=flagImage --image-pull-policy=Always",
cliInput: "--namespace=flagNS --sonobuoy-image=flagImage --image-pull-policy=Always --timeout 100",
expected: &config.Config{
Namespace: "flagNS",
WorkerImage: "flagImage",
Expand All @@ -176,6 +180,7 @@ func TestResolveConfig(t *testing.T) {
plugin.Selection{Name: "systemd-logs"},
},
PluginSearchPath: defaultPluginSearchPath,
Aggregation: plugin.AggregationConfig{TimeoutSeconds: 100},
},
}, {
name: "Flags shouldn't override the config settings unless set",
Expand All @@ -190,6 +195,7 @@ func TestResolveConfig(t *testing.T) {
plugin.Selection{Name: "systemd-logs"},
},
PluginSearchPath: defaultPluginSearchPath,
Aggregation: plugin.AggregationConfig{TimeoutSeconds: 500},
},
}, {
name: "Flags shouldn't override the config settings unless set",
Expand All @@ -204,6 +210,7 @@ func TestResolveConfig(t *testing.T) {
plugin.Selection{Name: "systemd-logs"},
},
PluginSearchPath: defaultPluginSearchPath,
Aggregation: plugin.AggregationConfig{TimeoutSeconds: 500},
},
}, {
name: "Manually specified plugins should result in empty selection",
Expand All @@ -215,6 +222,7 @@ func TestResolveConfig(t *testing.T) {
ImagePullPolicy: "IfNotPresent",
PluginSelections: nil,
PluginSearchPath: defaultPluginSearchPath,
Aggregation: defaultAggr,
},
},
}
Expand Down Expand Up @@ -244,6 +252,10 @@ func TestResolveConfig(t *testing.T) {
t.Errorf("Expected image pull policy %v but got %v", tc.expected.ImagePullPolicy, conf.ImagePullPolicy)
}

if conf.Aggregation.TimeoutSeconds != tc.expected.Aggregation.TimeoutSeconds {
t.Errorf("Expected timeout %v but got %v", tc.expected.Aggregation.TimeoutSeconds, conf.Aggregation.TimeoutSeconds)
}

if len(conf.PluginSelections) != len(tc.expected.PluginSelections) {
t.Fatalf("expected %v plugin selections but found %v", tc.expected.PluginSelections, conf.PluginSelections)
}
Expand Down
3 changes: 2 additions & 1 deletion cmd/sonobuoy/app/testdata/sonobuoy.conf
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
{
"Namespace":"configNS",
"WorkerImage":"configImage",
"ImagePullPolicy":"Never"
"ImagePullPolicy":"Never",
"Server":{"timeoutseconds":500}
}
4 changes: 3 additions & 1 deletion pkg/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@ const (
DefaultAggregationServerBindPort = 8080
// DefaultAggregationServerBindAddress is the default address for the aggregation server to bind to.
DefaultAggregationServerBindAddress = "0.0.0.0"
// DefaultAggregationServerTimeoutSeconds is the default amount of time the aggregation server will wait for all plugins to complete.
DefaultAggregationServerTimeoutSeconds = 10800 // 180 min
// MasterPodName is the name of the main pod that runs plugins and collects results.
MasterPodName = "sonobuoy"
// MasterContainerName is the name of the main container in the master pod.
Expand Down Expand Up @@ -290,7 +292,7 @@ func New() *Config {

cfg.Aggregation.BindAddress = DefaultAggregationServerBindAddress
cfg.Aggregation.BindPort = DefaultAggregationServerBindPort
cfg.Aggregation.TimeoutSeconds = 10800 // 180 minutes
cfg.Aggregation.TimeoutSeconds = DefaultAggregationServerTimeoutSeconds

cfg.PluginSearchPath = []string{
"./plugins.d",
Expand Down
5 changes: 4 additions & 1 deletion pkg/plugin/aggregation/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,10 @@ func Run(client kubernetes.Interface, plugins []plugin.Interface, cfg plugin.Agg
// Give the plugins a chance to cleanup before a hard timeout occurs
shutdownPlugins := time.After(time.Duration(cfg.TimeoutSeconds-plugin.GracefulShutdownPeriod) * time.Second)
// Ensure we only wait for results for a certain time
timeout := time.After(time.Duration(cfg.TimeoutSeconds) * time.Second)
var timeout <-chan time.Time
if cfg.TimeoutSeconds > 0 {
timeout = time.After(time.Duration(cfg.TimeoutSeconds) * time.Second)
}

// 6. Wait for aggr to show that all results are accounted for
for {
Expand Down

0 comments on commit aaa5e13

Please sign in to comment.