Merge pull request #135880 from hoskeri/remote-apiserver

Delay building discovery transport until needed

staging/src/k8s.io/kube-aggregator/pkg/controllers/status/remote/remote_available_controller.go

@@ -22,6 +22,7 @@ import (
 	"net/http"
 	"net/url"
 	"reflect"
+	"slices"
 	"sync"
 	"time"
 
@@ -173,35 +174,6 @@ func (c *AvailableConditionController) sync(key string) error {
 
 	apiService := originalAPIService.DeepCopy()
 
-	// if a particular transport was specified, use that otherwise build one
-	// construct an http client that will ignore TLS verification (if someone owns the network and messes with your status
-	// that's not so bad) and sets a very short timeout.  This is a best effort GET that provides no additional information
-	transportConfig := &transport.Config{
-		TLS: transport.TLSConfig{
-			Insecure: true,
-		},
-		DialHolder: c.proxyTransportDial,
-	}
-
-	if c.proxyCurrentCertKeyContent != nil {
-		proxyClientCert, proxyClientKey := c.proxyCurrentCertKeyContent()
-
-		transportConfig.TLS.CertData = proxyClientCert
-		transportConfig.TLS.KeyData = proxyClientKey
-	}
-	restTransport, err := transport.New(transportConfig)
-	if err != nil {
-		return err
-	}
-	discoveryClient := &http.Client{
-		Transport: restTransport,
-		// the request should happen quickly.
-		Timeout: 5 * time.Second,
-		CheckRedirect: func(req *http.Request, via []*http.Request) error {
-			return http.ErrUseLastResponse
-		},
-	}
-
 	availableCondition := apiregistrationv1.APIServiceCondition{
 		Type:               apiregistrationv1.Available,
 		Status:             apiregistrationv1.ConditionTrue,
@@ -262,26 +234,7 @@ func (c *AvailableConditionController) sync(key string) error {
 			_, err := c.updateAPIServiceStatus(originalAPIService, apiService)
 			return err
 		}
-		hasActiveEndpoints := false
-	outer:
-		for _, slice := range endpointSlices {
-			ready := false
-			for _, endpoint := range slice.Endpoints {
-				if endpoint.Conditions.Ready == nil || *endpoint.Conditions.Ready {
-					ready = true
-					break
-				}
-			}
-			if !ready {
-				continue
-			}
-			for _, endpointPort := range slice.Ports {
-				if endpointPort.Name != nil && *endpointPort.Name == portName && endpointPort.Port != nil {
-					hasActiveEndpoints = true
-					break outer
-				}
-			}
-		}
+		hasActiveEndpoints := hasAvailableEndpoint(portName, endpointSlices...)
 		if !hasActiveEndpoints {
 			availableCondition.Status = apiregistrationv1.ConditionFalse
 			availableCondition.Reason = "MissingEndpoints"
@@ -293,9 +246,38 @@ func (c *AvailableConditionController) sync(key string) error {
 	}
 	// actually try to hit the discovery endpoint when it isn't local and when we're routing as a service.
 	if apiService.Spec.Service != nil && c.serviceResolver != nil {
+		// if a particular transport was specified, use that otherwise build one
+		// construct an http client that will ignore TLS verification (if someone owns the network and messes with your status
+		// that's not so bad) and sets a very short timeout.  This is a best effort GET that provides no additional information
+		transportConfig := &transport.Config{
+			TLS: transport.TLSConfig{
+				Insecure: true,
+			},
+			DialHolder: c.proxyTransportDial,
+		}
+
+		if c.proxyCurrentCertKeyContent != nil {
+			proxyClientCert, proxyClientKey := c.proxyCurrentCertKeyContent()
+
+			transportConfig.TLS.CertData = proxyClientCert
+			transportConfig.TLS.KeyData = proxyClientKey
+		}
+		restTransport, err := transport.New(transportConfig)
+		if err != nil {
+			return err
+		}
+		discoveryClient := &http.Client{
+			Transport: restTransport,
+			// the request should happen quickly.
+			Timeout: 5 * time.Second,
+			CheckRedirect: func(req *http.Request, via []*http.Request) error {
+				return http.ErrUseLastResponse
+			},
+		}
+
 		attempts := 5
 		results := make(chan error, attempts)
-		for i := 0; i < attempts; i++ {
+		for range attempts {
 			go func() {
 				discoveryURL, err := c.serviceResolver.ResolveEndpoint(apiService.Spec.Service.Namespace, apiService.Spec.Service.Name, *apiService.Spec.Service.Port)
 				if err != nil {
@@ -352,7 +334,7 @@ func (c *AvailableConditionController) sync(key string) error {
 		}
 
 		var lastError error
-		for i := 0; i < attempts; i++ {
+		for range attempts {
 			lastError = <-results
 			// if we had at least one success, we are successful overall and we can return now
 			if lastError == nil {
@@ -382,6 +364,20 @@ func (c *AvailableConditionController) sync(key string) error {
 	return err
 }
 
+func hasAvailableEndpoint(portName string, es ...*discoveryv1.EndpointSlice) bool {
+	return slices.ContainsFunc(es, func(s *discoveryv1.EndpointSlice) bool {
+		if !slices.ContainsFunc(s.Endpoints, func(e discoveryv1.Endpoint) bool {
+			return e.Conditions.Ready == nil || *e.Conditions.Ready
+		}) {
+			return false
+		}
+
+		return slices.ContainsFunc(s.Ports, func(p discoveryv1.EndpointPort) bool {
+			return p.Name != nil && *p.Name == portName && p.Port != nil
+		})
+	})
+}
+
 // updateAPIServiceStatus only issues an update if a change is detected.  We have a tight resync loop to quickly detect dead
 // apiservices. Doing that means we don't want to quickly issue no-op updates.
 func (c *AvailableConditionController) updateAPIServiceStatus(originalAPIService, newAPIService *apiregistrationv1.APIService) (*apiregistrationv1.APIService, error) {
@@ -433,7 +429,7 @@ func (c *AvailableConditionController) Run(workers int, stopCh <-chan struct{})
 		return
 	}
 
-	for i := 0; i < workers; i++ {
+	for range workers {
 		go wait.Until(c.runWorker, time.Second, stopCh)
 	}
 
@@ -474,7 +470,7 @@ func (c *AvailableConditionController) addAPIService(obj interface{}) {
 	c.queue.Add(castObj.Name)
 }
 
-func (c *AvailableConditionController) updateAPIService(oldObj, newObj interface{}) {
+func (c *AvailableConditionController) updateAPIService(oldObj, newObj any) {
 	castObj := newObj.(*apiregistrationv1.APIService)
 	oldCastObj := oldObj.(*apiregistrationv1.APIService)
 	klog.V(4).Infof("Updating %s", oldCastObj.Name)

staging/src/k8s.io/kube-aggregator/pkg/controllers/status/remote/remote_available_controller_test.go

@@ -173,12 +173,12 @@ func BenchmarkBuildCache(b *testing.B) {
 	apiServiceName := "remote.group"
 	// model 1 APIService pointing at a given service, and 30 pointing at local group/versions
 	apiServices := []runtime.Object{newRemoteAPIService(apiServiceName)}
-	for i := 0; i < 30; i++ {
+	for i := range 30 {
 		apiServices = append(apiServices, newLocalAPIService(fmt.Sprintf("local.group%d", i)))
 	}
 	// model one service backing an API service, and 100 unrelated services
 	services := []*v1.Service{newService("foo", "bar", testServicePort, testServicePortName)}
-	for i := 0; i < 100; i++ {
+	for i := range 100 {
 		services = append(services, newService("foo", fmt.Sprintf("bar%d", i), testServicePort, testServicePortName))
 	}
 	c, _ := setupAPIServices(b, apiServices)