From 6c4828f9d49ad5ccf5f75ce93c57e92419a8bac8 Mon Sep 17 00:00:00 2001 From: Madhu Rajanna Date: Tue, 29 Oct 2024 13:05:15 +0100 Subject: [PATCH] use predictable name for csiaddons object This PR does the below changes * Use the deployment/Daemonset as the owner of the csiaddons instead of pod * Use predictable name which includes the nodeID, owner name and the kind to ensure that we get unique names per resources also if the pod is restarting we will reuse the same resource name for the csiAddonsNode object. Using Resource Kind as csiAddonsNode object is a namespaced resouces * Take care of worst case where resource char limit is not meet. Signed-off-by: Madhu Rajanna --- .../internal/csiaddonsnode/csiaddonsnode.go | 113 +++++++++++++----- .../csiaddonsnode/csiaddonsnode_test.go | 66 ++++++++++ sidecar/main.go | 1 + 3 files changed, 147 insertions(+), 33 deletions(-) diff --git a/sidecar/internal/csiaddonsnode/csiaddonsnode.go b/sidecar/internal/csiaddonsnode/csiaddonsnode.go index c714d6142..f3badd838 100644 --- a/sidecar/internal/csiaddonsnode/csiaddonsnode.go +++ b/sidecar/internal/csiaddonsnode/csiaddonsnode.go @@ -18,6 +18,8 @@ package csiaddonsnode import ( "context" + "crypto/sha256" + "encoding/hex" "errors" "fmt" "time" @@ -27,11 +29,12 @@ import ( apierrors "k8s.io/apimachinery/pkg/api/errors" v1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/runtime/serializer" - "k8s.io/apimachinery/pkg/types" + "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/util/wait" + "k8s.io/client-go/kubernetes" "k8s.io/client-go/rest" "k8s.io/klog/v2" + ctrlClient "sigs.k8s.io/controller-runtime/pkg/client" ) const ( @@ -58,6 +61,9 @@ type Manager struct { // Config is a ReST Config for the Kubernets API. Config *rest.Config + // kubernetes client to interact with the Kubernetes API. + KubeClinent *kubernetes.Clientset + // Node is the hostname of the system where the sidecar is running. Node string @@ -108,31 +114,22 @@ func (mgr *Manager) Deploy() error { // If the CSIAddonsNode object already exists, it will not be re-created or // modified, and the existing object is kept as-is. func (mgr *Manager) newCSIAddonsNode(node *csiaddonsv1alpha1.CSIAddonsNode) error { - scheme, err := csiaddonsv1alpha1.SchemeBuilder.Build() - if err != nil { - return fmt.Errorf("failed to add scheme: %w", err) + s := runtime.Scheme{} + if err := csiaddonsv1alpha1.SchemeBuilder.AddToScheme(&s); err != nil { + return fmt.Errorf("failed to register scheme: %w", err) } - crdConfig := *mgr.Config - crdConfig.GroupVersion = &csiaddonsv1alpha1.GroupVersion - crdConfig.APIPath = "/apis" - crdConfig.NegotiatedSerializer = serializer.NewCodecFactory(scheme) - crdConfig.UserAgent = rest.DefaultKubernetesUserAgent() - - c, err := rest.UnversionedRESTClientFor(&crdConfig) + cli, err := ctrlClient.New(mgr.Config, ctrlClient.Options{Scheme: &s}) if err != nil { - return fmt.Errorf("failed to get REST Client: %w", err) + return fmt.Errorf("failed to create controller-runtime client: %w", err) } - - err = c.Post(). - Resource("csiaddonsnodes"). - Namespace(node.Namespace). - Name(node.Name). - Body(node). - Do(context.TODO()). - Error() - - if err != nil && !apierrors.IsAlreadyExists(err) { + ctx := context.TODO() + err = cli.Create(ctx, node) + if err != nil { + if apierrors.IsAlreadyExists(err) { + klog.V(5).Infof("CSIAddonsNode %s/%s already exists", node.Namespace, node.Name) + return cli.Update(ctx, node) + } return fmt.Errorf("failed to create csiaddonsnode object: %w", err) } @@ -166,18 +163,45 @@ func (mgr *Manager) getCSIAddonsNode() (*csiaddonsv1alpha1.CSIAddonsNode, error) errInvalidConfig) } + // Get the owner of the pod as we want to set the owner of the CSIAddonsNode + // so that it won't get deleted when the pod is deleted rather it will be deleted + // when the owner is deleted. + pod, err := mgr.KubeClinent.CoreV1().Pods(mgr.PodNamespace).Get(context.TODO(), mgr.PodName, v1.GetOptions{}) + if err != nil { + return nil, fmt.Errorf("failed to get pod: %w", err) + } + + if len(pod.OwnerReferences) == 0 { + return nil, fmt.Errorf("%w: pod has no owner", errInvalidConfig) + } + + ownerReferences := []v1.OwnerReference{} + if pod.OwnerReferences[0].Kind == "ReplicaSet" { + // If the pod is owned by a ReplicaSet, we need to get the owner of the ReplicaSet i.e. Deployment + rs, err := mgr.KubeClinent.AppsV1().ReplicaSets(mgr.PodNamespace).Get(context.TODO(), pod.OwnerReferences[0].Name, v1.GetOptions{}) + if err != nil { + return nil, fmt.Errorf("failed to get replicaset: %w", err) + } + if len(rs.OwnerReferences) == 0 { + return nil, fmt.Errorf("%w: replicaset has no owner", errInvalidConfig) + } + ownerReferences = append(ownerReferences, rs.OwnerReferences[0]) + } else { + // If the pod is owned by DeamonSet or StatefulSet get the owner of the pod. + ownerReferences = append(ownerReferences, pod.OwnerReferences[0]) + } + // we need to have the constant name for the CSIAddonsNode object. + // We will use the nodeID and the ownerName for the CSIAddonsNode object name. + name, err := generateName(mgr.Node, ownerReferences[0].Kind, ownerReferences[0].Name) + if err != nil { + return nil, fmt.Errorf("failed to generate name: %w", err) + } + return &csiaddonsv1alpha1.CSIAddonsNode{ ObjectMeta: v1.ObjectMeta{ - Name: mgr.PodName, - Namespace: mgr.PodNamespace, - OwnerReferences: []v1.OwnerReference{ - { - APIVersion: "v1", - Kind: "Pod", - Name: mgr.PodName, - UID: types.UID(mgr.PodUID), - }, - }, + Name: name, + Namespace: mgr.PodNamespace, + OwnerReferences: ownerReferences, }, Spec: csiaddonsv1alpha1.CSIAddonsNodeSpec{ Driver: csiaddonsv1alpha1.CSIAddonsNodeDriver{ @@ -188,3 +212,26 @@ func (mgr *Manager) getCSIAddonsNode() (*csiaddonsv1alpha1.CSIAddonsNode, error) }, }, nil } + +func generateName(nodeID, ownerKind, ownerName string) (string, error) { + if nodeID == "" { + return "", fmt.Errorf("nodeID is required") + } + if ownerKind == "" { + return "", fmt.Errorf("ownerKind is required") + } + if ownerName == "" { + return "", fmt.Errorf("ownerName is required") + } + base := fmt.Sprintf("%s-%s-%s", nodeID, ownerKind, ownerName) + if len(base) > 253 { + // Generate a UUID based on nodeID, ownerKind, and ownerName + data := nodeID + ownerKind + ownerName + hash := sha256.Sum256([]byte(data)) + uuid := hex.EncodeToString(hash[:8]) // Use the first 8 characters of the hash as a UUID-like string + finalName := fmt.Sprintf("%s-%s", base[:251-len(uuid)-1], uuid) // Ensure total length is within 253 + return finalName, nil + } + + return base, nil +} diff --git a/sidecar/internal/csiaddonsnode/csiaddonsnode_test.go b/sidecar/internal/csiaddonsnode/csiaddonsnode_test.go index d989818cf..d87088660 100644 --- a/sidecar/internal/csiaddonsnode/csiaddonsnode_test.go +++ b/sidecar/internal/csiaddonsnode/csiaddonsnode_test.go @@ -17,6 +17,9 @@ limitations under the License. package csiaddonsnode import ( + "crypto/sha256" + "encoding/hex" + "fmt" "reflect" "testing" @@ -161,3 +164,66 @@ func Test_getCSIAddonsNode(t *testing.T) { }) } } + +func Test_generateName(t *testing.T) { + type args struct { + nodeID string + ownerKind string + ownerName string + } + tests := []struct { + name string + args args + want string + }{ + { + name: "Normal case", + args: args{ + nodeID: "example-node-id", + ownerKind: "Deployment", + ownerName: "example-owner-name", + }, + want: "example-node-id-Deployment-example-owner-name", + }, + { + name: "Case with long names", + args: args{ + nodeID: "a-very-long-node-id-that-exceeds-the-limit-of-253-characters-and-should-trigger-uuid-generation-12345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890", + ownerKind: "Deployment", + ownerName: "another-long-owner-name-that-is-definitely-more-than-253-characters-long-12345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890", + }, + // This should be computed dynamically in the expected function + want: generateExpectedLongName("a-very-long-node-id-that-exceeds-the-limit-of-253-characters-and-should-trigger-uuid-generation-12345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890", "Deployment", "another-long-owner-name-that-is-definitely-more-than-253-characters-long-12345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890"), + }, + { + name: "Exact 253 characters", + args: args{ + nodeID: "a-very-long-node-id-that-is-exactly-128-characters-long-because-of-some-conditions-and-requirements-conditionsandrequirements", + ownerKind: "Deployment", + ownerName: "another-long-owner-name-that-is-very-unique-and-specifies-exactly-how-the-naming-should-work-conditions-requirements", + }, + want: "a-very-long-node-id-that-is-exactly-128-characters-long-because-of-some-conditions-and-requirements-conditionsandrequirements-Deployment-another-long-owner-name-that-is-very-unique-and-specifies-exactly-how-the-naming-should-work-conditions-requirements", // Adjust based on hash + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := generateName(tt.args.nodeID, tt.args.ownerKind, tt.args.ownerName) + if err != nil { + t.Errorf("generateName() error = %v", err) + } + if got != tt.want { + t.Errorf("generateName() = %v, want %v", got, tt.want) + } + }) + } +} + +func generateExpectedLongName(nodeID, ownerKind, ownerName string) string { + data := nodeID + ownerKind + ownerName + hash := sha256.Sum256([]byte(data)) + uuid := hex.EncodeToString(hash[:8]) + base := fmt.Sprintf("%s-%s-%s", nodeID, ownerKind, ownerName) + fmt.Println(len(base)) + finalName := fmt.Sprintf("%s-%s", base[:251-len(uuid)-1], uuid) + return finalName +} diff --git a/sidecar/main.go b/sidecar/main.go index e2e6d410d..a83d4ca43 100644 --- a/sidecar/main.go +++ b/sidecar/main.go @@ -98,6 +98,7 @@ func main() { nodeMgr := &csiaddonsnode.Manager{ Client: csiClient, Config: cfg, + KubeClinent: kubeClient, Node: *nodeID, Endpoint: controllerEndpoint, PodName: *podName,