Skip to content

Commit

Permalink
[release 0.17] Cherrypick #79 and #82 (#86)
Browse files Browse the repository at this point in the history
* Retry Creates on resourcequota conflicts. (#79)

This follows @vaikas PR here: knative/eventing#3215

The kubernetes issue is tracked here: kubernetes/kubernetes#67761

Fixes: knative/test-infra#2346

* Include Get in the UpdateRetry. (#82)

This adjusts the update retry around the ingress update in #79 to include the Get.

The original change was to guard against issues updating gke-resource-quotas, but there is a low incidence of conflicts simply updating the kingress itself.

Here's an example from net-contour:

```
=== CONT  TestIngressConformance/5/update
    update.go:88: Error updating Ingress: Operation cannot be fulfilled on ingresses.networking.internal.knative.dev "ingress-conformance-5-update-eghinekn": the object has been modified; please apply your changes to the latest version and try again
```

However, to resolve this, we actually have to refetch the kingress shell we've stuck the desired IngressSpec into otherwise it will just retry until it has exhausted its attempts because the resourceVersion we're sending back is never changed (and this is what the optimistic concurrency keys off of).
  • Loading branch information
mattmoor authored Aug 12, 2020
1 parent 2ae98bd commit 2fb9051
Showing 1 changed file with 27 additions and 15 deletions.
42 changes: 27 additions & 15 deletions test/conformance/ingress/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ import (
"knative.dev/networking/test"
"knative.dev/networking/test/types"
"knative.dev/pkg/network"
"knative.dev/pkg/reconciler"
pkgTest "knative.dev/pkg/test"
"knative.dev/pkg/test/logging"
)
Expand Down Expand Up @@ -535,8 +536,10 @@ func createService(t *testing.T, clients *test.Clients, svc *corev1.Service) con
test.CleanupOnInterrupt(func() {
clients.KubeClient.Kube.CoreV1().Services(svc.Namespace).Delete(svc.Name, &metav1.DeleteOptions{})
})
svc, err := clients.KubeClient.Kube.CoreV1().Services(svc.Namespace).Create(svc)
if err != nil {
if err := reconciler.RetryUpdateConflicts(func(attempts int) error {
_, err := clients.KubeClient.Kube.CoreV1().Services(svc.Namespace).Create(svc)
return err
}); err != nil {
t.Fatal("Error creating Service:", err)
}

Expand All @@ -554,8 +557,10 @@ func createPodAndService(t *testing.T, clients *test.Clients, pod *corev1.Pod, s
t.Helper()

test.CleanupOnInterrupt(func() { clients.KubeClient.Kube.CoreV1().Pods(pod.Namespace).Delete(pod.Name, &metav1.DeleteOptions{}) })
pod, err := clients.KubeClient.Kube.CoreV1().Pods(pod.Namespace).Create(pod)
if err != nil {
if err := reconciler.RetryUpdateConflicts(func(attempts int) error {
_, err := clients.KubeClient.Kube.CoreV1().Pods(pod.Namespace).Create(pod)
return err
}); err != nil {
t.Fatal("Error creating Pod:", err)
}
cancel := func() {
Expand All @@ -568,8 +573,10 @@ func createPodAndService(t *testing.T, clients *test.Clients, pod *corev1.Pod, s
test.CleanupOnInterrupt(func() {
clients.KubeClient.Kube.CoreV1().Services(svc.Namespace).Delete(svc.Name, &metav1.DeleteOptions{})
})
svc, err = clients.KubeClient.Kube.CoreV1().Services(svc.Namespace).Create(svc)
if err != nil {
if err := reconciler.RetryUpdateConflicts(func(attempts int) error {
_, err := clients.KubeClient.Kube.CoreV1().Services(svc.Namespace).Create(svc)
return err
}); err != nil {
cancel()
t.Fatal("Error creating Service:", err)
}
Expand Down Expand Up @@ -621,8 +628,10 @@ func CreateIngress(t *testing.T, clients *test.Clients, spec v1alpha1.IngressSpe
Spec: spec,
}
test.CleanupOnInterrupt(func() { clients.NetworkingClient.Ingresses.Delete(ing.Name, &metav1.DeleteOptions{}) })
ing, err := clients.NetworkingClient.Ingresses.Create(ing)
if err != nil {
if err := reconciler.RetryUpdateConflicts(func(attempts int) (err error) {
ing, err = clients.NetworkingClient.Ingresses.Create(ing)
return err
}); err != nil {
t.Fatal("Error creating Ingress:", err)
}

Expand Down Expand Up @@ -682,14 +691,17 @@ func CreateIngressReady(t *testing.T, clients *test.Clients, spec v1alpha1.Ingre
func UpdateIngress(t *testing.T, clients *test.Clients, name string, spec v1alpha1.IngressSpec) {
t.Helper()

ing, err := clients.NetworkingClient.Ingresses.Get(name, metav1.GetOptions{})
if err != nil {
t.Fatal("Error getting Ingress:", err)
}
if err := reconciler.RetryUpdateConflicts(func(attempts int) error {
ing, err := clients.NetworkingClient.Ingresses.Get(name, metav1.GetOptions{})
if err != nil {
return err
}

ing.Spec = spec
if _, err := clients.NetworkingClient.Ingresses.Update(ing); err != nil {
t.Fatal("Error updating Ingress:", err)
ing.Spec = spec
_, err = clients.NetworkingClient.Ingresses.Update(ing)
return err
}); err != nil {
t.Fatal("Error fetching and updating Ingress:", err)
}
}

Expand Down

0 comments on commit 2fb9051

Please sign in to comment.