From f5d1caed983b59674a0db59c8c0632939625e66f Mon Sep 17 00:00:00 2001 From: jm96441n Date: Wed, 7 Feb 2024 16:41:37 -0500 Subject: [PATCH] Generalize the crud hooks for gateways --- .../resources/gateway_controller_crud.go | 208 +++++++++++++++++ .../resources/gateway_controller_setup.go | 34 --- .../resources/mesh_gateway_controller.go | 221 +----------------- 3 files changed, 220 insertions(+), 243 deletions(-) create mode 100644 control-plane/controllers/resources/gateway_controller_crud.go diff --git a/control-plane/controllers/resources/gateway_controller_crud.go b/control-plane/controllers/resources/gateway_controller_crud.go new file mode 100644 index 0000000000..fe2fb55b5e --- /dev/null +++ b/control-plane/controllers/resources/gateway_controller_crud.go @@ -0,0 +1,208 @@ +package resources + +import ( + "context" + "fmt" + + "github.com/hashicorp/consul-k8s/control-plane/api/mesh/v2beta1" + meshv2beta1 "github.com/hashicorp/consul-k8s/control-plane/api/mesh/v2beta1" + "github.com/hashicorp/consul-k8s/control-plane/gateways" + appsv1 "k8s.io/api/apps/v1" + corev1 "k8s.io/api/core/v1" + rbacv1 "k8s.io/api/rbac/v1" + k8serr "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" +) + +type gatewayConfigs struct { + gcc *meshv2beta1.GatewayClassConfig + gatewayConfig gateways.GatewayConfig +} + +// onCreateUpdate is responsible for creating/updating all K8s resources that +// are required in order to run a meshv2beta1.XGateway. These are created/updated +// in dependency order. +// 1. ServiceAccount +// 2. Deployment +// 3. Service +// 4. Role +// 5. RoleBinding +func onCreateUpdate[T gateways.Gateway](ctx context.Context, k8sClient client.Client, cfg gatewayConfigs, resource T) error { + builder := gateways.NewGatewayBuilder[T](resource, cfg.gatewayConfig, cfg.gcc) + + // Create ServiceAccount + desiredAccount := builder.ServiceAccount() + existingAccount := &corev1.ServiceAccount{ObjectMeta: metav1.ObjectMeta{Namespace: desiredAccount.Namespace, Name: desiredAccount.Name}} + + upsertOp := func(ctx context.Context, _, object client.Object) error { + _, err := controllerutil.CreateOrUpdate(ctx, k8sClient, object, func() error { return nil }) + return err + } + + err := opIfNewOrOwned(ctx, resource, k8sClient, existingAccount, desiredAccount, upsertOp) + if err != nil { + return fmt.Errorf("unable to create service account: %w", err) + } + + // Create Role + desiredRole := builder.Role() + existingRole := &rbacv1.Role{ObjectMeta: metav1.ObjectMeta{Namespace: desiredRole.Namespace, Name: desiredRole.Name}} + + err = opIfNewOrOwned(ctx, resource, k8sClient, existingRole, desiredRole, upsertOp) + if err != nil { + return fmt.Errorf("unable to create role: %w", err) + } + + // Create RoleBinding + desiredBinding := builder.RoleBinding() + existingBinding := &rbacv1.RoleBinding{ObjectMeta: metav1.ObjectMeta{Namespace: desiredBinding.Namespace, Name: desiredBinding.Name}} + + err = opIfNewOrOwned(ctx, resource, k8sClient, existingBinding, desiredBinding, upsertOp) + if err != nil { + return fmt.Errorf("unable to create role binding: %w", err) + } + + // Create Service + desiredService := builder.Service() + existingService := &corev1.Service{ObjectMeta: metav1.ObjectMeta{Namespace: desiredService.Namespace, Name: desiredService.Name}} + + mergeServiceOp := func(ctx context.Context, existingObj, desiredObj client.Object) error { + existing := existingObj.(*corev1.Service) + desired := desiredObj.(*corev1.Service) + + _, err := controllerutil.CreateOrUpdate(ctx, k8sClient, existing, func() error { + gateways.MergeService(existing, desired) + return nil + }) + return err + } + + err = opIfNewOrOwned(ctx, resource, k8sClient, existingService, desiredService, mergeServiceOp) + if err != nil { + return fmt.Errorf("unable to create service: %w", err) + } + + // Create Deployment + desiredDeployment, err := builder.Deployment() + if err != nil { + return fmt.Errorf("unable to create deployment: %w", err) + } + existingDeployment := &appsv1.Deployment{ObjectMeta: metav1.ObjectMeta{Namespace: desiredDeployment.Namespace, Name: desiredDeployment.Name}} + + mergeDeploymentOp := func(ctx context.Context, existingObj, desiredObj client.Object) error { + existing := existingObj.(*appsv1.Deployment) + desired := desiredObj.(*appsv1.Deployment) + + _, err = controllerutil.CreateOrUpdate(ctx, k8sClient, existing, func() error { + gateways.MergeDeployment(existing, desired) + return nil + }) + return err + } + + err = opIfNewOrOwned(ctx, resource, k8sClient, existingDeployment, desiredDeployment, mergeDeploymentOp) + if err != nil { + return fmt.Errorf("unable to create deployment: %w", err) + } + + return nil +} + +// onDelete is responsible for cleaning up any side effects of onCreateUpdate. +// We only clean up side effects because all resources that we create explicitly +// have an owner reference and will thus be cleaned up by the K8s garbage collector +// once the owning meshv2beta1.XGateway is deleted. +func onDelete[T gateways.Gateway](ctx context.Context, req ctrl.Request, k8sClient client.Client, resource T) error { + // TODO NET-6392 NET-6393 + return nil +} + +// ownedObjectOp represents an operation that needs to be applied +// only if the newObject does not yet exist or if the existingObject +// has an owner reference pointing to the XGateway being reconciled. +// +// The existing and new object are available in case any merging needs +// to occur, such as unknown annotations and values from the existing object +// that need to be carried forward onto the new object. +type ownedObjectOp func(ctx context.Context, existing, desired client.Object) error + +// opIfNewOrOwned runs a given ownedObjectOp to create, update, or delete a resource. +// The purpose of opIfNewOrOwned is to ensure that we aren't updating or deleting a +// resource that was not created by us. If this scenario is encountered, we error. +func opIfNewOrOwned(ctx context.Context, gateway client.Object, k8sClient client.Client, existing, desired client.Object, op ownedObjectOp) error { + // Ensure owner reference is always set on objects that we write + if err := ctrl.SetControllerReference(gateway, desired, k8sClient.Scheme()); err != nil { + return err + } + + key := client.ObjectKey{ + Namespace: existing.GetNamespace(), + Name: existing.GetName(), + } + + exists := false + if err := k8sClient.Get(ctx, key, existing); err != nil { + // We failed to fetch the object in a way that doesn't tell us about its existence + if !k8serr.IsNotFound(err) { + return err + } + } else { + // We successfully fetched the object, so it exists + exists = true + } + + // None exists, so we need only execute the operation + if !exists { + return op(ctx, existing, desired) + } + + // Ensure the existing object was put there by us so that we don't overwrite random objects + owned := false + for _, reference := range existing.GetOwnerReferences() { + if reference.UID == gateway.GetUID() && reference.Name == gateway.GetName() { + owned = true + break + } + } + if !owned { + return errResourceNotOwned + } + return op(ctx, existing, desired) +} + +func getGatewayClassConfigByGatewayClassName(ctx context.Context, k8sClient client.Client, className string) (*meshv2beta1.GatewayClassConfig, error) { + gatewayClass, err := getGatewayClassByName(ctx, k8sClient, className) + if err != nil { + return nil, err + } + + if gatewayClass == nil { + return nil, nil + } + + gatewayClassConfig := &meshv2beta1.GatewayClassConfig{} + if ref := gatewayClass.Spec.ParametersRef; ref != nil { + if ref.Group != meshv2beta1.MeshGroup || ref.Kind != v2beta1.KindGatewayClassConfig { + // TODO @Gateway-Management additionally check for controller name when available + return nil, nil + } + + if err := k8sClient.Get(ctx, types.NamespacedName{Name: ref.Name}, gatewayClassConfig); err != nil { + return nil, client.IgnoreNotFound(err) + } + } + return gatewayClassConfig, nil +} + +func getGatewayClassByName(ctx context.Context, k8sClient client.Client, className string) (*meshv2beta1.GatewayClass, error) { + var gatewayClass meshv2beta1.GatewayClass + + if err := k8sClient.Get(ctx, types.NamespacedName{Name: className}, &gatewayClass); err != nil { + return nil, client.IgnoreNotFound(err) + } + return &gatewayClass, nil +} diff --git a/control-plane/controllers/resources/gateway_controller_setup.go b/control-plane/controllers/resources/gateway_controller_setup.go index 257b6733e4..684d591b86 100644 --- a/control-plane/controllers/resources/gateway_controller_setup.go +++ b/control-plane/controllers/resources/gateway_controller_setup.go @@ -76,40 +76,6 @@ func setupGatewayControllerWithManager[L gatewayList](mgr ctrl.Manager, obj clie Complete(gwc) } -// TODO: uncomment when moving the CRUD hooks from mesh gateway controller -//func getGatewayClassConfigByGatewayClassName(ctx context.Context, k8sClient client.Client, className string) (*meshv2beta1.GatewayClassConfig, error) { -// gatewayClass, err := getGatewayClassByName(ctx, k8sClient, className) -// if err != nil { -// return nil, err -// } -// -// if gatewayClass == nil { -// return nil, nil -// } -// -// gatewayClassConfig := &meshv2beta1.GatewayClassConfig{} -// if ref := gatewayClass.Spec.ParametersRef; ref != nil { -// if ref.Group != meshv2beta1.MeshGroup || ref.Kind != v2beta1.KindGatewayClassConfig { -// // TODO @Gateway-Management additionally check for controller name when available -// return nil, nil -// } -// -// if err := k8sClient.Get(ctx, types.NamespacedName{Name: ref.Name}, gatewayClassConfig); err != nil { -// return nil, client.IgnoreNotFound(err) -// } -// } -// return gatewayClassConfig, nil -//} -// -//func getGatewayClassByName(ctx context.Context, k8sClient client.Client, className string) (*meshv2beta1.GatewayClass, error) { -//var gatewayClass meshv2beta1.GatewayClass -// -// if err := k8sClient.Get(ctx, types.NamespacedName{Name: className}, &gatewayClass); err != nil { -// return nil, client.IgnoreNotFound(err) -// } -// return &gatewayClass, nil -//} - // getGatewayClassesReferencingGatewayClassConfig queries all GatewayClass resources in the // cluster and returns any that reference the given GatewayClassConfig by name. func getGatewayClassesReferencingGatewayClassConfig(ctx context.Context, k8sClient client.Client, configName string) (*meshv2beta1.GatewayClassList, error) { diff --git a/control-plane/controllers/resources/mesh_gateway_controller.go b/control-plane/controllers/resources/mesh_gateway_controller.go index 4fb9b0518c..a49b2bdb40 100644 --- a/control-plane/controllers/resources/mesh_gateway_controller.go +++ b/control-plane/controllers/resources/mesh_gateway_controller.go @@ -6,19 +6,13 @@ package resources import ( "context" "errors" - "fmt" "github.com/go-logr/logr" - appsv1 "k8s.io/api/apps/v1" - corev1 "k8s.io/api/core/v1" - rbacv1 "k8s.io/api/rbac/v1" k8serr "k8s.io/apimachinery/pkg/api/errors" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" - "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" meshv2beta1 "github.com/hashicorp/consul-k8s/control-plane/api/mesh/v2beta1" "github.com/hashicorp/consul-k8s/control-plane/gateways" @@ -56,11 +50,21 @@ func (r *MeshGatewayController) Reconcile(ctx context.Context, req ctrl.Request) if !resource.GetDeletionTimestamp().IsZero() { logger.Info("deletion event") - if err := r.onDelete(ctx, req, resource); err != nil { + if err := onDelete(ctx, req, r.Client, resource); err != nil { return ctrl.Result{}, err } } else { - if err := r.onCreateUpdate(ctx, req, resource); err != nil { + // Fetch GatewayClassConfig for the gateway + gcc, err := getGatewayClassConfigByGatewayClassName(ctx, r.Client, resource.Spec.GatewayClassName) + if err != nil { + r.Log.Error(err, "unable to get gatewayclassconfig for gateway: %s gatewayclass: %s", resource.Name, resource.Spec.GatewayClassName) + return ctrl.Result{}, err + } + + if err := onCreateUpdate(ctx, r.Client, gatewayConfigs{ + gcc: gcc, + gatewayConfig: r.GatewayConfig, + }, resource); err != nil { return ctrl.Result{}, err } } @@ -79,204 +83,3 @@ func (r *MeshGatewayController) UpdateStatus(ctx context.Context, obj client.Obj func (r *MeshGatewayController) SetupWithManager(mgr ctrl.Manager) error { return setupGatewayControllerWithManager[*meshv2beta1.MeshGatewayList](mgr, &meshv2beta1.MeshGateway{}, r.Client, r, MeshGateway_GatewayClassIndex) } - -// onCreateUpdate is responsible for creating/updating all K8s resources that -// are required in order to run a meshv2beta1.MeshGateway. These are created/updated -// in dependency order. -// 1. ServiceAccount -// 2. Deployment -// 3. Service -// 4. Role -// 5. RoleBinding -func (r *MeshGatewayController) onCreateUpdate(ctx context.Context, req ctrl.Request, resource *meshv2beta1.MeshGateway) error { - // Fetch GatewayClassConfig for the gateway - gcc, err := r.getGatewayClassConfigForGateway(ctx, resource) - if err != nil { - r.Log.Error(err, "unable to get gatewayclassconfig for gateway: %s gatewayclass: %s", resource.Name, resource.Spec.GatewayClassName) - return err - } - - builder := gateways.NewGatewayBuilder[*meshv2beta1.MeshGateway](resource, r.GatewayConfig, gcc) - - // Create ServiceAccount - desiredAccount := builder.ServiceAccount() - existingAccount := &corev1.ServiceAccount{ObjectMeta: metav1.ObjectMeta{Namespace: desiredAccount.Namespace, Name: desiredAccount.Name}} - - upsertOp := func(ctx context.Context, _, object client.Object) error { - _, err := controllerutil.CreateOrUpdate(ctx, r.Client, object, func() error { return nil }) - return err - } - - err = r.opIfNewOrOwned(ctx, resource, existingAccount, desiredAccount, upsertOp) - if err != nil { - return fmt.Errorf("unable to create service account: %w", err) - } - - // Create Role - desiredRole := builder.Role() - existingRole := &rbacv1.Role{ObjectMeta: metav1.ObjectMeta{Namespace: desiredRole.Namespace, Name: desiredRole.Name}} - - err = r.opIfNewOrOwned(ctx, resource, existingRole, desiredRole, upsertOp) - if err != nil { - return fmt.Errorf("unable to create role: %w", err) - } - - // Create RoleBinding - desiredBinding := builder.RoleBinding() - existingBinding := &rbacv1.RoleBinding{ObjectMeta: metav1.ObjectMeta{Namespace: desiredBinding.Namespace, Name: desiredBinding.Name}} - - err = r.opIfNewOrOwned(ctx, resource, existingBinding, desiredBinding, upsertOp) - if err != nil { - return fmt.Errorf("unable to create role binding: %w", err) - } - - // Create Service - desiredService := builder.Service() - existingService := &corev1.Service{ObjectMeta: metav1.ObjectMeta{Namespace: desiredService.Namespace, Name: desiredService.Name}} - - mergeServiceOp := func(ctx context.Context, existingObj, desiredObj client.Object) error { - existing := existingObj.(*corev1.Service) - desired := desiredObj.(*corev1.Service) - - _, err := controllerutil.CreateOrUpdate(ctx, r.Client, existing, func() error { - gateways.MergeService(existing, desired) - return nil - }) - return err - } - - err = r.opIfNewOrOwned(ctx, resource, existingService, desiredService, mergeServiceOp) - if err != nil { - return fmt.Errorf("unable to create service: %w", err) - } - - // Create Deployment - desiredDeployment, err := builder.Deployment() - if err != nil { - return fmt.Errorf("unable to create deployment: %w", err) - } - existingDeployment := &appsv1.Deployment{ObjectMeta: metav1.ObjectMeta{Namespace: desiredDeployment.Namespace, Name: desiredDeployment.Name}} - - mergeDeploymentOp := func(ctx context.Context, existingObj, desiredObj client.Object) error { - existing := existingObj.(*appsv1.Deployment) - desired := desiredObj.(*appsv1.Deployment) - - _, err = controllerutil.CreateOrUpdate(ctx, r.Client, existing, func() error { - gateways.MergeDeployment(existing, desired) - return nil - }) - return err - } - - err = r.opIfNewOrOwned(ctx, resource, existingDeployment, desiredDeployment, mergeDeploymentOp) - if err != nil { - return fmt.Errorf("unable to create deployment: %w", err) - } - - return nil -} - -// onDelete is responsible for cleaning up any side effects of onCreateUpdate. -// We only clean up side effects because all resources that we create explicitly -// have an owner reference and will thus be cleaned up by the K8s garbage collector -// once the owning meshv2beta1.MeshGateway is deleted. -func (r *MeshGatewayController) onDelete(ctx context.Context, req ctrl.Request, resource *meshv2beta1.MeshGateway) error { - // TODO NET-6392 NET-6393 - return nil -} - -// ownedObjectOp represents an operation that needs to be applied -// only if the newObject does not yet exist or if the existingObject -// has an owner reference pointing to the MeshGateway being reconciled. -// -// The existing and new object are available in case any merging needs -// to occur, such as unknown annotations and values from the existing object -// that need to be carried forward onto the new object. -type ownedObjectOp func(ctx context.Context, existing, desired client.Object) error - -// opIfNewOrOwned runs a given ownedObjectOp to create, update, or delete a resource. -// The purpose of opIfNewOrOwned is to ensure that we aren't updating or deleting a -// resource that was not created by us. If this scenario is encountered, we error. -func (r *MeshGatewayController) opIfNewOrOwned(ctx context.Context, gateway *meshv2beta1.MeshGateway, existing, desired client.Object, op ownedObjectOp) error { - // Ensure owner reference is always set on objects that we write - if err := ctrl.SetControllerReference(gateway, desired, r.Client.Scheme()); err != nil { - return err - } - - key := client.ObjectKey{ - Namespace: existing.GetNamespace(), - Name: existing.GetName(), - } - - exists := false - if err := r.Get(ctx, key, existing); err != nil { - // We failed to fetch the object in a way that doesn't tell us about its existence - if !k8serr.IsNotFound(err) { - return err - } - } else { - // We successfully fetched the object, so it exists - exists = true - } - - // None exists, so we need only execute the operation - if !exists { - return op(ctx, existing, desired) - } - - // Ensure the existing object was put there by us so that we don't overwrite random objects - owned := false - for _, reference := range existing.GetOwnerReferences() { - if reference.UID == gateway.GetUID() && reference.Name == gateway.GetName() { - owned = true - break - } - } - if !owned { - return errResourceNotOwned - } - return op(ctx, existing, desired) -} - -func (r *MeshGatewayController) getGatewayClassConfigForGateway(ctx context.Context, gateway *meshv2beta1.MeshGateway) (*meshv2beta1.GatewayClassConfig, error) { - gatewayClass, err := r.getGatewayClassForGateway(ctx, gateway) - if err != nil { - return nil, err - } - - gatewayClassConfig, err := r.getGatewayClassConfigForGatewayClass(ctx, gatewayClass) - if err != nil { - return nil, err - } - - return gatewayClassConfig, nil -} - -func (r *MeshGatewayController) getGatewayClassConfigForGatewayClass(ctx context.Context, gatewayClass *meshv2beta1.GatewayClass) (*meshv2beta1.GatewayClassConfig, error) { - if gatewayClass == nil { - // if we don't have a gateway class we can't fetch the corresponding config - return nil, nil - } - - config := &meshv2beta1.GatewayClassConfig{} - if ref := gatewayClass.Spec.ParametersRef; ref != nil { - if ref.Group != meshv2beta1.MeshGroup || ref.Kind != meshv2beta1.KindGatewayClassConfig { - // TODO @Gateway-Management additionally check for controller name when available - return nil, nil - } - - if err := r.Client.Get(ctx, types.NamespacedName{Name: ref.Name}, config); err != nil { - return nil, client.IgnoreNotFound(err) - } - } - return config, nil -} - -func (r *MeshGatewayController) getGatewayClassForGateway(ctx context.Context, gateway *meshv2beta1.MeshGateway) (*meshv2beta1.GatewayClass, error) { - var gatewayClass meshv2beta1.GatewayClass - - if err := r.Client.Get(ctx, types.NamespacedName{Name: string(gateway.Spec.GatewayClassName)}, &gatewayClass); err != nil { - return nil, client.IgnoreNotFound(err) - } - return &gatewayClass, nil -}