From dca2ff788f6bbd7c1341b312844c74014f1e2a44 Mon Sep 17 00:00:00 2001 From: Connor Catlett Date: Fri, 24 May 2024 16:35:52 +0000 Subject: [PATCH] Greatly clarify misleading metadata logging Signed-off-by: Connor Catlett --- cmd/main.go | 30 +++++++++++++++-------------- pkg/cloud/metadata/ec2.go | 1 - pkg/cloud/metadata/metadata.go | 24 +++++++++++++++-------- pkg/cloud/metadata/metadata_test.go | 2 +- pkg/driver/node.go | 4 ---- 5 files changed, 33 insertions(+), 28 deletions(-) diff --git a/cmd/main.go b/cmd/main.go index 47962f7af4..458fa9ea3a 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -147,24 +147,26 @@ func main() { var md metadata.MetadataService var metadataErr error - if region == "" { - klog.V(5).InfoS("[Debug] Retrieving region from metadata service") - md, metadataErr = metadata.NewMetadataService(cfg, region) - if metadataErr != nil { - klog.ErrorS(metadataErr, "Could not determine region from any metadata service. The region can be manually supplied via the AWS_REGION environment variable.") - panic(metadataErr) + if region != "" { + klog.InfoS("Region provided via AWS_REGION environment variable", "region", region) + if options.Mode != driver.ControllerMode { + klog.InfoS("Node service requires metadata even if AWS_REGION provided, initializing metadata") + md, metadataErr = metadata.NewMetadataService(cfg, region) } - region = md.GetRegion() + } else { + klog.InfoS("No region provided via AWS_REGION environment variable, initializing metadata") + md, metadataErr = metadata.NewMetadataService(cfg, region) + } - if md == nil { - if options.Mode == driver.NodeMode || options.Mode == driver.AllMode { - md, metadataErr = metadata.NewMetadataService(cfg, region) - if metadataErr != nil { - klog.ErrorS(metadataErr, "failed to initialize metadata service") - klog.FlushAndExit(klog.ExitFlushTimeout, 1) - } + if metadataErr != nil { + klog.ErrorS(metadataErr, "Failed to initialize metadata when it is required") + if options.Mode == driver.ControllerMode { + klog.InfoS("The region can be manually supplied via the AWS_REGION environment variable") } + klog.FlushAndExit(klog.ExitFlushTimeout, 1) + } else if region == "" { + region = md.GetRegion() } cloud, err := cloud.NewCloud(region, options.AwsSdkDebugLog, options.UserAgentExtra, options.Batching) diff --git a/pkg/cloud/metadata/ec2.go b/pkg/cloud/metadata/ec2.go index a0a365b9d6..c35c3c4054 100644 --- a/pkg/cloud/metadata/ec2.go +++ b/pkg/cloud/metadata/ec2.go @@ -51,7 +51,6 @@ var DefaultEC2MetadataClient = func() (EC2Metadata, error) { func EC2MetadataInstanceInfo(svc EC2Metadata, regionFromSession string) (*Metadata, error) { docOutput, err := svc.GetInstanceIdentityDocument(context.Background(), &imds.GetInstanceIdentityDocumentInput{}) - klog.InfoS("Retrieving EC2 instance identity metadata", "regionFromSession", regionFromSession) if err != nil { return nil, fmt.Errorf("could not get EC2 instance identity metadata: %w", err) } diff --git a/pkg/cloud/metadata/metadata.go b/pkg/cloud/metadata/metadata.go index 2b81e9e445..1497a9944b 100644 --- a/pkg/cloud/metadata/metadata.go +++ b/pkg/cloud/metadata/metadata.go @@ -45,18 +45,19 @@ var _ MetadataService = &Metadata{} func NewMetadataService(cfg MetadataServiceConfig, region string) (MetadataService, error) { metadata, err := retrieveEC2Metadata(cfg.EC2MetadataClient, region) if err == nil { - klog.InfoS("ec2 metadata is available") - return metadata, nil + klog.InfoS("Retrieved metadata from IMDS") + return metadata.overrideRegion(region), nil } + klog.ErrorS(err, "Retrieving IMDS metadata failed, falling back to Kubernetes metadata") - klog.InfoS("failed to retrieve instance data from ec2 metadata; retrieving instance data from kubernetes api", "err", err) metadata, err = retrieveK8sMetadata(cfg.K8sAPIClient) if err == nil { - klog.InfoS("kubernetes api is available") - return metadata, nil + klog.InfoS("Retrieved metadata from Kubernetes") + return metadata.overrideRegion(region), nil } + klog.ErrorS(err, "Retrieving Kubernetes metadata failed") - return nil, fmt.Errorf("error getting instance data from ec2 metadata or kubernetes api") + return nil, fmt.Errorf("IMDS metadata and Kubernetes metadata are both unavailable") } func retrieveEC2Metadata(ec2MetadataClient EC2MetadataClient, region string) (*Metadata, error) { @@ -67,7 +68,7 @@ func retrieveEC2Metadata(ec2MetadataClient EC2MetadataClient, region string) (*M svc, err := ec2MetadataClient() if err != nil { - klog.ErrorS(err, "Failed to initialize EC2 Metadata client") + klog.ErrorS(err, "failed to initialize EC2 Metadata client") return nil, err } return EC2MetadataInstanceInfo(svc, region) @@ -76,13 +77,20 @@ func retrieveEC2Metadata(ec2MetadataClient EC2MetadataClient, region string) (*M func retrieveK8sMetadata(k8sAPIClient KubernetesAPIClient) (*Metadata, error) { clientset, err := k8sAPIClient() if err != nil { - klog.InfoS("error creating kubernetes api client", "err", err) return nil, err } return KubernetesAPIInstanceInfo(clientset) } +// Override the region on a Metadata object if it is non-empty +func (m *Metadata) overrideRegion(region string) *Metadata { + if region != "" { + m.Region = region + } + return m +} + // GetInstanceID returns the instance identification. func (m *Metadata) GetInstanceID() string { return m.InstanceID diff --git a/pkg/cloud/metadata/metadata_test.go b/pkg/cloud/metadata/metadata_test.go index 31fbe7a013..7fb66ea610 100644 --- a/pkg/cloud/metadata/metadata_test.go +++ b/pkg/cloud/metadata/metadata_test.go @@ -76,7 +76,7 @@ func TestNewMetadataService(t *testing.T) { region: "us-west-2", ec2MetadataError: errors.New("EC2 metadata error"), k8sAPIError: errors.New("K8s API error"), - expectedError: errors.New("error getting instance data from ec2 metadata or kubernetes api"), + expectedError: errors.New("IMDS metadata and Kubernetes metadata are both unavailable"), }, } diff --git a/pkg/driver/node.go b/pkg/driver/node.go index 3ea04dc4ac..a088ce3b41 100644 --- a/pkg/driver/node.go +++ b/pkg/driver/node.go @@ -92,10 +92,6 @@ type NodeService struct { // NewNodeService creates a new node service func NewNodeService(o *Options, md metadata.MetadataService, m mounter.Mounter, k kubernetes.Interface) *NodeService { - klog.V(5).InfoS("[Debug] Retrieving node info from metadata service") - region := os.Getenv("AWS_REGION") - klog.InfoS("regionFromSession Node service", "region", region) - if k != nil { // Remove taint from node to indicate driver startup success // This is done at the last possible moment to prevent race conditions or false positive removals