Skip to content

Commit

Permalink
set firelens mem_buf_limit
Browse files Browse the repository at this point in the history
  • Loading branch information
yinyic committed Oct 23, 2024
1 parent 79f17a5 commit f9662dc
Show file tree
Hide file tree
Showing 8 changed files with 210 additions and 41 deletions.
19 changes: 19 additions & 0 deletions agent/api/container/container.go
Original file line number Diff line number Diff line change
Expand Up @@ -1272,6 +1272,25 @@ func (c *Container) GetNetworkModeFromHostConfig() string {
return hostConfig.NetworkMode.NetworkName()
}

// GetMemoryReservationFromHostConfig returns the container memory reservation
func (c *Container) GetMemoryReservationFromHostConfig() int64 {
c.lock.RLock()
defer c.lock.RUnlock()

if c.DockerConfig.HostConfig == nil {
return 0
}

hostConfig := &dockercontainer.HostConfig{}
err := json.Unmarshal([]byte(*c.DockerConfig.HostConfig), hostConfig)
if err != nil {
seelog.Warnf("Encountered error when trying to get memory reservation for container %s: %v", c.RuntimeID, err)
return 0
}

return hostConfig.MemoryReservation
}

// GetHostConfig returns the container's host config.
func (c *Container) GetHostConfig() *string {
c.lock.RLock()
Expand Down
42 changes: 42 additions & 0 deletions agent/api/container/container_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -751,6 +751,48 @@ func TestGetNetworkModeFromHostConfig(t *testing.T) {
}
}

func TestGetMemoryReservationFromHostConfig(t *testing.T) {
getContainer := func(hostConfig *string) *Container {
c := &Container{
Name: "c",
}
c.DockerConfig.HostConfig = hostConfig
return c
}

getStrPtr := func(s string) *string {
return &s
}

testCases := []struct {
name string
container *Container
expectedOutput int64
}{
{
name: "happy case",
container: getContainer(getStrPtr("{\"MemoryReservation\":50}")),
expectedOutput: 50,
},
{
name: "invalid case",
container: getContainer(getStrPtr("invalid")),
expectedOutput: 0,
},
{
name: "nil case",
container: getContainer(nil),
expectedOutput: 0,
},
}

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
assert.Equal(t, tc.expectedOutput, tc.container.GetMemoryReservationFromHostConfig())
})
}
}

func TestShouldCreateWithEnvfiles(t *testing.T) {
cases := []struct {
in Container
Expand Down
17 changes: 15 additions & 2 deletions agent/api/task/task.go
Original file line number Diff line number Diff line change
Expand Up @@ -1246,7 +1246,7 @@ func (task *Task) initializeFirelensResource(config *config.Config, resourceFiel

for _, container := range task.Containers {
firelensConfig := container.GetFirelensConfig()
if firelensConfig != nil {
if firelensConfig != nil { // is Firelens container
var ec2InstanceID string
if container.Environment != nil && container.Environment[awsExecutionEnvKey] == ec2ExecutionEnv {
ec2InstanceID = resourceFields.EC2InstanceID
Expand All @@ -1260,9 +1260,22 @@ func (task *Task) initializeFirelensResource(config *config.Config, resourceFiel
} else {
networkMode = container.GetNetworkModeFromHostConfig()
}
// Resolve Firelens container memory limit to be used for calculating Firelens memory buffer limit
// We will use the container 'memoryReservation' (soft limit) if present, otherwise the container 'memory' (hard limit)
// If neither is present, we will pass in 0 indicating that a memory limit is not specified, and firelens will
// fall back to use a default memory buffer limit value
// see - agent/taskresource/firelens/firelensconfig_unix.go:resolveMemBufLimit()
var containerMemoryLimit int64
if memoryReservation := container.GetMemoryReservationFromHostConfig(); memoryReservation > 0 {
containerMemoryLimit = memoryReservation
} else if container.Memory > 0 {
containerMemoryLimit = int64(container.Memory)
} else {
containerMemoryLimit = 0
}
firelensResource, err := firelens.NewFirelensResource(config.Cluster, task.Arn, task.Family+":"+task.Version,
ec2InstanceID, config.DataDir, firelensConfig.Type, config.AWSRegion, networkMode, firelensConfig.Options, containerToLogOptions,
credentialsManager, task.ExecutionCredentialsID)
credentialsManager, task.ExecutionCredentialsID, containerMemoryLimit)
if err != nil {
return errors.Wrap(err, "unable to initialize firelens resource")
}
Expand Down
2 changes: 1 addition & 1 deletion agent/taskresource/firelens/firelens_unimplemented.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ type FirelensResource struct{}
// NewFirelensResource returns a new FirelensResource.
func NewFirelensResource(cluster, taskARN, taskDefinition, ec2InstanceID, dataDir, firelensConfigType, region, networkMode string,
firelensOptions map[string]string, containerToLogOptions map[string]map[string]string, credentialsManager credentials.Manager,
executionCredentialsID string) (*FirelensResource, error) {
executionCredentialsID string, containerMemoryLimit int64) (*FirelensResource, error) {
return nil, errors.New("not implemented")
}

Expand Down
4 changes: 3 additions & 1 deletion agent/taskresource/firelens/firelens_unix.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ type FirelensResource struct {
networkMode string
ioutil ioutilwrapper.IOUtil
s3ClientCreator factory.S3ClientCreator
containerMemoryLimit int64

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

fields := strings.Split(taskARN, "/")
Expand Down
46 changes: 24 additions & 22 deletions agent/taskresource/firelens/firelens_unix_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ const (
testExecutionCredentialsID = "testexecutioncredentialsid"
testExternalConfigType = "testexternalconfigtype"
testExternalConfigValue = "testexternalconfigvalue"
testContainerMemoryLimit = 100
)

var (
Expand Down Expand Up @@ -105,7 +106,7 @@ func mockMkdirAllError() func() {

func newMockFirelensResource(firelensConfigType, networkMode string, lopOptions map[string]string,
mockIOUtil *mock_ioutilwrapper.MockIOUtil, mockCredentialsManager *mock_credentials.MockManager,
mockS3ClientCreator *mock_factory.MockS3ClientCreator) *FirelensResource {
mockS3ClientCreator *mock_factory.MockS3ClientCreator, containerMemoryReservation int64) *FirelensResource {
return &FirelensResource{
cluster: testCluster,
taskARN: testTaskARN,
Expand All @@ -122,6 +123,7 @@ func newMockFirelensResource(firelensConfigType, networkMode string, lopOptions
credentialsManager: mockCredentialsManager,
ioutil: mockIOUtil,
s3ClientCreator: mockS3ClientCreator,
containerMemoryLimit: containerMemoryReservation,
}
}

Expand Down Expand Up @@ -158,7 +160,7 @@ func TestCreateFirelensResourceFluentdBridgeMode(t *testing.T) {
defer done()

firelensResource := newMockFirelensResource(FirelensConfigTypeFluentd, bridgeNetworkMode, testFluentdOptions, mockIOUtil,
mockCredentialsManager, mockS3ClientCreator)
mockCredentialsManager, mockS3ClientCreator, testContainerMemoryLimit)

defer mockRename()()
gomock.InOrder(
Expand All @@ -173,7 +175,7 @@ func TestCreateFirelensResourceFluentdAWSVPCMode(t *testing.T) {
defer done()

firelensResource := newMockFirelensResource(FirelensConfigTypeFluentd, awsvpcNetworkMode, testFluentdOptions, mockIOUtil,
mockCredentialsManager, mockS3ClientCreator)
mockCredentialsManager, mockS3ClientCreator, testContainerMemoryLimit)

defer mockRename()()
gomock.InOrder(
Expand All @@ -188,7 +190,7 @@ func TestCreateFirelensResourceFluentdDefaultMode(t *testing.T) {
defer done()

firelensResource := newMockFirelensResource(FirelensConfigTypeFluentd, "", testFluentdOptions, mockIOUtil,
mockCredentialsManager, mockS3ClientCreator)
mockCredentialsManager, mockS3ClientCreator, testContainerMemoryLimit)

defer mockRename()()
gomock.InOrder(
Expand All @@ -203,7 +205,7 @@ func TestCreateFirelensResourceFluentbit(t *testing.T) {
defer done()

firelensResource := newMockFirelensResource(FirelensConfigTypeFluentbit, bridgeNetworkMode, testFluentbitOptions, mockIOUtil,
mockCredentialsManager, mockS3ClientCreator)
mockCredentialsManager, mockS3ClientCreator, testContainerMemoryLimit)

defer mockRename()()
gomock.InOrder(
Expand All @@ -218,7 +220,7 @@ func TestCreateFirelensResourceInvalidType(t *testing.T) {
defer done()

firelensResource := newMockFirelensResource(FirelensConfigTypeFluentd, bridgeNetworkMode, testFluentdOptions, mockIOUtil,
mockCredentialsManager, mockS3ClientCreator)
mockCredentialsManager, mockS3ClientCreator, testContainerMemoryLimit)
firelensResource.firelensConfigType = "invalid"

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

firelensResource := newMockFirelensResource(FirelensConfigTypeFluentd, bridgeNetworkMode, testFluentdOptions, mockIOUtil,
mockCredentialsManager, mockS3ClientCreator)
mockCredentialsManager, mockS3ClientCreator, testContainerMemoryLimit)

defer mockMkdirAllError()()

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

firelensResource := newMockFirelensResource(FirelensConfigTypeFluentd, bridgeNetworkMode, testFluentdOptions, mockIOUtil,
mockCredentialsManager, mockS3ClientCreator)
mockCredentialsManager, mockS3ClientCreator, testContainerMemoryLimit)

defer mockMkdirAllError()()

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

firelensResource := newMockFirelensResource(FirelensConfigTypeFluentd, bridgeNetworkMode, testFluentdOptions, mockIOUtil,
mockCredentialsManager, mockS3ClientCreator)
mockCredentialsManager, mockS3ClientCreator, testContainerMemoryLimit)
firelensResource.containerToLogOptions = map[string]map[string]string{
"container": {
"invalid": "invalid",
Expand All @@ -272,7 +274,7 @@ func TestCreateFirelensResourceCreateTempFileError(t *testing.T) {
defer done()

firelensResource := newMockFirelensResource(FirelensConfigTypeFluentd, bridgeNetworkMode, testFluentdOptions, mockIOUtil,
mockCredentialsManager, mockS3ClientCreator)
mockCredentialsManager, mockS3ClientCreator, testContainerMemoryLimit)

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

firelensResource := newMockFirelensResource(FirelensConfigTypeFluentd, bridgeNetworkMode, testFluentdOptions, mockIOUtil,
mockCredentialsManager, mockS3ClientCreator)
mockCredentialsManager, mockS3ClientCreator, testContainerMemoryLimit)

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

firelensResource := newMockFirelensResource(FirelensConfigTypeFluentd, bridgeNetworkMode, testFluentdOptions, mockIOUtil,
mockCredentialsManager, mockS3ClientCreator)
mockCredentialsManager, mockS3ClientCreator, testContainerMemoryLimit)

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

firelensResource := newMockFirelensResource(FirelensConfigTypeFluentd, bridgeNetworkMode, testFluentdOptions, mockIOUtil,
mockCredentialsManager, mockS3ClientCreator)
mockCredentialsManager, mockS3ClientCreator, testContainerMemoryLimit)

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

firelensResource := newMockFirelensResource(FirelensConfigTypeFluentd, bridgeNetworkMode, testFluentdOptions, mockIOUtil,
mockCredentialsManager, mockS3ClientCreator)
mockCredentialsManager, mockS3ClientCreator, testContainerMemoryLimit)

err := firelensResource.parseOptions(testFirelensOptionsS3)
require.NoError(t, err)
Expand Down Expand Up @@ -385,7 +387,7 @@ func TestCreateFirelensResourceWithS3ConfigMissingCredentials(t *testing.T) {
defer done()

firelensResource := newMockFirelensResource(FirelensConfigTypeFluentd, bridgeNetworkMode, testFluentdOptions, mockIOUtil,
mockCredentialsManager, mockS3ClientCreator)
mockCredentialsManager, mockS3ClientCreator, testContainerMemoryLimit)

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

firelensResource := newMockFirelensResource(FirelensConfigTypeFluentd, bridgeNetworkMode, testFluentdOptions, mockIOUtil,
mockCredentialsManager, mockS3ClientCreator)
mockCredentialsManager, mockS3ClientCreator, testContainerMemoryLimit)

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

firelensResource := newMockFirelensResource(FirelensConfigTypeFluentd, bridgeNetworkMode, testFluentdOptions, mockIOUtil,
mockCredentialsManager, mockS3ClientCreator)
mockCredentialsManager, mockS3ClientCreator, testContainerMemoryLimit)

err := firelensResource.parseOptions(testFirelensOptionsS3)
require.NoError(t, err)
Expand Down Expand Up @@ -450,7 +452,7 @@ func TestCleanupFirelensResource(t *testing.T) {
defer done()

firelensResource := newMockFirelensResource(FirelensConfigTypeFluentd, bridgeNetworkMode, testFluentdOptions, mockIOUtil,
mockCredentialsManager, mockS3ClientCreator)
mockCredentialsManager, mockS3ClientCreator, testContainerMemoryLimit)

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

firelensResource := newMockFirelensResource(FirelensConfigTypeFluentd, bridgeNetworkMode, testFluentdOptions, mockIOUtil,
mockCredentialsManager, mockS3ClientCreator)
mockCredentialsManager, mockS3ClientCreator, testContainerMemoryLimit)

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

firelensResource := newMockFirelensResource(FirelensConfigTypeFluentd, bridgeNetworkMode, testFluentdOptions, nil, nil,
nil)
nil, testContainerMemoryLimit)
firelensResource.Initialize(&taskresource.ResourceFields{
ResourceFieldsCommon: &taskresource.ResourceFieldsCommon{
CredentialsManager: mockCredentialsManager,
Expand All @@ -494,7 +496,7 @@ func TestInitializeFirelensResource(t *testing.T) {

func TestSetKnownStatus(t *testing.T) {
firelensResource := newMockFirelensResource(FirelensConfigTypeFluentd, bridgeNetworkMode, testFluentdOptions, nil, nil,
nil)
nil, testContainerMemoryLimit)
firelensResource.appliedStatusUnsafe = resourcestatus.ResourceStatus(FirelensCreated)

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

func TestSetKnownStatusNoAppliedStatusUpdate(t *testing.T) {
firelensResource := newMockFirelensResource(FirelensConfigTypeFluentd, bridgeNetworkMode, testFluentdOptions, nil, nil,
nil)
nil, testContainerMemoryLimit)
firelensResource.appliedStatusUnsafe = resourcestatus.ResourceStatus(FirelensCreated)

firelensResource.SetKnownStatus(resourcestatus.ResourceStatus(FirelensStatusNone))
Expand Down
Loading

0 comments on commit f9662dc

Please sign in to comment.