Unverified Commit 455fbd0e authored by OpenShift Merge Robot's avatar OpenShift Merge Robot Committed by GitHub
Browse files

Merge pull request #569 from smarterclayton/min_ready

Bug 1959194: Ingress rollouts should specify minReadySeconds
parents a8a89f16 35babc88
......@@ -200,6 +200,16 @@ func desiredRouterDeployment(ci *operatorv1.IngressController, ingressController
gracePeriod := int64(60 * 60)
deployment.Spec.Template.Spec.TerminationGracePeriodSeconds = &gracePeriod
// Services behind load balancers should roll out new instances only after we are certain
// the new instance is part of rotation. This is set based on the highest value across all
// platforms, excluding custom load balancers like an F5, but our recommendation for these
// values for those should be indentical to the slowest cloud, AWS (which does not allow
// health checks to be more frequent than 10 seconds).
deployment.Spec.MinReadySeconds =
(2 + /* max healthy checks required to be brought into rotation across all platforms */
1) * /* we could miss one */
10 /* the longest health check interval on any platform */
volumes := deployment.Spec.Template.Spec.Volumes
routerVolumeMounts := deployment.Spec.Template.Spec.Containers[0].VolumeMounts
......@@ -994,6 +1004,7 @@ func hashableDeployment(deployment *appsv1.Deployment, onlyTemplate bool) *appsv
// Copy metadata and spec fields to which any changes should trigger an
// update of the deployment but should not trigger a rolling update.
hashableDeployment.Labels = deployment.Labels
hashableDeployment.Spec.MinReadySeconds = deployment.Spec.MinReadySeconds
hashableDeployment.Spec.Strategy = deployment.Spec.Strategy
var replicas *int32
if deployment.Spec.Replicas != nil && *deployment.Spec.Replicas != int32(1) {
......@@ -1178,6 +1189,7 @@ func deploymentConfigChanged(current, expected *appsv1.Deployment) (bool, *appsv
replicas = *expected.Spec.Replicas
}
updated.Spec.Replicas = &replicas
updated.Spec.MinReadySeconds = expected.Spec.MinReadySeconds
return true, updated
}
......
......@@ -1015,6 +1015,20 @@ func TestDeploymentConfigChanged(t *testing.T) {
},
expect: true,
},
{
description: "if .spec.minReadySeconds changes to non-zero",
mutate: func(deployment *appsv1.Deployment) {
deployment.Spec.MinReadySeconds = 10
},
expect: true,
},
{
description: "if .spec.minReadySeconds changes to none",
mutate: func(deployment *appsv1.Deployment) {
deployment.Spec.MinReadySeconds = 0
},
expect: true,
},
}
for _, tc := range testCases {
......@@ -1027,6 +1041,7 @@ func TestDeploymentConfigChanged(t *testing.T) {
UID: "1",
},
Spec: appsv1.DeploymentSpec{
MinReadySeconds: 30,
Strategy: appsv1.DeploymentStrategy{
Type: appsv1.RollingUpdateDeploymentStrategyType,
RollingUpdate: &appsv1.RollingUpdateDeployment{
......
......@@ -212,7 +212,7 @@ func TestHTTPHeaderBufferSize(t *testing.T) {
}
// Wait for the new router pod for the updated ingresscontroller to become ready.
pollErr = wait.PollImmediate(2*time.Second, 3*time.Minute, func() (bool, error) {
pollErr = wait.PollImmediate(2*time.Second, 5*time.Minute, func() (bool, error) {
podList := &corev1.PodList{}
if err := kclient.List(context.TODO(), podList, client.InNamespace(deployment.Namespace), client.MatchingLabels(labels)); err != nil {
t.Errorf("failed to list pods for ingress controllers %s: %v", ic.Name, err)
......@@ -220,16 +220,19 @@ func TestHTTPHeaderBufferSize(t *testing.T) {
for _, pod := range podList.Items {
if pod.Name == oldRouterPodName {
continue
// wait until the old router pod is gone before continuing
return false, nil
}
}
for _, pod := range podList.Items {
for _, cond := range pod.Status.Conditions {
if cond.Type == corev1.PodReady && cond.Status == corev1.ConditionTrue {
return true, nil
}
}
}
t.Logf("waiting for new router pod for %s to become ready", ic.Name)
return false, nil
})
......
......@@ -4,6 +4,7 @@ package e2e
import (
"bufio"
"bytes"
"context"
"crypto/tls"
"crypto/x509"
......@@ -557,7 +558,7 @@ func TestIngressControllerScale(t *testing.T) {
}
// Wait for the deployment scale up to be observed.
if err := waitForAvailableReplicas(t, kclient, ic, 2*time.Minute, newReplicas); err != nil {
if err := waitForAvailableReplicas(t, kclient, ic, 4*time.Minute, newReplicas); err != nil {
t.Fatalf("failed waiting deployment %s to scale to %d: %v", defaultName, newReplicas, err)
}
......@@ -1552,7 +1553,7 @@ func TestHTTPHeaderCapture(t *testing.T) {
if err != nil {
t.Fatalf("failed to create kube client: %v", err)
}
err = wait.PollImmediate(1*time.Second, 1*time.Minute, func() (bool, error) {
err = wait.PollImmediate(1*time.Second, 3*time.Minute, func() (bool, error) {
for _, pod := range podList.Items {
readCloser, err := client.CoreV1().Pods(pod.Namespace).GetLogs(pod.Name, &corev1.PodLogOptions{
Container: "logs",
......@@ -1562,7 +1563,8 @@ func TestHTTPHeaderCapture(t *testing.T) {
t.Errorf("failed to read logs from pod %s: %v", pod.Name, err)
continue
}
scanner := bufio.NewScanner(readCloser)
data, _ := ioutil.ReadAll(readCloser)
scanner := bufio.NewScanner(bytes.NewBuffer(data))
var found bool
for scanner.Scan() {
line := scanner.Text()
......@@ -1575,6 +1577,9 @@ func TestHTTPHeaderCapture(t *testing.T) {
if err := readCloser.Close(); err != nil {
t.Errorf("failed to close logs reader for pod %s: %v", pod.Name, err)
}
if !found {
t.Logf("failed to find output:\n\n%s", string(data))
}
return found, nil
}
return false, nil
......@@ -2153,14 +2158,16 @@ func waitForIngressControllerCondition(t *testing.T, cl client.Client, timeout t
}
func assertIngressControllerDeleted(t *testing.T, cl client.Client, ing *operatorv1.IngressController) {
if err := deleteIngressController(cl, ing, 2*time.Minute); err != nil {
t.Helper()
if err := deleteIngressController(t, cl, ing, 2*time.Minute); err != nil {
t.Fatalf("WARNING: cloud resources may have been leaked! failed to delete ingresscontroller %s: %v", ing.Name, err)
} else {
t.Logf("deleted ingresscontroller %s", ing.Name)
}
}
func deleteIngressController(cl client.Client, ic *operatorv1.IngressController, timeout time.Duration) error {
func deleteIngressController(t *testing.T, cl client.Client, ic *operatorv1.IngressController, timeout time.Duration) error {
t.Helper()
name := types.NamespacedName{Namespace: ic.Namespace, Name: ic.Name}
if err := cl.Delete(context.TODO(), ic); err != nil {
return fmt.Errorf("failed to delete ingresscontroller: %v", err)
......@@ -2171,6 +2178,7 @@ func deleteIngressController(cl client.Client, ic *operatorv1.IngressController,
if errors.IsNotFound(err) {
return true, nil
}
t.Logf("failed to delete ingress controller %s/%s: %v", ic.Namespace, ic.Name, err)
return false, nil
}
return false, nil
......
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment