Skip to content

Commit 9be31fc

Browse files
perdasilvaperdasilva
committed
Update unit tests
Signed-off-by: perdasilva <[email protected]> Upstream-repository: perdasilva Upstream-commit: fd1011590bdd0fb1200188bb26bffb76020a0024
1 parent 4cb10a8 commit 9be31fc

File tree

10 files changed

+308
-59
lines changed

10 files changed

+308
-59
lines changed

‎staging/operator-lifecycle-manager/pkg/controller/install/certresources.go

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,8 @@ const (
4646
OLMManagedLabelValue = "true"
4747

4848
// service deletion timeout
49-
serviceDeletionTimeout = 3 * time.Minute
49+
serviceDeletionTimeout = 3 * time.Minute
50+
defaultServiceDeletionPollingInterval = 5 * time.Second
5051
)
5152

5253
type certResource interface {
@@ -265,7 +266,7 @@ func (i *StrategyDeploymentInstaller) installCertRequirementsForDeployment(deplo
265266
service.SetOwnerReferences(existingService.GetOwnerReferences())
266267

267268
// Delete the Service to replace
268-
if err := i.deleteServiceWithTimeout(existingService, serviceDeletionTimeout); err != nil {
269+
if err := i.deleteServiceWithTimeout(service, serviceDeletionTimeout); err != nil {
269270
return nil, nil, err
270271
}
271272
}
@@ -546,8 +547,7 @@ func (i *StrategyDeploymentInstaller) deleteServiceWithTimeout(service *corev1.S
546547
timeout, cancelFn := context.WithTimeout(context.Background(), deletionTimeout)
547548
defer cancelFn()
548549

549-
const pollingInterval = 5 * time.Second
550-
err := wait.PollImmediateInfiniteWithContext(timeout, pollingInterval, func(ctx context.Context) (bool, error) {
550+
err := wait.PollImmediateInfiniteWithContext(timeout, i.serviceDeletionPollingInterval, func(ctx context.Context) (bool, error) {
551551
// try to delete service
552552
err := i.strategyClient.GetOpClient().DeleteService(service.GetNamespace(), service.GetName(), &metav1.DeleteOptions{})
553553
if err != nil {
@@ -558,7 +558,8 @@ func (i *StrategyDeploymentInstaller) deleteServiceWithTimeout(service *corev1.S
558558
}
559559

560560
// ensure it is deleted
561-
_, err = i.strategyClient.GetOpLister().CoreV1().ServiceLister().Services(i.owner.GetNamespace()).Get(service.GetName())
561+
_, err = i.strategyClient.GetOpClient().GetService(i.owner.GetNamespace(), service.GetName())
562+
// _, err = i.strategyClient.GetOpLister().CoreV1().ServiceLister().Services(i.owner.GetNamespace()).Get(service.GetName())
562563

563564
// successfully deleted
564565
if err != nil && apierrors.IsNotFound(err) {

‎staging/operator-lifecycle-manager/pkg/controller/install/certresources_test.go

Lines changed: 246 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -165,6 +165,7 @@ func TestInstallCertRequirementsForDeployment(t *testing.T) {
165165
Selector: selector(t, "test=label").MatchLabels,
166166
},
167167
}
168+
mockOpClient.EXPECT().GetService("", "test-service").Return(nil, errors.NewNotFound(corev1.Resource("service"), "test-service"))
168169
mockOpClient.EXPECT().CreateService(&service).Return(&service, nil)
169170

170171
hosts := []string{
@@ -389,6 +390,8 @@ func TestInstallCertRequirementsForDeployment(t *testing.T) {
389390
Selector: selector(t, "test=label").MatchLabels,
390391
},
391392
}
393+
394+
mockOpClient.EXPECT().GetService(namespace, "test-service").Return(nil, errors.NewNotFound(corev1.Resource("service"), "test-service"))
392395
mockOpClient.EXPECT().CreateService(&service).Return(&service, nil)
393396

394397
hosts := []string{
@@ -606,6 +609,8 @@ func TestInstallCertRequirementsForDeployment(t *testing.T) {
606609
Selector: selector(t, "test=label").MatchLabels,
607610
},
608611
}
612+
613+
mockOpClient.EXPECT().GetService(namespace, "test-service").Return(nil, errors.NewNotFound(corev1.Resource("service"), "test-service"))
609614
mockOpClient.EXPECT().CreateService(&service).Return(&service, nil)
610615

611616
hosts := []string{
@@ -825,6 +830,239 @@ func TestInstallCertRequirementsForDeployment(t *testing.T) {
825830
},
826831
},
827832
},
833+
{
834+
name: "retries deletion if service is still present",
835+
mockExternal: func(mockOpClient *operatorclientmocks.MockClientInterface, fakeLister *operatorlisterfakes.FakeOperatorLister, namespace string, args args) {
836+
mockOpClient.EXPECT().DeleteService(namespace, "test-service", &metav1.DeleteOptions{}).Return(nil)
837+
service := corev1.Service{
838+
ObjectMeta: metav1.ObjectMeta{
839+
Name: "test-service",
840+
OwnerReferences: []metav1.OwnerReference{
841+
ownerutil.NonBlockingOwner(&v1alpha1.ClusterServiceVersion{}),
842+
},
843+
},
844+
Spec: corev1.ServiceSpec{
845+
Ports: args.ports,
846+
Selector: selector(t, "test=label").MatchLabels,
847+
},
848+
}
849+
mockOpClient.EXPECT().GetService(namespace, "test-service").Return(&service, nil)
850+
851+
// retry
852+
mockOpClient.EXPECT().DeleteService(namespace, "test-service", &metav1.DeleteOptions{}).Return(nil)
853+
mockOpClient.EXPECT().GetService(namespace, "test-service").Return(&service, nil)
854+
mockOpClient.EXPECT().DeleteService(namespace, "test-service", &metav1.DeleteOptions{}).Return(nil)
855+
856+
// done
857+
mockOpClient.EXPECT().GetService(namespace, "test-service").Return(nil, errors.NewNotFound(corev1.Resource("service"), "test-service"))
858+
859+
mockOpClient.EXPECT().CreateService(&service).Return(&service, nil)
860+
861+
hosts := []string{
862+
fmt.Sprintf("%s.%s", service.GetName(), namespace),
863+
fmt.Sprintf("%s.%s.svc", service.GetName(), namespace),
864+
}
865+
servingPair, err := certGenerator.Generate(args.rotateAt, Organization, args.ca, hosts)
866+
require.NoError(t, err)
867+
868+
// Create Secret for serving cert
869+
certPEM, privPEM, err := servingPair.ToPEM()
870+
require.NoError(t, err)
871+
872+
secret := &corev1.Secret{
873+
ObjectMeta: metav1.ObjectMeta{
874+
Name: "test-service-cert",
875+
Namespace: namespace,
876+
Annotations: map[string]string{OLMCAHashAnnotationKey: caHash},
877+
Labels: map[string]string{OLMManagedLabelKey: OLMManagedLabelValue},
878+
},
879+
Data: map[string][]byte{
880+
"tls.crt": certPEM,
881+
"tls.key": privPEM,
882+
OLMCAPEMKey: caPEM,
883+
},
884+
Type: corev1.SecretTypeTLS,
885+
}
886+
mockOpClient.EXPECT().UpdateSecret(secret).Return(secret, nil)
887+
888+
secretRole := &rbacv1.Role{
889+
ObjectMeta: metav1.ObjectMeta{
890+
Name: secret.GetName(),
891+
Namespace: namespace,
892+
},
893+
Rules: []rbacv1.PolicyRule{
894+
{
895+
Verbs: []string{"get"},
896+
APIGroups: []string{""},
897+
Resources: []string{"secrets"},
898+
ResourceNames: []string{secret.GetName()},
899+
},
900+
},
901+
}
902+
mockOpClient.EXPECT().UpdateRole(secretRole).Return(secretRole, nil)
903+
904+
roleBinding := &rbacv1.RoleBinding{
905+
ObjectMeta: metav1.ObjectMeta{
906+
Name: secret.GetName(),
907+
Namespace: namespace,
908+
},
909+
Subjects: []rbacv1.Subject{
910+
{
911+
Kind: "ServiceAccount",
912+
APIGroup: "",
913+
Name: "test-sa",
914+
Namespace: namespace,
915+
},
916+
},
917+
RoleRef: rbacv1.RoleRef{
918+
APIGroup: "rbac.authorization.k8s.io",
919+
Kind: "Role",
920+
Name: secretRole.GetName(),
921+
},
922+
}
923+
mockOpClient.EXPECT().UpdateRoleBinding(roleBinding).Return(roleBinding, nil)
924+
925+
authDelegatorClusterRoleBinding := &rbacv1.ClusterRoleBinding{
926+
ObjectMeta: metav1.ObjectMeta{
927+
Name: service.GetName() + "-system:auth-delegator",
928+
},
929+
Subjects: []rbacv1.Subject{
930+
{
931+
Kind: "ServiceAccount",
932+
APIGroup: "",
933+
Name: "test-sa",
934+
Namespace: namespace,
935+
},
936+
},
937+
RoleRef: rbacv1.RoleRef{
938+
APIGroup: "rbac.authorization.k8s.io",
939+
Kind: "ClusterRole",
940+
Name: "system:auth-delegator",
941+
},
942+
}
943+
944+
mockOpClient.EXPECT().UpdateClusterRoleBinding(authDelegatorClusterRoleBinding).Return(authDelegatorClusterRoleBinding, nil)
945+
946+
authReaderRoleBinding := &rbacv1.RoleBinding{
947+
Subjects: []rbacv1.Subject{
948+
{
949+
Kind: "ServiceAccount",
950+
APIGroup: "",
951+
Name: args.depSpec.Template.Spec.ServiceAccountName,
952+
Namespace: namespace,
953+
},
954+
},
955+
RoleRef: rbacv1.RoleRef{
956+
APIGroup: "rbac.authorization.k8s.io",
957+
Kind: "Role",
958+
Name: "extension-apiserver-authentication-reader",
959+
},
960+
}
961+
authReaderRoleBinding.SetName(service.GetName() + "-auth-reader")
962+
authReaderRoleBinding.SetNamespace(KubeSystem)
963+
964+
mockOpClient.EXPECT().UpdateRoleBinding(authReaderRoleBinding).Return(authReaderRoleBinding, nil)
965+
},
966+
state: fakeState{
967+
existingService: &corev1.Service{
968+
ObjectMeta: metav1.ObjectMeta{
969+
OwnerReferences: []metav1.OwnerReference{
970+
ownerutil.NonBlockingOwner(&v1alpha1.ClusterServiceVersion{}),
971+
},
972+
},
973+
},
974+
existingSecret: &corev1.Secret{
975+
ObjectMeta: metav1.ObjectMeta{},
976+
},
977+
existingRole: &rbacv1.Role{
978+
ObjectMeta: metav1.ObjectMeta{},
979+
},
980+
existingRoleBinding: &rbacv1.RoleBinding{
981+
ObjectMeta: metav1.ObjectMeta{},
982+
},
983+
existingClusterRoleBinding: &rbacv1.ClusterRoleBinding{
984+
ObjectMeta: metav1.ObjectMeta{},
985+
},
986+
},
987+
fields: fields{
988+
owner: &v1alpha1.ClusterServiceVersion{},
989+
previousStrategy: nil,
990+
templateAnnotations: nil,
991+
initializers: nil,
992+
apiServiceDescriptions: []certResource{},
993+
webhookDescriptions: []certResource{},
994+
},
995+
args: args{
996+
deploymentName: "test",
997+
ca: ca,
998+
rotateAt: time.Now().Add(time.Hour),
999+
ports: []corev1.ServicePort{},
1000+
depSpec: appsv1.DeploymentSpec{
1001+
Selector: selector(t, "test=label"),
1002+
Template: corev1.PodTemplateSpec{
1003+
Spec: corev1.PodSpec{
1004+
ServiceAccountName: "test-sa",
1005+
},
1006+
ObjectMeta: metav1.ObjectMeta{
1007+
Annotations: map[string]string{
1008+
"foo": "bar",
1009+
},
1010+
},
1011+
},
1012+
},
1013+
},
1014+
want: &appsv1.DeploymentSpec{
1015+
Selector: selector(t, "test=label"),
1016+
Template: corev1.PodTemplateSpec{
1017+
ObjectMeta: metav1.ObjectMeta{
1018+
Annotations: map[string]string{
1019+
"foo": "bar",
1020+
OLMCAHashAnnotationKey: caHash},
1021+
},
1022+
Spec: corev1.PodSpec{
1023+
ServiceAccountName: "test-sa",
1024+
Volumes: []corev1.Volume{
1025+
{
1026+
Name: "apiservice-cert",
1027+
VolumeSource: corev1.VolumeSource{
1028+
Secret: &corev1.SecretVolumeSource{
1029+
SecretName: "test-service-cert",
1030+
Items: []corev1.KeyToPath{
1031+
{
1032+
Key: "tls.crt",
1033+
Path: "apiserver.crt",
1034+
},
1035+
{
1036+
Key: "tls.key",
1037+
Path: "apiserver.key",
1038+
},
1039+
},
1040+
},
1041+
},
1042+
},
1043+
{
1044+
Name: "webhook-cert",
1045+
VolumeSource: corev1.VolumeSource{
1046+
Secret: &corev1.SecretVolumeSource{
1047+
SecretName: "test-service-cert",
1048+
Items: []corev1.KeyToPath{
1049+
{
1050+
Key: "tls.crt",
1051+
Path: "tls.crt",
1052+
},
1053+
{
1054+
Key: "tls.key",
1055+
Path: "tls.key",
1056+
},
1057+
},
1058+
},
1059+
},
1060+
},
1061+
},
1062+
},
1063+
},
1064+
},
1065+
},
8281066
}
8291067
for _, tt := range tests {
8301068
t.Run(tt.name, func(t *testing.T) {
@@ -839,13 +1077,14 @@ func TestInstallCertRequirementsForDeployment(t *testing.T) {
8391077

8401078
client := wrappers.NewInstallStrategyDeploymentClient(mockOpClient, fakeLister, tt.fields.owner.GetNamespace())
8411079
i := &StrategyDeploymentInstaller{
842-
strategyClient: client,
843-
owner: tt.fields.owner,
844-
previousStrategy: tt.fields.previousStrategy,
845-
templateAnnotations: tt.fields.templateAnnotations,
846-
initializers: tt.fields.initializers,
847-
apiServiceDescriptions: tt.fields.apiServiceDescriptions,
848-
webhookDescriptions: tt.fields.webhookDescriptions,
1080+
strategyClient: client,
1081+
owner: tt.fields.owner,
1082+
previousStrategy: tt.fields.previousStrategy,
1083+
templateAnnotations: tt.fields.templateAnnotations,
1084+
initializers: tt.fields.initializers,
1085+
apiServiceDescriptions: tt.fields.apiServiceDescriptions,
1086+
webhookDescriptions: tt.fields.webhookDescriptions,
1087+
serviceDeletionPollingInterval: shortDeletionPollingInterval,
8491088
}
8501089
got, _, err := i.installCertRequirementsForDeployment(tt.args.deploymentName, tt.args.ca, tt.args.rotateAt, tt.args.depSpec, tt.args.ports)
8511090
if (err != nil) != tt.wantErr {

‎staging/operator-lifecycle-manager/pkg/controller/install/deployment.go

Lines changed: 21 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -23,15 +23,16 @@ import (
2323
const DeploymentSpecHashLabelKey = "olm.deployment-spec-hash"
2424

2525
type StrategyDeploymentInstaller struct {
26-
strategyClient wrappers.InstallStrategyDeploymentInterface
27-
owner ownerutil.Owner
28-
previousStrategy Strategy
29-
templateAnnotations map[string]string
30-
initializers DeploymentInitializerFuncChain
31-
apiServiceDescriptions []certResource
32-
webhookDescriptions []certResource
33-
certificateExpirationTime time.Time
34-
certificatesRotated bool
26+
strategyClient wrappers.InstallStrategyDeploymentInterface
27+
owner ownerutil.Owner
28+
previousStrategy Strategy
29+
templateAnnotations map[string]string
30+
initializers DeploymentInitializerFuncChain
31+
apiServiceDescriptions []certResource
32+
webhookDescriptions []certResource
33+
certificateExpirationTime time.Time
34+
certificatesRotated bool
35+
serviceDeletionPollingInterval time.Duration
3536
}
3637

3738
var _ Strategy = &v1alpha1.StrategyDetailsDeployment{}
@@ -68,7 +69,7 @@ func (c DeploymentInitializerFuncChain) Apply(deployment *appsv1.Deployment) (er
6869
// the given context.
6970
type DeploymentInitializerBuilderFunc func(owner ownerutil.Owner) DeploymentInitializerFunc
7071

71-
func NewStrategyDeploymentInstaller(strategyClient wrappers.InstallStrategyDeploymentInterface, templateAnnotations map[string]string, owner ownerutil.Owner, previousStrategy Strategy, initializers DeploymentInitializerFuncChain, apiServiceDescriptions []v1alpha1.APIServiceDescription, webhookDescriptions []v1alpha1.WebhookDescription) StrategyInstaller {
72+
func NewStrategyDeploymentInstaller(strategyClient wrappers.InstallStrategyDeploymentInterface, templateAnnotations map[string]string, owner ownerutil.Owner, previousStrategy Strategy, initializers DeploymentInitializerFuncChain, apiServiceDescriptions []v1alpha1.APIServiceDescription, webhookDescriptions []v1alpha1.WebhookDescription, serviceDeletionPollingInterval time.Duration) StrategyInstaller {
7273
apiDescs := make([]certResource, len(apiServiceDescriptions))
7374
for i := range apiServiceDescriptions {
7475
apiDescs[i] = &apiServiceDescriptionsWithCAPEM{apiServiceDescriptions[i], []byte{}}
@@ -80,15 +81,16 @@ func NewStrategyDeploymentInstaller(strategyClient wrappers.InstallStrategyDeplo
8081
}
8182

8283
return &StrategyDeploymentInstaller{
83-
strategyClient: strategyClient,
84-
owner: owner,
85-
previousStrategy: previousStrategy,
86-
templateAnnotations: templateAnnotations,
87-
initializers: initializers,
88-
apiServiceDescriptions: apiDescs,
89-
webhookDescriptions: webhookDescs,
90-
certificatesRotated: false,
91-
certificateExpirationTime: time.Time{},
84+
strategyClient: strategyClient,
85+
owner: owner,
86+
previousStrategy: previousStrategy,
87+
templateAnnotations: templateAnnotations,
88+
initializers: initializers,
89+
apiServiceDescriptions: apiDescs,
90+
webhookDescriptions: webhookDescs,
91+
certificatesRotated: false,
92+
certificateExpirationTime: time.Time{},
93+
serviceDeletionPollingInterval: serviceDeletionPollingInterval,
9294
}
9395
}
9496

0 commit comments

Comments
 (0)