Skip to content

Commit fae2aec

Browse files
aleskandroaleskandro
committed
OCPBUGS-27335: use InfrastructureTopology for clusters using external CP as the console deploys on the worker nodes
1 parent d390563 commit fae2aec

File tree

3 files changed

+159
-24
lines changed

3 files changed

+159
-24
lines changed

‎pkg/console/subresource/deployment/deployment.go‎

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -122,9 +122,18 @@ func DefaultDownloadsDeployment(
122122
return downloadsDeployment
123123
}
124124

125+
// ShouldDeployHA returns true if the console should be deployed in HA mode.
126+
// If the control plane is externalized, the console should be deployed in HA mode based on the InfrastructureTopology,
127+
// otherwise it should be deployed in HA mode based on the ControlPlaneTopology.
128+
func ShouldDeployHA(infrastructureConfig *configv1.Infrastructure) bool {
129+
return infrastructureConfig.Status.ControlPlaneTopology == configv1.HighlyAvailableTopologyMode ||
130+
(infrastructureConfig.Status.ControlPlaneTopology == configv1.ExternalTopologyMode &&
131+
infrastructureConfig.Status.InfrastructureTopology == configv1.HighlyAvailableTopologyMode)
132+
}
133+
125134
func withReplicas(deployment *appsv1.Deployment, infrastructureConfig *configv1.Infrastructure) {
126135
replicas := int32(SingleNodeConsoleReplicas)
127-
if infrastructureConfig.Status.ControlPlaneTopology != configv1.SingleReplicaTopologyMode {
136+
if ShouldDeployHA(infrastructureConfig) {
128137
replicas = int32(DefaultConsoleReplicas)
129138
}
130139
deployment.Spec.Replicas = &replicas
@@ -136,7 +145,7 @@ func withAffinity(
136145
component string,
137146
) {
138147
affinity := &corev1.Affinity{}
139-
if infrastructureConfig.Status.ControlPlaneTopology != configv1.SingleReplicaTopologyMode {
148+
if ShouldDeployHA(infrastructureConfig) {
140149
affinity = &corev1.Affinity{
141150
PodAntiAffinity: &corev1.PodAntiAffinity{
142151
RequiredDuringSchedulingIgnoredDuringExecution: []corev1.PodAffinityTerm{{
@@ -159,7 +168,7 @@ func withAffinity(
159168

160169
func withStrategy(deployment *appsv1.Deployment, infrastructureConfig *configv1.Infrastructure) {
161170
rollingUpdateParams := &appsv1.RollingUpdateDeployment{}
162-
if infrastructureConfig.Status.InfrastructureTopology != configv1.SingleReplicaTopologyMode {
171+
if ShouldDeployHA(infrastructureConfig) {
163172
rollingUpdateParams = &appsv1.RollingUpdateDeployment{
164173
MaxSurge: &intstr.IntOrString{
165174
IntVal: int32(3),

‎pkg/console/subresource/deployment/deployment_test.go‎

Lines changed: 144 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -166,9 +166,12 @@ func TestDefaultDeployment(t *testing.T) {
166166
},
167167
}
168168

169-
infrastructureConfigHighlyAvailable := infrastructureConfigWithTopology(configv1.HighlyAvailableTopologyMode)
170-
infrastructureConfigSingleReplica := infrastructureConfigWithTopology(configv1.SingleReplicaTopologyMode)
171-
infrastructureConfigExternalTopologyMode := infrastructureConfigWithTopology(configv1.ExternalTopologyMode)
169+
infrastructureConfigHighlyAvailable := infrastructureConfigWithTopology(configv1.HighlyAvailableTopologyMode,
170+
configv1.HighlyAvailableTopologyMode)
171+
infrastructureConfigSingleReplica := infrastructureConfigWithTopology(configv1.SingleReplicaTopologyMode,
172+
configv1.SingleReplicaTopologyMode)
173+
infrastructureConfigExternalTopologyMode := infrastructureConfigWithTopology(configv1.ExternalTopologyMode,
174+
configv1.HighlyAvailableTopologyMode)
172175
consoleDeploymentTemplate := resourceread.ReadDeploymentV1OrDie(bindata.MustAsset("assets/deployments/console-deployment.yaml"))
173176
withConsoleContainerImage(consoleDeploymentTemplate, consoleOperatorConfig, proxyConfig)
174177
withConsoleVolumes(consoleDeploymentTemplate, &corev1.ConfigMap{
@@ -670,8 +673,14 @@ func TestWithReplicas(t *testing.T) {
670673
infrastructureConfig *configv1.Infrastructure
671674
}
672675

673-
infrastructureConfigHighlyAvailable := infrastructureConfigWithTopology(configv1.HighlyAvailableTopologyMode)
674-
infrastructureConfigSingleReplica := infrastructureConfigWithTopology(configv1.SingleReplicaTopologyMode)
676+
infrastructureConfigHighlyAvailable := infrastructureConfigWithTopology(configv1.HighlyAvailableTopologyMode,
677+
configv1.HighlyAvailableTopologyMode)
678+
infrastructureConfigSingleReplica := infrastructureConfigWithTopology(configv1.SingleReplicaTopologyMode,
679+
configv1.SingleReplicaTopologyMode)
680+
infrastructureConfigExternalCPSingleReplica := infrastructureConfigWithTopology(configv1.ExternalTopologyMode,
681+
configv1.SingleReplicaTopologyMode)
682+
infrastructureConfigExternalCPHighlyAvailable := infrastructureConfigWithTopology(configv1.ExternalTopologyMode,
683+
configv1.HighlyAvailableTopologyMode)
675684

676685
tests := []struct {
677686
name string
@@ -706,6 +715,34 @@ func TestWithReplicas(t *testing.T) {
706715
},
707716
},
708717
},
718+
{
719+
name: "Test External CP with Single Replica workers",
720+
args: args{
721+
deployment: &appsv1.Deployment{
722+
Spec: appsv1.DeploymentSpec{},
723+
},
724+
infrastructureConfig: infrastructureConfigExternalCPSingleReplica,
725+
},
726+
want: &appsv1.Deployment{
727+
Spec: appsv1.DeploymentSpec{
728+
Replicas: &singleNodeReplicaCount,
729+
},
730+
},
731+
},
732+
{
733+
name: "Test External CP with Highly Available workers",
734+
args: args{
735+
deployment: &appsv1.Deployment{
736+
Spec: appsv1.DeploymentSpec{},
737+
},
738+
infrastructureConfig: infrastructureConfigExternalCPHighlyAvailable,
739+
},
740+
want: &appsv1.Deployment{
741+
Spec: appsv1.DeploymentSpec{
742+
Replicas: &defaultReplicaCount,
743+
},
744+
},
745+
},
709746
}
710747
for _, tt := range tests {
711748
t.Run(tt.name, func(t *testing.T) {
@@ -724,8 +761,12 @@ func TestWithAffinity(t *testing.T) {
724761
component string
725762
}
726763

727-
infrastructureConfigHighlyAvailable := infrastructureConfigWithTopology(configv1.HighlyAvailableTopologyMode)
728-
infrastructureConfigSingleReplica := infrastructureConfigWithTopology(configv1.SingleReplicaTopologyMode)
764+
infrastructureConfigHighlyAvailable := infrastructureConfigWithTopology(configv1.HighlyAvailableTopologyMode, configv1.HighlyAvailableTopologyMode)
765+
infrastructureConfigSingleReplica := infrastructureConfigWithTopology(configv1.SingleReplicaTopologyMode, configv1.SingleReplicaTopologyMode)
766+
infrastructureConfigExternalCPSingleReplica := infrastructureConfigWithTopology(configv1.ExternalTopologyMode,
767+
configv1.SingleReplicaTopologyMode)
768+
infrastructureConfigExternalCPHighlyAvailable := infrastructureConfigWithTopology(configv1.ExternalTopologyMode,
769+
configv1.HighlyAvailableTopologyMode)
729770

730771
tests := []struct {
731772
name string
@@ -785,6 +826,59 @@ func TestWithAffinity(t *testing.T) {
785826
},
786827
},
787828
},
829+
{
830+
name: "Test Single Replica Affinity in externalized control plane with Single Replica workers",
831+
args: args{
832+
deployment: &appsv1.Deployment{
833+
Spec: appsv1.DeploymentSpec{},
834+
},
835+
infrastructureConfig: infrastructureConfigExternalCPSingleReplica,
836+
component: "ui",
837+
},
838+
want: &appsv1.Deployment{
839+
Spec: appsv1.DeploymentSpec{
840+
Template: corev1.PodTemplateSpec{
841+
Spec: corev1.PodSpec{
842+
Affinity: &corev1.Affinity{},
843+
},
844+
},
845+
},
846+
},
847+
},
848+
{
849+
name: "Test Highly Available Affinity in externalized control plane with Highly Available workers",
850+
args: args{
851+
deployment: &appsv1.Deployment{
852+
Spec: appsv1.DeploymentSpec{},
853+
},
854+
infrastructureConfig: infrastructureConfigExternalCPHighlyAvailable,
855+
component: "foobar",
856+
},
857+
want: &appsv1.Deployment{
858+
Spec: appsv1.DeploymentSpec{
859+
Template: corev1.PodTemplateSpec{
860+
Spec: corev1.PodSpec{
861+
Affinity: &corev1.Affinity{
862+
PodAntiAffinity: &corev1.PodAntiAffinity{
863+
RequiredDuringSchedulingIgnoredDuringExecution: []corev1.PodAffinityTerm{{
864+
LabelSelector: &metav1.LabelSelector{
865+
MatchExpressions: []metav1.LabelSelectorRequirement{
866+
{
867+
Key: "component",
868+
Operator: metav1.LabelSelectorOpIn,
869+
Values: []string{"foobar"},
870+
},
871+
},
872+
},
873+
TopologyKey: "kubernetes.io/hostname",
874+
}},
875+
},
876+
},
877+
},
878+
},
879+
},
880+
},
881+
},
788882
}
789883
for _, tt := range tests {
790884
t.Run(tt.name, func(t *testing.T) {
@@ -1182,8 +1276,10 @@ func TestWithStrategy(t *testing.T) {
11821276
infrastructureConfig *configv1.Infrastructure
11831277
}
11841278

1185-
infrastructureConfigHighlyAvailable := infrastructureConfigWithTopology(configv1.HighlyAvailableTopologyMode)
1186-
infrastructureConfigSingleReplica := infrastructureConfigWithTopology(configv1.SingleReplicaTopologyMode)
1279+
infrastructureConfigHighlyAvailable := infrastructureConfigWithTopology(configv1.HighlyAvailableTopologyMode, configv1.HighlyAvailableTopologyMode)
1280+
infrastructureConfigSingleReplica := infrastructureConfigWithTopology(configv1.SingleReplicaTopologyMode, configv1.SingleReplicaTopologyMode)
1281+
infrastructureConfigExternalTopologyHighlyAvailable := infrastructureConfigWithTopology(configv1.ExternalTopologyMode, configv1.HighlyAvailableTopologyMode)
1282+
infrastructureConfigExternalTopologySingleReplica := infrastructureConfigWithTopology(configv1.ExternalTopologyMode, configv1.SingleReplicaTopologyMode)
11871283

11881284
singleReplicaStrategy := appsv1.RollingUpdateDeployment{}
11891285
highAvailStrategy := appsv1.RollingUpdateDeployment{
@@ -1228,6 +1324,34 @@ func TestWithStrategy(t *testing.T) {
12281324
},
12291325
},
12301326
},
1327+
{
1328+
name: "Test Single Replica Strategy in externalized control plane with Single Replica workers",
1329+
args: args{
1330+
deployment: &appsv1.Deployment{},
1331+
infrastructureConfig: infrastructureConfigExternalTopologySingleReplica,
1332+
},
1333+
want: &appsv1.Deployment{
1334+
Spec: appsv1.DeploymentSpec{
1335+
Strategy: appsv1.DeploymentStrategy{
1336+
RollingUpdate: &singleReplicaStrategy,
1337+
},
1338+
},
1339+
},
1340+
},
1341+
{
1342+
name: "Test Highly Available Strategy in externalized control plane with Highly Available workers",
1343+
args: args{
1344+
deployment: &appsv1.Deployment{},
1345+
infrastructureConfig: infrastructureConfigExternalTopologyHighlyAvailable,
1346+
},
1347+
want: &appsv1.Deployment{
1348+
Spec: appsv1.DeploymentSpec{
1349+
Strategy: appsv1.DeploymentStrategy{
1350+
RollingUpdate: &highAvailStrategy,
1351+
},
1352+
},
1353+
},
1354+
},
12311355
}
12321356
for _, tt := range tests {
12331357
t.Run(tt.name, func(t *testing.T) {
@@ -1245,9 +1369,10 @@ func TestWithConsoleNodeSelector(t *testing.T) {
12451369
infrastructureConfig *configv1.Infrastructure
12461370
}
12471371

1248-
infrastructureConfigSingleReplica := infrastructureConfigWithTopology(configv1.SingleReplicaTopologyMode)
1249-
infrastructureConfigExternalTopology := infrastructureConfigSingleReplica
1250-
infrastructureConfigExternalTopology.Status.ControlPlaneTopology = configv1.ExternalTopologyMode
1372+
infrastructureConfigSingleReplica := infrastructureConfigWithTopology(configv1.SingleReplicaTopologyMode,
1373+
configv1.SingleReplicaTopologyMode)
1374+
infrastructureConfigExternalTopology := infrastructureConfigWithTopology(configv1.ExternalTopologyMode,
1375+
configv1.SingleReplicaTopologyMode)
12511376
defaultDeployment := appsv1.Deployment{
12521377
Spec: appsv1.DeploymentSpec{
12531378
Template: corev1.PodTemplateSpec{
@@ -1344,8 +1469,10 @@ func TestDefaultDownloadsDeployment(t *testing.T) {
13441469
Finalizers: nil,
13451470
}
13461471

1347-
infrastructureConfigHighlyAvailable := infrastructureConfigWithTopology(configv1.HighlyAvailableTopologyMode)
1348-
infrastructureConfigSingleReplica := infrastructureConfigWithTopology(configv1.SingleReplicaTopologyMode)
1472+
infrastructureConfigHighlyAvailable := infrastructureConfigWithTopology(configv1.HighlyAvailableTopologyMode,
1473+
configv1.HighlyAvailableTopologyMode)
1474+
infrastructureConfigSingleReplica := infrastructureConfigWithTopology(configv1.SingleReplicaTopologyMode,
1475+
configv1.SingleReplicaTopologyMode)
13491476

13501477
downloadsDeploymentPodSpecSingleReplica := corev1.PodSpec{
13511478
NodeSelector: map[string]string{
@@ -1791,13 +1918,13 @@ func TestIsAvailableAndUpdated(t *testing.T) {
17911918

17921919
}
17931920

1794-
func infrastructureConfigWithTopology(topologyMode configv1.TopologyMode) *configv1.Infrastructure {
1921+
func infrastructureConfigWithTopology(controlPlaneTopologyMode, infrastructureTopologyMode configv1.TopologyMode) *configv1.Infrastructure {
17951922
return &configv1.Infrastructure{
17961923
TypeMeta: metav1.TypeMeta{},
17971924
ObjectMeta: metav1.ObjectMeta{},
17981925
Status: configv1.InfrastructureStatus{
1799-
InfrastructureTopology: topologyMode,
1800-
ControlPlaneTopology: topologyMode,
1926+
InfrastructureTopology: infrastructureTopologyMode,
1927+
ControlPlaneTopology: controlPlaneTopologyMode,
18011928
},
18021929
}
18031930
}

‎test/e2e/deployment_replicas_test.go‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ import (
77

88
appsv1 "k8s.io/api/apps/v1"
99

10-
configv1 "github.com/openshift/api/config/v1"
1110
operatorsv1 "github.com/openshift/api/operator/v1"
1211
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
1312

@@ -34,10 +33,10 @@ func TestDeploymentsReplicas(t *testing.T) {
3433
}
3534

3635
var expectedReplicas int32
37-
if infrastructureConfig.Status.ControlPlaneTopology == configv1.SingleReplicaTopologyMode {
38-
expectedReplicas = int32(deploymentsub.SingleNodeConsoleReplicas)
39-
} else {
36+
if deploymentsub.ShouldDeployHA(infrastructureConfig) {
4037
expectedReplicas = int32(deploymentsub.DefaultConsoleReplicas)
38+
} else {
39+
expectedReplicas = int32(deploymentsub.SingleNodeConsoleReplicas)
4140
}
4241

4342
consoleDeployment, err := framework.GetConsoleDeployment(client)

0 commit comments

Comments
 (0)