From 454232d6e8fe9e6227ca2e122faecbd7d581fcfa Mon Sep 17 00:00:00 2001 From: Laurent Luce Date: Thu, 17 Oct 2024 18:06:36 -0400 Subject: [PATCH] Add generic apply resources func --- .golangci.yml | 1 - internal/controller/core/flow_controller.go | 6 +- kardinal/resources/resources.go | 255 ++------------------ kardinal/resources/utils.go | 2 + kardinal/topology/service.go | 7 +- kardinal/topology/topology.go | 83 +++++-- 6 files changed, 93 insertions(+), 261 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 1e0dc8c..5c03cd2 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -14,7 +14,6 @@ issues: - lll - path: "kardinal/*" linters: - - dupl - lll - path: "internal/*" linters: diff --git a/internal/controller/core/flow_controller.go b/internal/controller/core/flow_controller.go index e775c64..c7dc7da 100644 --- a/internal/controller/core/flow_controller.go +++ b/internal/controller/core/flow_controller.go @@ -23,7 +23,6 @@ import ( ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/log" - "sigs.k8s.io/controller-runtime/pkg/reconcile" corev1 "kardinal.dev/kardinal-operator/api/core/v1" "kardinal.dev/kardinal-operator/kardinal/reconciler" @@ -55,10 +54,7 @@ type FlowReconciler struct { func (r *FlowReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { _ = log.FromContext(ctx) - err := reconciler.Reconcile(ctx, r.Client) - if err != nil { - return reconcile.Result{}, err - } + _ = reconciler.Reconcile(ctx, r.Client) return ctrl.Result{}, nil } diff --git a/kardinal/resources/resources.go b/kardinal/resources/resources.go index 019c704..4bae1ca 100644 --- a/kardinal/resources/resources.go +++ b/kardinal/resources/resources.go @@ -2,7 +2,6 @@ package resources import ( "context" - "reflect" "strings" "github.com/kurtosis-tech/stacktrace" @@ -27,6 +26,8 @@ const ( appNameKubernetesLabelKey = "app.kubernetes.io/name" appLabelKey = "app" versionLabelKey = "version" + + fieldOwner = "kardinal-operator" ) type labeledResources interface { @@ -161,227 +162,6 @@ func AddAnnotations(obj *metav1.ObjectMeta, annotations map[string]string) { } } -// OPERATOR-TODO: Add create, update and delete global options -// OPERATOR-TODO: Refactor the Apply... functions - -func ApplyServiceResources(ctx context.Context, clusterResources *Resources, clusterTopologyResources *Resources, cl client.Client) error { - for _, namespace := range clusterResources.Namespaces { - clusterTopologyNamespace := clusterTopologyResources.GetNamespaceByName(namespace.Name) - if clusterTopologyNamespace != nil { - for _, service := range clusterTopologyNamespace.Services { - namespaceService := namespace.GetService(service.Name) - if namespaceService == nil { - logrus.Infof("Creating service %s", service.Name) - err := cl.Create(ctx, service) - if err != nil { - return stacktrace.Propagate(err, "An error occurred creating service %s", service.Name) - } - } else { - serviceLabels := service.Labels - isManaged, found := serviceLabels[kardinalManagedLabelKey] - if found && isManaged == trueStr { - if !reflect.DeepEqual(namespaceService.Spec, service.Spec) { - service.ResourceVersion = namespaceService.ResourceVersion - err := cl.Update(ctx, service) - if err != nil { - return stacktrace.Propagate(err, "An error occurred updating service %s", service.Name) - } - } - } - } - } - } - - for _, service := range namespace.Services { - serviceLabels := service.Labels - isManaged, found := serviceLabels[kardinalManagedLabelKey] - if found && isManaged == trueStr { - if clusterTopologyNamespace == nil || clusterTopologyNamespace.GetService(service.Name) == nil { - logrus.Infof("Deleting service %s", service.Name) - err := cl.Delete(ctx, service) - if err != nil { - return stacktrace.Propagate(err, "An error occurred deleting service %s", service.Name) - } - } - } - } - } - - return nil -} - -func ApplyDeploymentResources(ctx context.Context, clusterResources *Resources, clusterTopologyResources *Resources, cl client.Client) error { - for _, namespace := range clusterResources.Namespaces { - clusterTopologyNamespace := clusterTopologyResources.GetNamespaceByName(namespace.Name) - if clusterTopologyNamespace != nil { - for _, deployment := range clusterTopologyNamespace.Deployments { - namespaceDeployment := namespace.GetDeployment(deployment.Name) - if namespaceDeployment == nil { - logrus.Infof("Creating deployment %s", deployment.Name) - err := cl.Create(ctx, deployment) - if err != nil { - return stacktrace.Propagate(err, "An error occurred creating deployment %s", deployment.Name) - } - } else { - deploymentLabels := deployment.Labels - isManaged, found := deploymentLabels[kardinalManagedLabelKey] - if found && isManaged == trueStr { - if !reflect.DeepEqual(namespaceDeployment.Spec, deployment.Spec) { - deployment.ResourceVersion = namespaceDeployment.ResourceVersion - err := cl.Update(ctx, deployment) - if err != nil { - return stacktrace.Propagate(err, "An error occurred updating deployment %s", deployment.Name) - } - } - } - } - } - } - - for _, deployment := range namespace.Deployments { - deploymentLabels := deployment.Labels - isManaged, found := deploymentLabels[kardinalManagedLabelKey] - if found && isManaged == trueStr { - if clusterTopologyNamespace == nil || clusterTopologyNamespace.GetDeployment(deployment.Name) == nil { - logrus.Infof("Deleting deployment %s", deployment.Name) - err := cl.Delete(ctx, deployment) - if err != nil { - return stacktrace.Propagate(err, "An error occurred deleting deployment %s", deployment.Name) - } - } - } - /* else { - annotationsToAdd := map[string]string{ - "sidecar.istio.io/inject": "true", - // KARDINAL-TODO: make this a flag to help debugging - // One can view the logs with: kubeclt logs -f -l app= -n -c istio-proxy - "sidecar.istio.io/componentLogLevel": "lua:info", - } - AddAnnotations(&deployment.ObjectMeta, annotationsToAdd) - err := cl.Update(ctx, deployment) - if err != nil { - return stacktrace.Propagate(err, "An error occurred updating deployment %s", deployment.Name) - } - } */ - } - } - - return nil -} - -func ApplyVirtualServiceResources(ctx context.Context, clusterResources *Resources, clusterTopologyResources *Resources, cl client.Client) error { - for _, namespace := range clusterResources.Namespaces { - clusterTopologyNamespace := clusterTopologyResources.GetNamespaceByName(namespace.Name) - if clusterTopologyNamespace != nil { - for _, virtualService := range clusterTopologyNamespace.VirtualServices { - namespaceVirtualService := namespace.GetVirtualService(virtualService.Name) - if namespaceVirtualService == nil { - logrus.Infof("Creating virtual service %s", virtualService.Name) - err := cl.Create(ctx, virtualService) - if err != nil { - return stacktrace.Propagate(err, "An error occurred creating virtual service %s", virtualService.Name) - } - } else { - if !reflect.DeepEqual(&namespaceVirtualService.Spec, &virtualService.Spec) { - virtualService.ResourceVersion = namespaceVirtualService.ResourceVersion - err := cl.Update(ctx, virtualService) - if err != nil { - return stacktrace.Propagate(err, "An error occurred updating virtual service %s", virtualService.Name) - } - } - } - } - } - - for _, virtualService := range namespace.VirtualServices { - if clusterTopologyNamespace == nil || clusterTopologyNamespace.GetVirtualService(virtualService.Name) == nil { - logrus.Infof("Deleting virtual service %s", virtualService.Name) - err := cl.Delete(ctx, virtualService) - if err != nil { - return stacktrace.Propagate(err, "An error occurred deleting virtual service %s", virtualService.Name) - } - } - } - } - - return nil -} - -func ApplyDestinationRuleResources(ctx context.Context, clusterResources *Resources, clusterTopologyResources *Resources, cl client.Client) error { - for _, namespace := range clusterResources.Namespaces { - clusterTopologyNamespace := clusterTopologyResources.GetNamespaceByName(namespace.Name) - if clusterTopologyNamespace != nil { - for _, destinationRule := range clusterTopologyNamespace.DestinationRules { - namespaceDestinationRule := namespace.GetDestinationRule(destinationRule.Name) - if namespaceDestinationRule == nil { - logrus.Infof("Creating destination rule %s", destinationRule.Name) - err := cl.Create(ctx, destinationRule) - if err != nil { - return stacktrace.Propagate(err, "An error occurred creating destination rule %s", destinationRule.Name) - } - } else { - if !reflect.DeepEqual(&namespaceDestinationRule.Spec, &destinationRule.Spec) { - destinationRule.ResourceVersion = namespaceDestinationRule.ResourceVersion - err := cl.Update(ctx, destinationRule) - if err != nil { - return stacktrace.Propagate(err, "An error occurred updating destination rule %s", destinationRule.Name) - } - } - } - } - } - - for _, destinationRule := range namespace.DestinationRules { - if clusterTopologyNamespace == nil || clusterTopologyNamespace.GetDestinationRule(destinationRule.Name) == nil { - logrus.Infof("Deleting destination rule %s", destinationRule.Name) - err := cl.Delete(ctx, destinationRule) - if err != nil { - return stacktrace.Propagate(err, "An error occurred deleting destination rule %s", destinationRule.Name) - } - } - } - } - - return nil -} - -func ApplyIngressResources(ctx context.Context, clusterResources *Resources, clusterTopologyResources *Resources, cl client.Client) error { - for _, namespace := range clusterResources.Namespaces { - clusterTopologyNamespace := clusterTopologyResources.GetNamespaceByName(namespace.Name) - if clusterTopologyNamespace != nil { - for _, ingress := range clusterTopologyNamespace.Ingresses { namespaceIngress := namespace.GetIngress(ingress.Name) - if namespaceIngress == nil { - logrus.Infof("Creating ingress %s", ingress.Name) - err := cl.Create(ctx, ingress) - if err != nil { - return stacktrace.Propagate(err, "An error occurred creating ingress %s", ingress.Name) - } - } else { - if !reflect.DeepEqual(namespaceIngress.Spec, ingress.Spec) { - ingress.ResourceVersion = namespaceIngress.ResourceVersion - err := cl.Update(ctx, ingress) - if err != nil { - return stacktrace.Propagate(err, "An error occurred updating ingress %s", ingress.Name) - } - } - } - } - } - - for _, ingress := range namespace.Ingresses { - if clusterTopologyNamespace == nil || clusterTopologyNamespace.GetIngress(ingress.Name) == nil { - logrus.Infof("Deleting ingress %s", ingress.Name) - err := cl.Delete(ctx, ingress) - if err != nil { - return stacktrace.Propagate(err, "An error occurred deleting ingress %s", ingress.Name) - } - } - } - } - - return nil -} - func ApplyResources( ctx context.Context, clusterResources *Resources, @@ -389,27 +169,28 @@ func ApplyResources( cl client.Client, getObjectsFunc func(namespace *Namespace) []client.Object, getObjectFunc func(namespace *Namespace, name string) client.Object, - compareFunc func(object1 client.Object, object2 client.Object) bool) error { + compareObjectsFunc func(object1 client.Object, object2 client.Object) bool) error { for _, namespace := range clusterResources.Namespaces { clusterTopologyNamespace := clusterTopologyResources.GetNamespaceByName(namespace.Name) if clusterTopologyNamespace != nil { - for _, clusterToplogyNamespaceObject := range getObjectsFunc(clusterTopologyNamespace) { - namespaceObject := getObjectFunc(namespace, clusterToplogyNamespaceObject.GetName()) + for _, clusterTopologyNamespaceObject := range getObjectsFunc(clusterTopologyNamespace) { + namespaceObject := getObjectFunc(namespace, clusterTopologyNamespaceObject.GetName()) if namespaceObject == nil { - logrus.Infof("Creating service %s", clusterToplogyNamespaceObject.GetName()) - err := cl.Create(ctx, clusterToplogyNamespaceObject) + logrus.Infof("Creating %s %s", clusterTopologyNamespaceObject.GetObjectKind().GroupVersionKind().String(), clusterTopologyNamespaceObject.GetName()) + err := cl.Create(ctx, clusterTopologyNamespaceObject, client.FieldOwner(fieldOwner)) if err != nil { - return stacktrace.Propagate(err, "An error occurred creating service %s", clusterToplogyNamespaceObject.GetName()) + return stacktrace.Propagate(err, "An error occurred creating resource %s", clusterTopologyNamespaceObject.GetName()) } } else { namespaceObjectLabels := namespaceObject.GetLabels() isManaged, found := namespaceObjectLabels[kardinalManagedLabelKey] if found && isManaged == trueStr { - if !compareFunc(clusterToplogyNamespaceObject, namespaceObject) { - clusterToplogyNamespaceObject.SetResourceVersion(namespaceObject.GetResourceVersion()) - err := cl.Update(ctx, clusterToplogyNamespaceObject) + if !compareObjectsFunc(clusterTopologyNamespaceObject, namespaceObject) { + logrus.Infof("Updating %s %s", clusterTopologyNamespaceObject.GetObjectKind().GroupVersionKind().String(), clusterTopologyNamespaceObject.GetName()) + clusterTopologyNamespaceObject.SetResourceVersion(namespaceObject.GetResourceVersion()) + err := cl.Update(ctx, clusterTopologyNamespaceObject) if err != nil { - return stacktrace.Propagate(err, "An error occurred updating service %s", clusterToplogyNamespaceObject.GetName()) + return stacktrace.Propagate(err, "An error occurred updating resource %s", clusterTopologyNamespaceObject.GetName()) } } } @@ -421,17 +202,17 @@ func ApplyResources( namespaceObjectLabels := namespaceObject.GetLabels() isManaged, found := namespaceObjectLabels[kardinalManagedLabelKey] if found && isManaged == trueStr { - if clusterTopologyNamespace == nil || getObjectFunc(namespace, namespaceObject.GetName()) == nil { - logrus.Infof("Deleting service %s", namespaceObject.GetName()) - err := cl.Delete(ctx, namespaceObject) + if clusterTopologyNamespace == nil || getObjectFunc(clusterTopologyNamespace, namespaceObject.GetName()) == nil { + logrus.Infof("Deleting %s %s", namespaceObject.GetObjectKind().GroupVersionKind().String(), namespaceObject.GetName()) + err := cl.Delete(ctx, namespaceObject, client.PropagationPolicy(metav1.DeletePropagationForeground)) if err != nil { - return stacktrace.Propagate(err, "An error occurred deleting service %s", namespaceObject.GetName()) + return stacktrace.Propagate(err, "An error occurred deleting resource %s", namespaceObject.GetName()) } } } } } - + return nil } diff --git a/kardinal/resources/utils.go b/kardinal/resources/utils.go index cc51c0a..d754abe 100644 --- a/kardinal/resources/utils.go +++ b/kardinal/resources/utils.go @@ -9,3 +9,5 @@ func IsManaged(objectMeta *metav1.ObjectMeta) bool { } return false } + +func int64Ptr(i int64) *int64 { return &i } diff --git a/kardinal/topology/service.go b/kardinal/topology/service.go index e586a01..b6a7d98 100644 --- a/kardinal/topology/service.go +++ b/kardinal/topology/service.go @@ -214,8 +214,8 @@ func (service *Service) GetVirtualService(services []*Service) (*istioclient.Vir ObjectMeta: metav1.ObjectMeta{ Name: service.ServiceID, Namespace: service.Namespace, - Annotations: map[string]string{ - "kardinal.dev/managed": trueStr, + Labels: map[string]string{ + kardinalManagedLabelKey: trueStr, }, }, Spec: v1alpha3.VirtualService{ @@ -264,6 +264,9 @@ func (service *Service) GetDestinationRule(services []*Service) *istioclient.Des ObjectMeta: metav1.ObjectMeta{ Name: service.ServiceID, Namespace: service.Namespace, + Labels: map[string]string{ + kardinalManagedLabelKey: trueStr, + }, }, Spec: v1alpha3.DestinationRule{ Host: service.ServiceID, diff --git a/kardinal/topology/topology.go b/kardinal/topology/topology.go index 49aa06d..b4e47fb 100644 --- a/kardinal/topology/topology.go +++ b/kardinal/topology/topology.go @@ -3,6 +3,7 @@ package topology import ( "context" "fmt" + "reflect" "strings" "github.com/brunoga/deep" @@ -10,6 +11,7 @@ import ( "github.com/kurtosis-tech/stacktrace" "github.com/samber/lo" "github.com/sirupsen/logrus" + istioclient "istio.io/client-go/pkg/apis/networking/v1alpha3" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" net "k8s.io/api/networking/v1" @@ -229,38 +231,87 @@ func (clusterTopology *ClusterTopology) ApplyResources(ctx context.Context, clus return stacktrace.Propagate(err, "An error occurred retrieving the list of resources") } - err = resources.ApplyServiceResources(ctx, clusterResources, clusterTopologyResources, cl) + err = resources.ApplyResources( + ctx, clusterResources, clusterTopologyResources, cl, + func(namespace *resources.Namespace) []client.Object { + return lo.Map(namespace.Services, func(service *corev1.Service, _ int) client.Object { return service }) + }, + func(namespace *resources.Namespace, name string) client.Object { + service := namespace.GetService(name) + if service == nil { + // We have to return nil here so the interface returned is nil and not just the underlying object + return nil + } else { + return service + } + }, + func(object1 client.Object, object2 client.Object) bool { + return reflect.DeepEqual(object1.(*corev1.Service).Spec, object2.(*corev1.Service).Spec) + }, + ) if err != nil { return stacktrace.Propagate(err, "An error occurred applying the service resources") } + err = resources.ApplyResources( - ctx, - clusterResources, - clusterTopologyResources, - cl, + ctx, clusterResources, clusterTopologyResources, cl, func(namespace *resources.Namespace) []client.Object { - objects := make([]client.Object, len(namespace.Services)) - for i := range namespace.Services { - objects[i] = namespace.Services[i] + return lo.Map(namespace.Deployments, func(deployment *appsv1.Deployment, _ int) client.Object { return deployment }) + }, + func(namespace *resources.Namespace, name string) client.Object { + deployment := namespace.GetDeployment(name) + if deployment == nil { + // We have to return nil here so the interface returned is nil and not just the underlying object + return nil } - return objects + return deployment + }, + func(object1 client.Object, object2 client.Object) bool { + return reflect.DeepEqual(object1.(*appsv1.Deployment).Spec, object2.(*appsv1.Deployment).Spec) }, - func(namespace *resources.Namespace, name string) client.Object { return namespace.GetService(name) }, - func(object1 client.Object, object2 client.Object) bool { return true }, ) - - err = resources.ApplyDeploymentResources(ctx, clusterResources, clusterTopologyResources, cl) - if err != nil { return stacktrace.Propagate(err, "An error occurred applying the deployment resources") } - err = resources.ApplyVirtualServiceResources(ctx, clusterResources, clusterTopologyResources, cl) + err = resources.ApplyResources( + ctx, clusterResources, clusterTopologyResources, cl, + func(namespace *resources.Namespace) []client.Object { + return lo.Map(namespace.VirtualServices, func(virtualService *istioclient.VirtualService, _ int) client.Object { return virtualService }) + }, + func(namespace *resources.Namespace, name string) client.Object { + virtualService := namespace.GetVirtualService(name) + if virtualService == nil { + // We have to return nil here so the interface returned is nil and not just the underlying object + return nil + } + return virtualService + }, + func(object1 client.Object, object2 client.Object) bool { + return reflect.DeepEqual(&object1.(*istioclient.VirtualService).Spec, &object2.(*istioclient.VirtualService).Spec) + }, + ) if err != nil { return stacktrace.Propagate(err, "An error occurred applying the virtual service resources") } - err = resources.ApplyDestinationRuleResources(ctx, clusterResources, clusterTopologyResources, cl) + err = resources.ApplyResources( + ctx, clusterResources, clusterTopologyResources, cl, + func(namespace *resources.Namespace) []client.Object { + return lo.Map(namespace.DestinationRules, func(destinationRule *istioclient.DestinationRule, _ int) client.Object { return destinationRule }) + }, + func(namespace *resources.Namespace, name string) client.Object { + destinationRule := namespace.GetDestinationRule(name) + if destinationRule == nil { + // We have to return nil here so the interface returned is nil and not just the underlying object + return nil + } + return destinationRule + }, + func(object1 client.Object, object2 client.Object) bool { + return reflect.DeepEqual(&object1.(*istioclient.DestinationRule).Spec, &object2.(*istioclient.DestinationRule).Spec) + }, + ) if err != nil { return stacktrace.Propagate(err, "An error occurred applying the virtual service resources") }