Merge pull request #1037 from sanchezl/apirequestcount-currentHour

Bug 2021629: UPSTREAM: <carry>: api request counts for current hour are incorrect

openshift-kube-apiserver/filters/deprecatedapirequest/apiaccess_count_controller.go

@@ -8,8 +8,8 @@ import (
 	"time"
 
 	apiv1 "github.com/openshift/api/apiserver/v1"
-
-	apiclientv1 "github.com/openshift/client-go/apiserver/clientset/versioned/typed/apiserver/v1"
+	apiv1client "github.com/openshift/client-go/apiserver/clientset/versioned/typed/apiserver/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apimachinery/pkg/util/runtime"
 	"k8s.io/apimachinery/pkg/util/wait"
@@ -18,7 +18,7 @@ import (
 )
 
 // NewController returns a controller
-func NewController(client apiclientv1.APIRequestCountInterface, nodeName string) *controller {
+func NewController(client apiv1client.APIRequestCountInterface, nodeName string) *controller {
 	ret := &controller{
 		client:       client,
 		nodeName:     nodeName,
@@ -36,9 +36,10 @@ type APIRequestLogger interface {
 }
 
 type controller struct {
-	client       apiclientv1.APIRequestCountInterface
+	client       apiv1client.APIRequestCountInterface
 	nodeName     string
 	updatePeriod time.Duration
+	loadOnce     sync.Once
 
 	requestCountLock sync.RWMutex
 	requestCounts    *apiRequestCounts
@@ -58,7 +59,7 @@ func (c *controller) IsDeprecated(resource, version, group string) bool {
 func (c *controller) LogRequest(resource schema.GroupVersionResource, timestamp time.Time, user, userAgent, verb string) {
 	c.requestCountLock.RLock()
 	defer c.requestCountLock.RUnlock()
-	// we snip user agents to reduce cardinality and unique keys.  For well behaved agents, we see useragents about like
+	// we snip user agents to reduce cardinality and unique keys.  For well-behaved agents, we see useragents about like
 	// kube-controller-manager/v1.21.0 (linux/amd64) kubernetes/743bd58/kube-controller-manager
 	// so we will snip at the first space.
 	snippedUserAgent := userAgent
@@ -94,24 +95,43 @@ func (c *controller) Start(stop <-chan struct{}) {
 	}()
 
 	// write out logs every c.updatePeriod
-	go wait.NonSlidingUntilWithContext(ctx, c.persistRequestCountForAllResources, c.updatePeriod)
+	go wait.NonSlidingUntilWithContext(ctx, c.sync, c.updatePeriod)
+}
+func (c *controller) sync(ctx context.Context) {
+	currentHour := time.Now().Hour()
+	c.persistRequestCountForAllResources(ctx, currentHour)
 }
 
-func (c *controller) persistRequestCountForAllResources(ctx context.Context) {
-	klog.V(2).Infof("updating top APIRequest counts")
-	defer klog.V(2).Infof("finished updating top APIRequest counts")
+func (c *controller) persistRequestCountForAllResources(ctx context.Context, currentHour int) {
+	klog.V(4).Infof("updating top APIRequest counts")
+	defer klog.V(4).Infof("finished updating top APIRequest counts")
 
 	// get the current count to persist, start a new in-memory count
 	countsToPersist := c.resetRequestCount()
 
 	// remove stale data
-	expiredHour := (time.Now().Hour() + 1) % 24
-	currentHour := time.Now().Hour()
+	expiredHour := (currentHour + 1) % 24
 	countsToPersist.ExpireOldestCounts(expiredHour)
 
 	// when this function returns, add any remaining counts back to the total to be retried for update
 	defer c.requestCounts.Add(countsToPersist)
 
+	// Add resources that have an existing APIRequestCount so that the current and hourly logs
+	// continue to rotate even if the resource has not had a request since the last restart.
+	c.loadOnce.Do(func() {
+		// As resources are never fully removed from countsToPersist, we only need to do this once.
+		// After the request counts have been persisted, the resources will be added "back" to the
+		// in memory counts (c.requestCounts, see defer statement above).
+		arcs, err := c.client.List(ctx, metav1.ListOptions{})
+		if err != nil {
+			runtime.HandleError(err) // oh well, we tried
+			return
+		}
+		for _, arc := range arcs.Items {
+			countsToPersist.Resource(apiNameToResource(arc.Name))
+		}
+	})
+
 	var wg sync.WaitGroup
 	for gvr := range countsToPersist.resourceToRequestCount {
 		resourceCount := countsToPersist.Resource(gvr)
@@ -127,13 +147,14 @@ func (c *controller) persistRequestCountForAllResources(ctx context.Context) {
 func (c *controller) persistRequestCountForResource(ctx context.Context, wg *sync.WaitGroup, currentHour, expiredHour int, localResourceCount *resourceRequestCounts) {
 	defer wg.Done()
 
-	klog.V(2).Infof("updating top %v APIRequest counts", localResourceCount.resource)
-	defer klog.V(2).Infof("finished updating top %v APIRequest counts", localResourceCount.resource)
+	klog.V(4).Infof("updating top %v APIRequest counts", localResourceCount.resource)
+	defer klog.V(4).Infof("finished updating top %v APIRequest counts", localResourceCount.resource)
 
 	status, _, err := v1helpers.ApplyStatus(
 		ctx,
 		c.client,
 		resourceToAPIName(localResourceCount.resource),
+		nodeStatusDefaulter(c.nodeName, currentHour, expiredHour, localResourceCount.resource),
 		SetRequestCountsForNode(c.nodeName, currentHour, expiredHour, localResourceCount),
 	)
 	if err != nil {
@@ -161,10 +182,10 @@ func removePersistedRequestCounts(nodeName string, currentHour int, persistedSta
 		if persistedNodeCount.NodeName != nodeName {
 			continue
 		}
-		for _, peristedUserCount := range persistedNodeCount.ByUser {
+		for _, persistedUserCount := range persistedNodeCount.ByUser {
 			userKey := userKey{
-				user:      peristedUserCount.UserName,
-				userAgent: peristedUserCount.UserAgent,
+				user:      persistedUserCount.UserName,
+				userAgent: persistedUserCount.UserAgent,
 			}
 			localResourceCount.Hour(currentHour).RemoveUser(userKey)
 		}
@@ -187,3 +208,12 @@ func resourceToAPIName(resource schema.GroupVersionResource) string {
 	}
 	return apiName
 }
+
+func apiNameToResource(name string) schema.GroupVersionResource {
+	segments := strings.SplitN(name, ".", 3)
+	result := schema.GroupVersionResource{Resource: segments[0], Version: segments[1]}
+	if len(segments) > 2 {
+		result.Group = segments[2]
+	}
+	return result
+}