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.

Sign up for GitHub

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

Evergreen: Update to use stop-orchestration.sh #1621

Merged
merged 2 commits into from
Feb 5, 2025
Merged

Evergreen: Update to use stop-orchestration.sh #1621

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c80888e
Evergreen: Update to use stop-orchestration.sh
rozza Feb 4, 2025
6a3531f
Update .evergreen/.evg.yml
rozza Feb 4, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 1 addition & 9 deletions .evergreen/.evg.yml
View file
Original file line number Diff line number Diff line change
@@ -794,15 +794,7 @@ functions:
shell: bash
script: |
${PREPARE_SHELL}
cd "$MONGO_ORCHESTRATION_HOME"
# source the mongo-orchestration virtualenv if it exists
if [ -f venv/bin/activate ]; then
. venv/bin/activate
elif [ -f venv/Scripts/activate ]; then
. venv/Scripts/activate
fi
mongo-orchestration stop || true
Copy link
Collaborator

Choose a reason for hiding this comment

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

bash: line 20: mongo-orchestration: command not found

From the linked log, it looks like this did not cause a system failure, presumably because of the || true. Would there be a problem if we removed this from this command?

Copy link
Member Author

Choose a reason for hiding this comment

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

yeah that stops the system failure error that would happen without it.

Copy link
Member Author

Choose a reason for hiding this comment

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

To clarify - we could remove the || true part but if the command fails then there is no more clean up. Which I think is the reason it was added in the first place.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes, iirc it's because in some variants we don't start mongo orchestration but we always attempt to stop it.

Copy link
Contributor

Choose a reason for hiding this comment

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

The reason we always stop it is because if we put it in task which starts mongo-orchestration, it might be skipped if the testing failed; to avoid that ugliness, we put it into the clearup post step to ensure it is run always, even for static-analysis task.

Copy link
Contributor

Choose a reason for hiding this comment

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

In mongo-hibernate project, a separate post step is created (rather than as a step in clearup), so no need to use the || true hack.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@rozza I see. If this is corresponds only to "bootstrap mongo-orchestration", then perhaps we could set an env var in that func, and then check it here? I think we can ensure that we rm -rf (which is the last part of cleanup) with the following, though it's a bit long:


if [ -n "$MONGO_ORCHESTRATION_STARTED" ]; then
  MONGO_ORCHESTRATION_OUTPUT=$(mongo-orchestration stop)
  MONGO_ORCHESTRATION_EXIT_CODE=$?
  echo "$MONGO_ORCHESTRATION_OUTPUT"
fi

cd -
rm -rf "$DRIVERS_TOOLS"

if [ $MONGO_ORCHESTRATION_EXIT_CODE -ne 0 ]; then
  exit $MONGO_ORCHESTRATION_EXIT_CODE
fi

(Don't know if I have the cd - in the right place.)

Not sure if this is worth it - up to you. LGTM.

Copy link
Contributor

Choose a reason for hiding this comment

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

there must be some elegant way for sure; but conservativeness usually reigns in devops process.

I created my PR for mongo-hibernate at mongodb/mongo-hibernate#36. I assigned @rozza as one reviewer. Thanks for your reminder.

cd -
bash ${DRIVERS_TOOLS}/.evergreen/stop-orchestration.sh || true
rm -rf $DRIVERS_TOOLS || true

"fix absolute paths":