- Add unit tests(Ginkgo) for setting values of burst,qps

- Revert variable name to BROKER_K8S prefix

Author: msft-paddy14

pkg/syncer/broker/config.go

@@ -31,26 +31,14 @@ type brokerSpecification struct {
 	APIServer       string
 	APIServerToken  string
 	RemoteNamespace string
-	Insecure        bool `default:"false"`
+	Insecure        bool    `default:"false"`
 	Ca              string
 	Secret          string
+	QPS             float32 `default:"5"`
+	Burst           int     `default:"10"`
 }
 
-// clientSpecification defines the client throttling configuration.
-// These values are read from environment variables with the prefix "CLIENT_K8S_".
-//
-// Environment variables:
-//   - CLIENT_K8S_QPS: Maximum queries per second to the API server (default: 5)
-//   - CLIENT_K8S_BURST: Maximum burst for throttle to the API server (default: 10)
-type clientSpecification struct {
-	QPS   float32 `default:"5"`
-	Burst int     `default:"10"`
-}
-
-const (
-	brokerConfigPrefix = "broker_k8s"
-	clientConfigPrefix = "submariner_client_k8s"
-)
+const brokerConfigPrefix = "broker_k8s"
 
 func getBrokerSpecification() (*brokerSpecification, error) {
 	brokerSpec := brokerSpecification{}
@@ -63,17 +51,6 @@ func getBrokerSpecification() (*brokerSpecification, error) {
 	return &brokerSpec, nil
 }
 
-func getClientSpecification() (*clientSpecification, error) {
-	clientSpec := clientSpecification{}
-
-	err := envconfig.Process(clientConfigPrefix, &clientSpec)
-	if err != nil {
-		return nil, errors.Wrap(err, "error processing client env configuration")
-	}
-
-	return &clientSpec, nil
-}
-
 func EnvironmentVariable(setting string) string {
 	// Check the setting is known (ignoring case)
 	s := reflect.ValueOf(&brokerSpecification{})

pkg/syncer/broker/syncer.go

@@ -362,18 +362,18 @@ func (c *SyncerConfig) ensureClients() error {
 	var err error
 
 	// Get client QPS/Burst settings from environment variables
-	clientSpec, err := getClientSpecification()
+	brokerSpec, err := getBrokerSpecification()
 	if err != nil {
 		return err
 	}
 
 	// Apply QPS and Burst settings to local REST config
-	if c.LocalRestConfig != nil && clientSpec.QPS > 0 {
-		c.LocalRestConfig.QPS = clientSpec.QPS
+	if c.LocalRestConfig != nil && brokerSpec.QPS > 0 {
+		c.LocalRestConfig.QPS = brokerSpec.QPS
 	}
 
-	if c.LocalRestConfig != nil && clientSpec.Burst > 0 {
-		c.LocalRestConfig.Burst = clientSpec.Burst
+	if c.LocalRestConfig != nil && brokerSpec.Burst > 0 {
+		c.LocalRestConfig.Burst = brokerSpec.Burst
 	}
 
 	if c.RestMapper == nil {
@@ -397,12 +397,12 @@ func (c *SyncerConfig) ensureClients() error {
 	}
 
 	// Apply QPS and Burst settings to broker REST config
-	if c.BrokerRestConfig != nil && clientSpec.QPS > 0 {
-		c.BrokerRestConfig.QPS = clientSpec.QPS
+	if c.BrokerRestConfig != nil && brokerSpec.QPS > 0 {
+		c.BrokerRestConfig.QPS = brokerSpec.QPS
 	}
 
-	if c.BrokerRestConfig != nil && clientSpec.Burst > 0 {
-		c.BrokerRestConfig.Burst = clientSpec.Burst
+	if c.BrokerRestConfig != nil && brokerSpec.Burst > 0 {
+		c.BrokerRestConfig.Burst = brokerSpec.Burst
 	}
 
 	return nil

pkg/syncer/broker/syncer_test.go

@@ -74,6 +74,8 @@ var _ = Describe("Broker Syncer", func() {
 		os.Unsetenv("BROKER_K8S_REMOTENAMESPACE")
 		os.Unsetenv("BROKER_K8S_INSECURE")
 		os.Unsetenv("BROKER_K8S_SECRET")
+		os.Unsetenv("BROKER_K8S_QPS")
+		os.Unsetenv("BROKER_K8S_BURST")
 
 		expectInitError = false
 		actualBrokerRestConfig = nil
@@ -688,4 +690,48 @@ var _ = Describe("Broker Syncer", func() {
 			})
 		})
 	})
+
+	When("client QPS/Burst environment vars are specified", func() {
+		BeforeEach(func() {
+			os.Setenv("BROKER_K8S_QPS", "100")
+			os.Setenv("BROKER_K8S_BURST", "500")
+
+			config.LocalRestConfig = &rest.Config{
+				Host: "https://local",
+			}
+
+			config.BrokerRestConfig = &rest.Config{
+				Host: "https://broker",
+			}
+		})
+
+		It("should apply QPS and Burst to both local and broker rest configs", func() {
+			Expect(config.LocalRestConfig.QPS).To(Equal(float32(100)))
+			Expect(config.LocalRestConfig.Burst).To(Equal(500))
+			Expect(actualBrokerRestConfig.QPS).To(Equal(float32(100)))
+			Expect(actualBrokerRestConfig.Burst).To(Equal(500))
+		})
+	})
+
+	When("client QPS/Burst environment vars are not specified", func() {
+		BeforeEach(func() {
+			os.Unsetenv("BROKER_K8S_QPS")
+			os.Unsetenv("BROKER_K8S_BURST")
+
+			config.LocalRestConfig = &rest.Config{
+				Host: "https://local",
+			}
+
+			config.BrokerRestConfig = &rest.Config{
+				Host: "https://broker",
+			}
+		})
+
+		It("should apply default QPS (5) and Burst (10) to both local and broker rest configs", func() {
+			Expect(config.LocalRestConfig.QPS).To(Equal(float32(5)))
+			Expect(config.LocalRestConfig.Burst).To(Equal(10))
+			Expect(actualBrokerRestConfig.QPS).To(Equal(float32(5)))
+			Expect(actualBrokerRestConfig.Burst).To(Equal(10))
+		})
+	})
 })