Review comments

Author: carreter

cmd/agent/app/options/options.go

@@ -211,6 +211,9 @@ func (o *GrpcProxyAgentOptions) Validate() error {
 		if _, err := labels.Parse(o.ServerLeaseSelector); err != nil {
 			return fmt.Errorf("invalid server count lease selector: %w", err)
 		}
+		if o.KubeconfigPath == "" {
+			return fmt.Errorf("kubeconfig path must be provided if server count lease system is enabled")
+		}
 	}
 	if o.KubeconfigPath != "" {
 		if _, err := os.Stat(o.KubeconfigPath); os.IsNotExist(err) {

cmd/agent/app/server.go

@@ -155,6 +155,7 @@ func (a *Agent) runProxyConnection(o *options.GrpcProxyAgentOptions, drainCh, st
 			serverLeaseSelector,
 		)
 		if err != nil {
+			klog.Errorf("failed to create server lease counter: %v", err)
 			return nil, fmt.Errorf("failed to create server lease counter: %w", err)
 		}
 		cc.ServerLeaseCounter = serverLeaseCounter

cmd/server/app/options/options.go

@@ -305,21 +305,21 @@ func (o *ProxyRunOptions) Validate() error {
 	}
 
 	// Validate leasing parameters. All must be empty or have a value.
-	if o.LeaseLabelSelector != "" || o.LeaseDuration != 0 || o.LeaseGCInterval != 0 || o.LeaseRenewalInterval != 0 || o.LeaseNamespace != "" {
+	if o.IsLeaseCountingEnabled() {
 		if o.LeaseLabelSelector == "" {
 			return fmt.Errorf("LeaseLabelSelector cannot be empty when leasing system is enabled")
 		}
 		if o.LeaseNamespace == "" {
 			return fmt.Errorf("LeaseNamespace cannot be empty when leasing system is enabled")
 		}
-		if o.LeaseDuration == 0 {
-			return fmt.Errorf("LeaseDuration cannot be zero when leasing system is enabled")
+		if o.LeaseDuration <= 0 {
+			return fmt.Errorf("LeaseDuration cannot be zero or negative when leasing system is enabled")
 		}
-		if o.LeaseGCInterval == 0 {
-			return fmt.Errorf("LeaseGCInterval cannot be zero when leasing system is enabled")
+		if o.LeaseGCInterval <= 0 {
+			return fmt.Errorf("LeaseGCInterval cannot be zero or negative when leasing system is enabled")
 		}
-		if o.LeaseRenewalInterval == 0 {
-			return fmt.Errorf("LeaseRenewalInterval cannot be zero when leasing system is enabled")
+		if o.LeaseRenewalInterval <= 0 {
+			return fmt.Errorf("LeaseRenewalInterval cannot be zero or negative when leasing system is enabled")
 		}
 	}
 
@@ -398,3 +398,7 @@ func defaultServerID() string {
 	}
 	return uuid.New().String()
 }
+
+func (o *ProxyRunOptions) IsLeaseCountingEnabled() bool {
+	return o.LeaseLabelSelector != "" || o.LeaseDuration != 0 || o.LeaseGCInterval != 0 || o.LeaseRenewalInterval != 0 || o.LeaseNamespace != ""
+}

cmd/server/app/server.go

@@ -143,10 +143,10 @@ func (p *Proxy) Run(o *options.ProxyRunOptions, stopCh <-chan struct{}) error {
 		defer frontendStop()
 	}
 
-	if o.LeaseLabelSelector != "" || o.LeaseDuration != 0 || o.LeaseGCInterval != 0 || o.LeaseRenewalInterval != 0 {
+	if o.IsLeaseCountingEnabled() {
 		leaseLabels, err := labels.ConvertSelectorToLabelsMap(o.LeaseLabelSelector)
 		if err != nil {
-			return fmt.Errorf("could not parse lease label selector: %w", err)
+			return fmt.Errorf("could not parse lease label selector (%q): %w", o.LeaseLabelSelector, err)
 		}
 		leaseController := leases.NewController(k8sClient, o.ServerID, int32(o.LeaseDuration.Seconds()), o.LeaseRenewalInterval, o.LeaseGCInterval, fmt.Sprintf("konnectivity-proxy-server-%v", o.ServerID), o.LeaseNamespace, leaseLabels)
 		klog.V(1).Infoln("Starting lease acquisition and garbage collection controller.")