Skip to content

Commit e74b161

Browse files
piyush-tiwaripiyush-tiwari
committed
add tagging support for IngressClass
- Add support for defined and freeform tags via IngressClass annotations - Swap opc-retry-token in UpdateLoadbalancer call in IngressClass controller logic for an ifMatch
1 parent a48822c commit e74b161

File tree

5 files changed

+248
-73
lines changed

5 files changed

+248
-73
lines changed

‎pkg/controllers/ingressclass/ingressclass.go‎

Lines changed: 109 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import (
1414
"fmt"
1515
coreinformers "k8s.io/client-go/informers/core/v1"
1616
corelisters "k8s.io/client-go/listers/core/v1"
17+
"reflect"
1718
"time"
1819

1920
"github.com/oracle/oci-native-ingress-controller/pkg/client"
@@ -234,8 +235,7 @@ func (c *Controller) getLoadBalancer(ctx context.Context, ic *networkingv1.Ingre
234235
}
235236

236237
func (c *Controller) ensureLoadBalancer(ctx context.Context, ic *networkingv1.IngressClass) error {
237-
238-
lb, etag, err := c.getLoadBalancer(ctx, ic)
238+
lb, _, err := c.getLoadBalancer(ctx, ic)
239239
if err != nil {
240240
return err
241241
}
@@ -255,55 +255,13 @@ func (c *Controller) ensureLoadBalancer(ctx context.Context, ic *networkingv1.In
255255
}
256256
}
257257

258-
wrapperClient, ok := ctx.Value(util.WrapperClient).(*client.WrapperClient)
259-
if !ok {
260-
return fmt.Errorf(util.OciClientNotFoundInContextError)
261-
}
262-
compartmentId := common.String(util.GetIngressClassCompartmentId(icp, c.defaultCompartmentId))
263258
if lb == nil {
264-
klog.V(2).InfoS("Creating load balancer for ingress class", "ingressClass", ic.Name)
265-
266-
createDetails := ociloadbalancer.CreateLoadBalancerDetails{
267-
CompartmentId: compartmentId,
268-
DisplayName: common.String(util.GetIngressClassLoadBalancerName(ic, icp)),
269-
ShapeName: common.String("flexible"),
270-
SubnetIds: []string{util.GetIngressClassSubnetId(icp, c.defaultSubnetId)},
271-
IsPrivate: common.Bool(icp.Spec.IsPrivate),
272-
NetworkSecurityGroupIds: util.GetIngressClassNetworkSecurityGroupIds(ic),
273-
BackendSets: map[string]ociloadbalancer.BackendSetDetails{
274-
util.DefaultBackendSetName: {
275-
Policy: common.String("LEAST_CONNECTIONS"),
276-
HealthChecker: &ociloadbalancer.HealthCheckerDetails{
277-
Protocol: common.String("TCP"),
278-
},
279-
},
280-
},
281-
FreeformTags: map[string]string{OnicResource: "loadbalancer"},
282-
}
283-
284-
if icp.Spec.ReservedPublicAddressId != "" {
285-
createDetails.ReservedIps = []ociloadbalancer.ReservedIp{{Id: common.String(icp.Spec.ReservedPublicAddressId)}}
286-
}
287-
288-
createDetails.ShapeDetails = &ociloadbalancer.ShapeDetails{
289-
MinimumBandwidthInMbps: common.Int(icp.Spec.MinBandwidthMbps),
290-
MaximumBandwidthInMbps: common.Int(icp.Spec.MaxBandwidthMbps),
291-
}
292-
293-
createLbRequest := ociloadbalancer.CreateLoadBalancerRequest{
294-
// Use UID as retry token so multiple requests in 24 hours won't recreate the same LoadBalancer,
295-
// but recreate of the IngressClass will trigger an LB within 24 hours.
296-
// If you used ingress class name it would disallow creation of more LB's even in different clusters potentially.
297-
OpcRetryToken: common.String(fmt.Sprintf("create-lb-%s", ic.UID)),
298-
CreateLoadBalancerDetails: createDetails,
299-
}
300-
klog.Infof("Create lb request: %s", util.PrettyPrint(createLbRequest))
301-
lb, err = wrapperClient.GetLbClient().CreateLoadBalancer(context.Background(), createLbRequest)
259+
lb, err = c.createLoadBalancer(ctx, ic, icp)
302260
if err != nil {
303-
return err
261+
return fmt.Errorf("unable to create LoadBalancer for IngressClass %s: %w", ic.Name, err)
304262
}
305263
} else {
306-
err = c.checkForIngressClassParameterUpdates(ctx, lb, ic, icp, etag)
264+
err = c.checkForIngressClassParameterUpdates(ctx, ic, icp)
307265
if err != nil {
308266
return err
309267
}
@@ -314,6 +272,12 @@ func (c *Controller) ensureLoadBalancer(ctx context.Context, ic *networkingv1.In
314272
}
315273
}
316274

275+
wrapperClient, ok := ctx.Value(util.WrapperClient).(*client.WrapperClient)
276+
if !ok {
277+
return fmt.Errorf(util.OciClientNotFoundInContextError)
278+
}
279+
compartmentId := common.String(util.GetIngressClassCompartmentId(icp, c.defaultCompartmentId))
280+
317281
if *lb.Id != util.GetIngressClassLoadBalancerId(ic) {
318282
klog.InfoS("Adding load balancer id to ingress class", "lbId", *lb.Id, "ingressClass", klog.KObj(ic))
319283
patchError, done := util.PatchIngressClassWithAnnotation(wrapperClient.GetK8Client(), ic, util.IngressClassLoadBalancerIdAnnotation, *lb.Id)
@@ -334,6 +298,68 @@ func (c *Controller) ensureLoadBalancer(ctx context.Context, ic *networkingv1.In
334298
return nil
335299
}
336300

301+
func (c *Controller) createLoadBalancer(ctx context.Context, ic *networkingv1.IngressClass,
302+
icp *v1beta1.IngressClassParameters) (*ociloadbalancer.LoadBalancer, error) {
303+
klog.V(2).InfoS("Creating load balancer for ingress class", "ingressClass", ic.Name)
304+
compartmentId := common.String(util.GetIngressClassCompartmentId(icp, c.defaultCompartmentId))
305+
definedTags, err := util.GetIngressClassDefinedTags(ic)
306+
if err != nil {
307+
return nil, err
308+
}
309+
freeformTags, err := util.GetIngressClassFreeformTags(ic)
310+
if err != nil {
311+
return nil, err
312+
}
313+
314+
createDetails := ociloadbalancer.CreateLoadBalancerDetails{
315+
CompartmentId: compartmentId,
316+
DisplayName: common.String(util.GetIngressClassLoadBalancerName(ic, icp)),
317+
ShapeName: common.String("flexible"),
318+
SubnetIds: []string{util.GetIngressClassSubnetId(icp, c.defaultSubnetId)},
319+
IsPrivate: common.Bool(icp.Spec.IsPrivate),
320+
NetworkSecurityGroupIds: util.GetIngressClassNetworkSecurityGroupIds(ic),
321+
BackendSets: map[string]ociloadbalancer.BackendSetDetails{
322+
util.DefaultBackendSetName: {
323+
Policy: common.String("LEAST_CONNECTIONS"),
324+
HealthChecker: &ociloadbalancer.HealthCheckerDetails{
325+
Protocol: common.String("TCP"),
326+
},
327+
},
328+
},
329+
FreeformTags: freeformTags,
330+
DefinedTags: definedTags,
331+
}
332+
333+
if icp.Spec.ReservedPublicAddressId != "" {
334+
createDetails.ReservedIps = []ociloadbalancer.ReservedIp{{Id: common.String(icp.Spec.ReservedPublicAddressId)}}
335+
}
336+
337+
createDetails.ShapeDetails = &ociloadbalancer.ShapeDetails{
338+
MinimumBandwidthInMbps: common.Int(icp.Spec.MinBandwidthMbps),
339+
MaximumBandwidthInMbps: common.Int(icp.Spec.MaxBandwidthMbps),
340+
}
341+
342+
createLbRequest := ociloadbalancer.CreateLoadBalancerRequest{
343+
// Use UID as retry token so multiple requests in 24 hours won't recreate the same LoadBalancer,
344+
// but recreate of the IngressClass will trigger an LB within 24 hours.
345+
// If you used ingress class name it would disallow creation of more LB's even in different clusters potentially.
346+
OpcRetryToken: common.String(fmt.Sprintf("create-lb-%s", ic.UID)),
347+
CreateLoadBalancerDetails: createDetails,
348+
}
349+
klog.Infof("Create lb request: %s", util.PrettyPrint(createLbRequest))
350+
351+
wrapperClient, ok := ctx.Value(util.WrapperClient).(*client.WrapperClient)
352+
if !ok {
353+
return nil, fmt.Errorf(util.OciClientNotFoundInContextError)
354+
}
355+
lb, err := wrapperClient.GetLbClient().CreateLoadBalancer(context.Background(), createLbRequest)
356+
if err != nil {
357+
return nil, err
358+
}
359+
360+
return lb, nil
361+
}
362+
337363
func (c *Controller) setupWebApplicationFirewall(ctx context.Context, ic *networkingv1.IngressClass, compartmentId *string, lbId *string) error {
338364
wrapperClient, ok := ctx.Value(util.WrapperClient).(*client.WrapperClient)
339365
if !ok {
@@ -353,33 +379,43 @@ func (c *Controller) setupWebApplicationFirewall(ctx context.Context, ic *networ
353379
return nil
354380
}
355381

356-
func (c *Controller) checkForIngressClassParameterUpdates(ctx context.Context, lb *ociloadbalancer.LoadBalancer,
357-
ic *networkingv1.IngressClass, icp *v1beta1.IngressClassParameters, etag string) error {
358-
// check LoadBalancerName AND MinBandwidthMbps ,MaxBandwidthMbps
359-
displayName := util.GetIngressClassLoadBalancerName(ic, icp)
382+
func (c *Controller) checkForIngressClassParameterUpdates(ctx context.Context, ic *networkingv1.IngressClass, icp *v1beta1.IngressClassParameters) error {
383+
lb, etag, err := c.getLoadBalancer(ctx, ic)
384+
if err != nil {
385+
return err
386+
}
387+
360388
wrapperClient, ok := ctx.Value(util.WrapperClient).(*client.WrapperClient)
361389
if !ok {
362390
return fmt.Errorf(util.OciClientNotFoundInContextError)
363391
}
364-
if *lb.DisplayName != displayName {
365392

366-
detail := ociloadbalancer.UpdateLoadBalancerDetails{
367-
DisplayName: &displayName,
368-
}
369-
req := ociloadbalancer.UpdateLoadBalancerRequest{
370-
OpcRetryToken: common.String(fmt.Sprintf("update-lb-detail-%s", ic.UID)),
371-
UpdateLoadBalancerDetails: detail,
372-
LoadBalancerId: lb.Id,
373-
}
393+
// check LoadBalancerName, Defined and Freeform tags
394+
displayName := util.GetIngressClassLoadBalancerName(ic, icp)
395+
definedTags, err := util.GetIngressClassDefinedTags(ic)
396+
if err != nil {
397+
return err
398+
}
399+
freeformTags, err := util.GetIngressClassFreeformTags(ic)
400+
if err != nil {
401+
return err
402+
}
374403

375-
klog.Infof("Update lb details request: %s", util.PrettyPrint(req))
376-
_, err := wrapperClient.GetLbClient().UpdateLoadBalancer(context.Background(), req)
404+
if *lb.DisplayName != displayName || !reflect.DeepEqual(lb.DefinedTags, definedTags) || !reflect.DeepEqual(lb.FreeformTags, freeformTags) {
405+
_, err = wrapperClient.GetLbClient().UpdateLoadBalancer(context.Background(), *lb.Id, displayName, definedTags, freeformTags)
377406
if err != nil {
378407
return err
379408
}
380409

381410
}
382411

412+
// refresh lb, etag information after last call
413+
lb, etag, err = c.getLoadBalancer(ctx, ic)
414+
if err != nil {
415+
return err
416+
}
417+
418+
// check LB Shape
383419
if *lb.ShapeDetails.MaximumBandwidthInMbps != icp.Spec.MaxBandwidthMbps ||
384420
*lb.ShapeDetails.MinimumBandwidthInMbps != icp.Spec.MinBandwidthMbps {
385421
shapeDetails := &ociloadbalancer.ShapeDetails{
@@ -460,7 +496,7 @@ func (c *Controller) clearLoadBalancer(ctx context.Context, ic *networkingv1.Ing
460496
}
461497

462498
if lb == nil {
463-
klog.Infof("Tried to clear LB for ic %s/%s, but it is deleted", ic.Namespace, ic.Name)
499+
klog.Infof("Tried to clear LB for ic %s, but it is deleted", ic.Name)
464500
return nil
465501
}
466502

@@ -478,15 +514,21 @@ func (c *Controller) clearLoadBalancer(ctx context.Context, ic *networkingv1.Ing
478514
if len(nsgIds) > 0 {
479515
_, err = wrapperClient.GetLbClient().UpdateNetworkSecurityGroups(context.Background(), *lb.Id, make([]string, 0))
480516
if err != nil {
481-
klog.Errorf("While clearing LB %s, cannot clear NSG IDs due to %s, will proceed with IngressClass deletion for %s/%s",
482-
*lb.Id, err.Error(), ic.Namespace, ic.Name)
517+
klog.Errorf("While clearing LB %s, cannot clear NSG IDs due to %s, will proceed with IngressClass deletion for %s",
518+
*lb.Id, err.Error(), ic.Name)
483519
}
484520
}
485521

486522
err = wrapperClient.GetLbClient().DeleteBackendSet(context.Background(), *lb.Id, util.DefaultBackendSetName)
487523
if err != nil {
488-
klog.Errorf("While clearing LB %s, cannot clear BackendSet %s due to %s, will proceed with IngressClass deletion for %s/%s",
489-
*lb.Id, util.DefaultBackendSetName, err.Error(), ic.Namespace, ic.Name)
524+
klog.Errorf("While clearing LB %s, cannot clear BackendSet %s due to %s, will proceed with IngressClass deletion for %s",
525+
*lb.Id, util.DefaultBackendSetName, err.Error(), ic.Name)
526+
}
527+
528+
_, err = wrapperClient.GetLbClient().UpdateLoadBalancer(context.Background(), *lb.Id, *lb.DisplayName, map[string]map[string]interface{}{}, map[string]string{})
529+
if err != nil {
530+
klog.Errorf("While clearing LB %s, cannot clear tags due to %s, will proceed with IngressClass deletion for %s",
531+
*lb.Id, err.Error(), ic.Name)
490532
}
491533

492534
return nil

‎pkg/controllers/ingressclass/ingressclass_test.go‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -202,11 +202,8 @@ func TestCheckForIngressClassParameterUpdates(t *testing.T) {
202202
RegisterTestingT(t)
203203
ctx, cancel := context.WithCancel(context.Background())
204204
defer cancel()
205-
ingressClassList := util.GetIngressClassList()
205+
ingressClassList := util.GetIngressClassListWithLBSet("id")
206206
c := inits(ctx, ingressClassList)
207-
mockClient, err := c.client.GetClient(&MockConfigGetter{})
208-
Expect(err).To(BeNil())
209-
loadBalancer, _, _ := mockClient.GetLbClient().GetLoadBalancer(context.TODO(), "id")
210207
icp := v1beta1.IngressClassParameters{
211208
Spec: v1beta1.IngressClassParametersSpec{
212209
CompartmentId: "",
@@ -217,7 +214,7 @@ func TestCheckForIngressClassParameterUpdates(t *testing.T) {
217214
MaxBandwidthMbps: 400,
218215
},
219216
}
220-
err = c.checkForIngressClassParameterUpdates(getContextWithClient(c, ctx), loadBalancer, &ingressClassList.Items[0], &icp, "etag")
217+
err := c.checkForIngressClassParameterUpdates(getContextWithClient(c, ctx), &ingressClassList.Items[0], &icp)
221218
Expect(err).Should(BeNil())
222219
}
223220

‎pkg/loadbalancer/loadbalancer.go‎

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,9 @@ func (lbc *LoadBalancerClient) GetBackendSetHealth(ctx context.Context, lbID str
8888

8989
func (lbc *LoadBalancerClient) UpdateNetworkSecurityGroups(ctx context.Context, lbId string, nsgIds []string) (loadbalancer.UpdateNetworkSecurityGroupsResponse, error) {
9090
_, etag, err := lbc.GetLoadBalancer(ctx, lbId)
91+
if err != nil {
92+
return loadbalancer.UpdateNetworkSecurityGroupsResponse{}, err
93+
}
9194

9295
req := loadbalancer.UpdateNetworkSecurityGroupsRequest{
9396
LoadBalancerId: common.String(lbId),
@@ -128,7 +131,24 @@ func (lbc *LoadBalancerClient) UpdateLoadBalancerShape(ctx context.Context, req
128131
return resp, err
129132
}
130133

131-
func (lbc *LoadBalancerClient) UpdateLoadBalancer(ctx context.Context, req loadbalancer.UpdateLoadBalancerRequest) (*loadbalancer.LoadBalancer, error) {
134+
func (lbc *LoadBalancerClient) UpdateLoadBalancer(ctx context.Context, lbId string, displayName string, definedTags map[string]map[string]interface{},
135+
freeformTags map[string]string) (*loadbalancer.LoadBalancer, error) {
136+
_, etag, err := lbc.GetLoadBalancer(ctx, lbId)
137+
if err != nil {
138+
return nil, err
139+
}
140+
141+
req := loadbalancer.UpdateLoadBalancerRequest{
142+
LoadBalancerId: common.String(lbId),
143+
IfMatch: common.String(etag),
144+
UpdateLoadBalancerDetails: loadbalancer.UpdateLoadBalancerDetails{
145+
DisplayName: common.String(displayName),
146+
DefinedTags: definedTags,
147+
FreeformTags: freeformTags,
148+
},
149+
}
150+
151+
klog.Infof("Update lb details request: %s", util.PrettyPrint(req))
132152
resp, err := lbc.LbClient.UpdateLoadBalancer(ctx, req)
133153
if err != nil {
134154
return nil, err

‎pkg/util/util.go‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,8 @@ const (
6868
IngressClassFireWallIdAnnotation = "oci-native-ingress.oraclecloud.com/firewall-id"
6969
IngressClassNetworkSecurityGroupIdsAnnotation = "oci-native-ingress.oraclecloud.com/network-security-group-ids"
7070
IngressClassDeleteProtectionEnabledAnnotation = "oci-native-ingress.oraclecloud.com/delete-protection-enabled"
71+
IngressClassDefinedTagsAnnotation = "oci-native-ingress.oraclecloud.com/defined-tags"
72+
IngressClassFreeformTagsAnnotation = "oci-native-ingress.oraclecloud.com/freeform-tags"
7173

7274
IngressHealthCheckProtocolAnnotation = "oci-native-ingress.oraclecloud.com/healthcheck-protocol"
7375
IngressHealthCheckPortAnnotation = "oci-native-ingress.oraclecloud.com/healthcheck-port"
@@ -189,6 +191,44 @@ func GetIngressClassDeleteProtectionEnabled(ic *networkingv1.IngressClass) bool
189191
return result
190192
}
191193

194+
func GetIngressClassDefinedTags(ic *networkingv1.IngressClass) (map[string]map[string]interface{}, error) {
195+
annotation := IngressClassDefinedTagsAnnotation
196+
value, ok := ic.Annotations[annotation]
197+
198+
// value of defined tags can only be strings for now, but we will allow anything that fits the type
199+
// specified by LoadBalancer.DefinedTags
200+
definedTags := map[string]map[string]interface{}{}
201+
202+
if !ok || strings.TrimSpace(value) == "" {
203+
return definedTags, nil
204+
}
205+
206+
err := json.Unmarshal([]byte(value), &definedTags)
207+
if err != nil {
208+
return nil, fmt.Errorf("error parsing value %s for annotation %s: %w", value, annotation, err)
209+
}
210+
211+
return definedTags, nil
212+
}
213+
214+
func GetIngressClassFreeformTags(ic *networkingv1.IngressClass) (map[string]string, error) {
215+
annotation := IngressClassFreeformTagsAnnotation
216+
value, ok := ic.Annotations[annotation]
217+
218+
freeformTags := map[string]string{}
219+
220+
if !ok || strings.TrimSpace(value) == "" {
221+
return freeformTags, nil
222+
}
223+
224+
err := json.Unmarshal([]byte(value), &freeformTags)
225+
if err != nil {
226+
return nil, fmt.Errorf("error parsing value %s for annotation %s: %w", value, annotation, err)
227+
}
228+
229+
return freeformTags, nil
230+
}
231+
192232
func GetIngressProtocol(i *networkingv1.Ingress) string {
193233
protocol, ok := i.Annotations[IngressProtocolAnnotation]
194234
if !ok {

0 commit comments

Comments
 (0)