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

Wait for agent startup before updating CAPI tags #145

Merged
merged 5 commits into from
Dec 8, 2022

Conversation

sarah-witt
Copy link
Contributor

What does this PR do?

This PR fixes the conditions when waiting for the agent to start up before running the update script to inject DCA tags. Now the update_agent_config_restart script will not run until either the agent or dogstatsd process is running. This PR also adds more logging.

Description of the Change

Alternate Designs

Possible Drawbacks

Verification Process

Additional Notes

Release Notes

Review checklist (to be filled by reviewers)

  • PR title must be written as a CHANGELOG entry (see why)
  • Files changes must correspond to the primary purpose of the PR as described in the title (small unrelated changes should have their own PR)
  • PR must have one changelog/ label attached. If applicable it should have the backward-incompatible label attached.
  • PR should not have do-not-merge/ label attached.
  • If Applicable, issue must have kind/ and severity/ labels attached at least.

@sarah-witt sarah-witt marked this pull request as ready for review December 5, 2022 17:59
@sarah-witt sarah-witt requested a review from a team as a code owner December 5, 2022 17:59
@@ -1,13 +1,30 @@
#!/bin/sh

# wait for the buildpack scripts to finish
timeout=0
while { ! [ "$(pgrep -f ./agent)" = "" ] && ! [ "$(pgrep -f ./dogstatsd)" = "" ]; } && [ $timeout -lt 120 ]; do

Choose a reason for hiding this comment

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

Since these processes are started using a pid file and it's logically possible for the associated buildpack applications to have a binary with the same name, does use of -F make more sense with pgrep?

Copy link
Member

Choose a reason for hiding this comment

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

Yes, it does make more sense. However, when this script is invoked by the Garden API, the underlying process doesn't have the DATADOG_DIR ENV variable, which we would need to find the pid file (e.g.: ${DATADOG_DIR}/run/agent.pid) . @sarah-witt was using this approach as a temporary workaround. We didn't want to assume that DATADOG_DIR=/home/vcap/app/.datadog without careful consideration, as we noticed in some parts in the scripts DATADOG_DIR could be a user input.
We're going to make a separate PR to address this issue and then we'll resume this PR.

@NouemanKHAL NouemanKHAL force-pushed the sarah/wait-agent-startup branch from aa1f50d to 6f75187 Compare December 7, 2022 16:31
@NouemanKHAL NouemanKHAL added the changelog/Fixed Fixed features results into a bug fix version bump label Dec 8, 2022
@sarah-witt sarah-witt changed the title Properly wait for agent startup before updating DCA tags Wait for agent startup before updating CAPI tags Dec 8, 2022
@sarah-witt sarah-witt merged commit 9c20fd5 into master Dec 8, 2022
@sarah-witt sarah-witt deleted the sarah/wait-agent-startup branch December 8, 2022 15:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog/Fixed Fixed features results into a bug fix version bump
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants