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

Make metadata logging best-effort #1028

Merged
merged 2 commits into from
Sep 27, 2024
Merged

Make metadata logging best-effort #1028

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
dbdac92
Make metadata logging best-effort
sjenning Sep 24, 2024
90dbeb6
update golang from 1.22.5 to 1.22.7
sjenning Sep 25, 2024
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
2 changes: 1 addition & 1 deletion .ko.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
defaultBaseImage: registry.k8s.io/build-image/go-runner:v2.3.1-go1.22.5-bookworm.0
defaultBaseImage: registry.k8s.io/build-image/go-runner:v2.3.1-go1.22.7-bookworm.0
4 changes: 2 additions & 2 deletions Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,15 +14,15 @@
## BUILD ARGS ##
################################################################################
# This build arg allows the specification of a custom Golang image.
ARG GOLANG_IMAGE=golang:1.22.5
ARG GOLANG_IMAGE=golang:1.22.7

# The distroless image on which the CPI manager image is built.
#
# Please do not use "latest". Explicit tags should be used to provide
# deterministic builds. Follow what kubernetes uses to build
# kube-controller-manager, for example for 1.23.x:
# https://github.com/kubernetes/kubernetes/blob/release-1.24/build/common.sh#L94
ARG DISTROLESS_IMAGE=registry.k8s.io/build-image/go-runner:v2.3.1-go1.22.5-bookworm.0
ARG DISTROLESS_IMAGE=registry.k8s.io/build-image/go-runner:v2.3.1-go1.22.7-bookworm.0

################################################################################
## BUILD STAGE ##
Expand Down
2 changes: 1 addition & 1 deletion cloudbuild.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ steps:
- --platform=linux/amd64,linux/arm64
- .
# Build cloudbuild artifacts (for attestation)
- name: 'docker.io/library/golang:1.22.5-bookworm'
- name: 'docker.io/library/golang:1.22.7-bookworm'
id: cloudbuild-artifacts
entrypoint: make
env:
Expand Down
2 changes: 1 addition & 1 deletion go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
module k8s.io/cloud-provider-aws

go 1.22.5
go 1.22.7

require (
github.com/aws/aws-sdk-go v1.55.5
Expand Down
23 changes: 11 additions & 12 deletions pkg/providers/v1/aws_sdk.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -211,18 +211,17 @@ func (p *awsSDKProvider) Metadata() (config.EC2Metadata, error) {
p.addAPILoggingHandlers(&client.Handlers)

identity, err := client.GetInstanceIdentityDocument()
if err != nil {
return nil, fmt.Errorf("unable to get instance identity document: %v", err)
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you add a log line for this error?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the review!

The line would be logged for every invocation of the function and it would basically be a log line that says "I wanted to log something useful but the call failed so I'm logging the fact that I couldn't log something useful".

How often is this function called?

Copy link
Contributor

Choose a reason for hiding this comment

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

Calls to IMDS can fail for reasons other than AWS_EC2_METADATA_DISABLED, so I think its worth logging. It's only called once in an init():

cloud-provider-aws/pkg/providers/v1/aws.go

Line 473 in d7e05d5

metadata, err := newAWSSDKProvider(nil, cfg).Metadata()

}
klog.InfoS("instance metadata identity",
"region", identity.Region,
"availability-zone", identity.AvailabilityZone,
"instance-type", identity.InstanceType,
"architecture", identity.Architecture,
"instance-id", identity.InstanceID,
"private-ip", identity.PrivateIP,
"account-id", identity.AccountID,
"image-id", identity.ImageID)
if err == nil {
klog.InfoS("instance metadata identity",
"region", identity.Region,
"availability-zone", identity.AvailabilityZone,
"instance-type", identity.InstanceType,
"architecture", identity.Architecture,
"instance-id", identity.InstanceID,
"private-ip", identity.PrivateIP,
"account-id", identity.AccountID,
"image-id", identity.ImageID)
}
return client, nil
}

Expand Down
2 changes: 1 addition & 1 deletion tests/e2e/go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
module k8s.io/cloud-provider-aws/tests/e2e

go 1.22.5
go 1.22.7

require (
github.com/onsi/ginkgo/v2 v2.9.4
Expand Down