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
[release-4.6] Bug 1937090: Configure CoreDNS to shut down gracefully #248
[release-4.6] Bug 1937090: Configure CoreDNS to shut down gracefully #248
Conversation
@Miciah: This pull request references Bugzilla bug 1937090, which is invalid:
Comment 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. |
47520d4
to
08015c7
Compare
/retest |
/test e2e-aws-operator |
test/e2e/operator_test.go: Re-enable TestCoreDNSImageUpgrade by adding a cluster version override for the DNS operator deployment. This override allows TestCoreDNSImageUpgrade to safely modify the DNS deployment (specifically the operator's container image field) without contention from the CVO. The cluster version override is removed at the end of the test via a `defer` block.
TestCoreDNSImageUpgrade: Ensure that the CoreDNS image change for default DNS is completely reverted before moving onto the next test. TestDNSForwarding: Ensure that DNS pods are all available before verifying Corefile contents from each DNS pod. Also, log pod status if a given pod's Corefile doesn't meet the test's expectations. This commit enhances the DNS operator tests to resolve BZ#1908891, in which TestDNSForwarding is noted as very flakey due to TestDNSForwarding's image revert rollout not blocking the premature execution of TestDNSFowarding.
This commit is the same as commit f094ddf, which was reverted with commit a96c45e, except that this commit does not change DNS pods' termination grace period. This commit is related to bug 1884053. https://bugzilla.redhat.com/show_bug.cgi?id=1884053 * assets/dns/daemonset.yaml: Change the readiness probe to use :8181/ready. * pkg/manifests/bindata.go: Regenerate. * pkg/operator/controller/controller_dns_configmap.go (corefileTemplate): Configure CoreDNS's health plugin to sleep 20 seconds when CoreDNS is shut down. Enable CoreDNS's ready plugin in order to provide a readiness endpoint on :8181/ready, which doesn't report ready until all plugins are initialized and stops reporting ready when CoreDNS is shutting down. * pkg/operator/controller/controller_dns_configmap_test.go (TestDesiredDNSConfigmap): Adjust for changes to corefileTemplate.
Delete the TestCoreDNSImageUpgrade CI test. This test is unreliable, and we can achieve sufficient test coverage without it. * test/e2e/operator_test.go (TestCoreDNSImageUpgrade) (setVersion, setImage, checkCurrentDNSImage): Delete functions.
Add an end-to-end test that verifies that the operator reconciles changes to the dns-default daemonset. This new test adds a node selector to the daemonset and verifies that the operator reverts the change. The operator already has unit tests to verify that the daemonset update logic handles changes to image pullspecs and other important fields. Together, the new end-to-end test and the existing unit tests should provide sufficient test coverage for reconciliation of daemonsets. * test/e2e/operator_test.go (TestCoreDNSDaemonSetReconciliation): New test. Verify that the operator reconciles the dns-default daemonset.
08015c7
to
2d17b72
Compare
@Miciah: This pull request references Bugzilla bug 1937090, which is invalid:
Comment 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. |
/test e2e-aws |
/bugzilla refresh |
@sgreene570: This pull request references Bugzilla bug 1937090, which is valid. 6 validation(s) were run on this bug
Requesting review from QA contact: 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. |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Miciah, sgreene570 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 |
/retest Please review the full test history for this PR and help us cut down flakes. |
@Miciah: All pull requests linked via external trackers have merged: Bugzilla bug 1937090 has been moved to the MODIFIED state. 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. |
test/e2e: Re-enable
TestCoreDNSImageUprade
test/e2e/operator_test.go
: Re-enableTestCoreDNSImageUpgrade
by adding a cluster version override for the DNS operator deployment. This override allowsTestCoreDNSImageUpgrade
to safely modify the DNS deployment (specifically the operator's container image field) without contention from the CVO. The cluster version override is removed at the end of the test via adefer
block.test/e2e: Block on
TestCoreDNSImageUpgrade
image revertTestCoreDNSImageUpgrade:
Ensure that the CoreDNS image change for default DNS is completely reverted before moving onto the next test.TestDNSForwarding:
Ensure that DNS pods are all available before verifying Corefile contents from each DNS pod. Also,log pod status if a given pod's Corefile doesn't meet the test's expectations.This commit enhances the DNS operator tests to resolve BZ#1908891, in which
TestDNSForwarding
is noted as very flakey due toTestDNSForwarding
's image revert rollout not blocking the premature execution ofTestDNSFowarding.
Configure CoreDNS to shut down gracefully
This PR is the same as #205, which was reverted with #213, except that this PR does not change DNS pods' termination grace period.
assets/dns/daemonset.yaml
: Change the readiness probe to use:8181/ready
.pkg/manifests/bindata.go
: Regenerate.pkg/operator/controller/controller_dns_configmap.go
(corefileTemplate
): Configure CoreDNS's health plugin to sleep 20 seconds when CoreDNS is shut down. Enable CoreDNS's ready plugin in order to provide a readiness endpoint on:8181/ready
, which doesn't report ready until all plugins are initialized and stops reporting ready when CoreDNS is shutting down.pkg/operator/controller/controller_dns_configmap_test.go
(TestDesiredDNSConfigmap
): Adjust for changes tocorefileTemplate
.Delete
TestCoreDNSImageUpgrade
Delete the
TestCoreDNSImageUpgrade
CI test. This test is unreliable, and we can achieve sufficient test coverage without it.test/e2e/operator_test.go
(TestCoreDNSImageUpgrade
,setVersion
,setImage
): Delete functions.Add
TestCoreDNSDaemonSetReconciliation
Add an end-to-end test that verifies that the operator reconciles changes to the dns-default daemonset. This new test adds a node selector to the daemonset and verifies that the operator reverts the change.
The operator already has unit tests to verify that the daemonset update logic handles changes to image pullspecs and other important fields. Together, the new end-to-end test and the existing unit tests should provide sufficient test coverage for reconciliation of daemonsets.
test/e2e/operator_test.go
(TestCoreDNSDaemonSetReconciliation
): New test. Verify that the operator reconciles the dns-default daemonset.This is a manual cherry-pick of #237, #230, and #226.