Skip to content

Commit

Permalink
use predictable name for csiaddons object
Browse files Browse the repository at this point in the history
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 <[email protected]>
  • Loading branch information
Madhu-1 committed Oct 29, 2024
1 parent 54944f9 commit 3250532
Show file tree
Hide file tree
Showing 3 changed files with 147 additions and 33 deletions.
113 changes: 80 additions & 33 deletions sidecar/internal/csiaddonsnode/csiaddonsnode.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ package csiaddonsnode

import (
"context"
"crypto/sha256"
"encoding/hex"
"errors"
"fmt"
"time"
Expand All @@ -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 (
Expand All @@ -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

Expand Down Expand Up @@ -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)
}

Expand Down Expand Up @@ -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 wont get deleted when the pod is deleted rather it will be deleted

Check failure on line 167 in sidecar/internal/csiaddonsnode/csiaddonsnode.go

View workflow job for this annotation

GitHub Actions / codespell

wont ==> won't
// 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{
Expand All @@ -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
}
66 changes: 66 additions & 0 deletions sidecar/internal/csiaddonsnode/csiaddonsnode_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,9 @@ limitations under the License.
package csiaddonsnode

import (
"crypto/sha256"
"encoding/hex"
"fmt"
"reflect"
"testing"

Expand Down Expand Up @@ -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
}
1 change: 1 addition & 0 deletions sidecar/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@ func main() {
nodeMgr := &csiaddonsnode.Manager{
Client: csiClient,
Config: cfg,
KubeClinent: kubeClient,
Node: *nodeID,
Endpoint: controllerEndpoint,
PodName: *podName,
Expand Down

0 comments on commit 3250532

Please sign in to comment.