Skip to content

Conversation

feng-j678
Copy link
Contributor

@feng-j678 feng-j678 commented Apr 22, 2025

[x] current condition is if sudo_status is true and attempts is 1, this will not get logged it's misleading and create confusion that success attempt happens on 2nd attempts and so on
[x] address sudo check is success start tracking successful attempt at 1
- if sudo_status and attempts >= 1
- not altering for attempts in range(1, Constants.MAX_CHECK_SUDO_ATTEMPTS + 1) to for attempts in range(0, Constants.MAX_CHECK_SUDO_ATTEMPTS) because for loop is exclusive. current code is start at 1 ensure we reflect success attempt at 1 then max attempt at 6 instead of success attempt at 0 then max attempt at 5.
[x] reword retry to attempt
[x] add [BST] (boostrapper) prefix to logs indicate the log msg is from boostrapper class
[x] add [BST] (boostrapper) prefix to logs indicate the log msg is from boostrapper class
[x] add ut to handle build_out_container raise exception
prior code change:
image
image

post code change:
image
image
image
image

Copy link

codecov bot commented Apr 22, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 93.77%. Comparing base (cfdca4c) to head (be0add0).
Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #315      +/-   ##
==========================================
+ Coverage   93.73%   93.77%   +0.04%     
==========================================
  Files         103      103              
  Lines       17881    17923      +42     
==========================================
+ Hits        16760    16808      +48     
+ Misses       1121     1115       -6     
Flag Coverage Δ
python27 93.77% <100.00%> (+0.04%) ⬆️
python312 93.77% <100.00%> (+0.04%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@feng-j678 feng-j678 marked this pull request as ready for review April 23, 2025 06:19
@Copilot Copilot AI review requested due to automatic review settings April 23, 2025 06:19
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes the misleading sudo check logging and updates the logic to mark a successful attempt at 1 rather than after a retry. Key changes include:

  • Renaming and adjusting logic from check_sudo_status_with_retry to check_sudo_status_with_attempts.
  • Updating log messages by adding a [BST] prefix and reflecting attempt numbers correctly.
  • Adding new unit tests to handle exceptions during build container and auto-assessment log file reset.

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
src/core/tests/Test_Bootstrapper.py Updated test cases to use the new attempts-based sudo check logic.
src/core/src/bootstrap/Constants.py Renamed constant from MAX_CHECK_SUDO_RETRY_COUNT to MAX_CHECK_SUDO_ATTEMPTS.
src/core/src/bootstrap/Bootstrapper.py Updated log messages and function names to reflect the correct attempts logic.

@feng-j678 feng-j678 changed the title Bugfix/sudo check logs Bugfix: Fix sudo check logs and wordings Apr 23, 2025
@kjohn-msft kjohn-msft added the bug Something isn't working label Apr 23, 2025
@kjohn-msft kjohn-msft added the engg. hygiene Engineering hygiene related label Apr 28, 2025
Copy link
Collaborator

@kjohn-msft kjohn-msft left a comment

Choose a reason for hiding this comment

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

Inline comment

Copy link
Collaborator

@kjohn-msft kjohn-msft left a comment

Choose a reason for hiding this comment

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

Comments inline.

kjohn-msft
kjohn-msft previously approved these changes May 1, 2025
@kjohn-msft kjohn-msft added the OE PR is considered near complete due to OE sign-off. label May 1, 2025
Copy link
Contributor

@rane-rajasi rane-rajasi left a comment

Choose a reason for hiding this comment

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

Very minor comments. Overall okay to merge

…into Bugfix/sudo_check_logs

pull master changes
@feng-j678 feng-j678 force-pushed the Bugfix/sudo_check_logs branch from 8c9f3ad to be0add0 Compare May 7, 2025 16:07
@kjohn-msft kjohn-msft merged commit f86301d into master May 8, 2025
8 checks passed
@kjohn-msft kjohn-msft deleted the Bugfix/sudo_check_logs branch May 8, 2025 23:02
@feng-j678 feng-j678 mentioned this pull request May 9, 2025
feng-j678 added a commit that referenced this pull request May 9, 2025
this release includes:
[x] [Bugfix: Restore patch mode config on Image Default
#301](#301)
[x] [Clean-up: Envlayer dead-code clean-up for Environment Recorder and
Emulator #304](#304)
[x] [Bugfix: Reboot Manager behavior - multiple bugfixes & error rate
reduction #310](#310)
[x] [Bugfix: Fix sudo check logs and wordings
#315](#315)
[x] [Bugfix: Mitigation for environments with Python Unbuffered I/O
#320](#320)
@feng-j678 feng-j678 mentioned this pull request May 12, 2025
kjohn-msft pushed a commit that referenced this pull request May 12, 2025
this release includes:
[x] Bugfix: Restore patch mode config on Image Default
(#301)
[x] Clean-up: Envlayer dead-code clean-up for Environment Recorder and
Emulator ( #304)
[x] Bugfix: Reboot Manager behavior - multiple bugfixes & error rate
reduction (#310)
[x] Bugfix: Fix sudo check logs and wordings
(#315)
[x] Bugfix: Mitigation for environments with Python Unbuffered I/O 
 (#320)
[x] Bugfix: Correct reboot management
parameters(#322)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working engg. hygiene Engineering hygiene related OE PR is considered near complete due to OE sign-off.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants