Add jitter to client config retry

Also:
* Replaces labeled for/continue RETRY loops with wait helpers for improved readability
* Pulls secrets and nodes from cache for node password verification
* Migrate nodepassword tests to wrangler mocks for better code reuse

Signed-off-by: Brad Davidson <brad.davidson@rancher.com>

go.mod

@@ -19,7 +19,7 @@ replace (
 	github.com/opencontainers/runc => github.com/opencontainers/runc v1.1.8
 	github.com/opencontainers/runtime-spec => github.com/opencontainers/runtime-spec v1.0.3-0.20220909204839-494a5a6aca78
 	github.com/opencontainers/selinux => github.com/opencontainers/selinux v1.10.1
-	github.com/rancher/wrangler => github.com/rancher/wrangler v1.1.1-0.20230425173236-39a4707f0689
+	github.com/rancher/wrangler => github.com/rancher/wrangler v1.1.1-0.20230807182002-35cb42e6a915
 	go.etcd.io/etcd/api/v3 => github.com/k3s-io/etcd/api/v3 v3.5.9-k3s1
 	go.etcd.io/etcd/client/pkg/v3 => github.com/k3s-io/etcd/client/pkg/v3 v3.5.9-k3s1
 	go.etcd.io/etcd/client/v2 => github.com/k3s-io/etcd/client/v2 v2.305.9-k3s1
@@ -104,6 +104,7 @@ require (
 	github.com/go-bindata/go-bindata v3.1.2+incompatible
 	github.com/go-sql-driver/mysql v1.7.1
 	github.com/go-test/deep v1.0.7
+	github.com/golang/mock v1.6.0
 	github.com/google/cadvisor v0.47.2
 	github.com/google/uuid v1.3.0
 	github.com/gorilla/mux v1.8.0
@@ -124,7 +125,7 @@ require (
 	github.com/otiai10/copy v1.7.0
 	github.com/pkg/errors v0.9.1
 	github.com/rancher/dynamiclistener v0.3.6-rc2
-	github.com/rancher/lasso v0.0.0-20221227210133-6ea88ca2fbcc
+	github.com/rancher/lasso v0.0.0-20230629200414-8a54b32e6792
 	github.com/rancher/remotedialer v0.3.0
 	github.com/rancher/wharfie v0.5.3
 	github.com/rancher/wrangler v1.1.1
@@ -258,7 +259,6 @@ require (
 	github.com/gogo/protobuf v1.3.2 // indirect
 	github.com/golang-jwt/jwt/v4 v4.4.2 // indirect
 	github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da // indirect
-	github.com/golang/mock v1.6.0 // indirect
 	github.com/golang/protobuf v1.5.3 // indirect
 	github.com/google/btree v1.1.2 // indirect
 	github.com/google/cel-go v0.12.7 // indirect

go.sum

@@ -1049,14 +1049,14 @@ github.com/prometheus/procfs v0.10.1/go.mod h1:nwNm2aOCAYw8uTR/9bWRREkZFxAUcWzPH
 github.com/prometheus/tsdb v0.7.1/go.mod h1:qhTCs0VvXwvX/y3TZrWD7rabWM+ijKTux40TwIPHuXU=
 github.com/rancher/dynamiclistener v0.3.6-rc2 h1:Y1nai+Xv+4qqlB3c+hmrY2uBo1EcCDU9kmN5hbnmZhA=
 github.com/rancher/dynamiclistener v0.3.6-rc2/go.mod h1:wOh62hdJIgyqTdD/VAHO77UPKAbUsJJ5gYRjzgBL3Wo=
-github.com/rancher/lasso v0.0.0-20221227210133-6ea88ca2fbcc h1:29VHrInLV4qSevvcvhBj5UhQWkPShxrxv4AahYg2Scw=
-github.com/rancher/lasso v0.0.0-20221227210133-6ea88ca2fbcc/go.mod h1:dEfC9eFQigj95lv/JQ8K5e7+qQCacWs1aIA6nLxKzT8=
+github.com/rancher/lasso v0.0.0-20230629200414-8a54b32e6792 h1:IaPhDqppVYX2v/nCR8j2i0nqOLD5yggzzy39QUlcqDw=
+github.com/rancher/lasso v0.0.0-20230629200414-8a54b32e6792/go.mod h1:dNcwXjcqgdOuKFIVETNAPURRh3e5PAi/nWUjj+MLVZA=
 github.com/rancher/remotedialer v0.3.0 h1:y1EO8JCsgZo0RcqTUp6U8FXcBAv27R+TLnWRcpvX1sM=
 github.com/rancher/remotedialer v0.3.0/go.mod h1:BwwztuvViX2JrLLUwDlsYt5DiyUwHLlzynRwkZLAY0Q=
 github.com/rancher/wharfie v0.5.3 h1:6hiO26H7YTgChbLAE6JppxFRjaH3tbKfMItv/LqV0Q0=
 github.com/rancher/wharfie v0.5.3/go.mod h1:Ebpai7digxegLroBseeC54XRBt5we3DgFS6kAE2ho+o=
-github.com/rancher/wrangler v1.1.1-0.20230425173236-39a4707f0689 h1:otb4OjgXH2b8a4C9g76jCDuTF3opjaYffZ55SiVe7KU=
-github.com/rancher/wrangler v1.1.1-0.20230425173236-39a4707f0689/go.mod h1:D6Tu6oVX8aGtCHsMCtYaysgVK3ad920MTSeAu7rzb5U=
+github.com/rancher/wrangler v1.1.1-0.20230807182002-35cb42e6a915 h1:CNwXnngrb5PgPe4nr7G5jS298YkhS6udMnNvyBOboFQ=
+github.com/rancher/wrangler v1.1.1-0.20230807182002-35cb42e6a915/go.mod h1:0oPjv01nvzeavcmeuT0xMlGKs9IJaNk5NCPPQq1n8Ro=
 github.com/rasky/go-xdr v0.0.0-20170217172119-4930550ba2e2/go.mod h1:Nfe4efndBz4TibWycNE+lqyJZiMX4ycx+QKV8Ta0f/o=
 github.com/rcrowley/go-metrics v0.0.0-20200313005456-10cdbea86bc0/go.mod h1:bCqnVzQkZxMG4s8nGwiZ5l3QUCyqpo9Y+/ZMZ9VjZe4=
 github.com/robfig/cron/v3 v3.0.1 h1:WdRxkvbJztn8LMz/QEvLN5sBU+xKpSqwwUO1Pjr4qDs=

pkg/agent/config/config.go

@@ -33,6 +33,7 @@ import (
 	"github.com/rancher/wrangler/pkg/slice"
 	"github.com/sirupsen/logrus"
 	"k8s.io/apimachinery/pkg/util/json"
+	"k8s.io/apimachinery/pkg/util/wait"
 	utilsnet "k8s.io/utils/net"
 )
 
@@ -42,65 +43,69 @@ const (
 
 // Get returns a pointer to a completed Node configuration struct,
 // containing a merging of the local CLI configuration with settings from the server.
+// Node configuration includes client certificates, which requires node password verification,
+// so this is somewhat computationally expensive on the server side, and is retried with jitter
+// to avoid having clients hammer on the server at fixed periods.
 // A call to this will bock until agent configuration is successfully returned by the
 // server.
 func Get(ctx context.Context, agent cmds.Agent, proxy proxy.Proxy) *config.Node {
-	ticker := time.NewTicker(5 * time.Second)
-	defer ticker.Stop()
-RETRY:
-	for {
-		agentConfig, err := get(ctx, &agent, proxy)
+	var agentConfig *config.Node
+	var err error
+
+	// This would be more clear as wait.PollImmediateUntilWithContext, but that function
+	// does not support jittering, so we instead use wait.JitterUntilWithContext, and cancel
+	// the context on success.
+	ctx, cancel := context.WithCancel(ctx)
+	wait.JitterUntilWithContext(ctx, func(ctx context.Context) {
+		agentConfig, err = get(ctx, &agent, proxy)
 		if err != nil {
 			logrus.Infof("Waiting to retrieve agent configuration; server is not ready: %v", err)
-			for range ticker.C {
-				continue RETRY
-			}
+		} else {
+			cancel()
 		}
-		return agentConfig
-	}
+	}, 5*time.Second, 1.0, true)
+	return agentConfig
 }
 
 // KubeProxyDisabled returns a bool indicating whether or not kube-proxy has been disabled in the
 // server configuration. The server may not have a complete view of cluster configuration until
 // after all startup hooks have completed, so a call to this will block until after the server's
 // readyz endpoint returns OK.
 func KubeProxyDisabled(ctx context.Context, node *config.Node, proxy proxy.Proxy) bool {
-	ticker := time.NewTicker(5 * time.Second)
-	defer ticker.Stop()
-RETRY:
-	for {
-		disabled, err := getKubeProxyDisabled(ctx, node, proxy)
+	var disabled bool
+	var err error
+
+	wait.PollImmediateUntilWithContext(ctx, 5*time.Second, func(ctx context.Context) (bool, error) {
+		disabled, err = getKubeProxyDisabled(ctx, node, proxy)
 		if err != nil {
 			logrus.Infof("Waiting to retrieve kube-proxy configuration; server is not ready: %v", err)
-			for range ticker.C {
-				continue RETRY
-			}
+			return false, nil
 		}
-		return disabled
-	}
+		return true, nil
+	})
+	return disabled
 }
 
 // APIServers returns a list of apiserver endpoints, suitable for seeding client loadbalancer configurations.
 // This function will block until it can return a populated list of apiservers, or if the remote server returns
 // an error (indicating that it does not support this functionality).
 func APIServers(ctx context.Context, node *config.Node, proxy proxy.Proxy) []string {
-	ticker := time.NewTicker(5 * time.Second)
-	defer ticker.Stop()
-RETRY:
-	for {
-		addresses, err := getAPIServers(ctx, node, proxy)
+	var addresses []string
+	var err error
+
+	wait.PollImmediateUntilWithContext(ctx, 5*time.Second, func(ctx context.Context) (bool, error) {
+		addresses, err = getAPIServers(ctx, node, proxy)
 		if err != nil {
 			logrus.Infof("Failed to retrieve list of apiservers from server: %v", err)
-			return nil
+			return false, err
 		}
 		if len(addresses) == 0 {
 			logrus.Infof("Waiting for apiserver addresses")
-			for range ticker.C {
-				continue RETRY
-			}
+			return false, nil
 		}
-		return addresses
-	}
+		return true, nil
+	})
+	return addresses
 }
 
 type HTTPRequester func(u string, client *http.Client, username, password, token string) ([]byte, error)

pkg/node/controller.go

@@ -13,15 +13,14 @@ import (
 
 func Register(ctx context.Context,
 	modCoreDNS bool,
-	secretClient coreclient.SecretClient,
-	configMap coreclient.ConfigMapController,
+	secrets coreclient.SecretController,
+	configMaps coreclient.ConfigMapController,
 	nodes coreclient.NodeController,
 ) error {
 	h := &handler{
-		modCoreDNS:   modCoreDNS,
-		secretClient: secretClient,
-		configCache:  configMap.Cache(),
-		configClient: configMap,
+		modCoreDNS: modCoreDNS,
+		secrets:    secrets,
+		configMaps: configMaps,
 	}
 	nodes.OnChange(ctx, "node", h.onChange)
 	nodes.OnRemove(ctx, "node", h.onRemove)
@@ -30,10 +29,9 @@ func Register(ctx context.Context,
 }
 
 type handler struct {
-	modCoreDNS   bool
-	secretClient coreclient.SecretClient
-	configCache  coreclient.ConfigMapCache
-	configClient coreclient.ConfigMapClient
+	modCoreDNS bool
+	secrets    coreclient.SecretController
+	configMaps coreclient.ConfigMapController
 }
 
 func (h *handler) onChange(key string, node *core.Node) (*core.Node, error) {
@@ -78,7 +76,7 @@ func (h *handler) updateCoreDNSConfigMap(nodeName, nodeAddress string, removed b
 		return nil
 	}
 
-	configMapCache, err := h.configCache.Get("kube-system", "coredns")
+	configMapCache, err := h.configMaps.Cache().Get("kube-system", "coredns")
 	if err != nil || configMapCache == nil {
 		logrus.Warn(errors.Wrap(err, "Unable to fetch coredns config map"))
 		return nil
@@ -120,7 +118,7 @@ func (h *handler) updateCoreDNSConfigMap(nodeName, nodeAddress string, removed b
 	}
 	configMap.Data["NodeHosts"] = newHosts
 
-	if _, err := h.configClient.Update(configMap); err != nil {
+	if _, err := h.configMaps.Update(configMap); err != nil {
 		return err
 	}
 
@@ -135,5 +133,5 @@ func (h *handler) updateCoreDNSConfigMap(nodeName, nodeAddress string, removed b
 }
 
 func (h *handler) removeNodePassword(nodeName string) error {
-	return nodepassword.Delete(h.secretClient, nodeName)
+	return nodepassword.Delete(h.secrets, nodeName)
 }

pkg/nodepassword/nodepassword.go

@@ -51,9 +51,9 @@ func getSecretName(nodeName string) string {
 	return strings.ToLower(nodeName + ".node-password." + version.Program)
 }
 
-func verifyHash(secretClient coreclient.SecretClient, nodeName, pass string) error {
+func verifyHash(secretClient coreclient.SecretController, nodeName, pass string) error {
 	name := getSecretName(nodeName)
-	secret, err := secretClient.Get(metav1.NamespaceSystem, name, metav1.GetOptions{})
+	secret, err := secretClient.Cache().Get(metav1.NamespaceSystem, name)
 	if err != nil {
 		return &passwordError{node: nodeName, err: err}
 	}
@@ -67,7 +67,7 @@ func verifyHash(secretClient coreclient.SecretClient, nodeName, pass string) err
 }
 
 // Ensure will verify a node-password secret if it exists, otherwise it will create one
-func Ensure(secretClient coreclient.SecretClient, nodeName, pass string) error {
+func Ensure(secretClient coreclient.SecretController, nodeName, pass string) error {
 	err := verifyHash(secretClient, nodeName, pass)
 	if apierrors.IsNotFound(err) {
 		var hash string
@@ -88,12 +88,12 @@ func Ensure(secretClient coreclient.SecretClient, nodeName, pass string) error {
 }
 
 // Delete will remove a node-password secret
-func Delete(secretClient coreclient.SecretClient, nodeName string) error {
+func Delete(secretClient coreclient.SecretController, nodeName string) error {
 	return secretClient.Delete(metav1.NamespaceSystem, getSecretName(nodeName), &metav1.DeleteOptions{})
 }
 
 // MigrateFile moves password file entries to secrets
-func MigrateFile(secretClient coreclient.SecretClient, nodeClient coreclient.NodeClient, passwordFile string) error {
+func MigrateFile(secretClient coreclient.SecretController, nodeClient coreclient.NodeController, passwordFile string) error {
 	_, err := os.Stat(passwordFile)
 	if os.IsNotExist(err) {
 		return nil
@@ -108,11 +108,9 @@ func MigrateFile(secretClient coreclient.SecretClient, nodeClient coreclient.Nod
 	}
 
 	nodeNames := []string{}
-	nodeList, _ := nodeClient.List(metav1.ListOptions{})
-	if nodeList != nil {
-		for _, node := range nodeList.Items {
-			nodeNames = append(nodeNames, node.Name)
-		}
+	nodeList, _ := nodeClient.Cache().List(nil)
+	for _, node := range nodeList {
+		nodeNames = append(nodeNames, node.Name)
 	}
 	if len(nodeNames) == 0 {
 		nodeNames = append(nodeNames, passwd.Users()...)