Skip to content

Issue # 223 - Remove app finalizers during e2e fixture teardown#225

Merged
alexmt merged 1 commit intoargoproj:masterfrom
alexmt:223-e2e-cleanup
May 22, 2018
Merged

Issue # 223 - Remove app finalizers during e2e fixture teardown#225
alexmt merged 1 commit intoargoproj:masterfrom
alexmt:223-e2e-cleanup

Conversation

@alexmt
Copy link
Copy Markdown
Collaborator

@alexmt alexmt commented May 22, 2018

Closes #223

@alexmt alexmt requested review from jessesuen and merenbach May 22, 2018 15:54
Copy link
Copy Markdown
Contributor

@merenbach merenbach left a comment

Choose a reason for hiding this comment

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

Looks good to me. One tiny comment.

}
}
if err == nil {
err = f.KubeClient.CoreV1().Namespaces().Delete(f.Namespace, &metav1.DeleteOptions{})
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Wondering if this could be moved right after line 206, thus obviating the need for sequential inverse conditionals.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Logic would change if I move it.

TearDown method is checking if the previous operation successful and trying to delete namespace. Delete operation error is stored same err variable. Finally method checking if err is not nil which means something went wrong and report that teardown had issues. I hope I'm not missing something obvious.

@merenbach
Copy link
Copy Markdown
Contributor

merenbach commented May 22, 2018 via email

@alexmt alexmt merged commit d0479e6 into argoproj:master May 22, 2018
@alexmt alexmt deleted the 223-e2e-cleanup branch May 22, 2018 16:23
leoluz pushed a commit to leoluz/argo-cd that referenced this pull request Sep 29, 2023
…nd on them (argoproj#225)

* fix: sync should apply Namespaces and CRDs before resources that depend on them

Signed-off-by: Alexander Matyushentsev <AMatyushentsev@gmail.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.

2 participants