Skip to content

Commit e9c0422

Browse files
tinnywangtinnywang
authored
Merge branch 'dev' into aws-sdk-go-v2/credentials
2 parents 66b0f2b + 4843cd0 commit e9c0422

File tree

8 files changed

+212
-41
lines changed

8 files changed

+212
-41
lines changed

‎agent/api/container/container.go

+21
Original file line numberDiff line numberDiff line change
@@ -1272,6 +1272,27 @@ func (c *Container) GetNetworkModeFromHostConfig() string {
12721272
return hostConfig.NetworkMode.NetworkName()
12731273
}
12741274

1275+
// GetMemoryReservationFromHostConfig returns the container memory reservation
1276+
func (c *Container) GetMemoryReservationFromHostConfig() int64 {
1277+
c.lock.RLock()
1278+
defer c.lock.RUnlock()
1279+
1280+
if c.DockerConfig.HostConfig == nil {
1281+
return 0
1282+
}
1283+
1284+
hostConfig := &dockercontainer.HostConfig{}
1285+
err := json.Unmarshal([]byte(*c.DockerConfig.HostConfig), hostConfig)
1286+
if err != nil {
1287+
seelog.Warnf("Encountered error when trying to get memory reservation for container %s: %v", c.RuntimeID, err)
1288+
return 0
1289+
}
1290+
1291+
// Soft limit is specified in MiB units but translated to bytes while being transferred to Agent
1292+
// Converting back to MiB
1293+
return hostConfig.MemoryReservation / (1024 * 1024)
1294+
}
1295+
12751296
// GetHostConfig returns the container's host config.
12761297
func (c *Container) GetHostConfig() *string {
12771298
c.lock.RLock()

‎agent/api/container/container_test.go

+42
Original file line numberDiff line numberDiff line change
@@ -751,6 +751,48 @@ func TestGetNetworkModeFromHostConfig(t *testing.T) {
751751
}
752752
}
753753

754+
func TestGetMemoryReservationFromHostConfig(t *testing.T) {
755+
getContainer := func(hostConfig *string) *Container {
756+
c := &Container{
757+
Name: "c",
758+
}
759+
c.DockerConfig.HostConfig = hostConfig
760+
return c
761+
}
762+
763+
getStrPtr := func(s string) *string {
764+
return &s
765+
}
766+
767+
testCases := []struct {
768+
name string
769+
container *Container
770+
expectedOutput int64
771+
}{
772+
{
773+
name: "happy case",
774+
container: getContainer(getStrPtr("{\"MemoryReservation\":268435456}")),
775+
expectedOutput: 256,
776+
},
777+
{
778+
name: "invalid case",
779+
container: getContainer(getStrPtr("invalid")),
780+
expectedOutput: 0,
781+
},
782+
{
783+
name: "nil case",
784+
container: getContainer(nil),
785+
expectedOutput: 0,
786+
},
787+
}
788+
789+
for _, tc := range testCases {
790+
t.Run(tc.name, func(t *testing.T) {
791+
assert.Equal(t, tc.expectedOutput, tc.container.GetMemoryReservationFromHostConfig())
792+
})
793+
}
794+
}
795+
754796
func TestShouldCreateWithEnvfiles(t *testing.T) {
755797
cases := []struct {
756798
in Container

‎agent/api/task/task.go

+15-2
Original file line numberDiff line numberDiff line change
@@ -1246,7 +1246,7 @@ func (task *Task) initializeFirelensResource(config *config.Config, resourceFiel
12461246

12471247
for _, container := range task.Containers {
12481248
firelensConfig := container.GetFirelensConfig()
1249-
if firelensConfig != nil {
1249+
if firelensConfig != nil { // is Firelens container
12501250
var ec2InstanceID string
12511251
if container.Environment != nil && container.Environment[awsExecutionEnvKey] == ec2ExecutionEnv {
12521252
ec2InstanceID = resourceFields.EC2InstanceID
@@ -1260,9 +1260,22 @@ func (task *Task) initializeFirelensResource(config *config.Config, resourceFiel
12601260
} else {
12611261
networkMode = container.GetNetworkModeFromHostConfig()
12621262
}
1263+
// Resolve Firelens container memory limit to be used for calculating Firelens memory buffer limit
1264+
// We will use the container 'memoryReservation' (soft limit) if present, otherwise the container 'memory' (hard limit)
1265+
// If neither is present, we will pass in 0 indicating that a memory limit is not specified, and firelens will
1266+
// fall back to use a default memory buffer limit value
1267+
// see - agent/taskresource/firelens/firelensconfig_unix.go:resolveMemBufLimit()
1268+
var containerMemoryLimit int64
1269+
if memoryReservation := container.GetMemoryReservationFromHostConfig(); memoryReservation > 0 {
1270+
containerMemoryLimit = memoryReservation
1271+
} else if container.Memory > 0 {
1272+
containerMemoryLimit = int64(container.Memory)
1273+
} else {
1274+
containerMemoryLimit = 0
1275+
}
12631276
firelensResource, err := firelens.NewFirelensResource(config.Cluster, task.Arn, task.Family+":"+task.Version,
12641277
ec2InstanceID, config.DataDir, firelensConfig.Type, config.AWSRegion, networkMode, firelensConfig.Options, containerToLogOptions,
1265-
credentialsManager, task.ExecutionCredentialsID)
1278+
credentialsManager, task.ExecutionCredentialsID, containerMemoryLimit)
12661279
if err != nil {
12671280
return errors.Wrap(err, "unable to initialize firelens resource")
12681281
}

‎agent/taskresource/firelens/firelens_unimplemented.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ type FirelensResource struct{}
5353
// NewFirelensResource returns a new FirelensResource.
5454
func NewFirelensResource(cluster, taskARN, taskDefinition, ec2InstanceID, dataDir, firelensConfigType, region, networkMode string,
5555
firelensOptions map[string]string, containerToLogOptions map[string]map[string]string, credentialsManager credentials.Manager,
56-
executionCredentialsID string) (*FirelensResource, error) {
56+
executionCredentialsID string, containerMemoryLimit int64) (*FirelensResource, error) {
5757
return nil, errors.New("not implemented")
5858
}
5959

‎agent/taskresource/firelens/firelens_unix.go

+3-1
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,7 @@ type FirelensResource struct {
8383
networkMode string
8484
ioutil ioutilwrapper.IOUtil
8585
s3ClientCreator factory.S3ClientCreator
86+
containerMemoryLimit int64
8687

8788
// Fields for the common functionality of task resource. Access to these fields are protected by lock.
8889
createdAtUnsafe time.Time
@@ -98,7 +99,7 @@ type FirelensResource struct {
9899
// NewFirelensResource returns a new FirelensResource.
99100
func NewFirelensResource(cluster, taskARN, taskDefinition, ec2InstanceID, dataDir, firelensConfigType, region, networkMode string,
100101
firelensOptions map[string]string, containerToLogOptions map[string]map[string]string, credentialsManager credentials.Manager,
101-
executionCredentialsID string) (*FirelensResource, error) {
102+
executionCredentialsID string, containerMemoryLimit int64) (*FirelensResource, error) {
102103
firelensResource := &FirelensResource{
103104
cluster: cluster,
104105
taskARN: taskARN,
@@ -112,6 +113,7 @@ func NewFirelensResource(cluster, taskARN, taskDefinition, ec2InstanceID, dataDi
112113
s3ClientCreator: factory.NewS3ClientCreator(),
113114
executionCredentialsID: executionCredentialsID,
114115
credentialsManager: credentialsManager,
116+
containerMemoryLimit: containerMemoryLimit,
115117
}
116118

117119
fields := strings.Split(taskARN, "/")

‎agent/taskresource/firelens/firelens_unix_test.go

+24-22
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@ const (
5454
testExecutionCredentialsID = "testexecutioncredentialsid"
5555
testExternalConfigType = "testexternalconfigtype"
5656
testExternalConfigValue = "testexternalconfigvalue"
57+
testContainerMemoryLimit = 100
5758
)
5859

5960
var (
@@ -105,7 +106,7 @@ func mockMkdirAllError() func() {
105106

106107
func newMockFirelensResource(firelensConfigType, networkMode string, lopOptions map[string]string,
107108
mockIOUtil *mock_ioutilwrapper.MockIOUtil, mockCredentialsManager *mock_credentials.MockManager,
108-
mockS3ClientCreator *mock_factory.MockS3ClientCreator) *FirelensResource {
109+
mockS3ClientCreator *mock_factory.MockS3ClientCreator, containerMemoryReservation int64) *FirelensResource {
109110
return &FirelensResource{
110111
cluster: testCluster,
111112
taskARN: testTaskARN,
@@ -122,6 +123,7 @@ func newMockFirelensResource(firelensConfigType, networkMode string, lopOptions
122123
credentialsManager: mockCredentialsManager,
123124
ioutil: mockIOUtil,
124125
s3ClientCreator: mockS3ClientCreator,
126+
containerMemoryLimit: containerMemoryReservation,
125127
}
126128
}
127129

@@ -158,7 +160,7 @@ func TestCreateFirelensResourceFluentdBridgeMode(t *testing.T) {
158160
defer done()
159161

160162
firelensResource := newMockFirelensResource(FirelensConfigTypeFluentd, bridgeNetworkMode, testFluentdOptions, mockIOUtil,
161-
mockCredentialsManager, mockS3ClientCreator)
163+
mockCredentialsManager, mockS3ClientCreator, testContainerMemoryLimit)
162164

163165
defer mockRename()()
164166
gomock.InOrder(
@@ -173,7 +175,7 @@ func TestCreateFirelensResourceFluentdAWSVPCMode(t *testing.T) {
173175
defer done()
174176

175177
firelensResource := newMockFirelensResource(FirelensConfigTypeFluentd, awsvpcNetworkMode, testFluentdOptions, mockIOUtil,
176-
mockCredentialsManager, mockS3ClientCreator)
178+
mockCredentialsManager, mockS3ClientCreator, testContainerMemoryLimit)
177179

178180
defer mockRename()()
179181
gomock.InOrder(
@@ -188,7 +190,7 @@ func TestCreateFirelensResourceFluentdDefaultMode(t *testing.T) {
188190
defer done()
189191

190192
firelensResource := newMockFirelensResource(FirelensConfigTypeFluentd, "", testFluentdOptions, mockIOUtil,
191-
mockCredentialsManager, mockS3ClientCreator)
193+
mockCredentialsManager, mockS3ClientCreator, testContainerMemoryLimit)
192194

193195
defer mockRename()()
194196
gomock.InOrder(
@@ -203,7 +205,7 @@ func TestCreateFirelensResourceFluentbit(t *testing.T) {
203205
defer done()
204206

205207
firelensResource := newMockFirelensResource(FirelensConfigTypeFluentbit, bridgeNetworkMode, testFluentbitOptions, mockIOUtil,
206-
mockCredentialsManager, mockS3ClientCreator)
208+
mockCredentialsManager, mockS3ClientCreator, testContainerMemoryLimit)
207209

208210
defer mockRename()()
209211
gomock.InOrder(
@@ -218,7 +220,7 @@ func TestCreateFirelensResourceInvalidType(t *testing.T) {
218220
defer done()
219221

220222
firelensResource := newMockFirelensResource(FirelensConfigTypeFluentd, bridgeNetworkMode, testFluentdOptions, mockIOUtil,
221-
mockCredentialsManager, mockS3ClientCreator)
223+
mockCredentialsManager, mockS3ClientCreator, testContainerMemoryLimit)
222224
firelensResource.firelensConfigType = "invalid"
223225

224226
assert.Error(t, firelensResource.Create())
@@ -230,7 +232,7 @@ func TestCreateFirelensResourceCreateConfigDirError(t *testing.T) {
230232
defer done()
231233

232234
firelensResource := newMockFirelensResource(FirelensConfigTypeFluentd, bridgeNetworkMode, testFluentdOptions, mockIOUtil,
233-
mockCredentialsManager, mockS3ClientCreator)
235+
mockCredentialsManager, mockS3ClientCreator, testContainerMemoryLimit)
234236

235237
defer mockMkdirAllError()()
236238

@@ -243,7 +245,7 @@ func TestCreateFirelensResourceCreateSocketDirError(t *testing.T) {
243245
defer done()
244246

245247
firelensResource := newMockFirelensResource(FirelensConfigTypeFluentd, bridgeNetworkMode, testFluentdOptions, mockIOUtil,
246-
mockCredentialsManager, mockS3ClientCreator)
248+
mockCredentialsManager, mockS3ClientCreator, testContainerMemoryLimit)
247249

248250
defer mockMkdirAllError()()
249251

@@ -256,7 +258,7 @@ func TestCreateFirelensResourceGenerateConfigError(t *testing.T) {
256258
defer done()
257259

258260
firelensResource := newMockFirelensResource(FirelensConfigTypeFluentd, bridgeNetworkMode, testFluentdOptions, mockIOUtil,
259-
mockCredentialsManager, mockS3ClientCreator)
261+
mockCredentialsManager, mockS3ClientCreator, testContainerMemoryLimit)
260262
firelensResource.containerToLogOptions = map[string]map[string]string{
261263
"container": {
262264
"invalid": "invalid",
@@ -272,7 +274,7 @@ func TestCreateFirelensResourceCreateTempFileError(t *testing.T) {
272274
defer done()
273275

274276
firelensResource := newMockFirelensResource(FirelensConfigTypeFluentd, bridgeNetworkMode, testFluentdOptions, mockIOUtil,
275-
mockCredentialsManager, mockS3ClientCreator)
277+
mockCredentialsManager, mockS3ClientCreator, testContainerMemoryLimit)
276278

277279
gomock.InOrder(
278280
mockIOUtil.EXPECT().TempFile(testResourceDir, tempFile).Return(nil, errors.New("test error")),
@@ -287,7 +289,7 @@ func TestCreateFirelensResourceWriteConfigFileError(t *testing.T) {
287289
defer done()
288290

289291
firelensResource := newMockFirelensResource(FirelensConfigTypeFluentd, bridgeNetworkMode, testFluentdOptions, mockIOUtil,
290-
mockCredentialsManager, mockS3ClientCreator)
292+
mockCredentialsManager, mockS3ClientCreator, testContainerMemoryLimit)
291293

292294
mockFile.(*mock_oswrapper.MockFile).WriteImpl = func(bytes []byte) (i int, e error) {
293295
return 0, errors.New("test error")
@@ -306,7 +308,7 @@ func TestCreateFirelensResourceChmodError(t *testing.T) {
306308
defer done()
307309

308310
firelensResource := newMockFirelensResource(FirelensConfigTypeFluentd, bridgeNetworkMode, testFluentdOptions, mockIOUtil,
309-
mockCredentialsManager, mockS3ClientCreator)
311+
mockCredentialsManager, mockS3ClientCreator, testContainerMemoryLimit)
310312

311313
mockFile.(*mock_oswrapper.MockFile).ChmodImpl = func(mode os.FileMode) error {
312314
return errors.New("test error")
@@ -325,7 +327,7 @@ func TestCreateFirelensResourceRenameError(t *testing.T) {
325327
defer done()
326328

327329
firelensResource := newMockFirelensResource(FirelensConfigTypeFluentd, bridgeNetworkMode, testFluentdOptions, mockIOUtil,
328-
mockCredentialsManager, mockS3ClientCreator)
330+
mockCredentialsManager, mockS3ClientCreator, testContainerMemoryLimit)
329331

330332
gomock.InOrder(
331333
mockIOUtil.EXPECT().TempFile(testResourceDir, tempFile).Return(mockFile, nil),
@@ -347,7 +349,7 @@ func TestCreateFirelensResourceWithS3Config(t *testing.T) {
347349
defer done()
348350

349351
firelensResource := newMockFirelensResource(FirelensConfigTypeFluentd, bridgeNetworkMode, testFluentdOptions, mockIOUtil,
350-
mockCredentialsManager, mockS3ClientCreator)
352+
mockCredentialsManager, mockS3ClientCreator, testContainerMemoryLimit)
351353

352354
err := firelensResource.parseOptions(testFirelensOptionsS3)
353355
require.NoError(t, err)
@@ -385,7 +387,7 @@ func TestCreateFirelensResourceWithS3ConfigMissingCredentials(t *testing.T) {
385387
defer done()
386388

387389
firelensResource := newMockFirelensResource(FirelensConfigTypeFluentd, bridgeNetworkMode, testFluentdOptions, mockIOUtil,
388-
mockCredentialsManager, mockS3ClientCreator)
390+
mockCredentialsManager, mockS3ClientCreator, testContainerMemoryLimit)
389391

390392
err := firelensResource.parseOptions(testFirelensOptionsS3)
391393
require.NoError(t, err)
@@ -403,7 +405,7 @@ func TestCreateFirelensResourceWithS3ConfigInvalidS3ARN(t *testing.T) {
403405
defer done()
404406

405407
firelensResource := newMockFirelensResource(FirelensConfigTypeFluentd, bridgeNetworkMode, testFluentdOptions, mockIOUtil,
406-
mockCredentialsManager, mockS3ClientCreator)
408+
mockCredentialsManager, mockS3ClientCreator, testContainerMemoryLimit)
407409

408410
err := firelensResource.parseOptions(testFirelensOptionsS3)
409411
require.NoError(t, err)
@@ -422,7 +424,7 @@ func TestCreateFirelensResourceWithS3ConfigDownloadFailure(t *testing.T) {
422424
defer done()
423425

424426
firelensResource := newMockFirelensResource(FirelensConfigTypeFluentd, bridgeNetworkMode, testFluentdOptions, mockIOUtil,
425-
mockCredentialsManager, mockS3ClientCreator)
427+
mockCredentialsManager, mockS3ClientCreator, testContainerMemoryLimit)
426428

427429
err := firelensResource.parseOptions(testFirelensOptionsS3)
428430
require.NoError(t, err)
@@ -450,7 +452,7 @@ func TestCleanupFirelensResource(t *testing.T) {
450452
defer done()
451453

452454
firelensResource := newMockFirelensResource(FirelensConfigTypeFluentd, bridgeNetworkMode, testFluentdOptions, mockIOUtil,
453-
mockCredentialsManager, mockS3ClientCreator)
455+
mockCredentialsManager, mockS3ClientCreator, testContainerMemoryLimit)
454456

455457
assert.NoError(t, firelensResource.Cleanup())
456458
}
@@ -460,7 +462,7 @@ func TestCleanupFirelensResourceError(t *testing.T) {
460462
defer done()
461463

462464
firelensResource := newMockFirelensResource(FirelensConfigTypeFluentd, bridgeNetworkMode, testFluentdOptions, mockIOUtil,
463-
mockCredentialsManager, mockS3ClientCreator)
465+
mockCredentialsManager, mockS3ClientCreator, testContainerMemoryLimit)
464466

465467
removeAll = func(path string) error {
466468
return errors.New("test error")
@@ -478,7 +480,7 @@ func TestInitializeFirelensResource(t *testing.T) {
478480
defer done()
479481

480482
firelensResource := newMockFirelensResource(FirelensConfigTypeFluentd, bridgeNetworkMode, testFluentdOptions, nil, nil,
481-
nil)
483+
nil, testContainerMemoryLimit)
482484
firelensResource.Initialize(&taskresource.ResourceFields{
483485
ResourceFieldsCommon: &taskresource.ResourceFieldsCommon{
484486
CredentialsManager: mockCredentialsManager,
@@ -494,7 +496,7 @@ func TestInitializeFirelensResource(t *testing.T) {
494496

495497
func TestSetKnownStatus(t *testing.T) {
496498
firelensResource := newMockFirelensResource(FirelensConfigTypeFluentd, bridgeNetworkMode, testFluentdOptions, nil, nil,
497-
nil)
499+
nil, testContainerMemoryLimit)
498500
firelensResource.appliedStatusUnsafe = resourcestatus.ResourceStatus(FirelensCreated)
499501

500502
firelensResource.SetKnownStatus(resourcestatus.ResourceStatus(FirelensCreated))
@@ -504,7 +506,7 @@ func TestSetKnownStatus(t *testing.T) {
504506

505507
func TestSetKnownStatusNoAppliedStatusUpdate(t *testing.T) {
506508
firelensResource := newMockFirelensResource(FirelensConfigTypeFluentd, bridgeNetworkMode, testFluentdOptions, nil, nil,
507-
nil)
509+
nil, testContainerMemoryLimit)
508510
firelensResource.appliedStatusUnsafe = resourcestatus.ResourceStatus(FirelensCreated)
509511

510512
firelensResource.SetKnownStatus(resourcestatus.ResourceStatus(FirelensStatusNone))

0 commit comments

Comments
 (0)