Skip to content

Commit

Permalink
Merge pull request #857 from openshift-cherrypick-robot/cherry-pick-8…
Browse files Browse the repository at this point in the history
…10-to-release-4.12

[release-4.12] OCPBUGS-3517: Ingress controller should not have affinity policy in single-replica clusters
  • Loading branch information
openshift-merge-robot committed Apr 10, 2023
2 parents 992b43b + 23717f3 commit 6f3e22f
Show file tree
Hide file tree
Showing 2 changed files with 53 additions and 0 deletions.
22 changes: 22 additions & 0 deletions pkg/operator/controller/ingress/deployment.go
Original file line number Diff line number Diff line change
Expand Up @@ -282,6 +282,17 @@ func desiredRouterDeployment(ci *operatorv1.IngressController, ingressController
// node. Thus no affinity policy is required when using
// HostNetwork.
case operatorv1.PrivateStrategyType, operatorv1.LoadBalancerServiceStrategyType, operatorv1.NodePortServiceStrategyType:
// Non-HA ingress controllers should have default deployment
// strategy with no affinity policy. We need to check the cluster
// topology in order to determine whether the affinity rule is
// required. Just checking desiredReplicas would be incorrect
// because we need the rule on multi-node clusters, even if
// desiredReplicas is 1, so that a rolling update schedules
// the new pod to the same node as the old pod.
if singleReplica(ingressConfig, infraConfig) {
break
}

// To avoid downtime during a rolling update, we need two
// things: a deployment strategy and an affinity policy. First,
// the deployment strategy: During a rolling update, we want the
Expand Down Expand Up @@ -1690,3 +1701,14 @@ func capReloadIntervalValue(interval time.Duration) time.Duration {
return interval
}
}

// SingleReplica determines if ingress controllers are deployed in SingleReplica topology
func singleReplica(ingressConfig *configv1.Ingress, infraConfig *configv1.Infrastructure) bool {
// DefaultPlacement affects which topology field we're interested in
topology := infraConfig.Status.InfrastructureTopology
if ingressConfig.Status.DefaultPlacement == configv1.DefaultPlacementControlPlane {
topology = infraConfig.Status.ControlPlaneTopology
}

return topology == configv1.SingleReplicaTopologyMode
}
31 changes: 31 additions & 0 deletions pkg/operator/controller/ingress/deployment_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -804,6 +804,37 @@ func TestDesiredRouterDeploymentHostNetworkNil(t *testing.T) {
checkContainerPort(t, deployment, "metrics", 1936)
}

func TestDesiredRouterDeploymentSingleReplica(t *testing.T) {
ic, ingressConfig, _, apiConfig, networkConfig, _ := getRouterDeploymentComponents(t)

infraConfig := &configv1.Infrastructure{
Status: configv1.InfrastructureStatus{
PlatformStatus: &configv1.PlatformStatus{
Type: configv1.IBMCloudPlatformType,
},
ControlPlaneTopology: configv1.ExternalTopologyMode,
InfrastructureTopology: configv1.SingleReplicaTopologyMode,
},
}

deployment, err := desiredRouterDeployment(ic, ingressControllerImage, ingressConfig, infraConfig, apiConfig, networkConfig, false, false, nil)
if err != nil {
t.Fatalf("invalid router Deployment: %v", err)
}

if *deployment.Spec.Replicas != 1 {
t.Errorf("expected replicas to be 1, got %d", *deployment.Spec.Replicas)
}

if deployment.Spec.Template.Spec.Affinity != nil {
t.Errorf("expected no affinity, got %+v", *deployment.Spec.Template.Spec.Affinity)
}

if deployment.Spec.Strategy.Type != "" || deployment.Spec.Strategy.RollingUpdate != nil {
t.Errorf("expected default deployment strategy, got %s", deployment.Spec.Strategy.Type)
}
}

func checkContainerPort(t *testing.T, d *appsv1.Deployment, portName string, port int32) {
t.Helper()
for _, p := range d.Spec.Template.Spec.Containers[0].Ports {
Expand Down

0 comments on commit 6f3e22f

Please sign in to comment.