Skip to content

Commit

Permalink
Revert "Include ServerClaim UID in Node ProviderID (#29)"
Browse files Browse the repository at this point in the history
This reverts commit d6e03dc.
  • Loading branch information
defo89 committed Oct 30, 2024
1 parent d6e03dc commit 62079d8
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 23 deletions.
2 changes: 1 addition & 1 deletion .reuse/dep5
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ Source: https://github.com/ironcore-dev/cloud-provider-metal
Files:
.github/*
.gitignore
.golangci.yml
.golangci.yaml
.dockerignore
CODE_OF_CONDUCT.md
CODEOWNERS
Expand Down
24 changes: 6 additions & 18 deletions pkg/cloudprovider/metal/instances_v2.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,6 @@ import (
"fmt"
"strings"

"k8s.io/apimachinery/pkg/types"

metalv1alpha1 "github.com/ironcore-dev/metal-operator/api/v1alpha1"
corev1 "k8s.io/api/core/v1"
apierrors "k8s.io/apimachinery/pkg/api/errors"
Expand Down Expand Up @@ -122,7 +120,7 @@ func (o *metalInstancesV2) InstanceMetadata(ctx context.Context, node *corev1.No

providerID := node.Spec.ProviderID
if providerID == "" {
providerID = getProviderIDForServerClaim(serverClaim)
providerID = fmt.Sprintf("%s://%s/%s", ProviderName, o.metalNamespace, serverClaim.Name)
}

// TODO: use constants here
Expand Down Expand Up @@ -150,10 +148,6 @@ func (o *metalInstancesV2) InstanceMetadata(ctx context.Context, node *corev1.No
}, nil
}

func getProviderIDForServerClaim(serverClaim *metalv1alpha1.ServerClaim) string {
return fmt.Sprintf("%s://%s/%s/%s", ProviderName, serverClaim.Namespace, serverClaim.Name, serverClaim.UID)
}

func (o *metalInstancesV2) getServerClaimForNode(ctx context.Context, node *corev1.Node) (*metalv1alpha1.ServerClaim, error) {
var serverClaim *metalv1alpha1.ServerClaim
if node.Spec.ProviderID != "" {
Expand All @@ -180,7 +174,7 @@ func (o *metalInstancesV2) getServerClaimForNode(ctx context.Context, node *core
}

func (o *metalInstancesV2) getServerClaimFromProviderID(ctx context.Context, providerID string) (*metalv1alpha1.ServerClaim, error) {
objKey, objUID, err := getObjectKeyFromProviderID(providerID)
objKey, err := getObjectKeyFromProviderID(providerID)
if err != nil {
return nil, fmt.Errorf("failed to get object key for ProviderID %s: %w", providerID, err)
}
Expand All @@ -191,22 +185,16 @@ func (o *metalInstancesV2) getServerClaimFromProviderID(ctx context.Context, pro
}
return nil, fmt.Errorf("failed to get server claim object for ProviderID %s: %w", providerID, err)
}

// Check for UID equality. For backwards compatibility we only compare if the UID is part of the ProviderID.
if len(objUID) > 0 && serverClaim.UID != objUID {
return nil, cloudprovider.InstanceNotFound
}

return serverClaim, nil
}

func getObjectKeyFromProviderID(providerID string) (client.ObjectKey, types.UID, error) {
func getObjectKeyFromProviderID(providerID string) (client.ObjectKey, error) {
parts := strings.Split(strings.TrimPrefix(providerID, fmt.Sprintf("%s://", ProviderName)), "/")
if len(parts) != 3 {
return client.ObjectKey{}, "", fmt.Errorf("invalid format of ProviderID %s", providerID)
if len(parts) != 2 {
return client.ObjectKey{}, fmt.Errorf("invalid format of ProviderID %s", providerID)
}
return client.ObjectKey{
Namespace: parts[0],
Name: parts[1],
}, types.UID(parts[2]), nil
}, nil
}
12 changes: 8 additions & 4 deletions pkg/cloudprovider/metal/instances_v2_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ var _ = Describe("InstancesV2", func() {
instanceMetadata, err := instancesProvider.InstanceMetadata(ctx, node)
Expect(err).NotTo(HaveOccurred())
Eventually(instanceMetadata).Should(SatisfyAll(
HaveField("ProviderID", getProviderIDForServerClaim(serverClaim)),
HaveField("ProviderID", getProviderID(serverClaim.Namespace, serverClaim.Name)),
HaveField("InstanceType", "foo"),
HaveField("NodeAddresses", ContainElements(
corev1.NodeAddress{
Expand Down Expand Up @@ -163,7 +163,7 @@ var _ = Describe("InstancesV2", func() {
GenerateName: "test-",
},
Spec: corev1.NodeSpec{
ProviderID: getProviderIDForServerClaim(serverClaim),
ProviderID: getProviderID(serverClaim.Namespace, serverClaim.Name),
},
}
Expect(k8sClient.Create(ctx, node)).To(Succeed())
Expand All @@ -183,7 +183,7 @@ var _ = Describe("InstancesV2", func() {
instanceMetadata, err := instancesProvider.InstanceMetadata(ctx, node)
Expect(err).NotTo(HaveOccurred())
Eventually(instanceMetadata).Should(SatisfyAll(
HaveField("ProviderID", getProviderIDForServerClaim(serverClaim)),
HaveField("ProviderID", getProviderID(serverClaim.Namespace, serverClaim.Name)),
HaveField("InstanceType", "foo"),
HaveField("NodeAddresses", ContainElements(
corev1.NodeAddress{
Expand All @@ -207,7 +207,7 @@ var _ = Describe("InstancesV2", func() {
Name: "foo",
},
Spec: corev1.NodeSpec{
ProviderID: fmt.Sprintf("%s://%s/foo/bar", ProviderName, ns.Name),
ProviderID: getProviderID(ns.Name, "bar"),
},
}
Expect(k8sClient.Create(ctx, node)).To(Succeed())
Expand Down Expand Up @@ -251,3 +251,7 @@ var _ = Describe("InstancesV2", func() {
Expect(ok).To(BeFalse())
})
})

func getProviderID(namespace, serverClaimName string) string {
return fmt.Sprintf("%s://%s/%s", ProviderName, namespace, serverClaimName)
}

0 comments on commit 62079d8

Please sign in to comment.