Skip to content

Commit 3628e51

Browse files
openshift-merge-bot[bot]openshift-merge-bot[bot]
authored
Merge pull request #315 from lunarwhite/allow-configure-replicas
CM-718: Enhance tests for overriding replicas of HA deployment
2 parents d8947cd + 88786ee commit 3628e51

File tree

6 files changed

+370
-59
lines changed

6 files changed

+370
-59
lines changed

‎api/operator/v1alpha1/certmanager_types.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,8 @@ type DeploymentConfig struct {
8282
// +optional
8383
OverrideResources CertManagerResourceRequirements `json:"overrideResources,omitempty"`
8484

85-
// OverrideReplicas defines the number of replicas to run for an operand
85+
// OverrideReplicas defines the number of replicas for the operand deployment.
86+
// If not specified, the default replicas from the deployment manifest will be used.
8687
// +kubebuilder:validation:Optional
8788
// +kubebuilder:validation:Minimum=1
8889
// +optional

‎bundle/manifests/operator.openshift.io_certmanagers.yaml‎

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -178,8 +178,9 @@ spec:
178178
type: string
179179
type: object
180180
overrideReplicas:
181-
description: OverrideReplicas defines the number of replicas to
182-
run for an operand
181+
description: |-
182+
OverrideReplicas defines the number of replicas for the operand deployment.
183+
If not specified, the default replicas from the deployment manifest will be used.
183184
format: int32
184185
minimum: 1
185186
type: integer
@@ -420,8 +421,9 @@ spec:
420421
type: string
421422
type: object
422423
overrideReplicas:
423-
description: OverrideReplicas defines the number of replicas to
424-
run for an operand
424+
description: |-
425+
OverrideReplicas defines the number of replicas for the operand deployment.
426+
If not specified, the default replicas from the deployment manifest will be used.
425427
format: int32
426428
minimum: 1
427429
type: integer
@@ -707,8 +709,9 @@ spec:
707709
type: string
708710
type: object
709711
overrideReplicas:
710-
description: OverrideReplicas defines the number of replicas to
711-
run for an operand
712+
description: |-
713+
OverrideReplicas defines the number of replicas for the operand deployment.
714+
If not specified, the default replicas from the deployment manifest will be used.
712715
format: int32
713716
minimum: 1
714717
type: integer

‎config/crd/bases/operator.openshift.io_certmanagers.yaml‎

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -178,8 +178,9 @@ spec:
178178
type: string
179179
type: object
180180
overrideReplicas:
181-
description: OverrideReplicas defines the number of replicas to
182-
run for an operand
181+
description: |-
182+
OverrideReplicas defines the number of replicas for the operand deployment.
183+
If not specified, the default replicas from the deployment manifest will be used.
183184
format: int32
184185
minimum: 1
185186
type: integer
@@ -420,8 +421,9 @@ spec:
420421
type: string
421422
type: object
422423
overrideReplicas:
423-
description: OverrideReplicas defines the number of replicas to
424-
run for an operand
424+
description: |-
425+
OverrideReplicas defines the number of replicas for the operand deployment.
426+
If not specified, the default replicas from the deployment manifest will be used.
425427
format: int32
426428
minimum: 1
427429
type: integer
@@ -707,8 +709,9 @@ spec:
707709
type: string
708710
type: object
709711
overrideReplicas:
710-
description: OverrideReplicas defines the number of replicas to
711-
run for an operand
712+
description: |-
713+
OverrideReplicas defines the number of replicas for the operand deployment.
714+
If not specified, the default replicas from the deployment manifest will be used.
712715
format: int32
713716
minimum: 1
714717
type: integer

‎pkg/controller/deployment/deployment_helper_test.go‎

Lines changed: 62 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import (
1818
"k8s.io/apimachinery/pkg/watch"
1919
clienttesting "k8s.io/client-go/testing"
2020
"k8s.io/client-go/tools/cache"
21+
"k8s.io/utils/ptr"
2122
)
2223

2324
func TestMergeContainerResources(t *testing.T) {
@@ -946,8 +947,6 @@ func TestGetOverrideSchedulingFor(t *testing.T) {
946947
}
947948

948949
func TestGetOverrideReplicasFor(t *testing.T) {
949-
ptr := func(i int32) *int32 { return &i }
950-
951950
tests := []struct {
952951
name string
953952
certManagerObj v1alpha1.CertManager
@@ -962,27 +961,27 @@ func TestGetOverrideReplicasFor(t *testing.T) {
962961
},
963962
Spec: v1alpha1.CertManagerSpec{
964963
ControllerConfig: &v1alpha1.DeploymentConfig{
965-
OverrideReplicas: ptr(2),
964+
OverrideReplicas: ptr.To(int32(3)),
966965
},
967966
},
968967
},
969968
deploymentName: certmanagerControllerDeployment,
970-
expectedOverrideReplicas: ptr(2),
969+
expectedOverrideReplicas: ptr.To(int32(3)),
971970
},
972971
{
973-
name: "get override scheduling of cert manager webhook config",
972+
name: "get override replicas of cert manager webhook config",
974973
certManagerObj: v1alpha1.CertManager{
975974
ObjectMeta: metav1.ObjectMeta{
976975
Name: "cluster",
977976
},
978977
Spec: v1alpha1.CertManagerSpec{
979978
WebhookConfig: &v1alpha1.DeploymentConfig{
980-
OverrideReplicas: ptr(0),
979+
OverrideReplicas: ptr.To(int32(4)),
981980
},
982981
},
983982
},
984983
deploymentName: certmanagerWebhookDeployment,
985-
expectedOverrideReplicas: ptr(0),
984+
expectedOverrideReplicas: ptr.To(int32(4)),
986985
},
987986
{
988987
name: "get override replicas of cert manager cainjector config",
@@ -992,23 +991,69 @@ func TestGetOverrideReplicasFor(t *testing.T) {
992991
},
993992
Spec: v1alpha1.CertManagerSpec{
994993
CAInjectorConfig: &v1alpha1.DeploymentConfig{
995-
OverrideReplicas: ptr(2),
994+
OverrideReplicas: ptr.To(int32(2)),
996995
},
997996
},
998997
},
999998
deploymentName: certmanagerCAinjectorDeployment,
1000-
expectedOverrideReplicas: ptr(2),
999+
expectedOverrideReplicas: ptr.To(int32(2)),
1000+
},
1001+
{
1002+
name: "get nil override replicas when config exists but replicas field is nil for controller",
1003+
certManagerObj: v1alpha1.CertManager{
1004+
ObjectMeta: metav1.ObjectMeta{
1005+
Name: "cluster",
1006+
},
1007+
Spec: v1alpha1.CertManagerSpec{
1008+
ControllerConfig: &v1alpha1.DeploymentConfig{
1009+
OverrideArgs: []string{"--v=3"},
1010+
OverrideReplicas: nil,
1011+
},
1012+
},
1013+
},
1014+
deploymentName: certmanagerControllerDeployment,
1015+
expectedOverrideReplicas: nil,
1016+
},
1017+
{
1018+
name: "get nil override replicas when config exists but replicas field is nil for webhook",
1019+
certManagerObj: v1alpha1.CertManager{
1020+
ObjectMeta: metav1.ObjectMeta{
1021+
Name: "cluster",
1022+
},
1023+
Spec: v1alpha1.CertManagerSpec{
1024+
WebhookConfig: &v1alpha1.DeploymentConfig{
1025+
OverrideEnv: []corev1.EnvVar{{Name: "TEST", Value: "value"}},
1026+
OverrideReplicas: nil,
1027+
},
1028+
},
1029+
},
1030+
deploymentName: certmanagerWebhookDeployment,
1031+
expectedOverrideReplicas: nil,
10011032
},
10021033
{
1003-
name: "no override replicas configured",
1034+
name: "get nil override replicas when config exists but replicas field is nil for cainjector",
10041035
certManagerObj: v1alpha1.CertManager{
10051036
ObjectMeta: metav1.ObjectMeta{
10061037
Name: "cluster",
10071038
},
10081039
Spec: v1alpha1.CertManagerSpec{
1009-
// no configs set
1040+
CAInjectorConfig: &v1alpha1.DeploymentConfig{
1041+
OverrideLabels: map[string]string{"test": "label"},
1042+
OverrideReplicas: nil,
1043+
},
10101044
},
10111045
},
1046+
deploymentName: certmanagerCAinjectorDeployment,
1047+
expectedOverrideReplicas: nil,
1048+
},
1049+
{
1050+
name: "get nil override replicas when cert manager config is not set",
1051+
certManagerObj: v1alpha1.CertManager{
1052+
ObjectMeta: metav1.ObjectMeta{
1053+
Name: "cluster",
1054+
},
1055+
Spec: v1alpha1.CertManagerSpec{},
1056+
},
10121057
deploymentName: certmanagerControllerDeployment,
10131058
expectedOverrideReplicas: nil,
10141059
},
@@ -1085,7 +1130,12 @@ func TestGetOverrideReplicasFor(t *testing.T) {
10851130

10861131
actualOverrideReplicas, err := getOverrideReplicasFor(certManagerInformers, tc.deploymentName)
10871132
assert.NoError(t, err)
1088-
require.Equal(t, tc.expectedOverrideReplicas, actualOverrideReplicas)
1133+
if tc.expectedOverrideReplicas == nil {
1134+
assert.Nil(t, actualOverrideReplicas)
1135+
} else {
1136+
require.NotNil(t, actualOverrideReplicas)
1137+
assert.Equal(t, *tc.expectedOverrideReplicas, *actualOverrideReplicas)
1138+
}
10891139

10901140
// Delete the cert manager object using the fake client.
10911141
err = fakeClient.OperatorV1alpha1().CertManagers().Delete(ctx, tc.certManagerObj.Name, metav1.DeleteOptions{})

‎test/e2e/overrides_test.go‎

Lines changed: 136 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,13 @@ import (
1414
corev1 "k8s.io/api/core/v1"
1515
k8sresource "k8s.io/apimachinery/pkg/api/resource"
1616
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
17+
"k8s.io/utils/ptr"
18+
)
19+
20+
const (
21+
defaultControllerReplicasCount = 1
22+
defaultWebhookReplicasCount = 1
23+
defaultCAInjectorReplicasCount = 1
1724
)
1825

1926
var _ = Describe("Overrides test", Ordered, func() {
@@ -594,6 +601,135 @@ var _ = Describe("Overrides test", Ordered, func() {
594601
})
595602
})
596603

604+
Context("When adding valid cert-manager controller override replicas", func() {
605+
606+
It("should set the replicas for the cert-manager controller deployment", func() {
607+
608+
By("Adding cert-manager controller override replicas to the cert-manager operator object")
609+
replicas := ptr.To(int32(2))
610+
err := addOverrideReplicas(certmanageroperatorclient, certmanagerControllerDeployment, replicas)
611+
Expect(err).NotTo(HaveOccurred())
612+
613+
By("Waiting for cert-manager controller status to become available")
614+
err = verifyOperatorStatusCondition(certmanageroperatorclient.OperatorV1alpha1(), GenerateConditionMatchers(
615+
[]PrefixAndMatchTypeTuple{{certManagerController, MatchAllConditions}}, validOperatorStatusConditions,
616+
))
617+
Expect(err).NotTo(HaveOccurred())
618+
619+
By("Waiting for the replicas to be set on the cert-manager controller deployment")
620+
err = verifyDeploymentReplicas(k8sClientSet, certmanagerControllerDeployment, replicas, true)
621+
Expect(err).NotTo(HaveOccurred())
622+
623+
By("Waiting for the cert-manager controller deployment to be Available")
624+
err = pollTillDeploymentAvailable(context.Background(), k8sClientSet, operandNamespace, certmanagerControllerDeployment)
625+
Expect(err).NotTo(HaveOccurred())
626+
627+
By("Removing override replicas for cert-manager controller")
628+
err = addOverrideReplicas(certmanageroperatorclient, certmanagerControllerDeployment, nil)
629+
Expect(err).NotTo(HaveOccurred())
630+
631+
By("Waiting for cert-manager controller status to become available after removing override")
632+
err = verifyOperatorStatusCondition(certmanageroperatorclient.OperatorV1alpha1(), GenerateConditionMatchers(
633+
[]PrefixAndMatchTypeTuple{{certManagerController, MatchAllConditions}}, validOperatorStatusConditions,
634+
))
635+
Expect(err).NotTo(HaveOccurred())
636+
637+
By("Verifying cert-manager controller rollback to default replica count")
638+
err = verifyDeploymentReplicas(k8sClientSet, certmanagerControllerDeployment, ptr.To(int32(defaultControllerReplicasCount)), true)
639+
Expect(err).NotTo(HaveOccurred())
640+
641+
By("Waiting for the cert-manager controller deployment to be Available after rollback")
642+
err = pollTillDeploymentAvailable(context.Background(), k8sClientSet, operandNamespace, certmanagerControllerDeployment)
643+
Expect(err).NotTo(HaveOccurred())
644+
})
645+
})
646+
647+
Context("When adding valid cert-manager webhook override replicas", func() {
648+
649+
It("should set the replicas for the cert-manager webhook deployment", func() {
650+
651+
By("Adding cert-manager webhook override replicas to the cert-manager operator object")
652+
replicas := ptr.To(int32(3))
653+
err := addOverrideReplicas(certmanageroperatorclient, certmanagerWebhookDeployment, replicas)
654+
Expect(err).NotTo(HaveOccurred())
655+
656+
By("Waiting for cert-manager webhook status to become available")
657+
err = verifyOperatorStatusCondition(certmanageroperatorclient.OperatorV1alpha1(), GenerateConditionMatchers(
658+
[]PrefixAndMatchTypeTuple{{certManagerWebhook, MatchAllConditions}}, validOperatorStatusConditions,
659+
))
660+
Expect(err).NotTo(HaveOccurred())
661+
662+
By("Waiting for the replicas to be set on the cert-manager webhook deployment")
663+
err = verifyDeploymentReplicas(k8sClientSet, certmanagerWebhookDeployment, replicas, true)
664+
Expect(err).NotTo(HaveOccurred())
665+
666+
By("Waiting for the cert-manager webhook deployment to be Available")
667+
err = pollTillDeploymentAvailable(context.Background(), k8sClientSet, operandNamespace, certmanagerWebhookDeployment)
668+
Expect(err).NotTo(HaveOccurred())
669+
670+
By("Removing override replicas for cert-manager webhook")
671+
err = addOverrideReplicas(certmanageroperatorclient, certmanagerWebhookDeployment, nil)
672+
Expect(err).NotTo(HaveOccurred())
673+
674+
By("Waiting for cert-manager webhook status to become available after removing override")
675+
err = verifyOperatorStatusCondition(certmanageroperatorclient.OperatorV1alpha1(), GenerateConditionMatchers(
676+
[]PrefixAndMatchTypeTuple{{certManagerWebhook, MatchAllConditions}}, validOperatorStatusConditions,
677+
))
678+
Expect(err).NotTo(HaveOccurred())
679+
680+
By("Verifying cert-manager webhook rollback to default replica count")
681+
err = verifyDeploymentReplicas(k8sClientSet, certmanagerWebhookDeployment, ptr.To(int32(defaultWebhookReplicasCount)), true)
682+
Expect(err).NotTo(HaveOccurred())
683+
684+
By("Waiting for the cert-manager webhook deployment to be Available after rollback")
685+
err = pollTillDeploymentAvailable(context.Background(), k8sClientSet, operandNamespace, certmanagerWebhookDeployment)
686+
Expect(err).NotTo(HaveOccurred())
687+
})
688+
})
689+
690+
Context("When adding valid cert-manager cainjector override replicas", func() {
691+
692+
It("should set the replicas for the cert-manager cainjector deployment", func() {
693+
694+
By("Adding cert-manager cainjector override replicas to the cert-manager operator object")
695+
replicas := ptr.To(int32(2))
696+
err := addOverrideReplicas(certmanageroperatorclient, certmanagerCAinjectorDeployment, replicas)
697+
Expect(err).NotTo(HaveOccurred())
698+
699+
By("Waiting for cert-manager cainjector status to become available")
700+
err = verifyOperatorStatusCondition(certmanageroperatorclient.OperatorV1alpha1(), GenerateConditionMatchers(
701+
[]PrefixAndMatchTypeTuple{{certManagerCAInjector, MatchAllConditions}}, validOperatorStatusConditions,
702+
))
703+
Expect(err).NotTo(HaveOccurred())
704+
705+
By("Waiting for the replicas to be set on the cert-manager cainjector deployment")
706+
err = verifyDeploymentReplicas(k8sClientSet, certmanagerCAinjectorDeployment, replicas, true)
707+
Expect(err).NotTo(HaveOccurred())
708+
709+
By("Waiting for the cert-manager cainjector deployment to be Available")
710+
err = pollTillDeploymentAvailable(context.Background(), k8sClientSet, operandNamespace, certmanagerCAinjectorDeployment)
711+
Expect(err).NotTo(HaveOccurred())
712+
713+
By("Removing override replicas for cert-manager cainjector")
714+
err = addOverrideReplicas(certmanageroperatorclient, certmanagerCAinjectorDeployment, nil)
715+
Expect(err).NotTo(HaveOccurred())
716+
717+
By("Waiting for cert-manager cainjector status to become available after removing override")
718+
err = verifyOperatorStatusCondition(certmanageroperatorclient.OperatorV1alpha1(), GenerateConditionMatchers(
719+
[]PrefixAndMatchTypeTuple{{certManagerCAInjector, MatchAllConditions}}, validOperatorStatusConditions,
720+
))
721+
Expect(err).NotTo(HaveOccurred())
722+
723+
By("Verifying cert-manager cainjector rollback to default replica count")
724+
err = verifyDeploymentReplicas(k8sClientSet, certmanagerCAinjectorDeployment, ptr.To(int32(defaultCAInjectorReplicasCount)), true)
725+
Expect(err).NotTo(HaveOccurred())
726+
727+
By("Waiting for the cert-manager cainjector deployment to be Available after rollback")
728+
err = pollTillDeploymentAvailable(context.Background(), k8sClientSet, operandNamespace, certmanagerCAinjectorDeployment)
729+
Expect(err).NotTo(HaveOccurred())
730+
})
731+
})
732+
597733
AfterAll(func() {
598734
By("Reset cert-manager state")
599735
err := resetCertManagerState(context.Background(), certmanageroperatorclient, loader)

0 commit comments

Comments
 (0)