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

Move RestoreNodeDrainState up in clean.All() #121

Merged
merged 1 commit into from
Apr 29, 2021

Conversation

oribon
Copy link
Contributor

@oribon oribon commented Apr 29, 2021

Previously if the RestoreNodeDrainState was set to true but the test namespace didn't exist,
the restoration wouldn't happen because the func returns nil too early

@@ -15,15 +15,22 @@ var RestoreNodeDrainState bool
// All cleans all the dangling resources created by conformance tests.
// This includes pods, networks, policies and namespaces.
func All() error {
var err error
Copy link
Collaborator

Choose a reason for hiding this comment

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

please remove this just put

err := cluster.SetDisableNodeDrainState(clients, operatorNamespace, false)

and

err := namespaces.DeleteAndWait(clients, namespaces.Test, 5*time.Minute)

Previously if the RestoreNodeDrainState was set to true but the test namespace didn't exist,
the restoration wouldn't happen because the func returns nil too early
Copy link
Collaborator

@SchSeba SchSeba left a comment

Choose a reason for hiding this comment

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

/lgtm
/cc @mmirecki

@mmirecki
Copy link
Contributor

/lgtm

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.

3 participants