Skip to content

Commit 86f8c6d

Browse files
openshift-merge-bot[bot]openshift-merge-bot[bot]
authored
Merge pull request #399 from theobarberbany/add-cloud-controller-manager-service
OCPBUGS-53300: Adds service for cloud-controller-manager
2 parents fa106e6 + 6879a81 commit 86f8c6d

4 files changed

Lines changed: 146 additions & 30 deletions

File tree

‎pkg/cloud/cloud_test.go‎

Lines changed: 50 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -99,61 +99,69 @@ 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: 6,
138+
expectedResourceCount: 7,
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",
138144
"ValidatingAdmissionPolicyBinding/network-tier-annotation-binding",
139145
"ValidatingAdmissionPolicy/network-tier-annotation-validation-policy",
146+
"Service/gcp-cloud-controller-manager",
140147
},
141148
}, {
142149
name: "GCP resources returned as expected with single node cluster",
143150
testPlatform: platformsMap[string(configv1.GCPPlatformType)],
144-
expectedResourceCount: 5,
151+
expectedResourceCount: 6,
145152
singleReplica: true,
146153
expectedResourcesKindName: []string{
147154
"Deployment/gcp-cloud-controller-manager",
148155
"ClusterRole/gcp-cloud-controller-manager",
149156
"ClusterRoleBinding/gcp-cloud-controller-manager:cloud-provider",
150157
"ValidatingAdmissionPolicyBinding/network-tier-annotation-binding",
151158
"ValidatingAdmissionPolicy/network-tier-annotation-validation-policy",
159+
"Service/gcp-cloud-controller-manager",
152160
},
153161
}, {
154162
name: "Azure resources returned as expected",
155163
testPlatform: platformsMap[string(configv1.AzurePlatformType)],
156-
expectedResourceCount: 11,
164+
expectedResourceCount: 12,
157165
expectedResourcesKindName: []string{
158166
"Deployment/azure-cloud-controller-manager",
159167
"DaemonSet/azure-cloud-node-manager",
@@ -166,11 +174,12 @@ func TestGetResources(t *testing.T) {
166174
"ValidatingAdmissionPolicyBinding/azure-load-balancer-tcp-idle-timeout-validation-annotation-binding",
167175
"ValidatingAdmissionPolicy/azure-load-balancer-tcp-idle-timeout-annotation-validation-policy",
168176
"PodDisruptionBudget/azure-cloud-controller-manager",
177+
"Service/azure-cloud-controller-manager",
169178
},
170179
}, {
171180
name: "Azure resources returned as expected with single node cluster",
172181
testPlatform: platformsMap[string(configv1.AzurePlatformType)],
173-
expectedResourceCount: 10,
182+
expectedResourceCount: 11,
174183
singleReplica: true,
175184
expectedResourcesKindName: []string{
176185
"Deployment/azure-cloud-controller-manager",
@@ -183,33 +192,36 @@ func TestGetResources(t *testing.T) {
183192
"ValidatingAdmissionPolicyBinding/openshift-cloud-controller-manager-cloud-provider-azure-node-admission",
184193
"ValidatingAdmissionPolicyBinding/azure-load-balancer-tcp-idle-timeout-validation-annotation-binding",
185194
"ValidatingAdmissionPolicy/azure-load-balancer-tcp-idle-timeout-annotation-validation-policy",
195+
"Service/azure-cloud-controller-manager",
186196
},
187197
}, {
188198
name: "Azure Stack resources returned as expected",
189199
testPlatform: platformsMap["AzureStackHub"],
190-
expectedResourceCount: 5,
200+
expectedResourceCount: 6,
191201
expectedResourcesKindName: []string{
192202
"Deployment/azure-cloud-controller-manager",
193203
"DaemonSet/azure-cloud-node-manager",
194204
"Role/azure-cloud-provider",
195205
"RoleBinding/azure-cloud-provider:azure-cloud-provider",
196206
"PodDisruptionBudget/azure-cloud-controller-manager",
207+
"Service/azure-cloud-controller-manager",
197208
},
198209
}, {
199210
name: "Azure Stack resources returned as expected with single node",
200211
testPlatform: platformsMap["AzureStackHub"],
201-
expectedResourceCount: 4,
212+
expectedResourceCount: 5,
202213
singleReplica: true,
203214
expectedResourcesKindName: []string{
204215
"Deployment/azure-cloud-controller-manager",
205216
"DaemonSet/azure-cloud-node-manager",
206217
"Role/azure-cloud-provider",
207218
"RoleBinding/azure-cloud-provider:azure-cloud-provider",
219+
"Service/azure-cloud-controller-manager",
208220
},
209221
}, {
210222
name: "VSphere resources returned as expected",
211223
testPlatform: platformsMap[string(configv1.VSpherePlatformType)],
212-
expectedResourceCount: 8,
224+
expectedResourceCount: 9,
213225
expectedResourcesKindName: []string{
214226
"Deployment/vsphere-cloud-controller-manager",
215227
"PodDisruptionBudget/vsphere-cloud-controller-manager",
@@ -219,11 +231,12 @@ func TestGetResources(t *testing.T) {
219231
"ClusterRole/vsphere-cloud-controller-manager",
220232
"ClusterRoleBinding/vsphere-cloud-controller-manager:vsphere-cloud-controller-manager",
221233
"ClusterRoleBinding/vsphere-cloud-controller-manager:cloud-controller-manager",
234+
"Service/vsphere-cloud-controller-manager",
222235
},
223236
}, {
224237
name: "VSphere resources returned as expected with single node",
225238
testPlatform: platformsMap[string(configv1.VSpherePlatformType)],
226-
expectedResourceCount: 7,
239+
expectedResourceCount: 8,
227240
singleReplica: true,
228241
expectedResourcesKindName: []string{
229242
"Deployment/vsphere-cloud-controller-manager",
@@ -233,39 +246,48 @@ func TestGetResources(t *testing.T) {
233246
"ClusterRole/vsphere-cloud-controller-manager",
234247
"ClusterRoleBinding/vsphere-cloud-controller-manager:vsphere-cloud-controller-manager",
235248
"ClusterRoleBinding/vsphere-cloud-controller-manager:cloud-controller-manager",
249+
"Service/vsphere-cloud-controller-manager",
236250
},
237251
}, {
238252
name: "OVirt resources are empty, as the platform is not yet supported",
239253
testPlatform: platformsMap[string(configv1.OvirtPlatformType)],
240254
}, {
241255
name: "IBMCloud resources",
242256
testPlatform: platformsMap[string(configv1.IBMCloudPlatformType)],
243-
expectedResourceCount: 2,
257+
expectedResourceCount: 3,
244258
expectedResourcesKindName: []string{
245259
"Deployment/ibm-cloud-controller-manager",
246260
"PodDisruptionBudget/ibmcloud-cloud-controller-manager",
261+
"Service/ibmcloud-cloud-controller-manager",
247262
},
248263
}, {
249-
name: "IBMCloud resources with single node cluster",
250-
testPlatform: platformsMap[string(configv1.IBMCloudPlatformType)],
251-
expectedResourceCount: 1,
252-
singleReplica: true,
253-
expectedResourcesKindName: []string{"Deployment/ibm-cloud-controller-manager"},
264+
name: "IBMCloud resources with single node cluster",
265+
testPlatform: platformsMap[string(configv1.IBMCloudPlatformType)],
266+
expectedResourceCount: 2,
267+
singleReplica: true,
268+
expectedResourcesKindName: []string{
269+
"Deployment/ibm-cloud-controller-manager",
270+
"Service/ibmcloud-cloud-controller-manager",
271+
},
254272
}, {
255273
name: "PowerVS resources",
256274
testPlatform: platformsMap[string(configv1.PowerVSPlatformType)],
257-
expectedResourceCount: 2,
275+
expectedResourceCount: 3,
258276
singleReplica: false,
259277
expectedResourcesKindName: []string{
260278
"Deployment/powervs-cloud-controller-manager",
261279
"PodDisruptionBudget/powervs-cloud-controller-manager",
280+
"Service/powervs-cloud-controller-manager",
262281
},
263282
}, {
264-
name: "PowerVS resources with single node cluster",
265-
testPlatform: platformsMap[string(configv1.PowerVSPlatformType)],
266-
expectedResourceCount: 1,
267-
singleReplica: true,
268-
expectedResourcesKindName: []string{"Deployment/powervs-cloud-controller-manager"},
283+
name: "PowerVS resources with single node cluster",
284+
testPlatform: platformsMap[string(configv1.PowerVSPlatformType)],
285+
expectedResourceCount: 2,
286+
singleReplica: true,
287+
expectedResourcesKindName: []string{
288+
"Deployment/powervs-cloud-controller-manager",
289+
"Service/powervs-cloud-controller-manager",
290+
},
269291
}, {
270292
name: "Libvirt resources are empty",
271293
testPlatform: platformsMap[string(configv1.LibvirtPlatformType)],

‎pkg/cloud/common/resources.go‎

Lines changed: 39 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,37 @@ 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+
Selector: matchLabels,
90+
SessionAffinity: corev1.ServiceAffinityNone,
91+
},
92+
}
93+
}

‎pkg/controllers/clusteroperator_controller_test.go‎

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

‎pkg/controllers/resourceapply/resourceapply.go‎

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,8 @@ func ApplyResource(ctx context.Context, client coreclientv1.Client, recorder rec
8989
return applyValidatingAdmissionPolicy(ctx, client, recorder, t)
9090
case *admissionregistrationv1.ValidatingAdmissionPolicyBinding:
9191
return applyValidatingAdmissionPolicyBinding(ctx, client, recorder, t)
92+
case *corev1.Service:
93+
return applyService(ctx, client, recorder, t)
9294
default:
9395
return false, fmt.Errorf("unhandled type %T", resource)
9496
}
@@ -650,3 +652,56 @@ func applyValidatingAdmissionPolicyBinding(ctx context.Context, client coreclien
650652

651653
return true, nil
652654
}
655+
656+
func applyService(ctx context.Context, client coreclientv1.Client, recorder record.EventRecorder,
657+
requiredOriginal *corev1.Service) (bool, error) {
658+
required := requiredOriginal.DeepCopy()
659+
660+
existing := &corev1.Service{}
661+
err := client.Get(ctx, coreclientv1.ObjectKeyFromObject(requiredOriginal), existing)
662+
if apierrors.IsNotFound(err) {
663+
required := requiredOriginal.DeepCopy()
664+
if err := client.Create(ctx, required); err != nil {
665+
recorder.Event(required, corev1.EventTypeWarning, ResourceCreateFailedEvent, err.Error())
666+
return false, fmt.Errorf("service creation failed: %v", err)
667+
}
668+
recorder.Event(required, corev1.EventTypeNormal, ResourceCreateSuccessEvent, "Resource was successfully created")
669+
return true, nil
670+
} else if err != nil {
671+
recorder.Event(required, corev1.EventTypeWarning, ResourceUpdateFailedEvent, err.Error())
672+
return false, fmt.Errorf("failed to get service for update: %v", err)
673+
}
674+
675+
modified := false
676+
existingCopy := existing.DeepCopy()
677+
678+
// This will catch also changes between old `required.spec` and current `required.spec`, because
679+
// the annotation from SetSpecHashAnnotation will be different.
680+
resourcemerge.EnsureObjectMeta(&modified, &existingCopy.ObjectMeta, required.ObjectMeta)
681+
selectorSame := equality.Semantic.DeepEqual(existingCopy.Spec.Selector, required.Spec.Selector)
682+
683+
typeSame := false
684+
requiredIsEmpty := len(required.Spec.Type) == 0
685+
existingCopyIsCluster := existingCopy.Spec.Type == corev1.ServiceTypeClusterIP
686+
if (requiredIsEmpty && existingCopyIsCluster) || equality.Semantic.DeepEqual(existingCopy.Spec.Type, required.Spec.Type) {
687+
typeSame = true
688+
}
689+
690+
if selectorSame && typeSame && !modified {
691+
return false, nil
692+
}
693+
694+
// at this point we know that we're going to perform a write. We're just trying to get the object correct
695+
toWrite := existingCopy // shallow copy so the code reads easier
696+
toWrite.Spec = required.Spec
697+
698+
klog.V(2).Infof("Service %q changes: %v", required.GetNamespace()+"/"+required.GetName(), resourceapply.JSONPatchNoError(existing, toWrite))
699+
700+
if err := client.Update(ctx, existingCopy); err != nil {
701+
recorder.Event(required, corev1.EventTypeWarning, ResourceUpdateFailedEvent, err.Error())
702+
return false, err
703+
}
704+
recorder.Event(required, corev1.EventTypeNormal, ResourceUpdateSuccessEvent, "Resource was successfully updated")
705+
706+
return true, nil
707+
}

0 commit comments

Comments
 (0)