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

Support IP configuration for multicard instances #2031

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 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
20 changes: 9 additions & 11 deletions nodeadm/doc/api.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,7 @@

#### ClusterDetails

ClusterDetails contains the coordinates of your EKS cluster.
These details can be found using the [DescribeCluster API](https://docs.aws.amazon.com/eks/latest/APIReference/API_DescribeCluster.html).
ClusterDetails contains the coordinates of your EKS cluster. These details can be found using the [DescribeCluster API](https://docs.aws.amazon.com/eks/latest/APIReference/API_DescribeCluster.html).

_Appears in:_
- [NodeConfigSpec](#nodeconfigspec)
Expand All @@ -20,7 +19,7 @@ _Appears in:_
| --- | --- |
| `name` _string_ | Name is the name of your EKS cluster |
| `apiServerEndpoint` _string_ | APIServerEndpoint is the URL of your EKS cluster's kube-apiserver. |
| `certificateAuthority` _integer array_ | CertificateAuthority is a base64-encoded string of your cluster's certificate authority chain. |
| `certificateAuthority` _[byte](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.29/#byte-v1-meta) array_ | CertificateAuthority is a base64-encoded string of your cluster's certificate authority chain. |
| `cidr` _string_ | CIDR is your cluster's service CIDR block. This value is used to infer your cluster's DNS address. |
| `enableOutpost` _boolean_ | EnableOutpost determines how your node is configured when running on an AWS Outpost. |
| `id` _string_ | ID is an identifier for your cluster; this is only used when your node is running on an AWS Outpost. |
Expand All @@ -34,8 +33,8 @@ _Appears in:_

| Field | Description |
| --- | --- |
| `config` _string_ | Config is an inline [`containerd` configuration TOML](https://github.com/containerd/containerd/blob/main/docs/man/containerd-config.toml.5.md)<br />that will be merged with the defaults. |
| `baseRuntimeSpec` _object (keys:string, values:[RawExtension](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.29/#rawextension-runtime-pkg))_ | BaseRuntimeSpec is the OCI runtime specification upon which all containers will be based.<br />The provided spec will be merged with the default spec; so that a partial spec may be provided.<br />For more information, see: https://github.com/opencontainers/runtime-spec |
| `config` _string_ | Config is an inline [`containerd` configuration TOML](https://github.com/containerd/containerd/blob/main/docs/man/containerd-config.toml.5.md) that will be merged with the defaults. |
| `baseRuntimeSpec` _object (keys:string, values:RawExtension)_ | BaseRuntimeSpec is the OCI runtime specification upon which all containers will be based. The provided spec will be merged with the default spec; so that a partial spec may be provided. For more information, see: https://github.com/opencontainers/runtime-spec |

#### Feature

Expand Down Expand Up @@ -69,13 +68,12 @@ _Appears in:_

| Field | Description |
| --- | --- |
| `config` _object (keys:string, values:[RawExtension](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.29/#rawextension-runtime-pkg))_ | Config is a [`KubeletConfiguration`](https://kubernetes.io/docs/reference/config-api/kubelet-config.v1beta1/)<br />that will be merged with the defaults. |
| `flags` _string array_ | Flags are [command-line `kubelet` arguments](https://kubernetes.io/docs/reference/command-line-tools-reference/kubelet/).<br />that will be appended to the defaults. |
| `config` _object (keys:string, values:RawExtension)_ | Config is a [`KubeletConfiguration`](https://kubernetes.io/docs/reference/config-api/kubelet-config.v1beta1/) that will be merged with the defaults. |
| `flags` _string array_ | Flags are [command-line `kubelet` arguments](https://kubernetes.io/docs/reference/command-line-tools-reference/kubelet/). that will be appended to the defaults. |

#### LocalStorageOptions

LocalStorageOptions control how [EC2 instance stores](https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/InstanceStorage.html)
are used when available.
LocalStorageOptions control how [EC2 instance stores](https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/InstanceStorage.html) are used when available.

_Appears in:_
- [InstanceOptions](#instanceoptions)
Expand Down Expand Up @@ -104,8 +102,8 @@ NodeConfig is the primary configuration object for `nodeadm`.
| --- | --- |
| `apiVersion` _string_ | `node.eks.aws/v1alpha1`
| `kind` _string_ | `NodeConfig`
| `kind` _string_ | Kind is a string value representing the REST resource this object represents.<br />Servers may infer this from the endpoint the client submits requests to.<br />Cannot be updated.<br />In CamelCase.<br />More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds |
| `apiVersion` _string_ | APIVersion defines the versioned schema of this representation of an object.<br />Servers should convert recognized schemas to the latest internal value, and<br />may reject unrecognized values.<br />More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources |
| `kind` _string_ | Kind is a string value representing the REST resource this object represents. Servers may infer this from the endpoint the client submits requests to. Cannot be updated. In CamelCase. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds |
| `apiVersion` _string_ | APIVersion defines the versioned schema of this representation of an object. Servers should convert recognized schemas to the latest internal value, and may reject unrecognized values. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources |
| `metadata` _[ObjectMeta](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.29/#objectmeta-v1-meta)_ | Refer to Kubernetes API documentation for fields of `metadata`. |
| `spec` _[NodeConfigSpec](#nodeconfigspec)_ | |

Expand Down
78 changes: 78 additions & 0 deletions nodeadm/internal/api/status.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ package api
import (
"context"
"fmt"
"strconv"
"strings"
"time"

"github.com/aws/aws-sdk-go-v2/aws"
Expand Down Expand Up @@ -32,6 +34,10 @@ func GetInstanceDetails(ctx context.Context, featureGates map[Feature]bool, ec2C
return nil, err
}
}
networkCardDetails, err := getNetworkCardsDetails(ctx, imds.GetProperty)
if err != nil {
return nil, err
}

return &InstanceDetails{
ID: instanceIdenitityDocument.InstanceID,
Expand All @@ -40,6 +46,7 @@ func GetInstanceDetails(ctx context.Context, featureGates map[Feature]bool, ec2C
AvailabilityZone: instanceIdenitityDocument.AvailabilityZone,
MAC: string(mac),
PrivateDNSName: privateDNSName,
NetworkCards: networkCardDetails,
}, nil
}

Expand All @@ -64,3 +71,74 @@ func privateDNSNameAvailable(out *ec2.DescribeInstancesOutput) (bool, error) {
}
return aws.ToString(out.Reservations[0].Instances[0].PrivateDnsName) != "", nil
}

func getNetworkCardsDetails(ctx context.Context, imdsFunc func(ctx context.Context, prop imds.IMDSProperty) (string, error)) ([]NetworkCardDetails, error) {

allMacs, err := imdsFunc(ctx, "network/interfaces/macs/")
if err != nil {
return nil, fmt.Errorf("failed to get network interfaces from imds: %w", err)
}

availableMacs := parseAvailableMacs(allMacs)
details := []NetworkCardDetails{}

for _, mac := range availableMacs {
cardDetails, err := getNetworkCardDetail(ctx, imdsFunc, mac)
if err != nil {
if isNotFoundError(err) {
nkvetsinski marked this conversation as resolved.
Show resolved Hide resolved
continue
}
return nil, fmt.Errorf("failed to get network card details for MAC %s: %w", mac, err)
}
// ip address can be empty for efa-only cards
if cardDetails.IpAddress == "" {
continue
}

details = append(details, cardDetails)
}

return details, nil
}

func parseAvailableMacs(allMacs string) []string {
allMacs = strings.ReplaceAll(allMacs, "\n", "")
allMacs = strings.TrimSuffix(allMacs, "/")
allMacs = strings.TrimSpace(allMacs)

return strings.Split(allMacs, "/")
}

func getNetworkCardDetail(ctx context.Context, imdsFunc func(ctx context.Context, prop imds.IMDSProperty) (string, error), mac string) (NetworkCardDetails, error) {
// imds will return 404 if we query network-card object for instance that doesn't support multiple cards
cardIndexPath := imds.IMDSProperty(fmt.Sprintf("network/interfaces/macs/%s/network-card", mac))
// imds will return 404 if we query local-ipv4s object if ip-address is not confirured on the interface from EC2 (efa-only)
ipAddressPath := imds.IMDSProperty(fmt.Sprintf("network/interfaces/macs/%s/local-ipv4s", mac))

cardIndex, err := imdsFunc(ctx, cardIndexPath)
if err != nil {
return NetworkCardDetails{}, err
}
cardIndexInt, err := strconv.Atoi(cardIndex)
if err != nil {
return NetworkCardDetails{}, fmt.Errorf("invalid card index: %w", err)
}

ipAddress, err := imdsFunc(ctx, ipAddressPath)
if err != nil {
return NetworkCardDetails{}, err
}

return NetworkCardDetails{
MAC: mac,
CardIndex: cardIndexInt,
IpAddress: ipAddress,
}, nil
}

func isNotFoundError(err error) bool {
if err == nil {
return false
}
return strings.Contains(err.Error(), "StatusCode: 404")
}
143 changes: 143 additions & 0 deletions nodeadm/internal/api/status_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,143 @@
package api

import (
"context"
"errors"
"testing"

"github.com/awslabs/amazon-eks-ami/nodeadm/internal/aws/imds"
"github.com/stretchr/testify/assert"
)

var (
nonMulticardInstanceMac = `06:83:e7:fd:fd:fd/
`
validNetworkInterfacesMacs = `06:83:e7:fe:fe:fe/
06:83:e7:ff:ff:ff/
`
multicardNoIpOnOneCard = `06:83:e7:fb:fb:fb/
06:83:e7:fc:fc:fc/
06:83:e7:fc:fc:fd/
`
validTwoNetworkCardDetails = []NetworkCardDetails{
{MAC: "06:83:e7:fe:fe:fe", IpAddress: "1.2.3.4", CardIndex: 1},
{MAC: "06:83:e7:ff:ff:ff", IpAddress: "5.6.7.8", CardIndex: 0},
}

validOneNetworkCardDetails = []NetworkCardDetails{
{MAC: "06:83:e7:fb:fb:fb", IpAddress: "1.2.3.4", CardIndex: 0},
}
imds404 = errors.New("http response error StatusCode: 404, request to EC2 IMDS failed")
imdsGeneric = errors.New("IMDS error")
)

func mockGetPropertyImdsError(ctx context.Context, prop imds.IMDSProperty) (string, error) {
if prop == "network/interfaces/macs/" {
return "", imdsGeneric
}
return "", nil
}

func mockGetPropertyNonMulticardInstance(ctx context.Context, prop imds.IMDSProperty) (string, error) {
if prop == "network/interfaces/macs/" {
return nonMulticardInstanceMac, nil
}
if prop == "network/interfaces/macs/06:83:e7:fd:fd:fd/network-card" {
return "", imds404
}
return "", nil
}

func mockGetPropertyTwoValidCards(ctx context.Context, prop imds.IMDSProperty) (string, error) {
if prop == "network/interfaces/macs/" {
return validNetworkInterfacesMacs, nil
}
if prop == "network/interfaces/macs/06:83:e7:fe:fe:fe/network-card" {
return "1", nil
}
if prop == "network/interfaces/macs/06:83:e7:ff:ff:ff/network-card" {
return "0", nil
}
if prop == "network/interfaces/macs/06:83:e7:fe:fe:fe/local-ipv4s" {
return "1.2.3.4", nil
}
if prop == "network/interfaces/macs/06:83:e7:ff:ff:ff/local-ipv4s" {
return "5.6.7.8", nil
}

return "", nil
}

func mockGetPropertyNoIp(ctx context.Context, prop imds.IMDSProperty) (string, error) {
if prop == "network/interfaces/macs/" {
return multicardNoIpOnOneCard, nil
}
if prop == "network/interfaces/macs/06:83:e7:fb:fb:fb/network-card" {
return "0", nil
}
if prop == "network/interfaces/macs/06:83:e7:fc:fc:fc/network-card" {
return "1", nil
}
if prop == "network/interfaces/macs/06:83:e7:fc:fc:fd/network-card" {
return "2", nil
}
if prop == "network/interfaces/macs/06:83:e7:fb:fb:fb/local-ipv4s" {
return "1.2.3.4", nil
}
if prop == "network/interfaces/macs/06:83:e7:fc:fc:fc/local-ipv4s" {
return "", imds404
}
if prop == "network/interfaces/macs/06:83:e7:fc:fc:fd/local-ipv4s" {
return "", nil
}

return "", nil
}

func TestGetNetworkCardsDetails(t *testing.T) {

tests := []struct {
name string
mockGetProperty func(ctx context.Context, prop imds.IMDSProperty) (string, error)
expectedNetworkCardDetails []NetworkCardDetails
expectedError error
}{
{
name: "Success card 0 and card 1 available",
mockGetProperty: mockGetPropertyTwoValidCards,
expectedNetworkCardDetails: validTwoNetworkCardDetails,
expectedError: nil,
},
{
name: "Success non multicard instance",
mockGetProperty: mockGetPropertyNonMulticardInstance,
expectedNetworkCardDetails: []NetworkCardDetails{},
expectedError: nil,
},
{
name: "Success multicard instance no IP",
mockGetProperty: mockGetPropertyNoIp,
expectedNetworkCardDetails: validOneNetworkCardDetails,
expectedError: nil,
},
{
name: "Fail with IMDS error",
mockGetProperty: mockGetPropertyImdsError,
expectedNetworkCardDetails: nil,
expectedError: errors.New("failed to get network interfaces from imds: IMDS error"),
},
}

for _, test := range tests {
t.Logf("Running test: %s", test.name)
ctx := context.Background()
networkCardDetails, err := getNetworkCardsDetails(ctx, test.mockGetProperty)

assert.Equal(t, test.expectedNetworkCardDetails, networkCardDetails, t.Name())
if test.expectedError != nil {
assert.EqualError(t, err, test.expectedError.Error(), t.Name())
} else {
assert.NoError(t, err, t.Name())
}
}
}
19 changes: 13 additions & 6 deletions nodeadm/internal/api/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,13 +40,20 @@ type NodeConfigStatus struct {
Defaults DefaultOptions `json:"default,omitempty"`
}

type NetworkCardDetails struct {
MAC string `json:"mac,omitempty"`
IpAddress string `json:"ipAddress,omitempty"`
CardIndex int `json:"cardIndex,omitempty"`
}

type InstanceDetails struct {
ID string `json:"id,omitempty"`
Region string `json:"region,omitempty"`
Type string `json:"type,omitempty"`
AvailabilityZone string `json:"availabilityZone,omitempty"`
MAC string `json:"mac,omitempty"`
PrivateDNSName string `json:"privateDnsName,omitempty"`
ID string `json:"id,omitempty"`
Region string `json:"region,omitempty"`
Type string `json:"type,omitempty"`
AvailabilityZone string `json:"availabilityZone,omitempty"`
MAC string `json:"mac,omitempty"`
PrivateDNSName string `json:"privateDnsName,omitempty"`
NetworkCards []NetworkCardDetails `json:"networkCards,omitempty"`
}

type DefaultOptions struct {
Expand Down
24 changes: 22 additions & 2 deletions nodeadm/internal/api/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading