diff --git a/pkg/console/subresource/deployment/deployment.go b/pkg/console/subresource/deployment/deployment.go index 4a015ea94..52e3921fd 100644 --- a/pkg/console/subresource/deployment/deployment.go +++ b/pkg/console/subresource/deployment/deployment.go @@ -187,13 +187,32 @@ func withStrategy(deployment *appsv1.Deployment, infrastructureConfig *configv1. MaxUnavailable: &intstr.IntOrString{Type: intstr.String, StrVal: "25%"}, } if ShouldDeployHA(infrastructureConfig) { - rollingUpdateParams = &appsv1.RollingUpdateDeployment{ - MaxSurge: &intstr.IntOrString{ - IntVal: int32(3), - }, - MaxUnavailable: &intstr.IntOrString{ - IntVal: int32(1), - }, + topology := infrastructureConfig.Status.ControlPlaneTopology + if topology == configv1.DualReplicaTopologyMode || topology == configv1.HighlyAvailableArbiterMode { + // On 2-node topologies the required pod anti-affinity prevents + // scheduling a surge pod when every eligible node already runs + // a console pod, so maxUnavailable must be >= 1 to allow the + // rollout to make progress. + rollingUpdateParams = &appsv1.RollingUpdateDeployment{ + MaxSurge: &intstr.IntOrString{ + IntVal: int32(1), + }, + MaxUnavailable: &intstr.IntOrString{ + IntVal: int32(1), + }, + } + } else { + // On 3+ node topologies a free node is available for the surge + // pod, so maxUnavailable=0 guarantees zero-downtime rollouts: + // no old pod is terminated until its replacement is ready. + rollingUpdateParams = &appsv1.RollingUpdateDeployment{ + MaxSurge: &intstr.IntOrString{ + IntVal: int32(1), + }, + MaxUnavailable: &intstr.IntOrString{ + IntVal: int32(0), + }, + } } } deployment.Spec.Strategy.RollingUpdate = rollingUpdateParams diff --git a/pkg/console/subresource/deployment/deployment_test.go b/pkg/console/subresource/deployment/deployment_test.go index ea9592440..2e2a5557c 100644 --- a/pkg/console/subresource/deployment/deployment_test.go +++ b/pkg/console/subresource/deployment/deployment_test.go @@ -266,10 +266,10 @@ func TestDefaultDeployment(t *testing.T) { Type: appsv1.RollingUpdateDeploymentStrategyType, RollingUpdate: &appsv1.RollingUpdateDeployment{ MaxSurge: &intstr.IntOrString{ - IntVal: int32(3), + IntVal: int32(1), }, MaxUnavailable: &intstr.IntOrString{ - IntVal: int32(1), + IntVal: int32(0), }, }, }, @@ -347,10 +347,10 @@ func TestDefaultDeployment(t *testing.T) { Type: appsv1.RollingUpdateDeploymentStrategyType, RollingUpdate: &appsv1.RollingUpdateDeployment{ MaxSurge: &intstr.IntOrString{ - IntVal: int32(3), + IntVal: int32(1), }, MaxUnavailable: &intstr.IntOrString{ - IntVal: int32(1), + IntVal: int32(0), }, }, }, @@ -505,10 +505,10 @@ func TestDefaultDeployment(t *testing.T) { Type: appsv1.RollingUpdateDeploymentStrategyType, RollingUpdate: &appsv1.RollingUpdateDeployment{ MaxSurge: &intstr.IntOrString{ - IntVal: int32(3), + IntVal: int32(1), }, MaxUnavailable: &intstr.IntOrString{ - IntVal: int32(1), + IntVal: int32(0), }, }, }, @@ -1529,14 +1529,24 @@ func TestWithStrategy(t *testing.T) { infrastructureConfigSingleReplica := infrastructureConfigWithTopology(configv1.SingleReplicaTopologyMode, configv1.SingleReplicaTopologyMode) infrastructureConfigExternalTopologyHighlyAvailable := infrastructureConfigWithTopology(configv1.ExternalTopologyMode, configv1.HighlyAvailableTopologyMode) infrastructureConfigExternalTopologySingleReplica := infrastructureConfigWithTopology(configv1.ExternalTopologyMode, configv1.SingleReplicaTopologyMode) + infrastructureConfigDualReplica := infrastructureConfigWithTopology(configv1.DualReplicaTopologyMode, configv1.HighlyAvailableTopologyMode) + infrastructureConfigArbiter := infrastructureConfigWithTopology(configv1.HighlyAvailableArbiterMode, configv1.HighlyAvailableTopologyMode) singleReplicaStrategy := appsv1.RollingUpdateDeployment{ MaxSurge: &intstr.IntOrString{Type: intstr.String, StrVal: "25%"}, MaxUnavailable: &intstr.IntOrString{Type: intstr.String, StrVal: "25%"}, } - highAvailStrategy := appsv1.RollingUpdateDeployment{ + zeroDowntimeStrategy := appsv1.RollingUpdateDeployment{ MaxSurge: &intstr.IntOrString{ - IntVal: int32(3), + IntVal: int32(1), + }, + MaxUnavailable: &intstr.IntOrString{ + IntVal: int32(0), + }, + } + constrainedHAStrategy := appsv1.RollingUpdateDeployment{ + MaxSurge: &intstr.IntOrString{ + IntVal: int32(1), }, MaxUnavailable: &intstr.IntOrString{ IntVal: int32(1), @@ -1571,7 +1581,7 @@ func TestWithStrategy(t *testing.T) { want: &appsv1.Deployment{ Spec: appsv1.DeploymentSpec{ Strategy: appsv1.DeploymentStrategy{ - RollingUpdate: &highAvailStrategy, + RollingUpdate: &zeroDowntimeStrategy, }, }, }, @@ -1599,7 +1609,35 @@ func TestWithStrategy(t *testing.T) { want: &appsv1.Deployment{ Spec: appsv1.DeploymentSpec{ Strategy: appsv1.DeploymentStrategy{ - RollingUpdate: &highAvailStrategy, + RollingUpdate: &zeroDowntimeStrategy, + }, + }, + }, + }, + { + name: "Test DualReplica Strategy uses maxUnavailable=1", + args: args{ + deployment: &appsv1.Deployment{}, + infrastructureConfig: infrastructureConfigDualReplica, + }, + want: &appsv1.Deployment{ + Spec: appsv1.DeploymentSpec{ + Strategy: appsv1.DeploymentStrategy{ + RollingUpdate: &constrainedHAStrategy, + }, + }, + }, + }, + { + name: "Test Arbiter Strategy uses maxUnavailable=1", + args: args{ + deployment: &appsv1.Deployment{}, + infrastructureConfig: infrastructureConfigArbiter, + }, + want: &appsv1.Deployment{ + Spec: appsv1.DeploymentSpec{ + Strategy: appsv1.DeploymentStrategy{ + RollingUpdate: &constrainedHAStrategy, }, }, }, @@ -1914,10 +1952,10 @@ func TestDefaultDownloadsDeployment(t *testing.T) { Type: appsv1.RollingUpdateDeploymentStrategyType, RollingUpdate: &appsv1.RollingUpdateDeployment{ MaxSurge: &intstr.IntOrString{ - IntVal: int32(3), + IntVal: int32(1), }, MaxUnavailable: &intstr.IntOrString{ - IntVal: int32(1), + IntVal: int32(0), }, }, }, diff --git a/test-e2e.sh b/test-e2e.sh index 298bfe9e4..77e9c031b 100755 --- a/test-e2e.sh +++ b/test-e2e.sh @@ -9,7 +9,7 @@ OPENSHIFT_CI=${OPENSHIFT_CI:=false} echo "Running tests..." if [ "$OPENSHIFT_CI" = true ]; then - KUBERNETES_CONFIG=${KUBECONFIG} go test -timeout 30m -v ./test/e2e/ 2>&1 | tee "$ARTIFACT_DIR/test.out" + KUBERNETES_CONFIG=${KUBECONFIG} go test -timeout 40m -v ./test/e2e/ 2>&1 | tee "$ARTIFACT_DIR/test.out" RESULT="${PIPESTATUS[0]}" go-junit-report < "$ARTIFACT_DIR/test.out" > "$ARTIFACT_DIR/junit.xml" @@ -17,8 +17,8 @@ if [ "$OPENSHIFT_CI" = true ]; then exit 255 fi else - echo 'KUBERNETES_CONFIG=${KUBECONFIG} go test -timeout 30m -v ./test/e2e/' - KUBERNETES_CONFIG=${KUBECONFIG} go test -timeout 30m -v ./test/e2e/ + echo 'KUBERNETES_CONFIG=${KUBECONFIG} go test -timeout 40m -v ./test/e2e/' + KUBERNETES_CONFIG=${KUBECONFIG} go test -timeout 40m -v ./test/e2e/ fi echo "Success" \ No newline at end of file