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

Fix agent to mark conditions with appropriate status on uninstall #588

Merged
merged 1 commit into from
Jun 21, 2022

Conversation

shubham14bajpai
Copy link
Contributor

Signed-off-by: Shubham [email protected]

What this PR does / why we need it:

This PR marks the K8sComponentsInstallationSucceeded condition as false whenever a host a removed from the cluster and the uninstall is executed successfully. It also fixes an existing test by asserting the correct condition for it's status.

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #585

Additional information

Special notes for your reviewer

@codecov-commenter
Copy link

Codecov Report

Merging #588 (6118454) into main (9bcc762) will increase coverage by 0.36%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #588      +/-   ##
==========================================
+ Coverage   67.46%   67.83%   +0.36%     
==========================================
  Files          27       27              
  Lines        2410     2400      -10     
==========================================
+ Hits         1626     1628       +2     
+ Misses        676      664      -12     
  Partials      108      108              
Impacted Files Coverage Δ
agent/reconciler/host_reconciler.go 81.04% <100.00%> (+0.06%) ⬆️
...trollers/infrastructure/byoadmission_controller.go 81.81% <0.00%> (+0.33%) ⬆️
agent/main.go 20.83% <0.00%> (+1.38%) ⬆️

khannakshat7
khannakshat7 previously approved these changes Jun 10, 2022
Copy link
Contributor

@khannakshat7 khannakshat7 left a comment

Choose a reason for hiding this comment

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

LGTM 👍🏻

anusha94
anusha94 previously approved these changes Jun 14, 2022
Copy link
Contributor

@anusha94 anusha94 left a comment

Choose a reason for hiding this comment

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

lgtm

shamsher31
shamsher31 previously approved these changes Jun 15, 2022
Copy link
Contributor

@shamsher31 shamsher31 left a comment

Choose a reason for hiding this comment

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

Just a minor comment, otherwise LGTM. 👍

Copy link
Contributor

@shamsher31 shamsher31 left a comment

Choose a reason for hiding this comment

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

LGTM

@anusha94 anusha94 merged commit e2ca564 into vmware-tanzu:main Jun 21, 2022
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants