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.

Sign up for GitHub

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

Jump to bottom

fix context timeout associated with imds apis #1732

Merged
merged 1 commit into from
Mar 22, 2024
Merged

fix context timeout associated with imds apis #1732

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
8 changes: 4 additions & 4 deletions nodeadm/go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,10 @@ go 1.21
require (
github.com/aws/aws-sdk-go v1.50.6
github.com/aws/aws-sdk-go-v2/config v1.26.6
github.com/aws/aws-sdk-go-v2/feature/ec2/imds v1.14.11
github.com/aws/aws-sdk-go-v2/feature/ec2/imds v1.16.0
github.com/aws/aws-sdk-go-v2/service/ec2 v1.146.0
github.com/aws/aws-sdk-go-v2/service/ecr v1.24.7
github.com/aws/smithy-go v1.19.0
github.com/aws/smithy-go v1.20.1
github.com/containerd/containerd v1.7.13
github.com/coreos/go-systemd/v22 v22.5.0
github.com/integrii/flaggy v1.5.2
Expand All @@ -34,7 +34,7 @@ require (
require dario.cat/mergo v1.0.0 // direct

require (
github.com/aws/aws-sdk-go-v2 v1.24.1
github.com/aws/aws-sdk-go-v2 v1.26.0
github.com/aws/aws-sdk-go-v2/credentials v1.16.16 // indirect
github.com/aws/aws-sdk-go-v2/internal/configsources v1.2.10 // indirect
github.com/aws/aws-sdk-go-v2/internal/endpoints/v2 v2.5.10 // indirect
Expand All @@ -54,7 +54,7 @@ require (
github.com/google/go-cmp v0.6.0 // indirect
github.com/google/gofuzz v1.2.0 // indirect
github.com/inconshreveable/mousetrap v1.1.0 // indirect
github.com/jmespath/go-jmespath v0.4.0
github.com/jmespath/go-jmespath v0.4.0 // indirect
github.com/json-iterator/go v1.1.12 // indirect
github.com/matttproud/golang_protobuf_extensions/v2 v2.0.0 // indirect
github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd // indirect
Expand Down
12 changes: 6 additions & 6 deletions nodeadm/go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,14 @@ github.com/Microsoft/go-winio v0.6.1 h1:9/kr64B9VUZrLm5YYwbGtUJnMgqWVOdUAXu6Migc
github.com/Microsoft/go-winio v0.6.1/go.mod h1:LRdKpFKfdobln8UmuiYcKPot9D2v6svN5+sAH+4kjUM=
github.com/aws/aws-sdk-go v1.50.6 h1:FaXvNwHG3Ri1paUEW16Ahk9zLVqSAdqa1M3phjZR35Q=
github.com/aws/aws-sdk-go v1.50.6/go.mod h1:LF8svs817+Nz+DmiMQKTO3ubZ/6IaTpq3TjupRn3Eqk=
github.com/aws/aws-sdk-go-v2 v1.24.1 h1:xAojnj+ktS95YZlDf0zxWBkbFtymPeDP+rvUQIH3uAU=
github.com/aws/aws-sdk-go-v2 v1.24.1/go.mod h1:LNh45Br1YAkEKaAqvmE1m8FUx6a5b/V0oAKV7of29b4=
github.com/aws/aws-sdk-go-v2 v1.26.0 h1:/Ce4OCiM3EkpW7Y+xUnfAFpchU78K7/Ug01sZni9PgA=
github.com/aws/aws-sdk-go-v2 v1.26.0/go.mod h1:35hUlJVYd+M++iLI3ALmVwMOyRYMmRqUXpTtRGW+K9I=
github.com/aws/aws-sdk-go-v2/config v1.26.6 h1:Z/7w9bUqlRI0FFQpetVuFYEsjzE3h7fpU6HuGmfPL/o=
github.com/aws/aws-sdk-go-v2/config v1.26.6/go.mod h1:uKU6cnDmYCvJ+pxO9S4cWDb2yWWIH5hra+32hVh1MI4=
github.com/aws/aws-sdk-go-v2/credentials v1.16.16 h1:8q6Rliyv0aUFAVtzaldUEcS+T5gbadPbWdV1WcAddK8=
github.com/aws/aws-sdk-go-v2/credentials v1.16.16/go.mod h1:UHVZrdUsv63hPXFo1H7c5fEneoVo9UXiz36QG1GEPi0=
github.com/aws/aws-sdk-go-v2/feature/ec2/imds v1.14.11 h1:c5I5iH+DZcH3xOIMlz3/tCKJDaHFwYEmxvlh2fAcFo8=
github.com/aws/aws-sdk-go-v2/feature/ec2/imds v1.14.11/go.mod h1:cRrYDYAMUohBJUtUnOhydaMHtiK/1NZ0Otc9lIb6O0Y=
github.com/aws/aws-sdk-go-v2/feature/ec2/imds v1.16.0 h1:af5YzcLf80tv4Em4jWVD75lpnOHSBkPUZxZfGkrI3HI=
github.com/aws/aws-sdk-go-v2/feature/ec2/imds v1.16.0/go.mod h1:nQ3how7DMnFMWiU1SpECohgC82fpn4cKZ875NDMmwtA=
github.com/aws/aws-sdk-go-v2/internal/configsources v1.2.10 h1:vF+Zgd9s+H4vOXd5BMaPWykta2a6Ih0AKLq/X6NYKn4=
github.com/aws/aws-sdk-go-v2/internal/configsources v1.2.10/go.mod h1:6BkRjejp/GR4411UGqkX8+wFMbFbqsUIimfK4XjOKR4=
github.com/aws/aws-sdk-go-v2/internal/endpoints/v2 v2.5.10 h1:nYPe006ktcqUji8S2mqXf9c/7NdiKriOwMvWQHgYztw=
Expand All @@ -32,8 +32,8 @@ github.com/aws/aws-sdk-go-v2/service/ssooidc v1.21.7 h1:QPMJf+Jw8E1l7zqhZmMlFw6w
github.com/aws/aws-sdk-go-v2/service/ssooidc v1.21.7/go.mod h1:ykf3COxYI0UJmxcfcxcVuz7b6uADi1FkiUz6Eb7AgM8=
github.com/aws/aws-sdk-go-v2/service/sts v1.26.7 h1:NzO4Vrau795RkUdSHKEwiR01FaGzGOH1EETJ+5QHnm0=
github.com/aws/aws-sdk-go-v2/service/sts v1.26.7/go.mod h1:6h2YuIoxaMSCFf5fi1EgZAwdfkGMgDY+DVfa61uLe4U=
github.com/aws/smithy-go v1.19.0 h1:KWFKQV80DpP3vJrrA9sVAHQ5gc2z8i4EzrLhLlWXcBM=
github.com/aws/smithy-go v1.19.0/go.mod h1:NukqUGpCZIILqqiV0NIjeFh24kd/FAa4beRb6nbIUPE=
github.com/aws/smithy-go v1.20.1 h1:4SZlSlMr36UEqC7XOyRVb27XMeZubNcBNN+9IgEPIQw=
github.com/aws/smithy-go v1.20.1/go.mod h1:krry+ya/rV9RDcV/Q16kpu6ypI4K2czasz0NC3qS14E=
github.com/beorn7/perks v1.0.1 h1:VlbKKnNfV8bJzeqoa4cOKqO6bYr3WgKZxO8Z16+hsOM=
github.com/beorn7/perks v1.0.1/go.mod h1:G2ZrVWU2WbWT9wwq4/hrbKbnv/1ERSJQ0ibhJ6rlkpw=
github.com/blang/semver/v4 v4.0.0 h1:1PFHFE6yCCTv8C1TeyNNarDzntLi7wMI5i/pzqYIsAM=
Expand Down
18 changes: 17 additions & 1 deletion nodeadm/internal/aws/imds/imds.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,22 @@ package imds
import (
"context"
"io"
"time"

"github.com/aws/aws-sdk-go-v2/aws/retry"
"github.com/aws/aws-sdk-go-v2/feature/ec2/imds"
)

var client *imds.Client

func init() {
client = imds.New(imds.Options{})
client = imds.New(imds.Options{
DisableDefaultTimeout: true,
Retryer: retry.NewStandard(func(so *retry.StandardOptions) {
so.MaxAttempts = 15
so.MaxBackoff = 1 * time.Second
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this means 1 second between each attempt, right?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yea, the underlying exponential retryer just behaves linearly for 1 second backoff

}),
})
}

type IMDSProperty string
Expand All @@ -19,6 +27,14 @@ const (
ServicesDomain IMDSProperty = "services/domain"
)

func GetUserData() ([]byte, error) {
resp, err := client.GetUserData(context.TODO(), &imds.GetUserDataInput{})
if err != nil {
return nil, err
}
return io.ReadAll(resp.Content)
}

func GetProperty(prop IMDSProperty) (string, error) {
bytes, err := GetPropertyBytes(prop)
if err != nil {
Expand Down
28 changes: 4 additions & 24 deletions nodeadm/internal/configprovider/userdata.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,20 +2,17 @@ package configprovider

import (
"bytes"
"context"
"fmt"
"io"
"mime"
"mime/multipart"
"net/mail"
"strings"
"time"

"github.com/aws/aws-sdk-go-v2/aws/retry"
"github.com/aws/aws-sdk-go-v2/feature/ec2/imds"
"github.com/awslabs/amazon-eks-ami/nodeadm/api"
internalapi "github.com/awslabs/amazon-eks-ami/nodeadm/internal/api"
apibridge "github.com/awslabs/amazon-eks-ami/nodeadm/internal/api/bridge"
imds "github.com/awslabs/amazon-eks-ami/nodeadm/internal/aws/imds"
)

const (
Expand All @@ -25,23 +22,14 @@ const (
nodeConfigMediaType = "application/" + api.GroupName
)

type userDataConfigProvider struct {
imdsClient *imds.Client
}
type userDataConfigProvider struct{}

func NewUserDataConfigProvider() ConfigProvider {
return &userDataConfigProvider{
imdsClient: imds.New(imds.Options{
Retryer: retry.NewStandard(func(so *retry.StandardOptions) {
so.MaxAttempts = 15
so.MaxBackoff = 1 * time.Second
}),
}),
}
return &userDataConfigProvider{}
}

func (ics *userDataConfigProvider) Provide() (*internalapi.NodeConfig, error) {
userData, err := ics.getUserData()
userData, err := imds.GetUserData()
if err != nil {
return nil, err
}
Expand All @@ -62,14 +50,6 @@ func (ics *userDataConfigProvider) Provide() (*internalapi.NodeConfig, error) {
}
}

func (ics userDataConfigProvider) getUserData() ([]byte, error) {
resp, err := ics.imdsClient.GetUserData(context.TODO(), &imds.GetUserDataInput{})
if err != nil {
return nil, err
}
return io.ReadAll(resp.Content)
}

func getMIMEMultipartReader(data []byte) (*multipart.Reader, error) {
msg, err := mail.ReadMessage(bytes.NewReader(data))
if err != nil {
Expand Down
23 changes: 23 additions & 0 deletions nodeadm/test/e2e/cases/imds-timeouts/run.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
#!/usr/bin/env bash

set -o errexit
set -o nounset
set -o pipefail

source /helpers.sh

wait::dbus-ready
mock::kubelet 1.29.0

# configure without launching the imds mock service
IMDS_MOCK_ONLY_CONFIGURE=true mock::aws

if nodeadm init --skip run; then
echo "bootstrap should not succeed when EC2 IMDS APIs are not reachable."
exit 1
fi

# start the imds mock part way into the initialization to mimic
# delayed availability of IMDS
{ sleep 10 && AWS_MOCK_ONLY_CONFIGURE=true mock::aws; } &
nodeadm init --skip run
7 changes: 3 additions & 4 deletions nodeadm/test/e2e/cases/kubelet-config-new-instance-type/run.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,13 @@ set -o pipefail

source /helpers.sh

mock::instance-type
mock::aws
config_path=/tmp/aemm-default-config.json
cat /etc/aemm-default-config.json | jq '.metadata.values."instance-type" = "mock-type.large" | .dynamic.values."instance-identity-document".instanceType = "mock-type.large"' | tee ${config_path}
mock::aws ${config_path}
mock::kubelet 1.27.0
wait::dbus-ready

for config in config.*; do
nodeadm init --skip run --config-source file://${config}
assert::json-files-equal /etc/kubernetes/kubelet/config.json expected-kubelet-config.json
done

revert::mock::instance-type
13 changes: 2 additions & 11 deletions nodeadm/test/e2e/helpers.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,9 +102,9 @@ function wait::dbus-ready() {

function mock::aws() {
local CONFIG_PATH=${1:-/etc/aemm-default-config.json}
imds-mock --config-file $CONFIG_PATH &
[ "${IMDS_MOCK_ONLY_CONFIGURE:-}" = "true" ] || imds-mock --config-file $CONFIG_PATH &
export AWS_EC2_METADATA_SERVICE_ENDPOINT=http://localhost:1338
$HOME/.local/bin/moto_server -p5000 &
[ "${AWS_MOCK_ONLY_CONFIGURE:-}" = "true" ] || $HOME/.local/bin/moto_server -p5000 &
Comment on lines -105 to +107
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe just split this into 2 helpers, mock::imds and mock::aws? I combined them so that our standard set of mocks didn't get unruly, but it's not that big of a deal

Copy link
Member Author

@ndbaker1 ndbaker1 Mar 20, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i was considering it, and I would probably still use an ONLY_CONFIGURE env so that i can enable the service part way through the run if that's fine.
I could just export the variable myself but i figured its cleaner using the mock as the mechanism

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there's not much pressure since this is a pretty unique test so lets not worry about it for now :)

export AWS_ACCESS_KEY_ID='testing'
export AWS_SECRET_ACCESS_KEY='testing'
export AWS_SECURITY_TOKEN='testing'
Expand All @@ -114,12 +114,3 @@ function mock::aws() {
# ensure that our instance exists in the API
aws ec2 run-instances
}

function mock::instance-type() {
cat /etc/aemm-default-config.json | jq '.metadata.values."instance-type" = "mock-type.large" | .dynamic.values."instance-identity-document".instanceType = "mock-type.large"' | tee /etc/aemm-default-config.json
}

function revert::mock::instance-type() {
cat /etc/aemm-default-config.json | jq '.metadata.values."instance-type" = "m4.xlarge" | .dynamic.values."instance-identity-document".instanceType = "m4.xlarge"' | tee /etc/aemm-default-config.json

}
13 changes: 13 additions & 0 deletions nodeadm/test/e2e/infra/Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,19 @@ RUN pip install --user 'moto[server]'
# I know how this looks, but it lets us use moto with our mocked IMDS and for now the simplicity is worth the hack
RUN sed -i 's/= random_instance_id()/= "i-1234567890abcdef0"/g' $HOME/.local/lib/python*/site-packages/moto/ec2/models/instances.py
COPY --from=imds-mock-build /imds-mock /usr/local/bin/imds-mock
# The content of ec2 userdata in the 'aemm-default-config.json'
# file is the base64 encoding of a minimally viable NodeConfig.
# At the time of this change, it is equal to the following:
#
# ---
# apiVersion: node.eks.aws/v1alpha1
# kind: NodeConfig
# spec:
# cluster:
# name: my-cluster
# apiServerEndpoint: https://example.com
# certificateAuthority: Y2VydGlmaWNhdGVBdXRob3JpdHk=
# cidr: 10.100.0.0/16
COPY test/e2e/infra/aemm-default-config.json /etc/aemm-default-config.json
COPY --from=nodeadm-build /nodeadm /usr/local/bin/nodeadm
COPY test/e2e/infra/systemd/kubelet.service /usr/lib/systemd/system/kubelet.service
Expand Down
5 changes: 5 additions & 0 deletions nodeadm/test/e2e/infra/aemm-default-config.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,5 +20,10 @@
"region": "us-west-2"
}
}
},
"userdata": {
"values": {
"userdata": "LS0tCmFwaVZlcnNpb246IG5vZGUuZWtzLmF3cy92MWFscGhhMQpraW5kOiBOb2RlQ29uZmlnCnNwZWM6CiAgY2x1c3RlcjoKICAgIG5hbWU6IG15LWNsdXN0ZXIKICAgIGFwaVNlcnZlckVuZHBvaW50OiBodHRwczovL2V4YW1wbGUuY29tCiAgICBjZXJ0aWZpY2F0ZUF1dGhvcml0eTogWTJWeWRHbG1hV05oZEdWQmRYUm9iM0pwZEhrPQogICAgY2lkcjogMTAuMTAwLjAuMC8xNgo="
}
}
}
2 changes: 1 addition & 1 deletion nodeadm/vendor/github.com/aws/aws-sdk-go-v2/aws/go_module_metadata.go
View file
Open in desktop

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

10 changes: 5 additions & 5 deletions nodeadm/vendor/github.com/aws/aws-sdk-go-v2/aws/middleware/middleware.go
View file
Open in desktop

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

12 changes: 8 additions & 4 deletions nodeadm/vendor/github.com/aws/aws-sdk-go-v2/aws/middleware/request_id_retriever.go
View file
Open in desktop

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

Loading
Loading