From ac5b65466639d75e9065e06cfe4ff9ae5a286aad Mon Sep 17 00:00:00 2001 From: Matt Merkes Date: Sat, 10 Feb 2024 00:00:19 +0000 Subject: [PATCH] Implements InstancesV2 interface Signed-off-by: Matt Merkes --- pkg/providers/v1/aws.go | 3 +- pkg/providers/v1/aws_test.go | 4 + pkg/providers/v1/instances_v2.go | 99 ++++++++++++ pkg/providers/v1/instances_v2_test.go | 215 ++++++++++++++++++++++++++ 4 files changed, 319 insertions(+), 2 deletions(-) create mode 100644 pkg/providers/v1/instances_v2.go create mode 100644 pkg/providers/v1/instances_v2_test.go diff --git a/pkg/providers/v1/aws.go b/pkg/providers/v1/aws.go index 1e276f237a..866b5532e3 100644 --- a/pkg/providers/v1/aws.go +++ b/pkg/providers/v1/aws.go @@ -1502,9 +1502,8 @@ func (c *Cloud) Instances() (cloudprovider.Instances, bool) { } // InstancesV2 returns an implementation of InstancesV2 for Amazon Web Services. -// TODO: implement ONLY for external cloud provider func (c *Cloud) InstancesV2() (cloudprovider.InstancesV2, bool) { - return nil, false + return c, true } // Zones returns an implementation of Zones for Amazon Web Services. diff --git a/pkg/providers/v1/aws_test.go b/pkg/providers/v1/aws_test.go index e3f4145a98..0e0e7dcec8 100644 --- a/pkg/providers/v1/aws_test.go +++ b/pkg/providers/v1/aws_test.go @@ -559,6 +559,10 @@ func testHasNodeAddress(t *testing.T, addrs []v1.NodeAddress, addressType v1.Nod t.Errorf("Did not find expected address: %s:%s in %v", addressType, address, addrs) } +func makeMinimalInstance(instanceID string) ec2.Instance { + return makeInstance(instanceID, "", "", "", "", nil, false) +} + func makeInstance(instanceID string, privateIP, publicIP, privateDNSName, publicDNSName string, ipv6s []string, setNetInterface bool) ec2.Instance { var tag ec2.Tag tag.Key = aws.String(TagNameKubernetesClusterLegacy) diff --git a/pkg/providers/v1/instances_v2.go b/pkg/providers/v1/instances_v2.go new file mode 100644 index 0000000000..887e5bbbec --- /dev/null +++ b/pkg/providers/v1/instances_v2.go @@ -0,0 +1,99 @@ +/* +Copyright 2024 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +// This file implements the InstancesV2 interface. +// InstancesV2 is an abstract, pluggable interface for cloud provider instances. +// Unlike the Instances interface, it is designed for external cloud providers and should only be used by them. + +package aws + +import ( + "context" + + v1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/types" + cloudprovider "k8s.io/cloud-provider" +) + +func (c *Cloud) getProviderID(ctx context.Context, node *v1.Node) (string, error) { + if node.Spec.ProviderID != "" { + return node.Spec.ProviderID, nil + } + + instanceID, err := c.InstanceID(ctx, types.NodeName(node.Name)) + if err != nil { + return "", err + } + + return c.ProviderName() + "://" + instanceID, nil +} + +// InstanceExists returns true if the instance for the given node exists according to the cloud provider. +// Use the node.name or node.spec.providerID field to find the node in the cloud provider. +func (c *Cloud) InstanceExists(ctx context.Context, node *v1.Node) (bool, error) { + providerID, err := c.getProviderID(ctx, node) + if err != nil { + return false, err + } + + return c.InstanceExistsByProviderID(ctx, providerID) +} + +// InstanceShutdown returns true if the instance is shutdown according to the cloud provider. +// Use the node.name or node.spec.providerID field to find the node in the cloud provider. +func (c *Cloud) InstanceShutdown(ctx context.Context, node *v1.Node) (bool, error) { + providerID, err := c.getProviderID(ctx, node) + if err != nil { + return false, err + } + + return c.InstanceShutdownByProviderID(ctx, providerID) +} + +// InstanceMetadata returns the instance's metadata. The values returned in InstanceMetadata are +// translated into specific fields and labels in the Node object on registration. +// Implementations should always check node.spec.providerID first when trying to discover the instance +// for a given node. In cases where node.spec.providerID is empty, implementations can use other +// properties of the node like its name, labels and annotations. +func (c *Cloud) InstanceMetadata(ctx context.Context, node *v1.Node) (*cloudprovider.InstanceMetadata, error) { + providerID, err := c.getProviderID(ctx, node) + if err != nil { + return nil, err + } + + instanceType, err := c.InstanceTypeByProviderID(ctx, providerID) + if err != nil { + return nil, err + } + + zone, err := c.GetZoneByProviderID(ctx, providerID) + if err != nil { + return nil, err + } + + nodeAddresses, err := c.NodeAddressesByProviderID(ctx, providerID) + if err != nil { + return nil, err + } + + return &cloudprovider.InstanceMetadata{ + ProviderID: providerID, + InstanceType: instanceType, + NodeAddresses: nodeAddresses, + Zone: zone.FailureDomain, + Region: zone.Region, + }, nil +} diff --git a/pkg/providers/v1/instances_v2_test.go b/pkg/providers/v1/instances_v2_test.go new file mode 100644 index 0000000000..b22b774ebe --- /dev/null +++ b/pkg/providers/v1/instances_v2_test.go @@ -0,0 +1,215 @@ +/* +Copyright 2024 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package aws + +import ( + "context" + "fmt" + "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/aws/awserr" + "github.com/aws/aws-sdk-go/service/ec2" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" + v1 "k8s.io/api/core/v1" + "testing" +) + +func TestGetProviderId(t *testing.T) { + for _, tc := range []struct { + name string + instanceID string + node v1.Node + expectedProviderID string + }{ + { + name: "ProviderID already set should be returned", + instanceID: "i-00000000000000000", + node: v1.Node{ + Spec: v1.NodeSpec{ + ProviderID: "obviously-custom-id", + }, + }, + expectedProviderID: "obviously-custom-id", + }, + { + name: "Should get ProviderID if not already set", + instanceID: "i-00000000000000001", + node: v1.Node{ + Spec: v1.NodeSpec{}, + }, + expectedProviderID: "aws:///us-east-1a/i-00000000000000001", + }, + } { + t.Run(tc.name, func(t *testing.T) { + instance := makeMinimalInstance(tc.instanceID) + c, _ := mockInstancesResp(&instance, []*ec2.Instance{&instance}) + + result, err := c.getProviderID(context.TODO(), &tc.node) + if err != nil { + t.Errorf("Should not error getting ProviderID: %s", err) + } + + if result != tc.expectedProviderID { + t.Errorf("Expected ProviderID to be %s. Got %s", tc.expectedProviderID, result) + } + }) + } +} + +func TestInstanceExists(t *testing.T) { + for _, tc := range []struct { + name string + instanceExists bool + instanceState string + expectedExists bool + }{ + { + name: "Should return false when instance is not found", + instanceExists: false, + instanceState: "", + expectedExists: false, + }, + { + name: "Should return true when instance is found and running", + instanceExists: true, + instanceState: ec2.InstanceStateNameRunning, + expectedExists: true, + }, + { + name: "Should return false when instance is found but terminated", + instanceExists: true, + instanceState: ec2.InstanceStateNameTerminated, + expectedExists: false, + }, + } { + t.Run(tc.name, func(t *testing.T) { + c := getCloudWithMockedDescribeInstances(tc.instanceExists, tc.instanceState) + + result, err := c.InstanceExists(context.TODO(), &v1.Node{ + Spec: v1.NodeSpec{ + ProviderID: "aws:///us-west-2c/1abc-2def/i-abc", + }, + }) + + assert.Nil(t, err) + if tc.expectedExists { + assert.True(t, result) + } else { + assert.False(t, result) + } + }) + } +} + +func TestInstanceShutdown(t *testing.T) { + for _, tc := range []struct { + name string + instanceExists bool + instanceState string + expectedShutdown bool + }{ + { + name: "Should return false when instance is found and running", + instanceExists: true, + instanceState: ec2.InstanceStateNameRunning, + expectedShutdown: false, + }, + { + name: "Should return false when instance is found and terminated", + instanceExists: true, + instanceState: ec2.InstanceStateNameTerminated, + expectedShutdown: false, + }, + { + name: "Should return true when instance is found and stopped", + instanceExists: true, + instanceState: ec2.InstanceStateNameStopped, + expectedShutdown: true, + }, + } { + t.Run(tc.name, func(t *testing.T) { + c := getCloudWithMockedDescribeInstances(tc.instanceExists, tc.instanceState) + + result, err := c.InstanceShutdown(context.TODO(), &v1.Node{ + Spec: v1.NodeSpec{ + ProviderID: "aws:///us-west-2c/1abc-2def/i-abc", + }, + }) + + assert.Nil(t, err) + if tc.expectedShutdown { + assert.True(t, result) + } else { + assert.False(t, result) + } + }) + } +} + +func TestInstanceMetadata(t *testing.T) { + t.Run("Should return populated InstanceMetadata", func(t *testing.T) { + instance := makeInstance("i-00000000000000000", "192.168.0.1", "1.2.3.4", "instance-same.ec2.internal", "instance-same.ec2.external", nil, true) + c, _ := mockInstancesResp(&instance, []*ec2.Instance{&instance}) + node := &v1.Node{ + Spec: v1.NodeSpec{ + ProviderID: fmt.Sprintf("aws:///us-west-2c/1abc-2def/%s", *instance.InstanceId), + }, + } + + result, err := c.InstanceMetadata(context.TODO(), node) + if err != nil { + t.Errorf("Should not error getting InstanceMetadata: %s", err) + } + + assert.Equal(t, "aws:///us-west-2c/1abc-2def/i-00000000000000000", result.ProviderID) + assert.Equal(t, "c3.large", result.InstanceType) + assert.Equal(t, []v1.NodeAddress{ + {Type: "InternalIP", Address: "192.168.0.1"}, + {Type: "ExternalIP", Address: "1.2.3.4"}, + {Type: "InternalDNS", Address: "instance-same.ec2.internal"}, + {Type: "Hostname", Address: "instance-same.ec2.internal"}, + {Type: "ExternalDNS", Address: "instance-same.ec2.external"}, + }, result.NodeAddresses) + assert.Equal(t, "us-east-1a", result.Zone) + assert.Equal(t, "us-east-1", result.Region) + }) +} + +func getCloudWithMockedDescribeInstances(instanceExists bool, instanceState string) *Cloud { + mockedEC2API := newMockedEC2API() + c := &Cloud{ec2: &awsSdkEC2{ec2: mockedEC2API}} + + if !instanceExists { + mockedEC2API.On("DescribeInstances", mock.Anything).Return(&ec2.DescribeInstancesOutput{}, awserr.New("InvalidInstanceID.NotFound", "Instance not found", nil)) + } else { + mockedEC2API.On("DescribeInstances", mock.Anything).Return(&ec2.DescribeInstancesOutput{ + Reservations: []*ec2.Reservation{ + { + Instances: []*ec2.Instance{ + { + State: &ec2.InstanceState{ + Name: aws.String(instanceState), + }, + }, + }, + }, + }, + }, nil) + } + + return c +}