Don't wait for hooks in kubectl cert-manager x install integration test
#4342
Conversation
…al chart for tests Signed-off-by: Inteon <42113979+inteon@users.noreply.github.com>
jetstack-bot
added
kind/bug
|
Hi @inteon. Thanks for your PR. I'm waiting for a jetstack or cert-manager member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
jetstack-bot
added
size/M
|
/ok-to-test |
jetstack-bot
added
ok-to-test
release-note-none
| // Helm supports 3 diffent combinations of the (Atomic, Wait) boolean couple: | ||
| // (False, False), (False, True) or (True, True) | ||
| // For simplicity, we want do not support Waiting without the Atomic option (False, True), | ||
| // this allows this cli to use a single --wait=(True|False) flag |
There was a problem hiding this comment.
I found this comment useful, but I also find the new, shorter comment, below, easier to read.
But what neither comment explains is why we don't also use Atomic=True, in the async install scenario (--wait=False).
That's my question....why don't we always set Atomic=True? Please answer that in a comment above the code, for future reference.
|
|
||
| o.client.Wait = o.Wait // Wait for resources to be ready | ||
| o.client.Atomic = o.Wait // If part of the install fails (& we are waiting), all resource installs are reverted; | ||
| o.client.DisableHooks = !o.Wait // Disable hooks if wait is disabled |
There was a problem hiding this comment.
And maybe explain why we disable hooks if wait is disabled. Is it simply for the sake of getting the tests to pass? If so, that's fine, but make it clear in the comment.
What if we introduce a future hook which performs some post-install editing of the CRDs, for example? Then we'd want the hooks to be run regardless of the --wait flag.
| func GetTestPath(path ...string) string { | ||
| return filepath.Join(append([]string{os.Getenv("RUNFILES_DIR"), "com_github_jetstack_cert_manager"}, path...)...) | ||
| } |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: inteon, wallrj The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
jetstack-bot
added
the
approved
|
I made the requested changes. see #4347 |
|
/cherry-pick release-1.5 I'm assuming this won't work... EDIT: oh yay it worked! |
|
@SgtCoDFish: new pull request created: #4356 In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
Integration tests are failing with a timeout error due to newly released Helm chart.
The integration test is performed against a fake kubernetes api-server, and the test only checks if the resources are created correctly.
The resources are not used to really deploy cert-manager in the tests, that is why we have to disable the post-install checks.
The newly-added post-install Helm hook in the v1.5 release was not yet disabled in the test; this causes a timeout error.
This PR disables the post-install Helm hook for the integration test.
Also included: use local chart instead of published chart in the integration test -> this should prevent this problem in the future
Release note:
/kind bug