Address comments

Signed-off-by: graysonwu <wgrayson@vmware.com>
antrea-io · Oct 20, 2023 · b2a0e00 · b2a0e00
1 parent 45a079e
commit b2a0e00
Show file tree

Hide file tree

Showing 4 changed files with 28 additions and 27 deletions.
diff --git a/pkg/agent/controller/egress/egress_controller.go b/pkg/agent/controller/egress/egress_controller.go
@@ -92,19 +92,20 @@ type egressState struct {
 	// The actual Pods of the Egress. Used to identify stale Pods when updating or deleting an Egress.
 	pods sets.Set[string]
 	// Rate-limit of this Egress.
-	rateLimit *rateLimit
+	rateLimitMeter *rateLimitMeter
 }
 
-type rateLimit struct {
-	Rate  uint32
-	Burst uint32
+type rateLimitMeter struct {
+	MeterID uint32
+	Rate    uint32
+	Burst   uint32
 }
 
-func (r *rateLimit) Equals(rateLimit *rateLimit) bool {
+func (r *rateLimitMeter) Equals(rateLimit *rateLimitMeter) bool {
 	if r == nil && rateLimit == nil {
 		return true
 	}
-	if (r == nil) != (rateLimit == nil) {
+	if r == nil || rateLimit == nil {
 		return false
 	}
 	if r.Rate == rateLimit.Rate && r.Burst == rateLimit.Burst {
@@ -531,49 +532,50 @@ func (c *EgressController) realizeEgressIP(egressName, egressIP string) (uint32,
 	return ipState.mark, nil
 }
 
-func bandwidthToRateLimit(bandwidth *crdv1b1.Bandwidth) *rateLimit {
+func bandwidthToRateLimitMeter(bandwidth *crdv1b1.Bandwidth, meterID uint32) *rateLimitMeter {
 	if bandwidth == nil {
 		return nil
 	}
 	rate, _ := resource.ParseQuantity(bandwidth.Rate)
 	burst, _ := resource.ParseQuantity(bandwidth.Burst)
-	return &rateLimit{
-		Rate:  uint32(rate.Value() / 1000),
-		Burst: uint32(burst.Value() / 1000),
+	return &rateLimitMeter{
+		MeterID: meterID,
+		Rate:    uint32(rate.Value() / 1000),
+		Burst:   uint32(burst.Value() / 1000),
 	}
 }
 
-func (c *EgressController) realizeEgressQoS(eState *egressState, egressName string, mark uint32, bandwidth *crdv1b1.Bandwidth, exist bool) error {
+func (c *EgressController) realizeEgressQoS(egressName string, eState *egressState, mark uint32, bandwidth *crdv1b1.Bandwidth) error {
 	if !c.trafficShapingEnabled {
 		if bandwidth != nil {
 			klog.InfoS("Bandwidth in the Egress is ignored because OVS meters are not supported or "+
 				"trafficShaping is not enabled in Antrea-agent config.", "EgressName", egressName)
 		}
 		return nil
 	}
-	var desiredRateLimit *rateLimit
+	var desiredRateLimit *rateLimitMeter
 	// QoS is desired only if the Egress is configured on this Node.
 	if mark != 0 {
-		desiredRateLimit = bandwidthToRateLimit(bandwidth)
+		desiredRateLimit = bandwidthToRateLimitMeter(bandwidth, mark)
 	}
 	// Nothing changes.
-	if eState.rateLimit.Equals(desiredRateLimit) {
+	if eState.rateLimitMeter.Equals(desiredRateLimit) {
 		return nil
 	}
 	// It's desired to have QoS on this Node, install/override it.
 	if desiredRateLimit != nil {
 		if err := c.ofClient.InstallEgressQoS(mark, desiredRateLimit.Rate, desiredRateLimit.Burst); err != nil {
 			return err
 		}
-		eState.rateLimit = desiredRateLimit
+		eState.rateLimitMeter = desiredRateLimit
 		return nil
 	}
 	// It's undesired to have QoS on this Node, uninstall it.
-	if eState.rateLimit != nil {
-		if err := c.ofClient.UninstallEgressQoS(eState.mark); err != nil {
+	if eState.rateLimitMeter != nil {
+		if err := c.ofClient.UninstallEgressQoS(eState.rateLimitMeter.MeterID); err != nil {
 			return err
 		}
-		eState.rateLimit = nil
+		eState.rateLimitMeter = nil
 	}
 	return nil
 }
@@ -802,7 +804,7 @@ func (c *EgressController) syncEgress(egressName string) error {
 		return err
 	}
 
-	if err = c.realizeEgressQoS(eState, egressName, mark, egress.Spec.Bandwidth, exist); err != nil {
+	if err = c.realizeEgressQoS(egressName, eState, mark, egress.Spec.Bandwidth); err != nil {
 		return err
 	}
 
@@ -883,8 +885,8 @@ func (c *EgressController) uninstallEgress(egressName string, eState *egressStat
 		return err
 	}
 	// Uninstall its meter.
-	if c.trafficShapingEnabled && eState.rateLimit != nil {
-		if err := c.ofClient.UninstallEgressQoS(eState.mark); err != nil {
+	if c.trafficShapingEnabled && eState.rateLimitMeter != nil {
+		if err := c.ofClient.UninstallEgressQoS(eState.rateLimitMeter.MeterID); err != nil {
 			return err
 		}
 	}

diff --git a/pkg/apis/crd/v1beta1/types.go b/pkg/apis/crd/v1beta1/types.go
@@ -864,9 +864,9 @@ type EgressSpec struct {
 }
 
 type Bandwidth struct {
-	// Rate specifies the maximum traffic rate. e.g. 300K, 10M
+	// Rate specifies the maximum traffic rate. e.g. 300k, 10M
 	Rate string `json:"rate"`
-	// Burst specifies maximum burst mbps for throttle. e.g. 300K, 10M
+	// Burst specifies maximum burst for throttle. e.g. 300k, 10M
 	Burst string `json:"burst"`
 }
 

diff --git a/pkg/apiserver/openapi/zz_generated.openapi.go b/pkg/apiserver/openapi/zz_generated.openapi.go
diff --git a/test/e2e/egress_test.go b/test/e2e/egress_test.go
@@ -716,6 +716,7 @@ func testEgressUpdateBandwidth(t *testing.T, data *TestData) {
 	if err != nil {
 		t.Fatalf("Error when waiting for Egress to be realized: %v", err)
 	}
+	defer failOnError(data.crdClient.CrdV1beta1().Egresses().Delete(context.TODO(), egress.Name, metav1.DeleteOptions{}), t)
 
 	// expectedBandwidth is Mbps
 	runIperf := func(cmd []string, expectedBandwidth int) {
@@ -746,8 +747,6 @@ func testEgressUpdateBandwidth(t *testing.T, data *TestData) {
 
 	runIperf([]string{"bash", "-c", "iperf3 -c 1.1.1.1 -f m -t 1|grep sender|awk '{print $7}'"}, transMap[bandwidth2.Rate]+transMap[bandwidth2.Burst])
 	runIperf([]string{"bash", "-c", "iperf3 -c 1.1.1.1 -f m -O 1|grep sender|awk '{print $7}'"}, transMap[bandwidth2.Rate])
-
-	failOnError(data.crdClient.CrdV1beta1().Egresses().Delete(context.TODO(), egress.Name, metav1.DeleteOptions{}), t)
 }
 
 func (data *TestData) checkEgressState(egressName, expectedIP, expectedNode, otherNode string, timeout time.Duration) (*v1beta1.Egress, error) {