Skip to content

Commit

Permalink
UPSTREAM: <carry>: skip posting failures to aggregated APIs to avoid …
Browse files Browse the repository at this point in the history
…getting false positives until the server becomes ready

the availability checks depend on fully initialized SDN
OpenShift carries a few reachability checks that affect /readyz protocol
we skip posting failures to avoid getting false positives until the server becomes ready
  • Loading branch information
p0lyn0mial committed Sep 14, 2021
1 parent d8043e1 commit 36aafa9
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -257,6 +257,7 @@ func (c completedConfig) NewWithDelegate(delegationTarget genericapiserver.Deleg
(func() ([]byte, []byte))(s.proxyCurrentCertKeyContent),
s.serviceResolver,
c.GenericConfig.EgressSelector,
c.GenericConfig.LifecycleSignals.HasBeenReady.Signaled(),
)
if err != nil {
return nil, err
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -152,6 +155,7 @@ func NewAvailableConditionController(
proxyCurrentCertKeyContent certKeyFunc,
serviceResolver ServiceResolver,
egressSelector *egressselector.EgressSelector,
hasBeenReady <-chan struct{},
) (*AvailableConditionController, error) {
c := &AvailableConditionController{
apiServiceClient: apiServiceClient,
Expand All @@ -171,6 +175,7 @@ func NewAvailableConditionController(
proxyCurrentCertKeyContent: proxyCurrentCertKeyContent,
tlsCache: &tlsTransportCache{transports: make(map[tlsCacheKey]http.RoundTripper)},
metrics: newAvailabilityMetrics(),
hasBeenReady: hasBeenReady,
}

if egressSelector != nil {
Expand Down Expand Up @@ -427,6 +432,16 @@ func (c *AvailableConditionController) sync(key string) error {
}

if lastError != nil {
// the availability checks depend on fully initialized SDN
// OpenShift carries a few reachability checks that affect /readyz protocol
// skip posting failures to avoid getting false positives until the server becomes ready
select {
case <-c.hasBeenReady: // the server has been ready and we can perform the checks
default:
// 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()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand All @@ -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)
Expand Down Expand Up @@ -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(),
Expand All @@ -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)

Expand Down

0 comments on commit 36aafa9

Please sign in to comment.