Skip to content

Commit 9422c76

Browse files
openshift-merge-bot[bot]openshift-merge-bot[bot]
authored
Merge pull request #407 from theobarberbany/tb/webhook-ports-4.17
[release-4.17] OCPBUGS-60211: Add Service using common resource templating
2 parents 32fd839 + 7ad40eb commit 9422c76

File tree

4 files changed

+152
-30
lines changed

4 files changed

+152
-30
lines changed

‎pkg/cloud/cloud_test.go‎

Lines changed: 50 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -99,97 +99,109 @@ func TestGetResources(t *testing.T) {
9999
}{{
100100
name: "AWS resources returned as expected",
101101
testPlatform: platformsMap[string(configv1.AWSPlatformType)],
102-
expectedResourceCount: 2,
102+
expectedResourceCount: 3,
103103
expectedResourcesKindName: []string{
104104
"Deployment/aws-cloud-controller-manager",
105105
"PodDisruptionBudget/aws-cloud-controller-manager",
106+
"Service/aws-cloud-controller-manager",
106107
},
107108
}, {
108-
name: "AWS resources returned as expected with single node cluster",
109-
testPlatform: platformsMap[string(configv1.AWSPlatformType)],
110-
expectedResourceCount: 1,
111-
singleReplica: true,
112-
expectedResourcesKindName: []string{"Deployment/aws-cloud-controller-manager"},
109+
name: "AWS resources returned as expected with single node cluster",
110+
testPlatform: platformsMap[string(configv1.AWSPlatformType)],
111+
expectedResourceCount: 2,
112+
singleReplica: true,
113+
expectedResourcesKindName: []string{
114+
"Deployment/aws-cloud-controller-manager",
115+
"Service/aws-cloud-controller-manager",
116+
},
113117
}, {
114118
name: "OpenStack resources returned as expected",
115119
testPlatform: platformsMap[string(configv1.OpenStackPlatformType)],
116-
expectedResourceCount: 2,
120+
expectedResourceCount: 3,
117121
expectedResourcesKindName: []string{
118122
"Deployment/openstack-cloud-controller-manager",
119123
"PodDisruptionBudget/openstack-cloud-controller-manager",
124+
"Service/openstack-cloud-controller-manager",
120125
},
121126
}, {
122127
name: "OpenStack resources returned as expected with signle node cluster",
123128
testPlatform: platformsMap[string(configv1.OpenStackPlatformType)],
124-
expectedResourceCount: 1,
129+
expectedResourceCount: 2,
125130
singleReplica: true,
126131
expectedResourcesKindName: []string{
127132
"Deployment/openstack-cloud-controller-manager",
133+
"Service/openstack-cloud-controller-manager",
128134
},
129135
}, {
130136
name: "GCP resources returned as expected",
131137
testPlatform: platformsMap[string(configv1.GCPPlatformType)],
132-
expectedResourceCount: 4,
138+
expectedResourceCount: 5,
133139
expectedResourcesKindName: []string{
134140
"Deployment/gcp-cloud-controller-manager",
135141
"PodDisruptionBudget/gcp-cloud-controller-manager",
136142
"ClusterRole/gcp-cloud-controller-manager",
137143
"ClusterRoleBinding/gcp-cloud-controller-manager:cloud-provider",
144+
"Service/gcp-cloud-controller-manager",
138145
},
139146
}, {
140147
name: "GCP resources returned as expected with single node cluster",
141148
testPlatform: platformsMap[string(configv1.GCPPlatformType)],
142-
expectedResourceCount: 3,
149+
expectedResourceCount: 4,
143150
singleReplica: true,
144151
expectedResourcesKindName: []string{
145152
"Deployment/gcp-cloud-controller-manager",
146153
"ClusterRole/gcp-cloud-controller-manager",
147154
"ClusterRoleBinding/gcp-cloud-controller-manager:cloud-provider",
155+
"Service/gcp-cloud-controller-manager",
148156
},
149157
}, {
150158
name: "Azure resources returned as expected",
151159
testPlatform: platformsMap[string(configv1.AzurePlatformType)],
152-
expectedResourceCount: 5,
160+
expectedResourceCount: 6,
153161
expectedResourcesKindName: []string{
154162
"Deployment/azure-cloud-controller-manager",
155163
"DaemonSet/azure-cloud-node-manager",
156164
"ClusterRole/azure-cloud-controller-manager",
157165
"ClusterRoleBinding/cloud-controller-manager:azure-cloud-controller-manager",
158166
"PodDisruptionBudget/azure-cloud-controller-manager",
167+
"Service/azure-cloud-controller-manager",
159168
},
160169
}, {
161170
name: "Azure resources returned as expected with single node cluster",
162171
testPlatform: platformsMap[string(configv1.AzurePlatformType)],
163-
expectedResourceCount: 4,
172+
expectedResourceCount: 5,
164173
singleReplica: true,
165174
expectedResourcesKindName: []string{
166175
"Deployment/azure-cloud-controller-manager",
167176
"DaemonSet/azure-cloud-node-manager",
168177
"ClusterRole/azure-cloud-controller-manager",
169178
"ClusterRoleBinding/cloud-controller-manager:azure-cloud-controller-manager",
179+
"Service/azure-cloud-controller-manager",
170180
},
171181
}, {
172182
name: "Azure Stack resources returned as expected",
173183
testPlatform: platformsMap["AzureStackHub"],
174-
expectedResourceCount: 3,
184+
expectedResourceCount: 4,
175185
expectedResourcesKindName: []string{
176186
"Deployment/azure-cloud-controller-manager",
177187
"DaemonSet/azure-cloud-node-manager",
178188
"PodDisruptionBudget/azure-cloud-controller-manager",
189+
"Service/azure-cloud-controller-manager",
179190
},
180191
}, {
181192
name: "Azure Stack resources returned as expected with single node",
182193
testPlatform: platformsMap["AzureStackHub"],
183-
expectedResourceCount: 2,
194+
expectedResourceCount: 3,
184195
singleReplica: true,
185196
expectedResourcesKindName: []string{
186197
"Deployment/azure-cloud-controller-manager",
187198
"DaemonSet/azure-cloud-node-manager",
199+
"Service/azure-cloud-controller-manager",
188200
},
189201
}, {
190202
name: "VSphere resources returned as expected",
191203
testPlatform: platformsMap[string(configv1.VSpherePlatformType)],
192-
expectedResourceCount: 8,
204+
expectedResourceCount: 9,
193205
expectedResourcesKindName: []string{
194206
"Deployment/vsphere-cloud-controller-manager",
195207
"PodDisruptionBudget/vsphere-cloud-controller-manager",
@@ -199,11 +211,12 @@ func TestGetResources(t *testing.T) {
199211
"ClusterRole/vsphere-cloud-controller-manager",
200212
"ClusterRoleBinding/vsphere-cloud-controller-manager:vsphere-cloud-controller-manager",
201213
"ClusterRoleBinding/vsphere-cloud-controller-manager:cloud-controller-manager",
214+
"Service/vsphere-cloud-controller-manager",
202215
},
203216
}, {
204217
name: "VSphere resources returned as expected with single node",
205218
testPlatform: platformsMap[string(configv1.VSpherePlatformType)],
206-
expectedResourceCount: 7,
219+
expectedResourceCount: 8,
207220
singleReplica: true,
208221
expectedResourcesKindName: []string{
209222
"Deployment/vsphere-cloud-controller-manager",
@@ -213,39 +226,48 @@ func TestGetResources(t *testing.T) {
213226
"ClusterRole/vsphere-cloud-controller-manager",
214227
"ClusterRoleBinding/vsphere-cloud-controller-manager:vsphere-cloud-controller-manager",
215228
"ClusterRoleBinding/vsphere-cloud-controller-manager:cloud-controller-manager",
229+
"Service/vsphere-cloud-controller-manager",
216230
},
217231
}, {
218232
name: "OVirt resources are empty, as the platform is not yet supported",
219233
testPlatform: platformsMap[string(configv1.OvirtPlatformType)],
220234
}, {
221235
name: "IBMCloud resources",
222236
testPlatform: platformsMap[string(configv1.IBMCloudPlatformType)],
223-
expectedResourceCount: 2,
237+
expectedResourceCount: 3,
224238
expectedResourcesKindName: []string{
225239
"Deployment/ibm-cloud-controller-manager",
226240
"PodDisruptionBudget/ibmcloud-cloud-controller-manager",
241+
"Service/ibmcloud-cloud-controller-manager",
227242
},
228243
}, {
229-
name: "IBMCloud resources with single node cluster",
230-
testPlatform: platformsMap[string(configv1.IBMCloudPlatformType)],
231-
expectedResourceCount: 1,
232-
singleReplica: true,
233-
expectedResourcesKindName: []string{"Deployment/ibm-cloud-controller-manager"},
244+
name: "IBMCloud resources with single node cluster",
245+
testPlatform: platformsMap[string(configv1.IBMCloudPlatformType)],
246+
expectedResourceCount: 2,
247+
singleReplica: true,
248+
expectedResourcesKindName: []string{
249+
"Deployment/ibm-cloud-controller-manager",
250+
"Service/ibmcloud-cloud-controller-manager",
251+
},
234252
}, {
235253
name: "PowerVS resources",
236254
testPlatform: platformsMap[string(configv1.PowerVSPlatformType)],
237-
expectedResourceCount: 2,
255+
expectedResourceCount: 3,
238256
singleReplica: false,
239257
expectedResourcesKindName: []string{
240258
"Deployment/powervs-cloud-controller-manager",
241259
"PodDisruptionBudget/powervs-cloud-controller-manager",
260+
"Service/powervs-cloud-controller-manager",
242261
},
243262
}, {
244-
name: "PowerVS resources with single node cluster",
245-
testPlatform: platformsMap[string(configv1.PowerVSPlatformType)],
246-
expectedResourceCount: 1,
247-
singleReplica: true,
248-
expectedResourcesKindName: []string{"Deployment/powervs-cloud-controller-manager"},
263+
name: "PowerVS resources with single node cluster",
264+
testPlatform: platformsMap[string(configv1.PowerVSPlatformType)],
265+
expectedResourceCount: 2,
266+
singleReplica: true,
267+
expectedResourcesKindName: []string{
268+
"Deployment/powervs-cloud-controller-manager",
269+
"Service/powervs-cloud-controller-manager",
270+
},
249271
}, {
250272
name: "Libvirt resources are empty",
251273
testPlatform: platformsMap[string(configv1.LibvirtPlatformType)],

‎pkg/cloud/common/resources.go‎

Lines changed: 44 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"fmt"
55
"strings"
66

7+
corev1 "k8s.io/api/core/v1"
78
policyv1 "k8s.io/api/policy/v1"
89
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
910
"k8s.io/apimachinery/pkg/util/intstr"
@@ -18,14 +19,17 @@ const (
1819
)
1920

2021
func GetCommonResources(config config.OperatorConfig) ([]client.Object, error) {
21-
commonResources := make([]client.Object, 0, 1)
22+
commonResources := []client.Object{}
2223
if !config.IsSingleReplica {
2324
pdb, err := getPDB(config)
2425
if err != nil {
2526
return nil, err
2627
}
2728
commonResources = append(commonResources, pdb)
2829
}
30+
31+
commonResources = append(commonResources, getService(config))
32+
2933
return commonResources, nil
3034
}
3135

@@ -53,3 +57,42 @@ func getPDB(config config.OperatorConfig) (*policyv1.PodDisruptionBudget, error)
5357
},
5458
}, nil
5559
}
60+
61+
// getService returns a common service for the cloud-controller-manager on port 10258,
62+
// for a given platform.
63+
func getService(config config.OperatorConfig) *corev1.Service {
64+
matchLabels := map[string]string{
65+
CloudControllerManagerProviderLabel: config.GetPlatformNameString(),
66+
}
67+
name := fmt.Sprintf("%s-cloud-controller-manager", strings.ToLower(config.GetPlatformNameString()))
68+
69+
return &corev1.Service{
70+
TypeMeta: metav1.TypeMeta{
71+
Kind: "Service",
72+
APIVersion: "core/v1",
73+
},
74+
ObjectMeta: metav1.ObjectMeta{
75+
Name: name,
76+
Namespace: config.ManagedNamespace,
77+
Labels: map[string]string{
78+
"k8s-app": name,
79+
},
80+
},
81+
Spec: corev1.ServiceSpec{
82+
Type: corev1.ServiceTypeClusterIP,
83+
Ports: []corev1.ServicePort{
84+
{
85+
Name: "https",
86+
Port: 10258,
87+
},
88+
{
89+
Name: "webhooks",
90+
Port: 10260,
91+
TargetPort: intstr.FromInt(10260),
92+
},
93+
},
94+
Selector: matchLabels,
95+
SessionAffinity: corev1.ServiceAffinityNone,
96+
},
97+
}
98+
}

‎pkg/controllers/clusteroperator_controller_test.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -351,7 +351,8 @@ var _ = Describe("Apply resources should", func() {
351351
updated, err := reconciler.applyResources(context.TODO(), resources)
352352
Expect(err).ShouldNot(HaveOccurred())
353353
Expect(updated).To(BeTrue())
354-
// two resources should report successful update, deployment and pdb
354+
// three resources should report successful update, deployment, pdb and service
355+
Eventually(recorder.Events).Should(Receive(ContainSubstring("Resource was successfully created")))
355356
Eventually(recorder.Events).Should(Receive(ContainSubstring("Resource was successfully created")))
356357
Eventually(recorder.Events).Should(Receive(ContainSubstring("Resource was successfully created")))
357358

‎pkg/controllers/resourceapply/resourceapply.go‎

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import (
2222
"sigs.k8s.io/controller-runtime/pkg/client"
2323
coreclientv1 "sigs.k8s.io/controller-runtime/pkg/client"
2424

25+
"github.com/openshift/library-go/pkg/operator/resource/resourceapply"
2526
"github.com/openshift/library-go/pkg/operator/resource/resourcemerge"
2627
)
2728

@@ -83,6 +84,8 @@ func ApplyResource(ctx context.Context, client coreclientv1.Client, recorder rec
8384
return applyRoleBinding(ctx, client, recorder, t)
8485
case *rbacv1.ClusterRoleBinding:
8586
return applyClusterRoleBinding(ctx, client, recorder, t)
87+
case *corev1.Service:
88+
return applyService(ctx, client, recorder, t)
8689
default:
8790
return false, fmt.Errorf("unhandled type %T", resource)
8891
}
@@ -560,3 +563,56 @@ func applyClusterRoleBinding(ctx context.Context, client coreclientv1.Client, re
560563
recorder.Event(required, corev1.EventTypeNormal, ResourceUpdateSuccessEvent, "Resource was successfully updated")
561564
return true, nil
562565
}
566+
567+
func applyService(ctx context.Context, client coreclientv1.Client, recorder record.EventRecorder,
568+
requiredOriginal *corev1.Service) (bool, error) {
569+
required := requiredOriginal.DeepCopy()
570+
571+
existing := &corev1.Service{}
572+
err := client.Get(ctx, coreclientv1.ObjectKeyFromObject(requiredOriginal), existing)
573+
if apierrors.IsNotFound(err) {
574+
required := requiredOriginal.DeepCopy()
575+
if err := client.Create(ctx, required); err != nil {
576+
recorder.Event(required, corev1.EventTypeWarning, ResourceCreateFailedEvent, err.Error())
577+
return false, fmt.Errorf("service creation failed: %v", err)
578+
}
579+
recorder.Event(required, corev1.EventTypeNormal, ResourceCreateSuccessEvent, "Resource was successfully created")
580+
return true, nil
581+
} else if err != nil {
582+
recorder.Event(required, corev1.EventTypeWarning, ResourceUpdateFailedEvent, err.Error())
583+
return false, fmt.Errorf("failed to get service for update: %v", err)
584+
}
585+
586+
modified := false
587+
existingCopy := existing.DeepCopy()
588+
589+
// This will catch also changes between old `required.spec` and current `required.spec`, because
590+
// the annotation from SetSpecHashAnnotation will be different.
591+
resourcemerge.EnsureObjectMeta(&modified, &existingCopy.ObjectMeta, required.ObjectMeta)
592+
selectorSame := equality.Semantic.DeepEqual(existingCopy.Spec.Selector, required.Spec.Selector)
593+
594+
typeSame := false
595+
requiredIsEmpty := len(required.Spec.Type) == 0
596+
existingCopyIsCluster := existingCopy.Spec.Type == corev1.ServiceTypeClusterIP
597+
if (requiredIsEmpty && existingCopyIsCluster) || equality.Semantic.DeepEqual(existingCopy.Spec.Type, required.Spec.Type) {
598+
typeSame = true
599+
}
600+
601+
if selectorSame && typeSame && !modified {
602+
return false, nil
603+
}
604+
605+
// at this point we know that we're going to perform a write. We're just trying to get the object correct
606+
toWrite := existingCopy // shallow copy so the code reads easier
607+
toWrite.Spec = required.Spec
608+
609+
klog.V(2).Infof("Service %q changes: %v", required.GetNamespace()+"/"+required.GetName(), resourceapply.JSONPatchNoError(existing, toWrite))
610+
611+
if err := client.Update(ctx, existingCopy); err != nil {
612+
recorder.Event(required, corev1.EventTypeWarning, ResourceUpdateFailedEvent, err.Error())
613+
return false, err
614+
}
615+
recorder.Event(required, corev1.EventTypeNormal, ResourceUpdateSuccessEvent, "Resource was successfully updated")
616+
617+
return true, nil
618+
}

0 commit comments

Comments
 (0)