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

Add scrapping ECS_CONTAINER_METADATA_URI_V4 for ECS #453

Merged
merged 29 commits into from
May 3, 2022
Merged
Changes from 14 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
41bcb1a
Add metadata point v4 and delete run in aws
khanhntd Apr 20, 2022
771ab69
Add log and delete flag
khanhntd Apr 22, 2022
4a3d227
Add some interface and add final endpoint v4
khanhntd Apr 22, 2022
dc97cff
Fix some typo and add interface
khanhntd Apr 22, 2022
13ca933
Fix typo
khanhntd Apr 22, 2022
bb5ec63
Delete apply interface
khanhntd Apr 22, 2022
39d79d1
Delete interface and debug
khanhntd Apr 22, 2022
8302a6e
Delete Ecs Util instance variable
khanhntd Apr 22, 2022
a080fcd
Add back ecsUtilInstance
khanhntd Apr 22, 2022
16d8af8
Add type assertion
khanhntd Apr 22, 2022
8713b5b
Delete type assertion
khanhntd Apr 22, 2022
d750aed
Add type assertion again
khanhntd Apr 22, 2022
5644636
Delete type assertion
khanhntd Apr 22, 2022
d77b840
Fix typo and nitpick
khanhntd Apr 22, 2022
50c4774
Add back mode EC2
khanhntd Apr 23, 2022
e28fef6
Add runinAws
khanhntd Apr 23, 2022
a483a6f
Add type assertion
khanhntd Apr 23, 2022
90ab672
Add type assertion
khanhntd Apr 23, 2022
e013ce1
delete type interface
khanhntd Apr 23, 2022
a5737cc
Add log for validating
khanhntd Apr 29, 2022
84f36d8
change to parse ecs region
khanhntd Apr 29, 2022
5d6f694
Build back image
khanhntd Apr 29, 2022
866f1b5
Revert back to original
khanhntd Apr 29, 2022
38845aa
Add back strings
khanhntd Apr 29, 2022
1bbd660
Add parseClusterName
khanhntd Apr 29, 2022
c3bfe4e
Parse the cluster
khanhntd Apr 29, 2022
b328b91
Delete parse cluster
khanhntd May 2, 2022
30098af
Delete go proxy
khanhntd May 2, 2022
9d414c9
Fix testing
khanhntd May 3, 2022
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: 0 additions & 2 deletions translator/config/envconst.go
Original file line number Diff line number Diff line change
@@ -6,8 +6,6 @@ package config
const (
RUN_IN_CONTAINER = "RUN_IN_CONTAINER"
RUN_IN_CONTAINER_TRUE = "True"
RUN_IN_AWS = "RUN_IN_AWS"
RUN_IN_AWS_TRUE = "True"
USE_DEFAULT_CONFIG = "USE_DEFAULT_CONFIG"
USE_DEFAULT_CONFIG_TRUE = "True"
HOST_NAME = "HOST_NAME"
18 changes: 12 additions & 6 deletions translator/util/ecsutil/ecsutil.go
Original file line number Diff line number Diff line change
@@ -17,6 +17,7 @@ import (
const (
v2MetadataEndpoint = "http://169.254.170.2/v2/metadata"
v3MetadataEndpointEnv = "ECS_CONTAINER_METADATA_URI"
v4MetadataEndpointEnv = "ECS_CONTAINER_METADATA_URI_V4"
)

type ecsMetadataResponse struct {
@@ -32,6 +33,7 @@ type ecsUtil struct {
}

var ecsUtilInstance *ecsUtil

var ecsUtilOnce sync.Once

func GetECSUtilSingleton() *ecsUtil {
@@ -48,6 +50,7 @@ func initECSUtilSingleton() (newInstance *ecsUtil) {
}
log.Println("I! attempt to access ECS task metadata to determine whether I'm running in ECS.")
ecsMetadataResponse, err := newInstance.getECSMetadata()

if err != nil {
log.Printf("I! access ECS task metadata fail with response %v, assuming I'm not running in ECS.\n", err)
return
@@ -65,26 +68,29 @@ func (e *ecsUtil) IsECS() bool {
}

func (e *ecsUtil) getECSMetadata() (em *ecsMetadataResponse, err error) {
// choose available endpoint
if v3MetadataEndpoint, ok := os.LookupEnv(v3MetadataEndpointEnv); !ok {
em, err = e.getMetadataResponse(v2MetadataEndpoint)
} else {
// Based on endpoint to get ECS metadata, for more information on the respond, https://docs.aws.amazon.com/AmazonECS/latest/developerguide/task-metadata-endpoint.html
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we still need to support v2 and v3? If endpoint is there but get metadata fails should we fall back?

Copy link
Contributor

Choose a reason for hiding this comment

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

IDC either way.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we still need to support v2 and v3 so long as people can create ECS clusters that still rely on them. I think we'd have to investigate further where the drop off points are for v2 and v3 before we could take out support for them.

if v4MetadataEndpoint, ok := os.LookupEnv(v4MetadataEndpointEnv); ok {
em, err = e.getMetadataResponse(v4MetadataEndpoint + "/task")
khanhntd marked this conversation as resolved.
Show resolved Hide resolved
} else if v3MetadataEndpoint, ok := os.LookupEnv(v3MetadataEndpointEnv); ok {
em, err = e.getMetadataResponse(v3MetadataEndpoint + "/task")
} else {
em, err = e.getMetadataResponse(v2MetadataEndpoint)
}
return
}

func (e *ecsUtil) getMetadataResponse(endpoint string) (em *ecsMetadataResponse, err error) {
em = &ecsMetadataResponse{}
resp, err := e.httpClient.Request(endpoint)

if err != nil {
return
}

err = json.Unmarshal(resp, em)
if err != nil {
log.Printf("E! unable to parse resp from ecsmetadata endpoint, error: %v", err)
log.Printf("D! resp content is %s", string(resp))
log.Printf("E! Unable to parse response from ecsmetadata endpoint, error: %v", err)
log.Printf("D! Content is %s", string(resp))
}
return
}
22 changes: 7 additions & 15 deletions translator/util/sdkutil.go
Original file line number Diff line number Diff line change
@@ -25,18 +25,12 @@ const (

var DetectRegion func(mode string, credsConfig map[string]string) string = detectRegion
var DetectCredentialsPath func() string = detectCredentialsPath
var runInAws = os.Getenv(config.RUN_IN_AWS)

func DetectAgentMode(configuredMode string) string {
if configuredMode != "auto" {
return configuredMode
}

if runInAws == config.RUN_IN_AWS_TRUE {
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 explain why you are removing this? From what I recall, this was a pretty impactful bug for running ECS where the agent couldn't determine that it's running on an EC2 but should have.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes I believe the issue was the agent was not able to scrap metadataendpoint on awsvpc network which happens on ECS using Task Definition > v1.39 because the issue happened around when the task definition new version got release. However, after adding the endpointECS_CONTAINER_METADATA_URI_V4, it would solve the issue. For upcoming release, I will talk with @sethAmazon to make an integration test based on different task definition and will help us in recognize the changes instead of relying on the flag. However, I could keeping the flag until the integration test got done to avoid the impact if that make sense.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah I would keep this until a integration tests is built for this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

On second though, should not delete this since EKS Fargate does not know how to detect onPrem or not so would need to keep back this flag.

fmt.Println("I! Detected from ENV instance is EC2")
return config.ModeEC2
}


if defaultEC2Region() != "" {
fmt.Println("I! Detected the instance is EC2")
return config.ModeEC2
@@ -93,15 +87,13 @@ func detectRegion(mode string, credsConfig map[string]string) (region string) {

// For ec2, fallback to metadata when no region info found in credential profile.
if region == "" && mode == config.ModeEC2 {
region = defaultEC2Region()
}

// try to get region from ecs metadata
if region == "" && mode == config.ModeEC2 {
fmt.Println("I! detect region from ecs")
region = defaultECSRegion()
fmt.Println("I! Detect region from EC2/ECS")
if region = defaultEC2Region(); region == "" {
// Try to get region from ecs metadata
region = defaultECSRegion()
}
}
SaxyPandaBear marked this conversation as resolved.
Show resolved Hide resolved

return
}