-
Notifications
You must be signed in to change notification settings - Fork 110
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
Disable component based on logs profile #1141
Disable component based on logs profile #1141
Conversation
pfrcks
commented
Dec 19, 2023
- Does not start fluentd and telegraf in DS if Logs and Events only profile set
- Updates livenessprobe to check for DCR updates and restart container if the logs profile changes
@controllerType = ENV['CONTROLLER_TYPE'] | ||
@containerType = ENV['CONTAINER_TYPE'] | ||
@daemonset = 'daemonset' | ||
@dcrConfigFileLocation = '/etc/mdsd.d/config-cache/configchunks/' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This one seems not used
@os_type = ENV['OS_TYPE'] | ||
@controllerType = ENV['CONTROLLER_TYPE'] | ||
@containerType = ENV['CONTAINER_TYPE'] | ||
@daemonset = 'daemonset' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if only reference once, might not need global var
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
which one in specific are you referring?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
daemonset one
touch /dev/write-to-traces | ||
|
||
if [ "${GENEVA_LOGS_INTEGRATION}" == "true" ] || [ "${GENEVA_LOGS_INTEGRATION_SERVICE_MODE}" == "true" ]; then | ||
checkAgentOnboardingStatus $AAD_MSI_AUTH_MODE 30 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
line 938 and 940 is the same, the if condition can be combined
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this part of code is old i just moved it up.
fluentd -c /etc/fluent/container.conf -o /var/opt/microsoft/docker-cimprov/log/fluentd.log --log-rotate-age 5 --log-rotate-size 20971520 & | ||
if [ "$LOGS_AND_EVENTS_ONLY" != "true" ]; then | ||
echo "*** starting fluentd v1 in daemonset" | ||
fluentd -c /etc/fluent/container.conf -o /var/opt/microsoft/docker-cimprov/log/fluentd.log --log-rotate-age 5 --log-rotate-size 20971520 & |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
although this is in old code, but where is this 20971520 number coming from? Is it adjustable from configmap?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i also dont know how this was picked. this part of code is quite old and its not configurable
* disables fluentd and telegraf in linux ds if logs and events only profile enabled --------- Co-authored-by: Amol Agrawal <[email protected]>
* disables fluentd and telegraf in linux ds if logs or events only streams enabled --------- Co-authored-by: Amol Agrawal <[email protected]> * address PR comments * added telemetry --------- Co-authored-by: Amol Agrawal <[email protected]>