Fix container collect all assignment #811
Conversation
| @@ -129,7 +129,7 @@ func NewHelmInstallation(e config.CommonEnvironment, args HelmInstallationArgs, | |||
| linuxInstallName += "-linux" | |||
| } | |||
|
|
|||
| values := buildLinuxHelmValues(installName, agentImagePath, agentImageTag, clusterAgentImagePath, clusterAgentImageTag, randomClusterAgentToken.Result, !args.DisableLogsContainerCollectAll) | |||
| values := buildLinuxHelmValues(installName, agentImagePath, agentImageTag, clusterAgentImagePath, clusterAgentImageTag, randomClusterAgentToken.Result, args.DisableLogsContainerCollectAll) | |||
There was a problem hiding this comment.
Are you sure this is what we want? The buildLinuxHelmValues has a parameter named logsContainerCollectAll that will enable collection of all container logs when enabled. If you change that when DisableLogsContainer is true we will enable logs collection.
There was a problem hiding this comment.
I thought so too, but after experimenting with the initial configuration, I found that the previous method set container_collect_all to true in the agent when WithoutLogsContainerCollectAll was applied, whereas with this configuration container_collect_all is correctly set to off.
There was a problem hiding this comment.
I think it was because I missed this: #814
The disableContainerCollectAll was never passed so no matter if you used WithoutLogsContainerCollectAll the default value was passed to buildLinuxHelmValues (ie !false) and I think that's why it was always enabled
There was a problem hiding this comment.
Ohh okay gotcha, thanks for the clarification.
What does this PR do?
Fixes disabling
container_collect_allin the agent running in Kubernetes.Which scenarios this will impact?
E2E tests in Kubernetes where the agent is orchestrated with
WithoutLogsContainerCollectAll().Motivation
Needed to be fixed for my E2E test to work.
Additional Notes