Skip to content

Improving logging and error readability for rke2-uninstall.sh script#6237

Merged
dereknola merged 9 commits intorancher:masterfrom
iamsarthakk:master
Oct 22, 2024
Merged

Improving logging and error readability for rke2-uninstall.sh script#6237
dereknola merged 9 commits intorancher:masterfrom
iamsarthakk:master

Conversation

@iamsarthakk
Copy link
Copy Markdown
Contributor

@iamsarthakk iamsarthakk commented Jun 21, 2024

Proposed Changes

Currently, whenever uninstall script fails it is not immediately clear that it failed and we need to address the failure issues. These changes adds clear messaging in red colour if the script fails at any point with better logging.

It doesn't need documentation changes

Types of Changes

Improving logging and error readability in rke2-uninstall script

Verification

Changes can be verified by running the uninstall script on rancher based system

Testing

It doesn't have any changes which need unit tests or integration tests

Linked Issues

User-Facing Change

NONE

Further Comments

@iamsarthakk iamsarthakk requested a review from a team as a code owner June 21, 2024 15:33
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Jun 24, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 16.62%. Comparing base (e1ba484) to head (6494952).
Report is 1 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #6237   +/-   ##
=======================================
  Coverage   16.62%   16.62%           
=======================================
  Files          32       32           
  Lines        3423     3423           
=======================================
  Hits          569      569           
  Misses       2812     2812           
  Partials       42       42           
Flag Coverage Δ
unittests 16.62% <ø> (ø)

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.

Comment thread bundle/bin/rke2-uninstall.sh
@iamsarthakk iamsarthakk requested a review from brandond August 19, 2024 10:33
@brandond
Copy link
Copy Markdown
Member

Please rebase your changes on top of current master.

@iamsarthakk
Copy link
Copy Markdown
Contributor Author

Please rebase your changes on top of current master.

Done @brandond

@iamsarthakk
Copy link
Copy Markdown
Contributor Author

@brandond Can you please merge the PR if there is no further issues?

@brandond
Copy link
Copy Markdown
Member

I would probably hold off until after the current release is done, since the install script gets bundled into system-agent-installer-rke2.

@iamsarthakk
Copy link
Copy Markdown
Contributor Author

iamsarthakk commented Sep 18, 2024

Okay @brandond , when is the release planned? I will remind after the release once. Thanks

@dereknola
Copy link
Copy Markdown
Member

@iamsarthakk Please resolve the conflicts (the [ -r /etc/amazon-linux-release ] needs to become the [ -r /etc/system-release ] and I will get this merged in this week. It will then be backported into the October patches

@brandond
Copy link
Copy Markdown
Member

looks like there are still conflicts. Did you pull before rebasing on top of master?

@dereknola dereknola merged commit 721ecb2 into rancher:master Oct 22, 2024
brandond pushed a commit to brandond/rke2 that referenced this pull request Jun 13, 2025
…ancher#6237)

* Added Descriptive logging and messaging in case the script fails

* Removing redundant piped error

* Added support for NO_COLOR env variable for default no printing of color codes

* Add data-dir to uninstall and killall scripts

(cherry picked from commit 721ecb2)
Signed-off-by: Jake Hyde <jakefhyde@gmail.com>
Co-authored-by: Sarthak Kumar <sarthak.kumar@cloudera.com>
Co-authored-by: Jake Hyde <jakefhyde@gmail.com>
Signed-off-by: Brad Davidson <brad.davidson@rancher.com>
brandond pushed a commit to brandond/rke2 that referenced this pull request Jun 13, 2025
…ancher#6237)

* Added Descriptive logging and messaging in case the script fails

* Removing redundant piped error

* Added support for NO_COLOR env variable for default no printing of color codes

* Add data-dir to uninstall and killall scripts

(cherry picked from commit 721ecb2)
Signed-off-by: Jake Hyde <jakefhyde@gmail.com>
Co-authored-by: Sarthak Kumar <sarthak.kumar@cloudera.com>
Co-authored-by: Jake Hyde <jakefhyde@gmail.com>
Signed-off-by: Brad Davidson <brad.davidson@rancher.com>
brandond pushed a commit that referenced this pull request Jun 13, 2025
…6237)

* Added Descriptive logging and messaging in case the script fails

* Removing redundant piped error

* Added support for NO_COLOR env variable for default no printing of color codes

* Add data-dir to uninstall and killall scripts

(cherry picked from commit 721ecb2)
Signed-off-by: Jake Hyde <jakefhyde@gmail.com>
Co-authored-by: Sarthak Kumar <sarthak.kumar@cloudera.com>
Co-authored-by: Jake Hyde <jakefhyde@gmail.com>
Signed-off-by: Brad Davidson <brad.davidson@rancher.com>
brandond pushed a commit that referenced this pull request Jun 13, 2025
…6237)

* Added Descriptive logging and messaging in case the script fails

* Removing redundant piped error

* Added support for NO_COLOR env variable for default no printing of color codes

* Add data-dir to uninstall and killall scripts

(cherry picked from commit 721ecb2)
Signed-off-by: Jake Hyde <jakefhyde@gmail.com>
Co-authored-by: Sarthak Kumar <sarthak.kumar@cloudera.com>
Co-authored-by: Jake Hyde <jakefhyde@gmail.com>
Signed-off-by: Brad Davidson <brad.davidson@rancher.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants