Add some review feedback

Author: bjee19

cmd/gateway/commands.go

@@ -179,7 +179,7 @@ func createStaticModeCommand() *cobra.Command {
 				}
 			}
 
-			flagKeys, flagValues := parseFlagKeysAndValues(cmd.Flags())
+			flagKeys, flagValues := parseFlags(cmd.Flags())
 
 			conf := config.Config{
 				GatewayCtlrName:          gatewayCtlrName.value,
@@ -218,9 +218,9 @@ func createStaticModeCommand() *cobra.Command {
 				Version:              version,
 				ExperimentalFeatures: gwExperimentalFeatures,
 				ImageSource:          imageSource,
-				FlagKeyValues: config.FlagKeyValues{
-					FlagKeys:   flagKeys,
-					FlagValues: flagValues,
+				Flags: config.Flags{
+					Names:  flagKeys,
+					Values: flagValues,
 				},
 			}
 
@@ -458,17 +458,17 @@ func createSleepCommand() *cobra.Command {
 	return cmd
 }
 
-func parseFlagKeysAndValues(flags *pflag.FlagSet) (flagKeys, flagValues []string) {
+func parseFlags(flags *pflag.FlagSet) (flagKeys, flagValues []string) {
 	flagKeys = []string{}
 	flagValues = []string{}
 
 	flags.VisitAll(
 		func(flag *pflag.Flag) {
 			flagKeys = append(flagKeys, flag.Name)
-			switch flag.Value.Type() {
-			case "bool":
+
+			if flag.Value.Type() == "bool" {
 				flagValues = append(flagValues, flag.Value.String())
-			default:
+			} else {
 				var val string
 				if flag.Value.String() == flag.DefValue {
 					val = "default"

cmd/gateway/commands_test.go

@@ -457,11 +457,11 @@ func TestSleepCmdFlagValidation(t *testing.T) {
 	}
 }
 
-func TestParseFlagKeysAndValues(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 parseFlagKeysAndValues loops over the flagSet it
+	// set SortFlags to false for testing purposes so when parseFlags loops over the flagSet it
 	// goes off of primordial order.
 	flagSet.SortFlags = false
 
@@ -582,7 +582,7 @@ func TestParseFlagKeysAndValues(t *testing.T) {
 		"user-defined",
 	}
 
-	flagKeys, flagValues := parseFlagKeysAndValues(flagSet)
+	flagKeys, flagValues := parseFlags(flagSet)
 
 	g.Expect(flagKeys).Should(Equal(expectedKeys))
 	g.Expect(flagValues).Should(Equal(expectedValues))

internal/mode/static/config/config.go

@@ -15,8 +15,8 @@ type Config struct {
 	ImageSource string
 	// AtomicLevel is an atomically changeable, dynamic logging level.
 	AtomicLevel zap.AtomicLevel
-	// FlagKeyValues contains the parsed NGF flag keys and values.
-	FlagKeyValues FlagKeyValues
+	// 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
@@ -108,12 +108,12 @@ type UsageReportConfig struct {
 	InsecureSkipVerify bool
 }
 
-// FlagKeyValues contains the parsed NGF flag keys and values.
-// Flag Key and Value are paired based off of index in slice.
-type FlagKeyValues struct {
-	// FlagKeys contains the name of the flag.
-	FlagKeys []string
-	// FlagValues contains the value of the flag in string form.
-	// Value will be either true or false for boolean flags and default or user-defined for non-boolean flags.
-	FlagValues []string
+// 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
 }

internal/mode/static/manager.go

@@ -252,8 +252,8 @@ func StartManager(cfg config.Config) error {
 				Namespace: cfg.GatewayPodConfig.Namespace,
 				Name:      cfg.GatewayPodConfig.Name,
 			},
-			ImageSource:   cfg.ImageSource,
-			FlagKeyValues: cfg.FlagKeyValues,
+			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)

internal/mode/static/telemetry/collector.go

@@ -56,7 +56,7 @@ type Data struct {
 	Arch              string
 	DeploymentID      string
 	ImageSource       string
-	FlagKeyValues     config.FlagKeyValues
+	Flags             config.Flags
 	NGFResourceCounts NGFResourceCounts
 	NodeCount         int
 	NGFReplicaCount   int
@@ -76,8 +76,8 @@ type DataCollectorConfig struct {
 	PodNSName types.NamespacedName
 	// ImageSource is the source of the NGF image.
 	ImageSource string
-	// FlagKeyValues contains the parsed NGF flag keys and values.
-	FlagKeyValues config.FlagKeyValues
+	// Flags contains the command-line NGF flag keys and values.
+	Flags config.Flags
 }
 
 // DataCollectorImpl is am implementation of DataCollector.
@@ -138,7 +138,7 @@ func (c DataCollectorImpl) Collect(ctx context.Context) (Data, error) {
 		ImageSource:     c.cfg.ImageSource,
 		Arch:            runtime.GOARCH,
 		DeploymentID:    deploymentID,
-		FlagKeyValues:   c.cfg.FlagKeyValues,
+		Flags:           c.cfg.Flags,
 	}
 
 	return data, nil

internal/mode/static/telemetry/collector_test.go

@@ -79,7 +79,7 @@ var _ = Describe("Collector", Ordered, func() {
 		ngfReplicaSet           *appsv1.ReplicaSet
 		kubeNamespace           *v1.Namespace
 		baseGetCalls            getCallsFunc
-		flagKeyValues           config.FlagKeyValues
+		flags                   config.Flags
 	)
 
 	BeforeAll(func() {
@@ -127,9 +127,9 @@ var _ = Describe("Collector", Ordered, func() {
 			},
 		}
 
-		flagKeyValues = config.FlagKeyValues{
-			FlagKeys:   []string{"boolFlag", "intFlag", "stringFlag"},
-			FlagValues: []string{"false", "default", "user-defined"},
+		flags = config.Flags{
+			Names:  []string{"boolFlag", "intFlag", "stringFlag"},
+			Values: []string{"false", "default", "user-defined"},
 		}
 	})
 
@@ -140,10 +140,14 @@ var _ = Describe("Collector", Ordered, func() {
 			NGFResourceCounts: telemetry.NGFResourceCounts{},
 			NGFReplicaCount:   1,
 			ClusterID:         string(kubeNamespace.GetUID()),
+<<<<<<< HEAD
 			ImageSource:       "local",
 			Arch:              runtime.GOARCH,
 			DeploymentID:      string(ngfReplicaSet.ObjectMeta.OwnerReferences[0].UID),
 			FlagKeyValues:     flagKeyValues,
+=======
+			Flags:             flags,
+>>>>>>> 9e04b8c (Add some review feedback)
 		}
 
 		k8sClientReader = &eventsfakes.FakeReader{}
@@ -159,8 +163,12 @@ var _ = Describe("Collector", Ordered, func() {
 			ConfigurationGetter: fakeConfigurationGetter,
 			Version:             version,
 			PodNSName:           podNSName,
+<<<<<<< HEAD
 			ImageSource:         "local",
 			FlagKeyValues:       flagKeyValues,
+=======
+			Flags:               flags,
+>>>>>>> 9e04b8c (Add some review feedback)
 		})
 
 		baseGetCalls = createGetCallsFunc(ngfPod, ngfReplicaSet, kubeNamespace)