Skip to content

Commit 5ca09b0

Browse files
openshift-merge-bot[bot]openshift-merge-bot[bot]
authored
Merge pull request #841 from aleskandro/master
OCPBUGS-27335: use InfrastructureTopology for clusters using external CP as the console deploys on the worker nodes
2 parents 67c10e6 + fae2aec commit 5ca09b0

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
@@ -125,9 +125,18 @@ func DefaultDownloadsDeployment(
125125
return downloadsDeployment
126126
}
127127

128+
// ShouldDeployHA returns true if the console should be deployed in HA mode.
129+
// If the control plane is externalized, the console should be deployed in HA mode based on the InfrastructureTopology,
130+
// otherwise it should be deployed in HA mode based on the ControlPlaneTopology.
131+
func ShouldDeployHA(infrastructureConfig *configv1.Infrastructure) bool {
132+
return infrastructureConfig.Status.ControlPlaneTopology == configv1.HighlyAvailableTopologyMode ||
133+
(infrastructureConfig.Status.ControlPlaneTopology == configv1.ExternalTopologyMode &&
134+
infrastructureConfig.Status.InfrastructureTopology == configv1.HighlyAvailableTopologyMode)
135+
}
136+
128137
func withReplicas(deployment *appsv1.Deployment, infrastructureConfig *configv1.Infrastructure) {
129138
replicas := int32(SingleNodeConsoleReplicas)
130-
if infrastructureConfig.Status.ControlPlaneTopology != configv1.SingleReplicaTopologyMode {
139+
if ShouldDeployHA(infrastructureConfig) {
131140
replicas = int32(DefaultConsoleReplicas)
132141
}
133142
deployment.Spec.Replicas = &replicas
@@ -139,7 +148,7 @@ func withAffinity(
139148
component string,
140149
) {
141150
affinity := &corev1.Affinity{}
142-
if infrastructureConfig.Status.ControlPlaneTopology != configv1.SingleReplicaTopologyMode {
151+
if ShouldDeployHA(infrastructureConfig) {
143152
affinity = &corev1.Affinity{
144153
PodAntiAffinity: &corev1.PodAntiAffinity{
145154
RequiredDuringSchedulingIgnoredDuringExecution: []corev1.PodAffinityTerm{{
@@ -162,7 +171,7 @@ func withAffinity(
162171

163172
func withStrategy(deployment *appsv1.Deployment, infrastructureConfig *configv1.Infrastructure) {
164173
rollingUpdateParams := &appsv1.RollingUpdateDeployment{}
165-
if infrastructureConfig.Status.InfrastructureTopology != configv1.SingleReplicaTopologyMode {
174+
if ShouldDeployHA(infrastructureConfig) {
166175
rollingUpdateParams = &appsv1.RollingUpdateDeployment{
167176
MaxSurge: &intstr.IntOrString{
168177
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{
@@ -669,8 +672,14 @@ func TestWithReplicas(t *testing.T) {
669672
infrastructureConfig *configv1.Infrastructure
670673
}
671674

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

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

726-
infrastructureConfigHighlyAvailable := infrastructureConfigWithTopology(configv1.HighlyAvailableTopologyMode)
727-
infrastructureConfigSingleReplica := infrastructureConfigWithTopology(configv1.SingleReplicaTopologyMode)
763+
infrastructureConfigHighlyAvailable := infrastructureConfigWithTopology(configv1.HighlyAvailableTopologyMode, configv1.HighlyAvailableTopologyMode)
764+
infrastructureConfigSingleReplica := infrastructureConfigWithTopology(configv1.SingleReplicaTopologyMode, configv1.SingleReplicaTopologyMode)
765+
infrastructureConfigExternalCPSingleReplica := infrastructureConfigWithTopology(configv1.ExternalTopologyMode,
766+
configv1.SingleReplicaTopologyMode)
767+
infrastructureConfigExternalCPHighlyAvailable := infrastructureConfigWithTopology(configv1.ExternalTopologyMode,
768+
configv1.HighlyAvailableTopologyMode)
728769

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

1184-
infrastructureConfigHighlyAvailable := infrastructureConfigWithTopology(configv1.HighlyAvailableTopologyMode)
1185-
infrastructureConfigSingleReplica := infrastructureConfigWithTopology(configv1.SingleReplicaTopologyMode)
1278+
infrastructureConfigHighlyAvailable := infrastructureConfigWithTopology(configv1.HighlyAvailableTopologyMode, configv1.HighlyAvailableTopologyMode)
1279+
infrastructureConfigSingleReplica := infrastructureConfigWithTopology(configv1.SingleReplicaTopologyMode, configv1.SingleReplicaTopologyMode)
1280+
infrastructureConfigExternalTopologyHighlyAvailable := infrastructureConfigWithTopology(configv1.ExternalTopologyMode, configv1.HighlyAvailableTopologyMode)
1281+
infrastructureConfigExternalTopologySingleReplica := infrastructureConfigWithTopology(configv1.ExternalTopologyMode, configv1.SingleReplicaTopologyMode)
11861282

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

1247-
infrastructureConfigSingleReplica := infrastructureConfigWithTopology(configv1.SingleReplicaTopologyMode)
1248-
infrastructureConfigExternalTopology := infrastructureConfigSingleReplica
1249-
infrastructureConfigExternalTopology.Status.ControlPlaneTopology = configv1.ExternalTopologyMode
1371+
infrastructureConfigSingleReplica := infrastructureConfigWithTopology(configv1.SingleReplicaTopologyMode,
1372+
configv1.SingleReplicaTopologyMode)
1373+
infrastructureConfigExternalTopology := infrastructureConfigWithTopology(configv1.ExternalTopologyMode,
1374+
configv1.SingleReplicaTopologyMode)
12501375
defaultDeployment := appsv1.Deployment{
12511376
Spec: appsv1.DeploymentSpec{
12521377
Template: corev1.PodTemplateSpec{
@@ -1343,8 +1468,10 @@ func TestDefaultDownloadsDeployment(t *testing.T) {
13431468
Finalizers: nil,
13441469
}
13451470

1346-
infrastructureConfigHighlyAvailable := infrastructureConfigWithTopology(configv1.HighlyAvailableTopologyMode)
1347-
infrastructureConfigSingleReplica := infrastructureConfigWithTopology(configv1.SingleReplicaTopologyMode)
1471+
infrastructureConfigHighlyAvailable := infrastructureConfigWithTopology(configv1.HighlyAvailableTopologyMode,
1472+
configv1.HighlyAvailableTopologyMode)
1473+
infrastructureConfigSingleReplica := infrastructureConfigWithTopology(configv1.SingleReplicaTopologyMode,
1474+
configv1.SingleReplicaTopologyMode)
13481475

13491476
downloadsDeploymentPodSpecSingleReplica := corev1.PodSpec{
13501477
NodeSelector: map[string]string{
@@ -1790,13 +1917,13 @@ func TestIsAvailableAndUpdated(t *testing.T) {
17901917

17911918
}
17921919

1793-
func infrastructureConfigWithTopology(topologyMode configv1.TopologyMode) *configv1.Infrastructure {
1920+
func infrastructureConfigWithTopology(controlPlaneTopologyMode, infrastructureTopologyMode configv1.TopologyMode) *configv1.Infrastructure {
17941921
return &configv1.Infrastructure{
17951922
TypeMeta: metav1.TypeMeta{},
17961923
ObjectMeta: metav1.ObjectMeta{},
17971924
Status: configv1.InfrastructureStatus{
1798-
InfrastructureTopology: topologyMode,
1799-
ControlPlaneTopology: topologyMode,
1925+
InfrastructureTopology: infrastructureTopologyMode,
1926+
ControlPlaneTopology: controlPlaneTopologyMode,
18001927
},
18011928
}
18021929
}

‎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)