Skip to content

Commit

Permalink
Collect Deployment Flag Options (nginx#1578)
Browse files Browse the repository at this point in the history
Problem: We want to collect deployment flag options so we can understand what a typical installation is using.

Solution: Collect the deployment flag options.
  • Loading branch information
miledxz committed Feb 28, 2024
1 parent 821159e commit 1445c5d
Show file tree
Hide file tree
Showing 7 changed files with 186 additions and 3 deletions.
30 changes: 30 additions & 0 deletions cmd/gateway/commands.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"time"

"github.com/spf13/cobra"
"github.com/spf13/pflag"
"go.uber.org/zap"
"k8s.io/apimachinery/pkg/types"
utilruntime "k8s.io/apimachinery/pkg/util/runtime"
Expand Down Expand Up @@ -178,6 +179,8 @@ func createStaticModeCommand() *cobra.Command {
}
}

flagKeys, flagValues := parseFlags(cmd.Flags())

conf := config.Config{
GatewayCtlrName: gatewayCtlrName.value,
ConfigName: configName.String(),
Expand Down Expand Up @@ -215,6 +218,10 @@ func createStaticModeCommand() *cobra.Command {
Version: version,
ExperimentalFeatures: gwExperimentalFeatures,
ImageSource: imageSource,
Flags: config.Flags{
Names: flagKeys,
Values: flagValues,
},
}

if err := static.StartManager(conf); err != nil {
Expand Down Expand Up @@ -450,3 +457,26 @@ func createSleepCommand() *cobra.Command {

return cmd
}

func parseFlags(flags *pflag.FlagSet) ([]string, []string) {
var flagKeys, flagValues []string

flags.VisitAll(
func(flag *pflag.Flag) {
flagKeys = append(flagKeys, flag.Name)

if flag.Value.Type() == "bool" {
flagValues = append(flagValues, flag.Value.String())
} else {
val := "user-defined"
if flag.Value.String() == flag.DefValue {
val = "default"
}

flagValues = append(flagValues, val)
}
},
)

return flagKeys, flagValues
}
127 changes: 127 additions & 0 deletions cmd/gateway/commands_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ import (

. "github.com/onsi/gomega"
"github.com/spf13/cobra"
"github.com/spf13/pflag"
"k8s.io/apimachinery/pkg/types"
)

type flagTestCase struct {
Expand Down Expand Up @@ -454,3 +456,128 @@ func TestSleepCmdFlagValidation(t *testing.T) {
})
}
}

func TestParseFlags(t *testing.T) {
g := NewWithT(t)

flagSet := pflag.NewFlagSet("flagSet", 0)
// set SortFlags to false for testing purposes so when parseFlags loops over the flagSet it
// goes off of primordial order.
flagSet.SortFlags = false

var boolFlagTrue bool
flagSet.BoolVar(
&boolFlagTrue,
"boolFlagTrue",
true,
"boolean true test flag",
)

var boolFlagFalse bool
flagSet.BoolVar(
&boolFlagFalse,
"boolFlagFalse",
false,
"boolean false test flag",
)

customIntFlagDefault := intValidatingValue{
validator: validatePort,
value: 8080,
}
flagSet.Var(
&customIntFlagDefault,
"customIntFlagDefault",
"default custom int test flag",
)

customIntFlagUserDefined := intValidatingValue{
validator: validatePort,
value: 8080,
}
flagSet.Var(
&customIntFlagUserDefined,
"customIntFlagUserDefined",
"user defined custom int test flag",
)
err := flagSet.Set("customIntFlagUserDefined", "8081")
g.Expect(err).To(Not(HaveOccurred()))

customStringFlagDefault := stringValidatingValue{
validator: validateResourceName,
value: "default-custom-string-test-flag",
}
flagSet.Var(
&customStringFlagDefault,
"customStringFlagDefault",
"default custom string test flag",
)

customStringFlagUserDefined := stringValidatingValue{
validator: validateResourceName,
value: "user-defined-custom-string-test-flag",
}
flagSet.Var(
&customStringFlagUserDefined,
"customStringFlagUserDefined",
"user defined custom string test flag",
)
err = flagSet.Set("customStringFlagUserDefined", "changed-test-flag-value")
g.Expect(err).To(Not(HaveOccurred()))

customStringFlagNoDefaultValueUnset := namespacedNameValue{
value: types.NamespacedName{},
}
flagSet.Var(
&customStringFlagNoDefaultValueUnset,
"customStringFlagNoDefaultValueUnset",
"no default value custom string test flag",
)

customStringFlagNoDefaultValueUserDefined := namespacedNameValue{
value: types.NamespacedName{},
}
flagSet.Var(
&customStringFlagNoDefaultValueUserDefined,
"customStringFlagNoDefaultValueUserDefined",
"no default value but with user defined namespacedName test flag",
)
userDefinedNamespacedName := types.NamespacedName{
Namespace: "changed-namespace",
Name: "changed-name",
}
err = flagSet.Set("customStringFlagNoDefaultValueUserDefined", userDefinedNamespacedName.String())
g.Expect(err).To(Not(HaveOccurred()))

expectedKeys := []string{
"boolFlagTrue",
"boolFlagFalse",

"customIntFlagDefault",
"customIntFlagUserDefined",

"customStringFlagDefault",
"customStringFlagUserDefined",

"customStringFlagNoDefaultValueUnset",
"customStringFlagNoDefaultValueUserDefined",
}
expectedValues := []string{
"true",
"false",

"default",
"user-defined",

"default",
"user-defined",

"default",
"user-defined",
}

flagKeys, flagValues := parseFlags(flagSet)

g.Expect(flagKeys).Should(Equal(expectedKeys))
g.Expect(flagValues).Should(Equal(expectedValues))
}
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ require (
github.com/prometheus/client_golang v1.18.0
github.com/prometheus/common v0.48.0
github.com/spf13/cobra v1.8.0
github.com/spf13/pflag v1.0.5
github.com/tsenart/vegeta/v12 v12.11.1
go.uber.org/zap v1.27.0
k8s.io/api v0.29.2
Expand Down Expand Up @@ -71,7 +72,6 @@ require (
github.com/prometheus/client_model v0.5.0 // indirect
github.com/prometheus/procfs v0.12.0 // indirect
github.com/rs/dnscache v0.0.0-20211102005908-e0241e321417 // indirect
github.com/spf13/pflag v1.0.5 // indirect
github.com/stretchr/testify v1.8.4 // indirect
go.uber.org/multierr v1.11.0 // indirect
golang.org/x/exp v0.0.0-20231006140011-7918f672742d // indirect
Expand Down
12 changes: 12 additions & 0 deletions internal/mode/static/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ type Config struct {
ImageSource string
// AtomicLevel is an atomically changeable, dynamic logging level.
AtomicLevel zap.AtomicLevel
// Flags contains the NGF command-line flag names and values.
Flags Flags
// GatewayNsName is the namespaced name of a Gateway resource that the Gateway will use.
// The Gateway will ignore all other Gateway resources.
GatewayNsName *types.NamespacedName
Expand Down Expand Up @@ -105,3 +107,13 @@ type UsageReportConfig struct {
// InsecureSkipVerify controls whether the client verifies the server cert.
InsecureSkipVerify bool
}

// Flags contains the NGF command-line flag names and values.
// Flag Names and Values are paired based off of index in slice.
type Flags struct {
// Names contains the name of the flag.
Names []string
// Values contains the value of the flag in string form.
// Each Value will be either true or false for boolean flags and default or user-defined for non-boolean flags.
Values []string
}
1 change: 1 addition & 0 deletions internal/mode/static/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -253,6 +253,7 @@ func StartManager(cfg config.Config) error {
Name: cfg.GatewayPodConfig.Name,
},
ImageSource: cfg.ImageSource,
Flags: cfg.Flags,
})
if err = mgr.Add(createTelemetryJob(cfg, dataCollector, nginxChecker.getReadyCh())); err != nil {
return fmt.Errorf("cannot register telemetry job: %w", err)
Expand Down
5 changes: 5 additions & 0 deletions internal/mode/static/telemetry/collector.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"k8s.io/apimachinery/pkg/types"
"sigs.k8s.io/controller-runtime/pkg/client"

"github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/config"
"github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/dataplane"
"github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/graph"
)
Expand Down Expand Up @@ -55,6 +56,7 @@ type Data struct {
Arch string
DeploymentID string
ImageSource string
Flags config.Flags
NGFResourceCounts NGFResourceCounts
NodeCount int
NGFReplicaCount int
Expand All @@ -74,6 +76,8 @@ type DataCollectorConfig struct {
PodNSName types.NamespacedName
// ImageSource is the source of the NGF image.
ImageSource string
// Flags contains the command-line NGF flag keys and values.
Flags config.Flags
}

// DataCollectorImpl is am implementation of DataCollector.
Expand Down Expand Up @@ -134,6 +138,7 @@ func (c DataCollectorImpl) Collect(ctx context.Context) (Data, error) {
ImageSource: c.cfg.ImageSource,
Arch: runtime.GOARCH,
DeploymentID: deploymentID,
Flags: c.cfg.Flags,
}

return data, nil
Expand Down
12 changes: 10 additions & 2 deletions internal/mode/static/telemetry/collector_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import (
gatewayv1 "sigs.k8s.io/gateway-api/apis/v1"

"github.com/nginxinc/nginx-gateway-fabric/internal/framework/events/eventsfakes"
"github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/config"
"github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/dataplane"
"github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/graph"
"github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/resolver"
Expand Down Expand Up @@ -77,8 +78,8 @@ var _ = Describe("Collector", Ordered, func() {
ngfPod *v1.Pod
ngfReplicaSet *appsv1.ReplicaSet
kubeNamespace *v1.Namespace

baseGetCalls getCallsFunc
baseGetCalls getCallsFunc
flags config.Flags
)

BeforeAll(func() {
Expand Down Expand Up @@ -125,6 +126,11 @@ var _ = Describe("Collector", Ordered, func() {
UID: "test-uid",
},
}

flags = config.Flags{
Names: []string{"boolFlag", "intFlag", "stringFlag"},
Values: []string{"false", "default", "user-defined"},
}
})

BeforeEach(func() {
Expand All @@ -137,6 +143,7 @@ var _ = Describe("Collector", Ordered, func() {
ImageSource: "local",
Arch: runtime.GOARCH,
DeploymentID: string(ngfReplicaSet.ObjectMeta.OwnerReferences[0].UID),
Flags: flags,
}

k8sClientReader = &eventsfakes.FakeReader{}
Expand All @@ -153,6 +160,7 @@ var _ = Describe("Collector", Ordered, func() {
Version: version,
PodNSName: podNSName,
ImageSource: "local",
Flags: flags,
})

baseGetCalls = createGetCallsFunc(ngfPod, ngfReplicaSet, kubeNamespace)
Expand Down

0 comments on commit 1445c5d

Please sign in to comment.