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

Improvements to the E2E tests #336

Merged
merged 3 commits into from
Apr 11, 2023
Merged

Improvements to the E2E tests #336

merged 3 commits into from
Apr 11, 2023

Conversation

liornoy
Copy link
Contributor

@liornoy liornoy commented Mar 28, 2023

This PR includes 2 small commits resulting from troubleshooting a bug:

  1. Improve a log output.
  2. Split the Delete function to Delete and DeleteAndCheck, to prevent failures from calling Delete on
    an Incorrect_metallb resource where it shouldn't do the additional check whether the deployments and
    daemonsets are deleted as well.

This commit changes the way the MetalLBReconciler logs an error
on the Reconcile function, added the error's content into the log.

This change is required as we faced flakes hitting this line and
not reporting the error, making it hard to debug.

Signed-off-by: liornoy <[email protected]>
@liornoy liornoy changed the title MetalLBReconciler: log the error on failure Improvements to the E2e tests Mar 28, 2023
@liornoy liornoy changed the title Improvements to the E2e tests Improvements to the E2E tests Mar 29, 2023
metallbutils.Delete(correct_metallb)
metallbutils.Delete(incorrect_metallb)
Copy link
Member

Choose a reason for hiding this comment

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

I think the right way to do is to revise Delete and have it "just" delete, and moving the check logic in another function, OR having a Delete and a DeleteAndCheck

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@liornoy liornoy force-pushed the printerror branch 2 times, most recently from c8a657f to f66d59a Compare April 2, 2023 06:29
Split the Delete function to Delete and DeleteAndCheck
so we'll be able to call only Delete on the incorrect_metallb resources
and DeleteAndCheck on metallb resources where the additional check is
required.

Signed-off-by: liornoy <[email protected]>
@fedepaol
Copy link
Member

Thanks!

@fedepaol fedepaol merged commit a116c4c into metallb:main Apr 11, 2023
@liornoy liornoy deleted the printerror branch April 13, 2023 06:51
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.

2 participants