Skip to content

Commit

Permalink
Watch Infrastructure object and update AWS user tags
Browse files Browse the repository at this point in the history
- Ingress controller now monitors changes to the Infrastructure object,
ensuring that modifications to user-defined AWS ResourceTags (platform.AWS.ResourceTags) trigger updates to the load balancer service.
- Consider awsLBAdditionalResourceTags annotation as a managed annotation.

Signed-off-by: chiragkyal <ckyal@redhat.com>
  • Loading branch information
chiragkyal committed Oct 15, 2024
1 parent 871b2b2 commit ae57009
Show file tree
Hide file tree
Showing 4 changed files with 27 additions and 9 deletions.
15 changes: 15 additions & 0 deletions pkg/operator/controller/ingress/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,8 @@ import (

const (
controllerName = "ingress_controller"
// clusterInfrastructureName is the name of the 'cluster' infrastructure object.
clusterInfrastructureName = "cluster"
)

// TODO: consider moving these to openshift/api
Expand Down Expand Up @@ -134,6 +136,12 @@ func New(mgr manager.Manager, config Config) (controller.Controller, error) {
if err := c.Watch(source.Kind[client.Object](operatorCache, &configv1.Proxy{}, handler.EnqueueRequestsFromMapFunc(reconciler.ingressConfigToIngressController))); err != nil {
return nil, err
}
// Watch for changes to infrastructure config to update user defined tags.
if err := c.Watch(source.Kind[client.Object](operatorCache, &configv1.Infrastructure{}, handler.EnqueueRequestsFromMapFunc(reconciler.ingressConfigToIngressController),
predicate.NewPredicateFuncs(hasName(clusterInfrastructureName)),
)); err != nil {
return nil, err
}
return c, nil
}

Expand Down Expand Up @@ -187,6 +195,13 @@ func enqueueRequestForOwningIngressController(namespace string) handler.EventHan
})
}

// hasName returns a predicate which checks whether an object has the given name.
func hasName(name string) func(o client.Object) bool {
return func(o client.Object) bool {
return o.GetName() == name
}
}

// Config holds all the things necessary for the controller to run.
type Config struct {
Namespace string
Expand Down
17 changes: 10 additions & 7 deletions pkg/operator/controller/ingress/load_balancer_service.go
Original file line number Diff line number Diff line change
Expand Up @@ -254,6 +254,11 @@ var (
//
// https://cloud.ibm.com/docs/containers?topic=containers-vpc-lbaas
iksLBEnableFeaturesAnnotation,
// awsLBAdditionalResourceTags annotation is populated
// by user tags present in
// Status.PlatformStatus.AWS.ResourceTags in the
// infrastructure config.
awsLBAdditionalResourceTags,
)

// Azure and GCP support switching between internal and external
Expand Down Expand Up @@ -751,12 +756,6 @@ func IsServiceInternal(service *corev1.Service) bool {
return false
}

// loadBalancerServiceTagsModified verifies that none of the managedAnnotations have been changed and also the AWS tags annotation
func loadBalancerServiceTagsModified(current, expected *corev1.Service) (bool, *corev1.Service) {
ignoredAnnotations := managedLoadBalancerServiceAnnotations.Union(sets.NewString(awsLBAdditionalResourceTags))
return loadBalancerServiceAnnotationsChanged(current, expected, ignoredAnnotations)
}

// loadBalancerServiceIsUpgradeable returns an error value indicating if the
// load balancer service is safe to upgrade. In particular, if the current
// service matches the desired service, then the service is upgradeable, and the
Expand All @@ -773,7 +772,11 @@ func loadBalancerServiceIsUpgradeable(ic *operatorv1.IngressController, deployme
return nil
}

changed, updated := loadBalancerServiceTagsModified(current, desired)
// Verify that none of the managedAnnotations have been changed by something or someone.
// Since the status logic runs after the controller sets the annotations, it checks for
// any discrepancy (in case modified) between the desired annotation values of the controller
// and the current annotation values.
changed, updated := loadBalancerServiceAnnotationsChanged(current, desired, managedLoadBalancerServiceAnnotations)
if changed {
diff := cmp.Diff(current, updated, cmpopts.EquateEmpty())
return fmt.Errorf("load balancer service has been modified; changes must be reverted before upgrading: %s", diff)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1141,7 +1141,7 @@ func Test_loadBalancerServiceChanged(t *testing.T) {
mutate: func(svc *corev1.Service) {
svc.Annotations["service.beta.kubernetes.io/aws-load-balancer-additional-resource-tags"] = "Key3=Value3,Key4=Value4"
},
expect: false,
expect: true,
},
{
description: "if the service.beta.kubernetes.io/aws-load-balancer-connection-idle-timeout annotation changes",
Expand Down
2 changes: 1 addition & 1 deletion pkg/operator/controller/ingress/status_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3022,7 +3022,7 @@ func Test_computeIngressUpgradeableCondition(t *testing.T) {
expect: true,
},
{
description: "if the service.beta.kubernetes.io/aws-load-balancer-additional-resource-tags annotation changes",
description: "if the service.beta.kubernetes.io/aws-load-balancer-additional-resource-tags annotation changes not by ingress controller",
mutate: func(svc *corev1.Service) {
svc.Annotations[awsLBAdditionalResourceTags] = "Key2=Value2"
},
Expand Down

0 comments on commit ae57009

Please sign in to comment.