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.

Sign up for GitHub

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

Jump to bottom

Resolve test flakes "the object has been modified" #4239

Merged
merged 2 commits into from
Jul 26, 2021
Merged

Resolve test flakes "the object has been modified" #4239

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ac67796
resolve test flakes 'the object has been modified'
inteon Jul 22, 2021
c377e0e
add comments
inteon Jul 22, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 34 additions & 10 deletions test/e2e/framework/helper/certificates.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,47 +32,71 @@ import (
e2eutil "github.com/jetstack/cert-manager/test/e2e/util"
)

func (h *Helper) handleResult(ns, name string, cert *cmapi.Certificate, err error) (*cmapi.Certificate, error) {
func (h *Helper) handleResult(ns, name string, cert *cmapi.Certificate, state string, err error) (*cmapi.Certificate, error) {
if err != nil {
log.Logf("Error waiting for Certificate to become Ready: %v", err)
log.Logf("Error waiting for Certificate to become %s: %v", state, err)
h.Kubectl(ns).DescribeResource("certificate", name)
h.Kubectl(ns).Describe("order", "challenge")
h.describeCertificateRequestFromCertificate(ns, cert)
}
return cert, err
}

// WaitForCertificateReady waits for the certificate resource to enter a Ready state.
// waitForCertificateNotIssuing waits for the certificate resource to leave the Issuing state.
func (h *Helper) waitForCertificateNotIssuing(ns, name string, timeout time.Duration) (*cmapi.Certificate, error) {
result, err := e2eutil.WaitForMissingCertificateCondition(h.CMClient.CertmanagerV1().Certificates(ns), name, cmapi.CertificateCondition{
Type: cmapi.CertificateConditionIssuing,
Status: cmmeta.ConditionTrue,
}, timeout)
return h.handleResult(ns, name, result, "Not Issuing", err)
}

// WaitForCertificateReady waits for the certificate resource to enter a Ready state and to leave the Issuing state.
func (h *Helper) WaitForCertificateReady(ns, name string, timeout time.Duration) (*cmapi.Certificate, error) {
result, err := e2eutil.WaitForCertificateCondition(h.CMClient.CertmanagerV1().Certificates(ns), name, cmapi.CertificateCondition{
Type: cmapi.CertificateConditionReady,
Status: cmmeta.ConditionTrue,
}, timeout)
return h.handleResult(ns, name, result, err)
if err != nil {
return h.handleResult(ns, name, result, "Ready", err)
}
// Making sure that the Certificate is stable (see #4239) by also waiting for the Issuing state to disappear.
// A certificate that has state Ready=True and Issuing not set, is stable and will not change without outside changes.
return h.waitForCertificateNotIssuing(ns, name, timeout)
}

// WaitForCertificateReadyUpdate waits for the certificate resource to enter a
// Ready state. If the provided cert was in a Ready state already, the function
// waits for a state transition to have happened.
// Ready state and to leave the Issuing state. If the provided cert was in a
// Ready state already, the function waits for a state transition to have happened.
func (h *Helper) WaitForCertificateReadyUpdate(cert *cmapi.Certificate, timeout time.Duration) (*cmapi.Certificate, error) {
result, err := e2eutil.WaitForCertificateConditionWithObservedGeneration(h.CMClient.CertmanagerV1().Certificates(cert.Namespace), cert.Name, cmapi.CertificateCondition{
Type: cmapi.CertificateConditionReady,
Status: cmmeta.ConditionTrue,
ObservedGeneration: cert.Generation,
}, timeout)
return h.handleResult(cert.Namespace, cert.Name, result, err)
if err != nil {
return h.handleResult(cert.Namespace, cert.Name, result, "Ready", err)
}
// Making sure that the Certificate is stable (see #4239) by also waiting for the Issuing state to disappear.
// A certificate that has state Ready=True and Issuing not set, is stable and will not change without outside changes.
return h.waitForCertificateNotIssuing(cert.Namespace, cert.Name, timeout)
}

// WaitForCertificateReadyUpdate waits for the certificate resource to enter a
// Ready=False state. If the provided cert was in a Ready=False state already,
// the function waits for a state transition to have happened.
// Ready=False state and to leave the Issuing state. If the provided cert was
// in a Ready=False state already, the function waits for a state transition to have happened.
func (h *Helper) WaitForCertificateNotReadyUpdate(cert *cmapi.Certificate, timeout time.Duration) (*cmapi.Certificate, error) {
result, err := e2eutil.WaitForCertificateConditionWithObservedGeneration(h.CMClient.CertmanagerV1().Certificates(cert.Namespace), cert.Name, cmapi.CertificateCondition{
Type: cmapi.CertificateConditionReady,
Status: cmmeta.ConditionFalse,
ObservedGeneration: cert.Generation,
}, timeout)
return h.handleResult(cert.Namespace, cert.Name, result, err)
if err != nil {
return h.handleResult(cert.Namespace, cert.Name, result, "Not Ready", err)
}
// Making sure that the Certificate is stable (see #4239) by also waiting for the Issuing state to disappear.
// A certificate that has state Ready=False and Issuing not set, is stable and will not change without outside changes.
return h.waitForCertificateNotIssuing(cert.Namespace, cert.Name, timeout)
}

func (h *Helper) deduplicateExtKeyUsages(us []x509.ExtKeyUsage) []x509.ExtKeyUsage {
Expand Down
9 changes: 6 additions & 3 deletions test/e2e/suite/serving/cainjector.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ var _ = framework.CertManagerDescribe("CA Injector", func() {

By("grabbing the corresponding secret")
var secret corev1.Secret
Eventually(func() error { return f.CRClient.Get(context.Background(), secretName, &secret) }, "10s", "2s").Should(Succeed())
Expect(f.CRClient.Get(context.Background(), secretName, &secret)).To(Succeed())

By("checking that all webhooks have a populated CA")
caData := secret.Data["ca.crt"]
Expand Down Expand Up @@ -158,11 +158,14 @@ var _ = framework.CertManagerDescribe("CA Injector", func() {
By("changing the name of the corresponding secret in the cert")
secretName := types.NamespacedName{Name: cert.Spec.SecretName, Namespace: f.Namespace.Name}
cert.Spec.DNSNames = append(cert.Spec.DNSNames, "something.com")
Eventually(func() error { return f.CRClient.Update(context.Background(), &cert) }, "10s", "2s").Should(Succeed())
Expect(f.CRClient.Update(context.Background(), &cert)).To(Succeed())

_, err := f.Helper().WaitForCertificateReadyUpdate(&cert, time.Second*30)
Expect(err).NotTo(HaveOccurred(), "failed to wait for Certificate to become updated")

By("grabbing the new secret")
var secret corev1.Secret
Eventually(func() error { return f.CRClient.Get(context.Background(), secretName, &secret) }, "10s", "2s").Should(Succeed())
Expect(f.CRClient.Get(context.Background(), secretName, &secret)).To(Succeed())

By("verifying that the hooks have the new data")
caData := secret.Data["ca.crt"]
Expand Down
21 changes: 21 additions & 0 deletions test/e2e/util/util.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,27 @@ func WaitForCertificateCondition(client clientset.CertificateInterface, name str
return certificate, wrapErrorWithCertificateStatusCondition(client, pollErr, name, condition.Type)
}

// WaitForMissingCertificateCondition waits for the status of the named Certificate to NOT contain
// a condition whose type and status matches the supplied one.
func WaitForMissingCertificateCondition(client clientset.CertificateInterface, name string, condition v1.CertificateCondition, timeout time.Duration) (*v1.Certificate, error) {
var certificate *v1.Certificate = nil
pollErr := wait.PollImmediate(500*time.Millisecond, timeout,
func() (bool, error) {
log.Logf("Waiting for Certificate %v condition %v=%v to be missing", name, condition.Type, condition.Status)
certificate, err := client.Get(context.TODO(), name, metav1.GetOptions{})
if nil != err {
return false, fmt.Errorf("error getting Certificate %v: %v", name, err)
}
if apiutil.CertificateHasCondition(certificate, condition) {
log.Logf("Expected Certificate %v condition %v=%v to be missing but it has: %v", name, condition.Type, condition.Status, condition.ObservedGeneration, certificate.Status.Conditions)
return false, nil
}
return true, nil
},
)
return certificate, wrapErrorWithCertificateStatusCondition(client, pollErr, name, condition.Type)
}

// WaitForCertificateConditionWithObservedGeneration waits for the status of the named Certificate to contain
// a condition whose type and status matches the supplied one.
func WaitForCertificateConditionWithObservedGeneration(client clientset.CertificateInterface, name string, condition v1.CertificateCondition, timeout time.Duration) (*v1.Certificate, error) {
Expand Down