Skip to content

Commit

Permalink
Watch infrastructure and update AWS tags
Browse files Browse the repository at this point in the history 
Signed-off-by: chiragkyal <[email protected]>
  • Loading branch information
@chiragkyal
chiragkyal committed Sep 25, 2024
1 parent 721cf7c commit 03dfa5c
Show file tree
Hide file tree
Showing 3 changed files with 95 additions and 25 deletions.
35 changes: 26 additions & 9 deletions pkg/controllers/awsloadbalancercontroller/controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import (

cco "github.com/openshift/cloud-credential-operator/pkg/apis/cloudcredential/v1"

configv1 "github.com/openshift/api/config/v1"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/builder"
"sigs.k8s.io/controller-runtime/pkg/client"
Expand All @@ -46,6 +47,8 @@ import (
const (
// the name of the AWSLoadBalancerController resource which will be reconciled
controllerName = "cluster"
// the name of the 'cluster' infrastructure object
clusterInfrastructureName = "cluster"
// the port on which controller metrics are served
controllerMetricsPort = 8080
// the port on which the controller webhook is served
Expand Down Expand Up @@ -120,6 +123,15 @@ func (r *AWSLoadBalancerControllerReconciler) Reconcile(ctx context.Context, req
}
}

infraConfig := &configv1.Infrastructure{}
if err := r.Client.Get(ctx, types.NamespacedName{Name: clusterInfrastructureName}, infraConfig); err != nil {
return ctrl.Result{}, fmt.Errorf("failed to get infrastructure %q: %w", clusterInfrastructureName, err)
}
platformStatus := infraConfig.Status.PlatformStatus
if platformStatus == nil {
return ctrl.Result{}, fmt.Errorf("failed to determine infrastructure platform status : PlatformStatus is nil")
}

if err := r.ensureIngressClass(ctx, lbController); err != nil {
return ctrl.Result{}, fmt.Errorf("failed to ensure default IngressClass for AWSLoadBalancerController %q: %v", req.Name, err)
}
Expand Down Expand Up @@ -187,7 +199,7 @@ func (r *AWSLoadBalancerControllerReconciler) Reconcile(ctx context.Context, req
return ctrl.Result{}, fmt.Errorf("failed to ensure ClusterRole and Binding for AWSLoadBalancerController %q: %w", req.Name, err)
}

deployment, err := r.ensureDeployment(ctx, sa, credSecretNsName.Name, servingSecretName, lbController, trustCAConfigMap)
deployment, err := r.ensureDeployment(ctx, sa, credSecretNsName.Name, servingSecretName, lbController, platformStatus, trustCAConfigMap)
if err != nil {
return ctrl.Result{}, fmt.Errorf("failed to ensure Deployment for AWSLoadbalancerController %q: %w", req.Name, err)
}
Expand Down Expand Up @@ -247,17 +259,17 @@ func (r *AWSLoadBalancerControllerReconciler) BuildManagedController(mgr ctrl.Ma
Owns(&arv1.ValidatingWebhookConfiguration{}).
Owns(&arv1.MutatingWebhookConfiguration{})

if r.TrustedCAConfigMapName != "" {
clusterALBCInstance := func(ctx context.Context, o client.Object) []reconcile.Request {
return []reconcile.Request{
{
NamespacedName: types.NamespacedName{
Name: controllerName,
},
clusterALBCInstance := func(ctx context.Context, o client.Object) []reconcile.Request {
return []reconcile.Request{
{
NamespacedName: types.NamespacedName{
Name: controllerName,
},
}
},
}
}

if r.TrustedCAConfigMapName != "" {
// Requeue the only (cluster) instance of AWSLoadBalancerController
// so that the main reconciliation loop can detect the changes in the trusted CA configmap's contents
// and redeploy the controller if needed.
Expand All @@ -270,6 +282,11 @@ func (r *AWSLoadBalancerControllerReconciler) BuildManagedController(mgr ctrl.Ma
predicate.NewPredicateFuncs(inNamespace(r.Namespace))),
predicate.NewPredicateFuncs(hasName(r.TrustedCAConfigMapName))))
}
// Watch Infrastructure object to detect changes in AWS user tags
bldr = bldr.Watches(&configv1.Infrastructure{},
handler.EnqueueRequestsFromMapFunc(clusterALBCInstance),
builder.WithPredicates(
predicate.NewPredicateFuncs(hasName(clusterInfrastructureName))))
return bldr
}

Expand Down
60 changes: 49 additions & 11 deletions pkg/controllers/awsloadbalancercontroller/deployment.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ import (
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
"sigs.k8s.io/controller-runtime/pkg/log"

configv1 "github.com/openshift/api/config/v1"

albo "github.com/openshift/aws-load-balancer-operator/api/v1"
)

Expand Down Expand Up @@ -69,7 +71,7 @@ const (
allCapabilities = "ALL"
)

func (r *AWSLoadBalancerControllerReconciler) ensureDeployment(ctx context.Context, sa *corev1.ServiceAccount, crSecretName, servingSecretName string, controller *albo.AWSLoadBalancerController, trustCAConfigMap *corev1.ConfigMap) (*appsv1.Deployment, error) {
func (r *AWSLoadBalancerControllerReconciler) ensureDeployment(ctx context.Context, sa *corev1.ServiceAccount, crSecretName, servingSecretName string, controller *albo.AWSLoadBalancerController, platformStatus *configv1.PlatformStatus, trustCAConfigMap *corev1.ConfigMap) (*appsv1.Deployment, error) {
deploymentName := fmt.Sprintf("%s-%s", controllerResourcePrefix, controller.Name)

reqLogger := log.FromContext(ctx).WithValues("deployment", deploymentName)
Expand All @@ -90,7 +92,7 @@ func (r *AWSLoadBalancerControllerReconciler) ensureDeployment(ctx context.Conte
trustCAConfigMapHash = configMapHash
}

desired := r.desiredDeployment(deploymentName, crSecretName, servingSecretName, controller, sa, trustCAConfigMapName, trustCAConfigMapHash)
desired := r.desiredDeployment(deploymentName, crSecretName, servingSecretName, controller, platformStatus, sa, trustCAConfigMapName, trustCAConfigMapHash)
err = controllerutil.SetControllerReference(controller, desired, r.Scheme)
if err != nil {
return nil, fmt.Errorf("failed to set owner reference on deployment %s: %w", deploymentName, err)
Expand Down Expand Up @@ -120,7 +122,7 @@ func (r *AWSLoadBalancerControllerReconciler) ensureDeployment(ctx context.Conte
return current, nil
}

func (r *AWSLoadBalancerControllerReconciler) desiredDeployment(name, credentialsRequestSecretName, servingSecret string, controller *albo.AWSLoadBalancerController, sa *corev1.ServiceAccount, trustedCAConfigMapName, trustedCAConfigMapHash string) *appsv1.Deployment {
func (r *AWSLoadBalancerControllerReconciler) desiredDeployment(name, credentialsRequestSecretName, servingSecret string, controller *albo.AWSLoadBalancerController, platformStatus *configv1.PlatformStatus, sa *corev1.ServiceAccount, trustedCAConfigMapName, trustedCAConfigMapHash string) *appsv1.Deployment {
d := &appsv1.Deployment{
ObjectMeta: metav1.ObjectMeta{
Name: name,
Expand All @@ -145,7 +147,7 @@ func (r *AWSLoadBalancerControllerReconciler) desiredDeployment(name, credential
{
Name: awsLoadBalancerControllerContainerName,
Image: r.Image,
Args: desiredContainerArgs(controller, r.ClusterName, r.VPCID),
Args: desiredContainerArgs(controller, platformStatus, r.ClusterName, r.VPCID),
Env: append([]corev1.EnvVar{
{
Name: awsRegionEnvVarName,
Expand Down Expand Up @@ -260,18 +262,22 @@ func (r *AWSLoadBalancerControllerReconciler) desiredDeployment(name, credential
return d
}

func desiredContainerArgs(controller *albo.AWSLoadBalancerController, clusterName, vpcID string) []string {
func desiredContainerArgs(controller *albo.AWSLoadBalancerController, platformStatus *configv1.PlatformStatus, clusterName, vpcID string) []string {
var args []string
args = append(args, fmt.Sprintf("--webhook-cert-dir=%s", webhookTLSDir))
args = append(args, fmt.Sprintf("--aws-vpc-id=%s", vpcID))
args = append(args, fmt.Sprintf("--cluster-name=%s", clusterName))

// if additional keys are present then sort them and append it to the arguments
if controller.Spec.AdditionalResourceTags != nil {
var tags []string
for _, t := range controller.Spec.AdditionalResourceTags {
tags = append(tags, fmt.Sprintf("%s=%s", t.Key, t.Value))
}
tags := mergeTags(controller, platformStatus)
// AWS supports a maximum of 50 tags per resource
// FIXME: AWSLoadBalancerController reserves 3 tags and allows 24 user tags
// platformStatus.AWS.ResourceTags allows 25 user tags
// (3+24+25) > 50 -- fix is needed
if len(tags) > 50 {
log.Log.Error(fmt.Errorf("max 50 tags are allowed"), "got", "tags", len(tags))
// TODO return error
}
if len(tags) > 0 {
sort.Strings(tags)
args = append(args, fmt.Sprintf(`--default-tags=%s`, strings.Join(tags, ",")))
}
Expand Down Expand Up @@ -542,3 +548,35 @@ func buildMapHash(data map[string]string) (string, error) {
}
return hex.EncodeToString(hash.Sum(nil)), nil
}

func mergeTags(controller *albo.AWSLoadBalancerController, platformStatus *configv1.PlatformStatus) []string {
// tagMap holds tags with unique keys
tagMap := make(map[string]string)

// Add tags from controller.Spec.AdditionalResourceTags since operator tags has higher precedence
if controller.Spec.AdditionalResourceTags != nil {
for _, t := range controller.Spec.AdditionalResourceTags {
tagMap[t.Key] = t.Value
}
}

// Add tags from platformStatus.AWS.ResourceTags to the map, only if the key doesn't exist
if platformStatus.AWS != nil && len(platformStatus.AWS.ResourceTags) > 0 {
log.Log.Info("@chirag: platform.AWS.ResourceTags", "tags", platformStatus.AWS.ResourceTags)
for _, t := range platformStatus.AWS.ResourceTags {
if len(t.Key) > 0 {
if _, exists := tagMap[t.Key]; !exists {
tagMap[t.Key] = t.Value
}
}
}
}

// Convert map back to a slice
var tags []string
for key, value := range tagMap {
tags = append(tags, fmt.Sprintf("%s=%s", key, value))
}

return tags
}
25 changes: 20 additions & 5 deletions pkg/controllers/awsloadbalancercontroller/deployment_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ import (
"github.com/google/go-cmp/cmp"
"sigs.k8s.io/controller-runtime/pkg/client/fake"

configv1 "github.com/openshift/api/config/v1"

albo "github.com/openshift/aws-load-balancer-operator/api/v1"
"github.com/openshift/aws-load-balancer-operator/pkg/utils/test"
)
Expand All @@ -26,6 +28,19 @@ const (
testAWSRegion = "us-east-1"
)

// TODO: example platformStatus
var platformStatus = &configv1.PlatformStatus{
Type: configv1.AWSPlatformType,
AWS: &configv1.AWSPlatformStatus{
ResourceTags: []configv1.AWSResourceTag{
// {
// Key: "Key1",
// Value: "Value1",
// },
},
},
}

func TestDesiredArgs(t *testing.T) {
for _, tc := range []struct {
name string
Expand Down Expand Up @@ -142,7 +157,7 @@ func TestDesiredArgs(t *testing.T) {
if tc.controller.Spec.IngressClass == "" {
tc.controller.Spec.IngressClass = "alb"
}
args := desiredContainerArgs(tc.controller, "test-cluster", "test-vpc")
args := desiredContainerArgs(tc.controller, platformStatus, "test-cluster", "test-vpc")

expected := sets.List(expectedArgs)
sort.Strings(expected)
Expand Down Expand Up @@ -743,11 +758,11 @@ func TestEnsureDeployment(t *testing.T) {
VPCID: "test-vpc",
AWSRegion: testAWSRegion,
}
_, err := r.ensureDeployment(context.Background(), tc.serviceAccount, "test-credentials", "test-serving", tc.controller, tc.trustedCAConfigMap)
_, err := r.ensureDeployment(context.Background(), tc.serviceAccount, "test-credentials", "test-serving", tc.controller, platformStatus, tc.trustedCAConfigMap)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
tc.expectedDeployment.Spec.Template.Spec.Containers[0].Args = desiredContainerArgs(tc.controller, "test-cluster", "test-vpc")
tc.expectedDeployment.Spec.Template.Spec.Containers[0].Args = desiredContainerArgs(tc.controller, platformStatus, "test-cluster", "test-vpc")
var deployment appsv1.Deployment
err = client.Get(context.Background(), types.NamespacedName{Namespace: "test-namespace", Name: fmt.Sprintf("%s-%s", controllerResourcePrefix, tc.controller.Name)}, &deployment)
if err != nil {
Expand Down Expand Up @@ -854,11 +869,11 @@ func TestEnsureDeploymentEnvVars(t *testing.T) {
VPCID: "test-vpc",
AWSRegion: testAWSRegion,
}
_, err := r.ensureDeployment(context.Background(), tc.serviceAccount, "test-credentials", "test-serving", tc.controller, nil)
_, err := r.ensureDeployment(context.Background(), tc.serviceAccount, "test-credentials", "test-serving", tc.controller, platformStatus, nil)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
tc.expectedDeployment.Spec.Template.Spec.Containers[0].Args = desiredContainerArgs(tc.controller, "test-cluster", "test-vpc")
tc.expectedDeployment.Spec.Template.Spec.Containers[0].Args = desiredContainerArgs(tc.controller, platformStatus, "test-cluster", "test-vpc")
var deployment appsv1.Deployment
err = client.Get(context.Background(), types.NamespacedName{Namespace: "test-namespace", Name: fmt.Sprintf("%s-%s", controllerResourcePrefix, tc.controller.Name)}, &deployment)
if err != nil {
Expand Down

0 comments on commit 03dfa5c

Please sign in to comment.