From 26b99e5af3461f0a87a9ea5e1a3362e94c727e99 Mon Sep 17 00:00:00 2001 From: "lukasz.widera@vshn.ch" Date: Fri, 6 Sep 2024 16:28:00 +0200 Subject: [PATCH 1/5] Removing bug in Cloudsclae exporter --- main.go | 22 +++++++---- pkg/cloudscale/objectstorage.go | 69 +++++++++++++++++++++++++-------- pkg/cmd/cloudscale.go | 8 ++-- pkg/kubernetes/client.go | 3 -- 4 files changed, 71 insertions(+), 31 deletions(-) diff --git a/main.go b/main.go index e6cdfa7..980f715 100644 --- a/main.go +++ b/main.go @@ -71,14 +71,22 @@ func main() { ctx, stop, app := newApp() defer stop() - go func() { - http.Handle("/metrics", promhttp.Handler()) - err := http.ListenAndServe(":2112", nil) - if err != nil { - fmt.Println("Error starting prometheus server: ", err.Error()) + go func(ctx context.Context) { + ctxx, cancel := context.WithCancel(ctx) + defer cancel() + select { + case <-ctxx.Done(): + fmt.Println("Shutting down prometheus server") + return + default: + http.Handle("/metrics", promhttp.Handler()) + err := http.ListenAndServe(":2112", nil) + if err != nil { + fmt.Println("Error starting prometheus server: ", err.Error()) + } + os.Exit(1) } - os.Exit(1) - }() + }(ctx) err := app.RunContext(ctx, os.Args) // If required flags aren't set, it will return with error before we could set up logging diff --git a/pkg/cloudscale/objectstorage.go b/pkg/cloudscale/objectstorage.go index 2b2f12d..3706a13 100644 --- a/pkg/cloudscale/objectstorage.go +++ b/pkg/cloudscale/objectstorage.go @@ -4,6 +4,7 @@ import ( "context" "errors" "fmt" + "strings" "time" "github.com/prometheus/client_golang/prometheus" @@ -38,6 +39,12 @@ const ( namespaceLabel = "crossplane.io/claim-namespace" ) +type ObjectStorageData struct { + cloudscale.BucketMetricsData + BucketDetail + Organization string +} + func NewObjectStorage(client *cloudscale.Client, k8sClient k8s.Client, controlApiClient k8s.Client, salesOrder, clusterId string, cloudZone string, uomMapping map[string]string, providerMetrics map[string]prometheus.Counter) (*ObjectStorage, error) { return &ObjectStorage{ client: client, @@ -61,8 +68,29 @@ func (o *ObjectStorage) GetMetrics(ctx context.Context, billingDate time.Time) ( if err != nil { o.providerMetrics["providerFailed"].Inc() return nil, err - } else { - o.providerMetrics["providerSucceeded"].Inc() + } + + bucketMap := make(map[string]*ObjectStorageData) + + // create a map with bucket name as key, this way we match buckets created manually and not via Appcat service + for i, bucketMetric := range bucketMetrics.Data { + bucketMap[bucketMetric.Subject.BucketName] = &ObjectStorageData{ + BucketMetricsData: bucketMetrics.Data[i], + } + } + + // Since our buckets are always created in the convention $namespace.$bucketname, we can extract the namespace from the bucket name by splitting it. + // However, we need to fetch the user details to get the actual namespace. + for _, bucket := range bucketMap { + // fetch bucket user by id + logger.Info("fetching user details", "userID", bucket.Subject.ObjectsUserID) + userDetails, err := o.client.ObjectsUsers.Get(ctx, bucket.Subject.ObjectsUserID) + if err != nil { + o.providerMetrics["providerFailed"].Inc() + logger.Error(err, "unknown userID, something broke here fatally") + return nil, err + } + bucket.BucketDetail.Namespace = strings.Split(userDetails.DisplayName, ".")[0] } // Fetch organisations in case salesOrder is missing @@ -71,6 +99,7 @@ func (o *ObjectStorage) GetMetrics(ctx context.Context, billingDate time.Time) ( logger.V(1).Info("Sales order id is missing, fetching namespaces to get the associated org id") nsTenants, err = kubernetes.FetchNamespaceWithOrganizationMap(ctx, o.k8sClient) if err != nil { + o.providerMetrics["providerFailed"].Inc() return nil, err } } @@ -79,37 +108,45 @@ func (o *ObjectStorage) GetMetrics(ctx context.Context, billingDate time.Time) ( buckets, err := fetchBuckets(ctx, o.k8sClient) if err != nil { + o.providerMetrics["providerFailed"].Inc() return nil, err } - allRecords := make([]odoo.OdooMeteredBillingRecord, 0) - for _, bucketMetricsData := range bucketMetrics.Data { - name := bucketMetricsData.Subject.BucketName - logger = logger.WithValues("bucket", name) - bd, ok := buckets[name] - if !ok { - logger.Info("unable to sync bucket, ObjectBucket not found") - continue + for bucket := range bucketMap { + bucketName := bucketMap[bucket].BucketMetricsData.Subject.BucketName + if val, ok := buckets[bucketName]; ok { + bucketMap[bucket].Zone = val.Zone } + + // assign organisation to bucketMap + if val, ok := nsTenants[bucketMap[bucket].Namespace]; ok { + bucketMap[bucket].Organization = val + } + + } + + allRecords := make([]odoo.OdooMeteredBillingRecord, 0) + for _, bucket := range bucketMap { + appuioManaged := true salesOrder := o.salesOrder if salesOrder == "" { appuioManaged = false - salesOrder, err = controlAPI.GetSalesOrder(ctx, o.controlApiClient, nsTenants[bd.Namespace]) + salesOrder, err = controlAPI.GetSalesOrder(ctx, o.controlApiClient, bucket.Organization) if err != nil { - logger.Error(err, "unable to sync bucket", "namespace", bd.Namespace) + logger.Error(err, "unable to sync bucket", "namespace", bucket) continue } } - records, err := o.createOdooRecord(bucketMetricsData, bd, appuioManaged, salesOrder, billingDate) + records, err := o.createOdooRecord(bucket.BucketMetricsData, bucket.BucketDetail, appuioManaged, salesOrder, billingDate) if err != nil { - logger.Error(err, "unable to create Odoo Record", "namespace", bd.Namespace) + logger.Error(err, "unable to create Odoo Record", "namespace", bucket.Namespace) continue } allRecords = append(allRecords, records...) - logger.V(1).Info("Created Odoo records", "namespace", bd.Namespace, "records", records) + logger.V(1).Info("Created Odoo records", "namespace", bucket, "records", records) } - + o.providerMetrics["providerSucceeded"].Inc() return allRecords, nil } diff --git a/pkg/cmd/cloudscale.go b/pkg/cmd/cloudscale.go index 38d4a16..cd1bec8 100644 --- a/pkg/cmd/cloudscale.go +++ b/pkg/cmd/cloudscale.go @@ -123,7 +123,7 @@ func CloudscaleCmds(allMetrics map[string]map[string]prometheus.Counter) *cli.Co wg.Add(1) go func() { for { - if time.Now().Hour() >= billingHour { + if true { billingDate := time.Now().In(location) if days != 0 { @@ -136,21 +136,19 @@ func CloudscaleCmds(allMetrics map[string]map[string]prometheus.Counter) *cli.Co logger.Error(err, "could not collect cloudscale bucket metrics") wg.Done() } - if len(metrics) == 0 { logger.Info("No data to export to odoo", "date", billingDate) time.Sleep(time.Hour) continue } - logger.Info("Exporting data to Odoo", "billingHour", billingHour, "date", billingDate) err = odooClient.SendData(metrics) if err != nil { logger.Error(err, "could not export cloudscale bucket metrics") } - time.Sleep(time.Hour * time.Duration(collectInterval)) + time.Sleep(time.Hour * time.Duration(15)) } - time.Sleep(time.Hour) + time.Sleep(time.Minute * 1) } }() wg.Wait() diff --git a/pkg/kubernetes/client.go b/pkg/kubernetes/client.go index 1366f21..e4dcf0a 100644 --- a/pkg/kubernetes/client.go +++ b/pkg/kubernetes/client.go @@ -5,7 +5,6 @@ import ( "fmt" orgv1 "github.com/appuio/control-api/apis/organization/v1" - "github.com/vshn/billing-collector-cloudservices/pkg/log" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime/schema" ctrl "sigs.k8s.io/controller-runtime" @@ -77,7 +76,6 @@ func restConfig(kubeconfig string, url string, token string) (*rest.Config, erro } func FetchNamespaceWithOrganizationMap(ctx context.Context, k8sClient client.Client) (map[string]string, error) { - logger := log.Logger(ctx) gvk := schema.GroupVersionKind{ Group: "", @@ -96,7 +94,6 @@ func FetchNamespaceWithOrganizationMap(ctx context.Context, k8sClient client.Cli for _, ns := range list.Items { orgLabel, ok := ns.GetLabels()[OrganizationLabel] if !ok { - logger.Info("Organization label not found in namespace", "namespace", ns.GetName()) continue } namespaces[ns.GetName()] = orgLabel From 22227d06eff131ba28e0ed8548ef6b00069e0db0 Mon Sep 17 00:00:00 2001 From: "lukasz.widera@vshn.ch" Date: Mon, 9 Sep 2024 12:14:05 +0200 Subject: [PATCH 2/5] restoring correct intervals in cloudscale.go --- pkg/cmd/cloudscale.go | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/pkg/cmd/cloudscale.go b/pkg/cmd/cloudscale.go index cd1bec8..38d4a16 100644 --- a/pkg/cmd/cloudscale.go +++ b/pkg/cmd/cloudscale.go @@ -123,7 +123,7 @@ func CloudscaleCmds(allMetrics map[string]map[string]prometheus.Counter) *cli.Co wg.Add(1) go func() { for { - if true { + if time.Now().Hour() >= billingHour { billingDate := time.Now().In(location) if days != 0 { @@ -136,19 +136,21 @@ func CloudscaleCmds(allMetrics map[string]map[string]prometheus.Counter) *cli.Co logger.Error(err, "could not collect cloudscale bucket metrics") wg.Done() } + if len(metrics) == 0 { logger.Info("No data to export to odoo", "date", billingDate) time.Sleep(time.Hour) continue } + logger.Info("Exporting data to Odoo", "billingHour", billingHour, "date", billingDate) err = odooClient.SendData(metrics) if err != nil { logger.Error(err, "could not export cloudscale bucket metrics") } - time.Sleep(time.Hour * time.Duration(15)) + time.Sleep(time.Hour * time.Duration(collectInterval)) } - time.Sleep(time.Minute * 1) + time.Sleep(time.Hour) } }() wg.Wait() From 89ca3f543c38a4230ac39bfbac6c391fcd0d06ad Mon Sep 17 00:00:00 2001 From: "lukasz.widera@vshn.ch" Date: Wed, 2 Oct 2024 11:22:53 +0200 Subject: [PATCH 3/5] defaulting organization to prevent fails --- pkg/cloudscale/objectstorage.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/pkg/cloudscale/objectstorage.go b/pkg/cloudscale/objectstorage.go index 3706a13..cb5d60b 100644 --- a/pkg/cloudscale/objectstorage.go +++ b/pkg/cloudscale/objectstorage.go @@ -132,9 +132,14 @@ func (o *ObjectStorage) GetMetrics(ctx context.Context, billingDate time.Time) ( salesOrder := o.salesOrder if salesOrder == "" { appuioManaged = false + if bucket.Organization == "" { + // in cases that our VSHN services are using buckets, then Organization is not set, we must default it to "vshn" + // we can't set it in cluster as for customers as then we might run into scheduling issues + bucket.Organization = "vshn" + } salesOrder, err = controlAPI.GetSalesOrder(ctx, o.controlApiClient, bucket.Organization) if err != nil { - logger.Error(err, "unable to sync bucket", "namespace", bucket) + logger.Error(err, "unable to sync bucket", "namespace", bucket, "reason", err) continue } } From ee7b527ef036fccb9e4e4815af15c30606dd8b8f Mon Sep 17 00:00:00 2001 From: "lukasz.widera@vshn.ch" Date: Wed, 2 Oct 2024 11:44:18 +0200 Subject: [PATCH 4/5] switching how we handle broken userID fetch --- pkg/cloudscale/objectstorage.go | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/pkg/cloudscale/objectstorage.go b/pkg/cloudscale/objectstorage.go index cb5d60b..6d5c536 100644 --- a/pkg/cloudscale/objectstorage.go +++ b/pkg/cloudscale/objectstorage.go @@ -81,14 +81,16 @@ func (o *ObjectStorage) GetMetrics(ctx context.Context, billingDate time.Time) ( // Since our buckets are always created in the convention $namespace.$bucketname, we can extract the namespace from the bucket name by splitting it. // However, we need to fetch the user details to get the actual namespace. - for _, bucket := range bucketMap { + for key, bucket := range bucketMap { // fetch bucket user by id logger.Info("fetching user details", "userID", bucket.Subject.ObjectsUserID) userDetails, err := o.client.ObjectsUsers.Get(ctx, bucket.Subject.ObjectsUserID) if err != nil { o.providerMetrics["providerFailed"].Inc() - logger.Error(err, "unknown userID, something broke here fatally") - return nil, err + logger.Error(err, "unknown userID, something broke here fatally", "userID", bucket.Subject.ObjectsUserID, "bucket", bucket) + // deleting this bucket as it's unsuable + delete(bucketMap, key) + continue } bucket.BucketDetail.Namespace = strings.Split(userDetails.DisplayName, ".")[0] } From c615442ad6313ed09144d16daa9ca5781edad5f1 Mon Sep 17 00:00:00 2001 From: "lukasz.widera@vshn.ch" Date: Wed, 2 Oct 2024 12:36:47 +0200 Subject: [PATCH 5/5] refactoring for loop --- pkg/cloudscale/objectstorage.go | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/pkg/cloudscale/objectstorage.go b/pkg/cloudscale/objectstorage.go index 6d5c536..5da11cd 100644 --- a/pkg/cloudscale/objectstorage.go +++ b/pkg/cloudscale/objectstorage.go @@ -114,17 +114,15 @@ func (o *ObjectStorage) GetMetrics(ctx context.Context, billingDate time.Time) ( return nil, err } - for bucket := range bucketMap { - bucketName := bucketMap[bucket].BucketMetricsData.Subject.BucketName - if val, ok := buckets[bucketName]; ok { - bucketMap[bucket].Zone = val.Zone + for name, bucket := range bucketMap { + if val, ok := buckets[name]; ok { + bucket.Zone = val.Zone } // assign organisation to bucketMap - if val, ok := nsTenants[bucketMap[bucket].Namespace]; ok { - bucketMap[bucket].Organization = val + if val, ok := nsTenants[bucket.Namespace]; ok { + bucket.Organization = val } - } allRecords := make([]odoo.OdooMeteredBillingRecord, 0)