From 607ebf4b03950da13f108677e54d90208c90a406 Mon Sep 17 00:00:00 2001 From: Laurent Luce Date: Wed, 9 Oct 2024 09:09:40 -0400 Subject: [PATCH 1/8] Refactoring apply resources --- kardinal/resources/resources.go | 52 +++++++++++++++++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/kardinal/resources/resources.go b/kardinal/resources/resources.go index 1a3a8a7..94f8e8d 100644 --- a/kardinal/resources/resources.go +++ b/kardinal/resources/resources.go @@ -338,6 +338,12 @@ func ApplyIngressResources(ctx context.Context, clusterResources *Resources, clu clusterTopologyNamespace := clusterTopologyResources.GetNamespaceByName(namespace.Name) if clusterTopologyNamespace != nil { for _, ingress := range clusterTopologyNamespace.Ingresses { + var obj client.Object + obj = ingress + switch obj := obj.(type) { + case *net.Ingress: + obj.Spec = + } namespaceIngress := namespace.GetService(ingress.Name) if namespaceIngress == nil { logrus.Infof("Creating ingress %s", ingress.Name) @@ -370,3 +376,49 @@ func ApplyIngressResources(ctx context.Context, clusterResources *Resources, clu return nil } + +func ApplyResources(ctx context.Context, clusterResources *Resources, clusterTopologyResources *Resources, cl client.Client, getObjectFunc func(namespace *Namespace, name string) *client.Object) error { + for _, namespace := range clusterResources.Namespaces { + clusterTopologyNamespace := clusterTopologyResources.GetNamespaceByName(namespace.Name) + if clusterTopologyNamespace != nil { + for _, service := range clusterTopologyNamespace.Services { + namespaceService := getObjectFunc(namespace, 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 +} From dee81b37d6d30ca53749ef567b88e742f6e149c2 Mon Sep 17 00:00:00 2001 From: Laurent Luce Date: Thu, 17 Oct 2024 12:29:35 -0400 Subject: [PATCH 2/8] Add generic apply resources func --- kardinal/resources/resources.go | 54 ++++++++++++++++----------------- kardinal/topology/topology.go | 16 ++++++++++ 2 files changed, 42 insertions(+), 28 deletions(-) diff --git a/kardinal/resources/resources.go b/kardinal/resources/resources.go index 94f8e8d..d7bbea5 100644 --- a/kardinal/resources/resources.go +++ b/kardinal/resources/resources.go @@ -337,14 +337,7 @@ func ApplyIngressResources(ctx context.Context, clusterResources *Resources, clu for _, namespace := range clusterResources.Namespaces { clusterTopologyNamespace := clusterTopologyResources.GetNamespaceByName(namespace.Name) if clusterTopologyNamespace != nil { - for _, ingress := range clusterTopologyNamespace.Ingresses { - var obj client.Object - obj = ingress - switch obj := obj.(type) { - case *net.Ingress: - obj.Spec = - } - namespaceIngress := namespace.GetService(ingress.Name) + 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) @@ -377,27 +370,32 @@ func ApplyIngressResources(ctx context.Context, clusterResources *Resources, clu return nil } -func ApplyResources(ctx context.Context, clusterResources *Resources, clusterTopologyResources *Resources, cl client.Client, getObjectFunc func(namespace *Namespace, name string) *client.Object) error { +func ApplyResources( + ctx context.Context, + clusterResources *Resources, + clusterTopologyResources *Resources, + 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 { for _, namespace := range clusterResources.Namespaces { clusterTopologyNamespace := clusterTopologyResources.GetNamespaceByName(namespace.Name) if clusterTopologyNamespace != nil { - for _, service := range clusterTopologyNamespace.Services { - namespaceService := getObjectFunc(namespace, service.Name) - if namespaceService == nil { - logrus.Infof("Creating service %s", service.Name) - err := cl.Create(ctx, service) + for _, clusterToplogyNamespaceObject := range getObjectsFunc(clusterTopologyNamespace) { + namespaceObject := getObjectFunc(namespace, clusterToplogyNamespaceObject.GetName()) + if namespaceObject == nil { + logrus.Infof("Creating service %s", clusterToplogyNamespaceObject.GetName()) + err := cl.Create(ctx, clusterToplogyNamespaceObject) if err != nil { - return stacktrace.Propagate(err, "An error occurred creating service %s", service.Name) + return stacktrace.Propagate(err, "An error occurred creating service %s", clusterToplogyNamespaceObject.GetName()) } } else { - serviceLabels := service.Labels - isManaged, found := serviceLabels[kardinalManagedLabelKey] + namespaceObjectLabels := namespaceObject.GetLabels() + isManaged, found := namespaceObjectLabels[kardinalManagedLabelKey] if found && isManaged == trueStr { - if !reflect.DeepEqual(namespaceService.Spec, service.Spec) { - service.ResourceVersion = namespaceService.ResourceVersion - err := cl.Update(ctx, service) + if !compareFunc(clusterToplogyNamespaceObject, namespaceObject) { + clusterToplogyNamespaceObject.SetResourceVersion(namespaceObject.GetResourceVersion()) + err := cl.Update(ctx, clusterToplogyNamespaceObject) if err != nil { - return stacktrace.Propagate(err, "An error occurred updating service %s", service.Name) + return stacktrace.Propagate(err, "An error occurred updating service %s", clusterToplogyNamespaceObject.GetName()) } } } @@ -405,15 +403,15 @@ func ApplyResources(ctx context.Context, clusterResources *Resources, clusterTop } } - for _, service := range namespace.Services { - serviceLabels := service.Labels - isManaged, found := serviceLabels[kardinalManagedLabelKey] + for _, namespaceObject := range getObjectsFunc(namespace) { + namespaceObjectLabels := namespaceObject.GetLabels() + isManaged, found := namespaceObjectLabels[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 clusterTopologyNamespace == nil || getObjectFunc(namespace, namespaceObject.GetName()) == nil { + logrus.Infof("Deleting service %s", namespaceObject.GetName()) + err := cl.Delete(ctx, namespaceObject) if err != nil { - return stacktrace.Propagate(err, "An error occurred deleting service %s", service.Name) + return stacktrace.Propagate(err, "An error occurred deleting service %s", namespaceObject.GetName()) } } } diff --git a/kardinal/topology/topology.go b/kardinal/topology/topology.go index 80bee44..98a92ca 100644 --- a/kardinal/topology/topology.go +++ b/kardinal/topology/topology.go @@ -223,8 +223,24 @@ func (clusterTopology *ClusterTopology) ApplyResources(ctx context.Context, clus if err != nil { return stacktrace.Propagate(err, "An error occurred applying the service resources") } + err = resources.ApplyResources( + 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 objects + }, + 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") } From 454232d6e8fe9e6227ca2e122faecbd7d581fcfa Mon Sep 17 00:00:00 2001 From: Laurent Luce Date: Thu, 17 Oct 2024 18:06:36 -0400 Subject: [PATCH 3/8] 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") } From 18336a705e147cd5e978a82addfe5a5e6763d7ca Mon Sep 17 00:00:00 2001 From: Laurent Luce Date: Thu, 17 Oct 2024 21:54:29 -0400 Subject: [PATCH 4/8] Cleanup --- kardinal/resources/resources.go | 5 +++++ kardinal/topology/topology.go | 2 -- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/kardinal/resources/resources.go b/kardinal/resources/resources.go index 4bae1ca..265c45a 100644 --- a/kardinal/resources/resources.go +++ b/kardinal/resources/resources.go @@ -164,11 +164,16 @@ func AddAnnotations(obj *metav1.ObjectMeta, annotations map[string]string) { func ApplyResources( ctx context.Context, + // Current cluster resources clusterResources *Resources, + // Base + flows resources to reconcile clusterTopologyResources *Resources, cl client.Client, + // Function to retrieve the namespace resources getObjectsFunc func(namespace *Namespace) []client.Object, + // Function to retrieve a resource by namespace and name getObjectFunc func(namespace *Namespace, name string) client.Object, + // Function to compare two resources compareObjectsFunc func(object1 client.Object, object2 client.Object) bool) error { for _, namespace := range clusterResources.Namespaces { clusterTopologyNamespace := clusterTopologyResources.GetNamespaceByName(namespace.Name) diff --git a/kardinal/topology/topology.go b/kardinal/topology/topology.go index b4e47fb..2c4c075 100644 --- a/kardinal/topology/topology.go +++ b/kardinal/topology/topology.go @@ -394,7 +394,6 @@ func (clusterTopology *ClusterTopology) Merge(clusterTopologies []*ClusterTopolo mergedClusterTopology.Ingress.ActiveFlowIDs = append(mergedClusterTopology.Ingress.ActiveFlowIDs, topology.Ingress.ActiveFlowIDs...) } mergedClusterTopology.Ingress.ActiveFlowIDs = lo.Uniq(mergedClusterTopology.Ingress.ActiveFlowIDs) - logrus.Infof("Services length: %d", len(mergedClusterTopology.Services)) // KARDINAL-TODO improve the filtering method, we could implement the `Service.Equal` method to compare and filter the services and inside this method we could use the k8s service marshall method (https://pkg.go.dev/k8s.io/api/core/v1#Service.Marsha) and also the same for other k8s fields it should be faster mergedClusterTopology.Services = lo.UniqBy(mergedClusterTopology.Services, func(service *Service) ServiceVersion { @@ -405,7 +404,6 @@ func (clusterTopology *ClusterTopology) Merge(clusterTopologies []*ClusterTopolo } return serviceVersion }) - logrus.Infof("Services length: %d", len(mergedClusterTopology.Services)) mergedClusterTopology.ServiceDependencies = lo.UniqBy(mergedClusterTopology.ServiceDependencies, func(serviceDependency *ServiceDependency) ServiceDependencyVersion { serviceDependencyVersion := ServiceDependencyVersion{ ServiceID: serviceDependency.Service.ServiceID, From 4640f898d08364ec37627511dd4d49a53672def3 Mon Sep 17 00:00:00 2001 From: Laurent Luce Date: Thu, 17 Oct 2024 22:01:18 -0400 Subject: [PATCH 5/8] Cleanup --- kardinal/resources/utils.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/kardinal/resources/utils.go b/kardinal/resources/utils.go index d754abe..cc51c0a 100644 --- a/kardinal/resources/utils.go +++ b/kardinal/resources/utils.go @@ -9,5 +9,3 @@ func IsManaged(objectMeta *metav1.ObjectMeta) bool { } return false } - -func int64Ptr(i int64) *int64 { return &i } From 3e87503d1b120be2eaa9a043b0dffca25f735ff8 Mon Sep 17 00:00:00 2001 From: Laurent Luce Date: Fri, 18 Oct 2024 11:28:21 -0400 Subject: [PATCH 6/8] Address comments --- NOTES | 3 +++ kardinal/resources/resources.go | 14 ++++++-------- 2 files changed, 9 insertions(+), 8 deletions(-) create mode 100644 NOTES diff --git a/NOTES b/NOTES new file mode 100644 index 0000000..1f713ae --- /dev/null +++ b/NOTES @@ -0,0 +1,3 @@ +Deployments need the following annotation "sidecar.istio.io/componentLogLevel": "lua:info" to be able to see the logs with: kubeclt logs -f -l app= -n -c istio-proxy + + diff --git a/kardinal/resources/resources.go b/kardinal/resources/resources.go index 265c45a..94d748f 100644 --- a/kardinal/resources/resources.go +++ b/kardinal/resources/resources.go @@ -162,18 +162,16 @@ func AddAnnotations(obj *metav1.ObjectMeta, annotations map[string]string) { } } +// ApplyResources compares the current cluster resources with the base + flows topology resources and applies the differences. +// getObjectFunc and getObjectsFunc are used to retrieve the namespace resources. +// compareObjectsFunc is used to compare two resources func ApplyResources( ctx context.Context, - // Current cluster resources clusterResources *Resources, - // Base + flows resources to reconcile clusterTopologyResources *Resources, cl client.Client, - // Function to retrieve the namespace resources getObjectsFunc func(namespace *Namespace) []client.Object, - // Function to retrieve a resource by namespace and name getObjectFunc func(namespace *Namespace, name string) client.Object, - // Function to compare two resources compareObjectsFunc func(object1 client.Object, object2 client.Object) bool) error { for _, namespace := range clusterResources.Namespaces { clusterTopologyNamespace := clusterTopologyResources.GetNamespaceByName(namespace.Name) @@ -184,7 +182,7 @@ func ApplyResources( 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 resource %s", clusterTopologyNamespaceObject.GetName()) + return stacktrace.Propagate(err, "An error occurred creating %s %s", clusterTopologyNamespaceObject.GetObjectKind().GroupVersionKind().String(), clusterTopologyNamespaceObject.GetName()) } } else { namespaceObjectLabels := namespaceObject.GetLabels() @@ -195,7 +193,7 @@ func ApplyResources( clusterTopologyNamespaceObject.SetResourceVersion(namespaceObject.GetResourceVersion()) err := cl.Update(ctx, clusterTopologyNamespaceObject) if err != nil { - return stacktrace.Propagate(err, "An error occurred updating resource %s", clusterTopologyNamespaceObject.GetName()) + return stacktrace.Propagate(err, "An error occurred updating %s %s", clusterTopologyNamespaceObject.GetObjectKind().GroupVersionKind().String(), clusterTopologyNamespaceObject.GetName()) } } } @@ -211,7 +209,7 @@ func ApplyResources( 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 resource %s", namespaceObject.GetName()) + return stacktrace.Propagate(err, "An error occurred deleting %s %s", namespaceObject.GetObjectKind().GroupVersionKind().String(), namespaceObject.GetName()) } } } From ef993e34896d0cf9de23161e4938cf0efe4294ce Mon Sep 17 00:00:00 2001 From: Laurent Luce Date: Fri, 18 Oct 2024 11:30:21 -0400 Subject: [PATCH 7/8] Linting --- kardinal/resources/resources.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/kardinal/resources/resources.go b/kardinal/resources/resources.go index 94d748f..b214a6a 100644 --- a/kardinal/resources/resources.go +++ b/kardinal/resources/resources.go @@ -164,7 +164,7 @@ func AddAnnotations(obj *metav1.ObjectMeta, annotations map[string]string) { // ApplyResources compares the current cluster resources with the base + flows topology resources and applies the differences. // getObjectFunc and getObjectsFunc are used to retrieve the namespace resources. -// compareObjectsFunc is used to compare two resources +// compareObjectsFunc is used to compare two resources func ApplyResources( ctx context.Context, clusterResources *Resources, @@ -209,7 +209,7 @@ func ApplyResources( 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 %s %s", namespaceObject.GetObjectKind().GroupVersionKind().String(), namespaceObject.GetName()) + return stacktrace.Propagate(err, "An error occurred deleting %s %s", namespaceObject.GetObjectKind().GroupVersionKind().String(), namespaceObject.GetName()) } } } From bb1428aadbcb4a74556ba8bf6bfa6abd4e916cba Mon Sep 17 00:00:00 2001 From: Laurent Luce Date: Fri, 18 Oct 2024 13:37:08 -0400 Subject: [PATCH 8/8] Bump golangci linter --- .golangci.yml | 2 +- Makefile | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 5c03cd2..600cca8 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -24,7 +24,7 @@ linters: enable: - dupl - errcheck - - exportloopref + - copyloopvar - ginkgolinter - goconst - gocyclo diff --git a/Makefile b/Makefile index cf27a38..5c67073 100644 --- a/Makefile +++ b/Makefile @@ -161,7 +161,7 @@ GOLANGCI_LINT = $(LOCALBIN)/golangci-lint KUSTOMIZE_VERSION ?= v5.4.3 CONTROLLER_TOOLS_VERSION ?= v0.16.1 ENVTEST_VERSION ?= release-0.19 -GOLANGCI_LINT_VERSION ?= v1.59.1 +GOLANGCI_LINT_VERSION ?= v1.61.0 .PHONY: kustomize kustomize: $(KUSTOMIZE) ## Download kustomize locally if necessary.