Merge pull request #955 from p0lyn0mial/release-4.8-gate-availability-controller-until-ready

Bug 1994655: openshift-apiserver should not set Available=False APIServicesAvailable on update

Author: openshift-merge-robot

staging/src/k8s.io/apiserver/pkg/server/config.go

@@ -464,6 +464,11 @@ func (c *Config) AddPostStartHookOrDie(name string, hook PostStartHookFunc) {
 	}
 }
 
+// HasBeenReadySignal exposes a server's lifecycle signal which is signaled when the readyz endpoint succeeds for the first time.
+func (c *Config) HasBeenReadySignal() <-chan struct{} {
+	return c.hasBeenReadyCh
+}
+
 // Complete fills in any fields not set that are required to have valid data and can be derived
 // from other fields. If you're going to `ApplyOptions`, do that first. It's mutating the receiver.
 func (c *Config) Complete(informers informers.SharedInformerFactory) CompletedConfig {

staging/src/k8s.io/kube-aggregator/pkg/apiserver/apiserver.go

@@ -257,6 +257,7 @@ func (c completedConfig) NewWithDelegate(delegationTarget genericapiserver.Deleg
 		(func() ([]byte, []byte))(s.proxyCurrentCertKeyContent),
 		s.serviceResolver,
 		c.GenericConfig.EgressSelector,
+		c.GenericConfig.HasBeenReadySignal(),
 	)
 	if err != nil {
 		return nil, err

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

@@ -100,6 +100,9 @@ type AvailableConditionController struct {
 
 	// metrics registered into legacy registry
 	metrics *availabilityMetrics
+
+	// hasBeenReady is signaled when the readyz endpoint succeeds for the first time.
+	hasBeenReady <-chan struct{}
 }
 
 type tlsTransportCache struct {
@@ -152,6 +155,7 @@ func NewAvailableConditionController(
 	proxyCurrentCertKeyContent certKeyFunc,
 	serviceResolver ServiceResolver,
 	egressSelector *egressselector.EgressSelector,
+	hasBeenReady <-chan struct{},
 ) (*AvailableConditionController, error) {
 	c := &AvailableConditionController{
 		apiServiceClient: apiServiceClient,
@@ -171,6 +175,7 @@ func NewAvailableConditionController(
 		proxyCurrentCertKeyContent: proxyCurrentCertKeyContent,
 		tlsCache:                   &tlsTransportCache{transports: make(map[tlsCacheKey]http.RoundTripper)},
 		metrics:                    newAvailabilityMetrics(),
+		hasBeenReady:               hasBeenReady,
 	}
 
 	if egressSelector != nil {
@@ -233,6 +238,18 @@ func (c *AvailableConditionController) sync(key string) error {
 		return err
 	}
 
+	// the availability checks depend on fully initialized SDN
+	// OpenShift carries a few reachability checks that affect /readyz protocol
+	// record availability of the server so that we can
+	// skip posting failures to avoid getting false positives until the server becomes ready
+	hasBeenReady := false
+	select {
+	case <-c.hasBeenReady:
+		hasBeenReady = true
+	default:
+		// continue, we will skip posting only potential failures
+	}
+
 	// 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
@@ -427,6 +444,11 @@ func (c *AvailableConditionController) sync(key string) error {
 		}
 
 		if lastError != nil {
+			if !hasBeenReady {
+				// returning an error will requeue the item in an exponential fashion
+				return fmt.Errorf("the server hasn't been ready yet, skipping updating availability of the aggreaged API until the server becomes ready to avoid false positives, lastError = %v", lastError)
+			}
+
 			availableCondition.Status = apiregistrationv1.ConditionFalse
 			availableCondition.Reason = "FailedDiscoveryCheck"
 			availableCondition.Message = lastError.Error()

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

@@ -121,6 +121,8 @@ func setupAPIServices(apiServices []*apiregistration.APIService) (*AvailableCond
 	for _, o := range apiServices {
 		apiServiceIndexer.Add(o)
 	}
+	alwaysReadyChan := make(chan struct{})
+	close(alwaysReadyChan)
 
 	c := AvailableConditionController{
 		apiServiceClient: fakeClient.ApiregistrationV1(),
@@ -134,8 +136,9 @@ func setupAPIServices(apiServices []*apiregistration.APIService) (*AvailableCond
 			// the maximum disruption time to a minimum, but it does prevent hot loops.
 			workqueue.NewItemExponentialFailureRateLimiter(5*time.Millisecond, 30*time.Second),
 			"AvailableConditionController"),
-		tlsCache: &tlsTransportCache{transports: make(map[tlsCacheKey]http.RoundTripper)},
-		metrics:  newAvailabilityMetrics(),
+		tlsCache:     &tlsTransportCache{transports: make(map[tlsCacheKey]http.RoundTripper)},
+		metrics:      newAvailabilityMetrics(),
+		hasBeenReady: alwaysReadyChan,
 	}
 	for _, svc := range apiServices {
 		c.addAPIService(svc)
@@ -400,6 +403,8 @@ func TestSync(t *testing.T) {
 				w.WriteHeader(http.StatusForbidden)
 			}))
 			defer testServer.Close()
+			alwaysReadyChan := make(chan struct{})
+			close(alwaysReadyChan)
 
 			c := AvailableConditionController{
 				apiServiceClient:           fakeClient.ApiregistrationV1(),
@@ -410,6 +415,7 @@ func TestSync(t *testing.T) {
 				proxyCurrentCertKeyContent: func() ([]byte, []byte) { return emptyCert(), emptyCert() },
 				tlsCache:                   &tlsTransportCache{transports: make(map[tlsCacheKey]http.RoundTripper)},
 				metrics:                    newAvailabilityMetrics(),
+				hasBeenReady:               alwaysReadyChan,
 			}
 			c.sync(tc.apiServiceName)