Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make NodeAddresses Configuration optional #44

Merged
merged 5 commits into from
Dec 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion pkg/cloudprovider/metal/cloud.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,12 @@ func (o *cloud) Initialize(clientBuilder cloudprovider.ControllerClientBuilder,
log.Fatalf("Failed to create new cluster: %v", err)
}

o.instancesV2 = newMetalInstancesV2(o.targetCluster.GetClient(), o.metalCluster.GetClient(), o.metalNamespace, o.cloudConfig.ClusterName)
o.instancesV2 = newMetalInstancesV2(
o.targetCluster.GetClient(),
o.metalCluster.GetClient(),
o.metalNamespace,
o.cloudConfig,
)

if err := o.metalCluster.GetFieldIndexer().IndexField(ctx, &metalv1alpha1.ServerClaim{}, serverClaimMetadataUIDField, func(object client.Object) []string {
serverClaim := object.(*metalv1alpha1.ServerClaim)
Expand Down
8 changes: 7 additions & 1 deletion pkg/cloudprovider/metal/cloud_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,13 @@ import (
)

var _ = Describe("Cloud", func() {
_, cp, _ := SetupTest()
cloudConfig := CloudConfig{
ClusterName: clusterName,
Networking: Networking{
ConfigureNodeAddresses: true,
},
}
_, cp, _ := SetupTest(cloudConfig)

It("should ensure the correct cloud provider setup", func() {
Expect((*cp).HasClusterID()).To(BeTrue())
Expand Down
7 changes: 6 additions & 1 deletion pkg/cloudprovider/metal/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,13 @@ type CloudProviderConfig struct {
cloudConfig CloudConfig
}

type Networking struct {
ConfigureNodeAddresses bool `json:"configureNodeAddresses"`
}

type CloudConfig struct {
ClusterName string `json:"clusterName"`
ClusterName string `json:"clusterName"`
Networking Networking `json:"networking"`
}

var (
Expand Down
25 changes: 14 additions & 11 deletions pkg/cloudprovider/metal/instances_v2.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,15 +20,15 @@ type metalInstancesV2 struct {
targetClient client.Client
metalClient client.Client
metalNamespace string
clusterName string
cloudConfig CloudConfig
}

func newMetalInstancesV2(targetClient client.Client, metalClient client.Client, namespace, clusterName string) cloudprovider.InstancesV2 {
func newMetalInstancesV2(targetClient client.Client, metalClient client.Client, namespace string, cloudConfig CloudConfig) cloudprovider.InstancesV2 {
return &metalInstancesV2{
targetClient: targetClient,
metalClient: metalClient,
metalNamespace: namespace,
clusterName: clusterName,
cloudConfig: cloudConfig,
}
}

Expand Down Expand Up @@ -98,7 +98,7 @@ func (o *metalInstancesV2) InstanceMetadata(ctx context.Context, node *corev1.No
if serverClaim.Labels == nil {
serverClaim.Labels = make(map[string]string)
}
serverClaim.Labels[LabelKeyClusterName] = o.clusterName
serverClaim.Labels[LabelKeyClusterName] = o.cloudConfig.ClusterName
klog.V(2).InfoS("Adding cluster name label to server claim object", "ServerClaim", client.ObjectKeyFromObject(serverClaim), "Node", node.Name)
if err := o.metalClient.Patch(ctx, serverClaim, client.MergeFrom(serverClaimBase)); err != nil {
return nil, fmt.Errorf("failed to patch server claim for Node %s: %w", node.Name, err)
Expand Down Expand Up @@ -139,13 +139,16 @@ func (o *metalInstancesV2) InstanceMetadata(ctx context.Context, node *corev1.No
klog.V(2).InfoS("No region label found for node instance", "Node", node.Name)
}

return &cloudprovider.InstanceMetadata{
ProviderID: providerID,
InstanceType: instanceType,
NodeAddresses: addresses,
Zone: zone,
Region: region,
}, nil
metaData := &cloudprovider.InstanceMetadata{
ProviderID: providerID,
InstanceType: instanceType,
Zone: zone,
Region: region,
}
if o.cloudConfig.Networking.ConfigureNodeAddresses {
metaData.NodeAddresses = addresses
}
return metaData, nil
}

func (o *metalInstancesV2) getServerClaimForNode(ctx context.Context, node *corev1.Node) (*metalv1alpha1.ServerClaim, error) {
Expand Down
113 changes: 109 additions & 4 deletions pkg/cloudprovider/metal/instances_v2_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,19 @@ import (
. "sigs.k8s.io/controller-runtime/pkg/envtest/komega"
)

var (
instancesProvider cloudprovider.InstancesV2
clusterName = "test"
)

var _ = Describe("InstancesV2", func() {
var (
instancesProvider cloudprovider.InstancesV2
)
ns, cp, clusterName := SetupTest()
cloudConfig := CloudConfig{
ClusterName: clusterName,
Networking: Networking{
ConfigureNodeAddresses: true,
},
}
ns, cp, clusterName := SetupTest(cloudConfig)

BeforeEach(func() {
By("Instantiating the instances v2 provider")
Expand Down Expand Up @@ -252,6 +260,103 @@ var _ = Describe("InstancesV2", func() {
})
})

var _ = Describe("InstancesV2 with modified CloudConfig", func() {
cloudConfig := CloudConfig{
ClusterName: clusterName,
Networking: Networking{
ConfigureNodeAddresses: false,
},
}
ns, cp, clusterName := SetupTest(cloudConfig)

BeforeEach(func() {
By("Instantiating the instances v2 provider")
var ok bool
instancesProvider, ok = (*cp).InstancesV2()
Expect(ok).To(BeTrue())
})

It("Should not configure node addresses if ConfigureNodeAddresses is false", func(ctx SpecContext) {
By("Creating a Server")
server := &metalv1alpha1.Server{
ObjectMeta: metav1.ObjectMeta{
GenerateName: "test-",
Labels: map[string]string{
"instance-type": "foo",
"zone": "a",
"region": "bar",
},
},
Spec: metalv1alpha1.ServerSpec{
UUID: "12345",
Power: "On",
},
}
Expect(k8sClient.Create(ctx, server)).To(Succeed())
DeferCleanup(k8sClient.Delete, server)

By("Patching the Server object to have a valid network interface status")
Eventually(UpdateStatus(server, func() {
server.Status.PowerState = metalv1alpha1.ServerOnPowerState
server.Status.NetworkInterfaces = []metalv1alpha1.NetworkInterface{{
Name: "my-nic",
IP: metalv1alpha1.MustParseIP("10.0.0.1"),
}}
})).Should(Succeed())

By("Creating a ServerClaim for a Node")
serverClaim := &metalv1alpha1.ServerClaim{
ObjectMeta: metav1.ObjectMeta{
Namespace: ns.Name,
GenerateName: "test-",
},
Spec: metalv1alpha1.ServerClaimSpec{
Power: "On",
ServerRef: &corev1.LocalObjectReference{Name: server.Name},
},
}
Expect(k8sClient.Create(ctx, serverClaim)).To(Succeed())
DeferCleanup(k8sClient.Delete, serverClaim)

By("Creating a Node object with a provider ID referencing the machine")
node := &corev1.Node{
ObjectMeta: metav1.ObjectMeta{
GenerateName: "test-",
},
Spec: corev1.NodeSpec{
ProviderID: getProviderID(serverClaim.Namespace, serverClaim.Name),
},
}
Expect(k8sClient.Create(ctx, node)).To(Succeed())
DeferCleanup(k8sClient.Delete, node)

By("Ensuring that an instance for a Node exists")
ok, err := instancesProvider.InstanceExists(ctx, node)
Expect(err).NotTo(HaveOccurred())
Expect(ok).To(BeTrue())

By("Ensuring that the instance is not shut down")
ok, err = instancesProvider.InstanceShutdown(ctx, node)
Expect(err).NotTo(HaveOccurred())
Expect(ok).To(BeFalse())

By("Ensuring that the instance meta data has empty addresses")
instanceMetadata, err := instancesProvider.InstanceMetadata(ctx, node)
Expect(err).NotTo(HaveOccurred())
Eventually(instanceMetadata).Should(SatisfyAll(
HaveField("ProviderID", getProviderID(serverClaim.Namespace, serverClaim.Name)),
HaveField("InstanceType", "foo"),
HaveField("NodeAddresses", BeEmpty()),
HaveField("Zone", "a"),
HaveField("Region", "bar")))

By("Ensuring cluster name label is added to ServerClaim object")
Eventually(Object(serverClaim)).Should(SatisfyAll(
HaveField("Labels", map[string]string{LabelKeyClusterName: clusterName}),
))
})
})

func getProviderID(namespace, serverClaimName string) string {
return fmt.Sprintf("%s://%s/%s", ProviderName, namespace, serverClaimName)
}
12 changes: 4 additions & 8 deletions pkg/cloudprovider/metal/suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,11 +97,10 @@ var _ = BeforeSuite(func() {
SetClient(k8sClient)
})

func SetupTest() (*corev1.Namespace, *cloudprovider.Interface, string) {
func SetupTest(cloudConfig CloudConfig) (*corev1.Namespace, *cloudprovider.Interface, string) {
var (
ns = &corev1.Namespace{}
cp cloudprovider.Interface
clusterName = "test"
ns = &corev1.Namespace{}
cp cloudprovider.Interface
)

BeforeEach(func(ctx SpecContext) {
Expand Down Expand Up @@ -145,9 +144,6 @@ func SetupTest() (*corev1.Namespace, *cloudprovider.Interface, string) {
defer func() {
_ = cloudConfigFile.Close()
}()
cloudConfig := CloudConfig{
ClusterName: clusterName,
}
cloudConfigData, err := yaml.Marshal(&cloudConfig)
Expect(err).NotTo(HaveOccurred())
Expect(os.WriteFile(cloudConfigFile.Name(), cloudConfigData, 0666)).To(Succeed())
Expand All @@ -164,5 +160,5 @@ func SetupTest() (*corev1.Namespace, *cloudprovider.Interface, string) {
cp.Initialize(clientBuilder, cloudProviderCtx.Done())
})

return ns, &cp, clusterName
return ns, &cp, cloudConfig.ClusterName
}
Loading