From ced7f3285a35d93ee8f6c24a263040d277daae72 Mon Sep 17 00:00:00 2001 From: Brad Davidson Date: Sat, 21 Oct 2023 00:14:12 +0000 Subject: [PATCH] Convert server, etcd, secrets-encrypt to use patch builder Signed-off-by: Brad Davidson --- pkg/etcd/member_controller.go | 26 +++++--- pkg/etcd/metadata_controller.go | 101 ++++++++++++++----------------- pkg/secretsencrypt/config.go | 22 ++++--- pkg/secretsencrypt/controller.go | 40 +++++------- pkg/server/secrets-encrypt.go | 28 +++++---- pkg/server/server.go | 58 +++++++++--------- 6 files changed, 140 insertions(+), 135 deletions(-) diff --git a/pkg/etcd/member_controller.go b/pkg/etcd/member_controller.go index ebf30ffb65c3..a40b4def65c9 100644 --- a/pkg/etcd/member_controller.go +++ b/pkg/etcd/member_controller.go @@ -6,12 +6,14 @@ import ( "strings" "github.com/k3s-io/k3s/pkg/util" + "github.com/k3s-io/k3s/pkg/util/jsonpatch" "github.com/k3s-io/k3s/pkg/version" "github.com/pkg/errors" controllerv1 "github.com/rancher/wrangler/pkg/generated/controllers/core/v1" "github.com/sirupsen/logrus" "go.etcd.io/etcd/api/v3/v3rpc/rpctypes" v1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/types" ) func registerMemberHandlers(ctx context.Context, etcd *ETCD) { @@ -68,9 +70,11 @@ func (e *etcdMemberHandler) sync(key string, node *v1.Node) (*v1.Node, error) { // If the current node name is not the same as the removed node name, clear the removal annotations, // as this indicates that the node has been re-added with a new name. logrus.WithFields(lf).Info("Resetting removed node flag as removed node name does not match current node name") - delete(node.Annotations, removedNodeNameAnnotation) - delete(node.Annotations, removalAnnotation) - return e.nodeController.Update(node) + b, err := jsonpatch.NewBuilder("metadata", "annotations").Remove(removedNodeNameAnnotation).Remove(removalAnnotation).Marshal() + if err != nil { + return node, err + } + return e.nodeController.Patch(node.Name, types.JSONPatchType, b) } // Current node name matches removed node name; don't need to do anything return node, nil @@ -85,8 +89,11 @@ func (e *etcdMemberHandler) sync(key string, node *v1.Node) (*v1.Node, error) { // the annotation again, once there are more cluster members. if errors.Is(err, rpctypes.ErrMemberNotEnoughStarted) { logrus.WithFields(lf).Errorf("etcd member removal rejected, clearing remove annotation: %v", err) - delete(node.Annotations, removalAnnotation) - return e.nodeController.Update(node) + b, err := jsonpatch.NewBuilder("metadata", "annotations").Remove(removalAnnotation).Marshal() + if err != nil { + return node, err + } + return e.nodeController.Patch(node.Name, types.JSONPatchType, b) } return node, err } @@ -94,10 +101,11 @@ func (e *etcdMemberHandler) sync(key string, node *v1.Node) (*v1.Node, error) { logrus.WithFields(lf).Info("etcd emember removed successfully") // Set the removed node name annotation and delete the etcd name and address annotations. // These will be re-set to their new value when the member rejoins the cluster. - node.Annotations[removedNodeNameAnnotation] = name - delete(node.Annotations, NodeNameAnnotation) - delete(node.Annotations, NodeAddressAnnotation) - return e.nodeController.Update(node) + b, err := jsonpatch.NewBuilder("metadata", "annotations").Add(name, removedNodeNameAnnotation).Remove(NodeNameAnnotation).Remove(NodeAddressAnnotation).Marshal() + if err != nil { + return node, err + } + return e.nodeController.Patch(node.Name, types.JSONPatchType, b) } // In the event that we had an unexpected removal annotation value, simply return. // Fallthrough to the non-op below. diff --git a/pkg/etcd/metadata_controller.go b/pkg/etcd/metadata_controller.go index da86e81263a7..736587324a98 100644 --- a/pkg/etcd/metadata_controller.go +++ b/pkg/etcd/metadata_controller.go @@ -7,12 +7,13 @@ import ( "time" "github.com/k3s-io/k3s/pkg/util" + "github.com/k3s-io/k3s/pkg/util/jsonpatch" controllerv1 "github.com/rancher/wrangler/pkg/generated/controllers/core/v1" "github.com/sirupsen/logrus" v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" - "k8s.io/client-go/util/retry" + "k8s.io/apimachinery/pkg/types" ) func registerMetadataHandlers(ctx context.Context, etcd *ETCD) { @@ -66,24 +67,22 @@ func (m *metadataHandler) checkReset() { logrus.Errorf("Failed to list etcd nodes: %v", err) return } - for _, n := range nodes.Items { - node := &n - err := retry.RetryOnConflict(retry.DefaultRetry, func() error { - _, remove := node.Annotations[removalAnnotation] - _, removed := node.Annotations[removedNodeNameAnnotation] - if remove || removed { - node = node.DeepCopy() - delete(node.Annotations, removalAnnotation) - delete(node.Annotations, removedNodeNameAnnotation) - node, err = m.nodeController.Update(node) - return err + for _, node := range nodes.Items { + ls := labels.Set(node.Annotations) + patch := jsonpatch.NewBuilder("metadata", "annotations"). + RemoveIfHas(ls, removalAnnotation). + RemoveIfHas(ls, removedNodeNameAnnotation) + if patch.Len() > 0 { + b, err := patch.Marshal() + if err != nil { + logrus.Errorf("Failed to marshal JSON patch: %v", err) + continue + } + if _, err = m.nodeController.Patch(node.Name, types.JSONPatchType, b); err != nil { + logrus.Errorf("Failed to clear removal annotations from node %s after cluster reset: %v", node.Name, err) + } else { + logrus.Infof("Cleared etcd member removal annotations from node %s after cluster reset", node.Name) } - return nil - }) - if err != nil { - logrus.Errorf("Failed to clear removal annotations from node %s after cluster reset: %v", node.Name, err) - } else { - logrus.Infof("Cleared etcd member removal annotations from node %s after cluster reset", node.Name) } } if err := m.etcd.clearReset(); err != nil { @@ -94,46 +93,40 @@ func (m *metadataHandler) checkReset() { func (m *metadataHandler) handleSelf(node *v1.Node) (*v1.Node, error) { if m.etcd.config.DisableETCD { - if node.Annotations[NodeNameAnnotation] == "" && - node.Annotations[NodeAddressAnnotation] == "" && - node.Labels[util.ETCDRoleLabelKey] == "" { - return node, nil - } - - node = node.DeepCopy() - if node.Annotations == nil { - node.Annotations = map[string]string{} - } - if node.Labels == nil { - node.Labels = map[string]string{} + patch := jsonpatch.NewBuilder() + ls := labels.Set(node.Annotations) + patch.WithPath("metadata", "annotations"). + RemoveIfHas(ls, NodeNameAnnotation). + RemoveIfHas(ls, NodeAddressAnnotation) + ls = labels.Set(node.Labels) + patch.WithPath("metadata", "labels"). + RemoveIfHas(ls, util.ETCDRoleLabelKey) + if patch.Len() > 0 { + b, err := patch.Marshal() + if err != nil { + return node, err + } + return m.nodeController.Patch(node.Name, types.JSONPatchType, b) } - - delete(node.Annotations, NodeNameAnnotation) - delete(node.Annotations, NodeAddressAnnotation) - delete(node.Labels, util.ETCDRoleLabelKey) - - return m.nodeController.Update(node) + return node, nil } m.once.Do(m.checkReset) - if node.Annotations[NodeNameAnnotation] == m.etcd.name && - node.Annotations[NodeAddressAnnotation] == m.etcd.address && - node.Labels[util.ETCDRoleLabelKey] == "true" { - return node, nil - } - - node = node.DeepCopy() - if node.Annotations == nil { - node.Annotations = map[string]string{} - } - if node.Labels == nil { - node.Labels = map[string]string{} + patch := jsonpatch.NewBuilder() + ls := labels.Set(node.Annotations) + patch.WithPath("metadata", "annotations"). + AddIfNotEqual(ls, NodeNameAnnotation, m.etcd.name). + AddIfNotEqual(ls, NodeAddressAnnotation, m.etcd.address) + ls = labels.Set(node.Labels) + patch.WithPath("metadata", "labels"). + AddIfNotEqual(ls, util.ETCDRoleLabelKey, "true") + if patch.Len() > 0 { + b, err := patch.Marshal() + if err != nil { + return node, err + } + return m.nodeController.Patch(node.Name, types.JSONPatchType, b) } - - node.Annotations[NodeNameAnnotation] = m.etcd.name - node.Annotations[NodeAddressAnnotation] = m.etcd.address - node.Labels[util.ETCDRoleLabelKey] = "true" - - return m.nodeController.Update(node) + return node, nil } diff --git a/pkg/secretsencrypt/config.go b/pkg/secretsencrypt/config.go index dd770c7d8b7a..00de2c74d797 100644 --- a/pkg/secretsencrypt/config.go +++ b/pkg/secretsencrypt/config.go @@ -9,11 +9,14 @@ import ( "github.com/k3s-io/k3s/pkg/daemons/config" "github.com/k3s-io/k3s/pkg/util" + "github.com/k3s-io/k3s/pkg/util/jsonpatch" "github.com/k3s-io/k3s/pkg/version" corev1 "k8s.io/api/core/v1" "github.com/sirupsen/logrus" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/labels" + "k8s.io/apimachinery/pkg/types" apiserverconfigv1 "k8s.io/apiserver/pkg/apis/config/v1" ) @@ -149,12 +152,12 @@ func getEncryptionHashFile(runtime *config.ControlRuntime) (string, error) { return string(curEncryptionByte), nil } -func BootstrapEncryptionHashAnnotation(node *corev1.Node, runtime *config.ControlRuntime) error { +func BootstrapEncryptionHashAnnotation(node *corev1.Node, runtime *config.ControlRuntime, patch jsonpatch.PatchBuilder) error { existingAnn, err := getEncryptionHashFile(runtime) if err != nil { return err } - node.Annotations[EncryptionHashAnnotation] = existingAnn + patch.WithPath("metadata", "annotations").AddIfNotEqual(labels.Set(node.Annotations), EncryptionHashAnnotation, existingAnn) return nil } @@ -163,13 +166,16 @@ func WriteEncryptionHashAnnotation(runtime *config.ControlRuntime, node *corev1. if err != nil { return err } - if node.Annotations == nil { - return fmt.Errorf("node annotations do not exist for %s", node.ObjectMeta.Name) - } ann := stage + "-" + encryptionConfigHash - node.Annotations[EncryptionHashAnnotation] = ann - if _, err = runtime.Core.Core().V1().Node().Update(node); err != nil { - return err + patch := jsonpatch.NewBuilder("metadata", "annotations").AddIfNotEqual(labels.Set(node.Annotations), EncryptionHashAnnotation, ann) + if patch.Len() > 0 { + b, err := patch.Marshal() + if err != nil { + return err + } + if _, err = runtime.Core.Core().V1().Node().Patch(node.Name, types.JSONPatchType, b); err != nil { + return err + } } logrus.Debugf("encryption hash annotation set successfully on node: %s\n", node.ObjectMeta.Name) return os.WriteFile(runtime.EncryptionHash, []byte(ann), 0600) diff --git a/pkg/secretsencrypt/controller.go b/pkg/secretsencrypt/controller.go index 7fb76f6ce6b7..4918fddfaaa2 100644 --- a/pkg/secretsencrypt/controller.go +++ b/pkg/secretsencrypt/controller.go @@ -8,6 +8,7 @@ import ( "github.com/k3s-io/k3s/pkg/cluster" "github.com/k3s-io/k3s/pkg/daemons/config" "github.com/k3s-io/k3s/pkg/util" + "github.com/k3s-io/k3s/pkg/util/jsonpatch" coreclient "github.com/rancher/wrangler/pkg/generated/controllers/core/v1" "github.com/sirupsen/logrus" corev1 "k8s.io/api/core/v1" @@ -20,7 +21,6 @@ import ( "k8s.io/client-go/kubernetes" "k8s.io/client-go/tools/pager" "k8s.io/client-go/tools/record" - "k8s.io/client-go/util/retry" ) const ( @@ -92,16 +92,13 @@ func (h *handler) onChangeNode(nodeName string, node *corev1.Node) (*corev1.Node } ann = EncryptionReencryptActive + "-" + reencryptHash - err = retry.RetryOnConflict(retry.DefaultRetry, func() error { - node, err = h.nodes.Get(nodeName, metav1.GetOptions{}) - if err != nil { - return err - } - node.Annotations[EncryptionHashAnnotation] = ann - _, err = h.nodes.Update(node) - return err - }) + patch := jsonpatch.NewBuilder("metadata", "annotations").Add(ann, EncryptionHashAnnotation) + b, err := patch.Marshal() if err != nil { + return node, err + } + + if _, err := h.nodes.Patch(nodeName, types.JSONPatchType, b); err != nil { h.recorder.Event(nodeRef, corev1.EventTypeWarning, secretsUpdateErrorEvent, err.Error()) return node, err } @@ -113,15 +110,15 @@ func (h *handler) onChangeNode(nodeName string, node *corev1.Node) (*corev1.Node // If skipping, revert back to the previous stage if h.controlConfig.EncryptSkip { - err = retry.RetryOnConflict(retry.DefaultRetry, func() error { - node, err = h.nodes.Get(nodeName, metav1.GetOptions{}) + patch := jsonpatch.NewBuilder() + BootstrapEncryptionHashAnnotation(node, h.controlConfig.Runtime, patch) + if patch.Len() > 0 { + b, err := patch.Marshal() if err != nil { - return err + return node, err } - BootstrapEncryptionHashAnnotation(node, h.controlConfig.Runtime) - _, err = h.nodes.Update(node) - return err - }) + _, err = h.nodes.Patch(node.Name, types.JSONPatchType, b) + } return node, err } @@ -142,14 +139,7 @@ func (h *handler) onChangeNode(nodeName string, node *corev1.Node) (*corev1.Node h.recorder.Event(nodeRef, corev1.EventTypeWarning, secretsUpdateErrorEvent, err.Error()) return node, err } - err = retry.RetryOnConflict(retry.DefaultRetry, func() error { - node, err = h.nodes.Get(nodeName, metav1.GetOptions{}) - if err != nil { - return err - } - return WriteEncryptionHashAnnotation(h.controlConfig.Runtime, node, EncryptionReencryptFinished) - }) - if err != nil { + if err = WriteEncryptionHashAnnotation(h.controlConfig.Runtime, node, EncryptionReencryptFinished); err != nil { h.recorder.Event(nodeRef, corev1.EventTypeWarning, secretsUpdateErrorEvent, err.Error()) return node, err } diff --git a/pkg/server/secrets-encrypt.go b/pkg/server/secrets-encrypt.go index 7e0a4dd0c03f..b2248fcbd1c6 100644 --- a/pkg/server/secrets-encrypt.go +++ b/pkg/server/secrets-encrypt.go @@ -18,10 +18,12 @@ import ( "github.com/k3s-io/k3s/pkg/daemons/config" "github.com/k3s-io/k3s/pkg/secretsencrypt" "github.com/k3s-io/k3s/pkg/util" + "github.com/k3s-io/k3s/pkg/util/jsonpatch" "github.com/rancher/wrangler/pkg/generated/controllers/core" "github.com/sirupsen/logrus" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" + "k8s.io/apimachinery/pkg/types" apiserverconfigv1 "k8s.io/apiserver/pkg/apis/config/v1" "k8s.io/utils/pointer" ) @@ -267,21 +269,22 @@ func encryptionReencrypt(ctx context.Context, server *config.Control, force bool server.EncryptForce = force server.EncryptSkip = skip nodeName := os.Getenv("NODE_NAME") - node, err := server.Runtime.Core.Core().V1().Node().Get(nodeName, metav1.GetOptions{}) + + reencryptHash, err := secretsencrypt.GenReencryptHash(server.Runtime, secretsencrypt.EncryptionReencryptRequest) if err != nil { return err } - reencryptHash, err := secretsencrypt.GenReencryptHash(server.Runtime, secretsencrypt.EncryptionReencryptRequest) + ann := secretsencrypt.EncryptionReencryptRequest + "-" + reencryptHash + patch := jsonpatch.NewBuilder("metadata", "annotations").Add(ann, secretsencrypt.EncryptionHashAnnotation) + b, err := patch.Marshal() if err != nil { return err } - ann := secretsencrypt.EncryptionReencryptRequest + "-" + reencryptHash - node.Annotations[secretsencrypt.EncryptionHashAnnotation] = ann - if _, err = server.Runtime.Core.Core().V1().Node().Update(node); err != nil { + if _, err = server.Runtime.Core.Core().V1().Node().Patch(nodeName, types.JSONPatchType, b); err != nil { return err } - logrus.Debugf("encryption hash annotation set successfully on node: %s\n", node.ObjectMeta.Name) + logrus.Debugf("encryption hash annotation set successfully on node: %s\n", nodeName) return nil } @@ -327,21 +330,22 @@ func encryptionRotateKeys(ctx context.Context, server *config.Control) error { func setReencryptAnnotation(server *config.Control) error { nodeName := os.Getenv("NODE_NAME") - node, err := server.Runtime.Core.Core().V1().Node().Get(nodeName, metav1.GetOptions{}) + + reencryptHash, err := secretsencrypt.GenReencryptHash(server.Runtime, secretsencrypt.EncryptionReencryptRequest) if err != nil { return err } - reencryptHash, err := secretsencrypt.GenReencryptHash(server.Runtime, secretsencrypt.EncryptionReencryptRequest) + ann := secretsencrypt.EncryptionReencryptRequest + "-" + reencryptHash + patch := jsonpatch.NewBuilder("metadata", "annotations").Add(ann, secretsencrypt.EncryptionHashAnnotation) + b, err := patch.Marshal() if err != nil { return err } - ann := secretsencrypt.EncryptionReencryptRequest + "-" + reencryptHash - node.Annotations[secretsencrypt.EncryptionHashAnnotation] = ann - if _, err = server.Runtime.Core.Core().V1().Node().Update(node); err != nil { + if _, err = server.Runtime.Core.Core().V1().Node().Patch(nodeName, types.JSONPatchType, b); err != nil { return err } - logrus.Debugf("encryption hash annotation set successfully on node: %s\n", node.ObjectMeta.Name) + logrus.Debugf("encryption hash annotation set successfully on node: %s\n", nodeName) return nil } diff --git a/pkg/server/server.go b/pkg/server/server.go index 860f40ecb348..d58911aef210 100644 --- a/pkg/server/server.go +++ b/pkg/server/server.go @@ -26,6 +26,7 @@ import ( "github.com/k3s-io/k3s/pkg/secretsencrypt" "github.com/k3s-io/k3s/pkg/static" "github.com/k3s-io/k3s/pkg/util" + "github.com/k3s-io/k3s/pkg/util/jsonpatch" "github.com/k3s-io/k3s/pkg/version" "github.com/pkg/errors" "github.com/rancher/wrangler/pkg/apply" @@ -35,6 +36,9 @@ import ( "github.com/sirupsen/logrus" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/labels" + "k8s.io/apimachinery/pkg/types" + "k8s.io/apimachinery/pkg/util/wait" clientset "k8s.io/client-go/kubernetes" "k8s.io/client-go/tools/clientcmd" ) @@ -567,47 +571,47 @@ func setNodeLabelsAndAnnotations(ctx context.Context, nodes v1.NodeClient, confi if config.DisableAgent || config.ControlConfig.DisableAPIServer { return nil } - for { + return wait.PollImmediateUntilWithContext(ctx, time.Second, func(ctx context.Context) (bool, error) { nodeName := os.Getenv("NODE_NAME") if nodeName == "" { logrus.Info("Waiting for control-plane node agent startup") - time.Sleep(1 * time.Second) - continue + return false, nil } node, err := nodes.Get(nodeName, metav1.GetOptions{}) if err != nil { logrus.Infof("Waiting for control-plane node %s startup: %v", nodeName, err) - time.Sleep(1 * time.Second) - continue - } - if node.Labels == nil { - node.Labels = make(map[string]string) - } - v, ok := node.Labels[util.ControlPlaneRoleLabelKey] - if !ok || v != "true" { - node.Labels[util.ControlPlaneRoleLabelKey] = "true" - node.Labels[util.MasterRoleLabelKey] = "true" + return false, nil } + ls := labels.Set(node.Labels) + patch := jsonpatch.NewBuilder() + patch.WithPath("metadata", "labels"). + AddIfNotEqual(ls, util.ControlPlaneRoleLabelKey, "true"). + AddIfNotEqual(ls, util.MasterRoleLabelKey, "true") if config.ControlConfig.EncryptSecrets { - if err = secretsencrypt.BootstrapEncryptionHashAnnotation(node, config.ControlConfig.Runtime); err != nil { - logrus.Infof("Unable to set encryption hash annotation %s", err.Error()) - break + if err = secretsencrypt.BootstrapEncryptionHashAnnotation(node, config.ControlConfig.Runtime, patch); err != nil { + logrus.Errorf("Failed to set encryption hash annotation: %v", err) + return false, nil } } - _, err = nodes.Update(node) - if err == nil { - logrus.Infof("Labels and annotations have been set successfully on node: %s", nodeName) - break - } - select { - case <-ctx.Done(): - return ctx.Err() - case <-time.After(time.Second): + if patch.Len() > 0 { + b, err := patch.Marshal() + if err != nil { + return false, err + } + + if _, err := nodes.Patch(nodeName, types.JSONPatchType, b); err != nil { + logrus.Errorf("Failed to set server annotations and labels on node %s: %v", nodeName, err) + return false, nil + } + logrus.Infof("Server annotations and labels have been set successfully on node: %s", nodeName) + return true, nil } - } - return nil + + logrus.Infof("Server annotations and labels have already set on node: %s", nodeName) + return true, nil + }) } func setClusterDNSConfig(ctx context.Context, config *Config, configMap v1.ConfigMapClient) error {