From 355091e1d5d0284ea58be7e6605f04c5e076d924 Mon Sep 17 00:00:00 2001 From: Rajagopalan Subrahmanian Date: Thu, 1 Aug 2024 20:40:10 -0400 Subject: [PATCH 1/7] add support for custom collectors --- apstra/api_custom_collector.go | 257 ++++++++++++++++++++++++++++ apstra/api_custom_collector_test.go | 141 +++++++++++++++ apstra/enum.go | 75 ++++++++ 3 files changed, 473 insertions(+) create mode 100644 apstra/api_custom_collector.go create mode 100644 apstra/api_custom_collector_test.go diff --git a/apstra/api_custom_collector.go b/apstra/api_custom_collector.go new file mode 100644 index 00000000..d71cebd4 --- /dev/null +++ b/apstra/api_custom_collector.go @@ -0,0 +1,257 @@ +package apstra + +import ( + "context" + "encoding/json" + "errors" + "fmt" + "net/http" + "strings" +) + +const ( + apiUrlCollectors = "/api/telemetry/collectors" + apiUrlCollectorsByServiceName = apiUrlCollectors + apiUrlPathDelim + "%s" +) + +//var ( +// _ json.Marshaler = (*Collector)(nil) +// _ json.Unmarshaler = (*Collector)(nil) +//) + +type CollectorPlatform struct { + OsType CollectorOSType + OsVersion CollectorOSVersion + OsFamily []CollectorOSVariant + Model string +} + +func (o *CollectorPlatform) UnmarshalJSON(data []byte) error { + var raw struct { + OsType string `json:"os_type"` + OsVersion string `json:"os_version"` + OsFamily string `json:"family"` + Model string `json:"model"` + } + + err := json.Unmarshal(data, &raw) + if err != nil { + return err + } + + err = o.OsType.FromString(raw.OsType) + if err != nil { + return err + } + + err = o.OsVersion.FromString(raw.OsVersion) + if err != nil { + return err + } + + o.Model = raw.Model + + for _, v := range strings.Split(raw.OsFamily, ",") { + var variant CollectorOSVariant + err = variant.FromString(v) + if err != nil { + return err + } + o.OsFamily = append(o.OsFamily, variant) + } + + return nil +} + +func (o *CollectorPlatform) MarshalJSON() ([]byte, error) { + var raw struct { + OsType string `json:"os_type"` + OsVersion string `json:"os_version"` + OsFamily string `json:"family"` + Model string `json:"model"` + } + raw.OsType = o.OsType.String() + raw.OsVersion = o.OsVersion.String() + raw.Model = o.Model + raw.OsFamily = o.OsFamily[0].String() + for _, v := range o.OsFamily[1:] { + raw.OsFamily = raw.OsFamily + "," + v.String() + } + return json.Marshal(raw) +} + +type Query struct { + Accessors map[string]string `json:"accessors"` + Keys map[string]string `json:"keys"` + Value string `json:"value"` + Filter string `json:"filter"` +} +type Collector struct { + ServiceName string + Platform CollectorPlatform `json:"platform"` + SourceType string `json:"source_type"` + Cli string `json:"cli"` + Query Query `json:"query"` + RelaxedSchemaValidation bool `json:"relaxed_schema_validation"` +} + +// GetAllCollectors gets all the Collectors for all services +func (o *Client) GetAllCollectors(ctx context.Context) ([]Collector, error) { + var response struct { + Items map[string]interface{} `json:"items"` + } + var collectors []Collector + // First Reach out to /collectors , we are interested really only in the keys, so that we can pull the collectors + err := o.talkToApstra(ctx, &talkToApstraIn{ + method: http.MethodGet, + urlStr: fmt.Sprintf(apiUrlCollectors), + apiResponse: &response, + }) + if err != nil { + return nil, convertTtaeToAceWherePossible(err) + } + + for k := range response.Items { + cs, err := o.GetCollectorsByServiceName(ctx, k) + if err != nil { + return nil, err + } + for _, v := range cs { + v.ServiceName = k + collectors = append(collectors, v) + } + } + return collectors, nil +} + +// GetCollectorsByServiceName gets all the Collectors that correspond to a particular service +func (o *Client) GetCollectorsByServiceName(ctx context.Context, name string) ([]Collector, error) { + var ace ClientErr + var Response struct { + Items []Collector `json:"items"` + } + err := o.talkToApstra(ctx, &talkToApstraIn{ + method: http.MethodGet, + urlStr: fmt.Sprintf(apiUrlCollectorsByServiceName, name), + apiResponse: &Response, + }) + + if err != nil { + err = convertTtaeToAceWherePossible(err) + if errors.As(err, &ace) && ace.Type() == ErrNotfound { + return Response.Items, nil + } + return nil, err + } + + for i := range Response.Items { + Response.Items[i].ServiceName = name + } + return Response.Items, nil +} + +// CreateCollector creates a collector +func (o *Client) CreateCollector(ctx context.Context, in *Collector) error { + // Check if this is the first collector for that service name + cs, err := o.GetCollectorsByServiceName(ctx, in.ServiceName) + if err != nil { + return err + } + var Request struct { + ServiceName string `json:"service_name"` + Items []Collector `json:"collectors"` + } + Request.ServiceName = in.ServiceName + Request.Items = append(Request.Items, *in) + // This is the first collector for this service name + // So we POST + if len(cs) == 0 { + err = o.talkToApstra(ctx, &talkToApstraIn{ + method: http.MethodPost, + urlStr: fmt.Sprintf(apiUrlCollectors), + apiInput: &Request, + }) + return err + } + + // There are other collectors, so this is a patch + err = o.talkToApstra(ctx, &talkToApstraIn{ + method: http.MethodPatch, + urlStr: fmt.Sprintf(apiUrlCollectorsByServiceName, in.ServiceName), + apiInput: &Request, + }) + return err +} + +// Update Collector Updates a collector +func (o *Client) UpdateCollector(ctx context.Context, in *Collector) error { + var Request struct { + Items []Collector `json:"collectors"` + } + Request.Items = append(Request.Items, *in) + return o.talkToApstra(ctx, &talkToApstraIn{ + method: http.MethodPatch, + urlStr: fmt.Sprintf(apiUrlCollectorsByServiceName, in.ServiceName), + apiInput: &Request, + }) +} + +// DeleteAllCollectorsInService deletes all the collectors under a service +func (o *Client) DeleteAllCollectorsInService(ctx context.Context, name string) error { + return o.talkToApstra(ctx, &talkToApstraIn{ + method: http.MethodDelete, + urlStr: fmt.Sprintf(apiUrlCollectorsByServiceName, name), + }) +} + +func (p1 *CollectorPlatform) Equals(p2 *CollectorPlatform) bool { + if p1.OsType != p2.OsType { + return false + } + if p1.OsVersion != p2.OsVersion { + return false + } + if p1.Model != p2.Model { + return false + } + if len(p1.OsFamily) != len(p2.OsFamily) { + return false + } + + m := make(map[CollectorOSVariant]bool, len(p1.OsFamily)) + for _, v := range p1.OsFamily { + m[v] = true + } + for _, v := range p2.OsFamily { + _, ok := m[v] + if !ok { + return false + } + } + return true +} + +// DeleteCollector deletes the collector described in the object +func (o *Client) DeleteCollector(ctx context.Context, in *Collector) error { + var Request struct { + ServiceName string `json:"service_name"` + Items []Collector `json:"collectors"` + } + + cs, err := o.GetCollectorsByServiceName(ctx, in.ServiceName) + if err != nil { + return err + } + + Request.ServiceName = in.ServiceName + for _, c := range cs { + if !c.Platform.Equals(&in.Platform) { + Request.Items = append(Request.Items, c) + } + } + return o.talkToApstra(ctx, &talkToApstraIn{ + method: http.MethodPut, + urlStr: fmt.Sprintf(apiUrlCollectorsByServiceName, in.ServiceName), + apiInput: &Request, + }) +} diff --git a/apstra/api_custom_collector_test.go b/apstra/api_custom_collector_test.go new file mode 100644 index 00000000..220ef396 --- /dev/null +++ b/apstra/api_custom_collector_test.go @@ -0,0 +1,141 @@ +//go:build integration +// +build integration + +package apstra + +import ( + "context" + "github.com/stretchr/testify/require" + "log" + "testing" +) + +func TestCollector(t *testing.T) { + ctx := context.Background() + + clients, err := getTestClients(ctx, t) + require.NoError(t, err) + + for clientName, client := range clients { + log.Printf("Testing Custom Collectors against %s %s (%s)", client.clientType, clientName, client.client.ApiVersion()) + + ts, err := client.client.GetAllTelemetryServiceRegistryEntries(ctx) + for _, tsr := range ts { + c, err := client.client.GetCollectorsByServiceName(ctx, tsr.ServiceName) + if err != nil { + t.Fatalf(err.Error()) + } + for _, d := range c { + log.Printf("%v", d) + } + } + + name := randString(10, "hex") + schema := `{ + "properties": { + "key": { + "properties": { + "schemakey1": { + "type": "string" + } + }, + "required": [ + "schemakey1" + ], + "type": "object" + }, + "value": { + "type": "string" + } + }, + "required": [ + "key", + "value" + ], + "type": "object" + }` + + entry := TelemetryServiceRegistryEntry{ + ServiceName: name, + StorageSchemaPath: StorageSchemaPathIBA_STRING_DATA, + ApplicationSchema: []byte(schema), + Builtin: false, + Description: "Test Service %s", + } + ServiceName, err := client.client.CreateTelemetryServiceRegistryEntry(ctx, &entry) + log.Printf("Service Name %s Created ", ServiceName) + require.NoError(t, err) + cs, err := client.client.GetCollectorsByServiceName(ctx, name) + require.NoError(t, err) + if len(cs) != 0 { + log.Println("There should be no collectors, this is a new service") + } + + c1 := Collector{ + ServiceName: name, + Platform: CollectorPlatform{ + OsType: CollectorOSTypeJunosEvo, + OsVersion: CollectorOSVersion22_2r2, + OsFamily: []CollectorOSVariant{CollectorOSVariantACX}, + Model: "", + }, + SourceType: "cli", + Cli: "cli show interfaces extensive", + Query: Query{ + Accessors: map[string]string{"telemetrykey1": "/interface-information/docsis-information/docsis-media-properties/downstream-buffers-free"}, + Keys: map[string]string{"schemakey1": "telemetrykey1"}, + Value: "telemetrykey1", + Filter: "", + }, + RelaxedSchemaValidation: true, + } + + err = client.client.CreateCollector(ctx, &c1) + require.NoError(t, err) + + cs, err = client.client.GetCollectorsByServiceName(ctx, name) + require.NoError(t, err) + if len(cs) != 1 { + log.Printf("There should be one collector, got %d", len(cs)) + } + + c1.Platform.OsFamily = []CollectorOSVariant{CollectorOSVariantACX_F, CollectorOSVariantJunos} + err = client.client.CreateCollector(ctx, &c1) + require.NoError(t, err) + cs, err = client.client.GetCollectorsByServiceName(ctx, name) + require.NoError(t, err) + if len(cs) != 2 { + log.Printf("There should be two collectors, got %d", len(cs)) + } + + c1.Query.Accessors["telemetrykey1"] = "/interface-information/docsis-information/docsis-media-properties/downstream-buffers-used" + err = client.client.UpdateCollector(ctx, &c1) + require.NoError(t, err) + cs, err = client.client.GetCollectorsByServiceName(ctx, name) + require.NoError(t, err) + if len(cs) != 2 { + log.Printf("There should be two collectors, got %d", len(cs)) + } + + err = client.client.DeleteCollector(ctx, &c1) + require.NoError(t, err) + cs, err = client.client.GetCollectorsByServiceName(ctx, name) + require.NoError(t, err) + if len(cs) != 1 { + log.Printf("There should be one collector, got %d", len(cs)) + } + + err = client.client.DeleteAllCollectorsInService(ctx, name) + require.NoError(t, err) + + cs, err = client.client.GetCollectorsByServiceName(ctx, name) + require.NoError(t, err) + if len(cs) != 0 { + log.Println("There should be no collectors, this is a new service") + } + + err = client.client.DeleteTelemetryServiceRegistryEntry(ctx, name) + require.NoError(t, err) + + } +} diff --git a/apstra/enum.go b/apstra/enum.go index 273f5740..0e136f58 100644 --- a/apstra/enum.go +++ b/apstra/enum.go @@ -211,6 +211,42 @@ var ( ResourcePoolTypeVlan, ResourcePoolTypeVni, ) + _ enum = new(CollectorOSType) + CollectorOSTypeJunos = CollectorOSType{Value: "junos"} + CollectorOSTypeJunosEvo = CollectorOSType{Value: "junos_evo"} + CollectorOSTypes = oenum.New( + CollectorOSTypeJunos, + CollectorOSTypeJunosEvo, + ) + _ enum = new(CollectorOSVariant) + CollectorOSVariantACX = CollectorOSVariant{Value: "acx"} + CollectorOSVariantACX_F = CollectorOSVariant{Value: "acx-f"} + CollectorOSVariantACX_QFX_7K = CollectorOSVariant{Value: "acx-qfx-7k"} + CollectorOSVariantPTX = CollectorOSVariant{Value: "ptx"} + CollectorOSVariantPTX1K = CollectorOSVariant{Value: "ptx1k"} + CollectorOSVariantQFX_MS_FIXED = CollectorOSVariant{Value: "qfx-ms-fixed"} + CollectorOSVariantJunos = CollectorOSVariant{Value: "junos"} + CollectorOSVariantJunos_EX = CollectorOSVariant{Value: "junos-ex"} + CollectorOSVariantJunos_QFX = CollectorOSVariant{Value: "junos-qfx"} + + CollectorOSVariants = oenum.New( + CollectorOSVariantACX, + CollectorOSVariantACX_F, + CollectorOSVariantACX_QFX_7K, + CollectorOSVariantPTX, + CollectorOSVariantPTX1K, + CollectorOSVariantQFX_MS_FIXED, + CollectorOSVariantJunos, + CollectorOSVariantJunos_EX, + CollectorOSVariantJunos_QFX, + ) + + CollectorOSVersion22_2r2 = CollectorOSVersion{Value: "22.2r2"} + CollectorOSVersion23_2r1 = CollectorOSVersion{Value: "23.2r1"} + CollectorOSVersions = oenum.New( + CollectorOSVersion22_2r2, + CollectorOSVersion23_2r1, + ) ) type DeployMode oenum.Member[string] @@ -452,3 +488,42 @@ func (o *ResourcePoolType) FromString(s string) error { o.Value = t.Value return nil } + +type CollectorOSType oenum.Member[string] + +func (o CollectorOSType) String() string { return o.Value } + +func (o *CollectorOSType) FromString(s string) error { + t := CollectorOSTypes.Parse(s) + if t == nil { + return fmt.Errorf("failed to parse CollectorOSType %q", s) + } + o.Value = t.Value + return nil +} + +type CollectorOSVersion oenum.Member[string] + +func (o CollectorOSVersion) String() string { return o.Value } + +func (o *CollectorOSVersion) FromString(s string) error { + t := CollectorOSVersions.Parse(s) + if t == nil { + return fmt.Errorf("failed to parse CollectorOSVersion %q", s) + } + o.Value = t.Value + return nil +} + +type CollectorOSVariant oenum.Member[string] + +func (o CollectorOSVariant) String() string { return o.Value } + +func (o *CollectorOSVariant) FromString(s string) error { + t := CollectorOSVariants.Parse(s) + if t == nil { + return fmt.Errorf("failed to parse CollectorOSVariant %q", s) + } + o.Value = t.Value + return nil +} From 05ae210adb3097935303df85357c3d6414b3b852 Mon Sep 17 00:00:00 2001 From: Rajagopalan Subrahmanian Date: Tue, 6 Aug 2024 23:02:16 -0400 Subject: [PATCH 2/7] code review fixes 1 --- apstra/api_custom_collector.go | 87 +++++++++++++++++++---------- apstra/api_custom_collector_test.go | 21 +++---- apstra/enum.go | 69 ++++++++++++++--------- 3 files changed, 112 insertions(+), 65 deletions(-) diff --git a/apstra/api_custom_collector.go b/apstra/api_custom_collector.go index d71cebd4..d402075c 100644 --- a/apstra/api_custom_collector.go +++ b/apstra/api_custom_collector.go @@ -5,6 +5,7 @@ import ( "encoding/json" "errors" "fmt" + "log" "net/http" "strings" ) @@ -14,15 +15,10 @@ const ( apiUrlCollectorsByServiceName = apiUrlCollectors + apiUrlPathDelim + "%s" ) -//var ( -// _ json.Marshaler = (*Collector)(nil) -// _ json.Unmarshaler = (*Collector)(nil) -//) - type CollectorPlatform struct { OsType CollectorOSType OsVersion CollectorOSVersion - OsFamily []CollectorOSVariant + OsFamily []CollectorOSFamily Model string } @@ -52,7 +48,7 @@ func (o *CollectorPlatform) UnmarshalJSON(data []byte) error { o.Model = raw.Model for _, v := range strings.Split(raw.OsFamily, ",") { - var variant CollectorOSVariant + var variant CollectorOSFamily err = variant.FromString(v) if err != nil { return err @@ -80,7 +76,7 @@ func (o *CollectorPlatform) MarshalJSON() ([]byte, error) { return json.Marshal(raw) } -type Query struct { +type CollectorQuery struct { Accessors map[string]string `json:"accessors"` Keys map[string]string `json:"keys"` Value string `json:"value"` @@ -88,11 +84,24 @@ type Query struct { } type Collector struct { ServiceName string - Platform CollectorPlatform `json:"platform"` - SourceType string `json:"source_type"` - Cli string `json:"cli"` - Query Query `json:"query"` - RelaxedSchemaValidation bool `json:"relaxed_schema_validation"` + Platform CollectorPlatform `json:"platform"` + SourceType CollectorSourceType `json:"source_type"` + Cli string `json:"cli"` + Query CollectorQuery `json:"query"` + RelaxedSchemaValidation bool `json:"relaxed_schema_validation"` +} + +func (o *CollectorSourceType) MarshalJSON() ([]byte, error) { + return json.Marshal(o.String()) +} + +func (o *CollectorSourceType) UnmarshalJSON(data []byte) error { + var s string + err := json.Unmarshal(data, &s) + if err != nil { + return err + } + return o.FromString(s) } // GetAllCollectors gets all the Collectors for all services @@ -139,7 +148,7 @@ func (o *Client) GetCollectorsByServiceName(ctx context.Context, name string) ([ if err != nil { err = convertTtaeToAceWherePossible(err) if errors.As(err, &ace) && ace.Type() == ErrNotfound { - return Response.Items, nil + return nil, nil } return nil, err } @@ -153,10 +162,10 @@ func (o *Client) GetCollectorsByServiceName(ctx context.Context, name string) ([ // CreateCollector creates a collector func (o *Client) CreateCollector(ctx context.Context, in *Collector) error { // Check if this is the first collector for that service name - cs, err := o.GetCollectorsByServiceName(ctx, in.ServiceName) - if err != nil { - return err - } + //cs, err := o.GetCollectorsByServiceName(ctx, in.ServiceName) + //if err != nil { + // return err + //} var Request struct { ServiceName string `json:"service_name"` Items []Collector `json:"collectors"` @@ -165,13 +174,15 @@ func (o *Client) CreateCollector(ctx context.Context, in *Collector) error { Request.Items = append(Request.Items, *in) // This is the first collector for this service name // So we POST - if len(cs) == 0 { - err = o.talkToApstra(ctx, &talkToApstraIn{ - method: http.MethodPost, - urlStr: fmt.Sprintf(apiUrlCollectors), - apiInput: &Request, - }) - return err + err := o.talkToApstra(ctx, &talkToApstraIn{ + method: http.MethodPost, + urlStr: fmt.Sprintf(apiUrlCollectors), + apiInput: &Request, + }) + err = convertTtaeToAceWherePossible(err) + var ace ClientErr + if !(errors.As(err, &ace) && ace.Type() == ErrConflict) { + return err // fatal error } // There are other collectors, so this is a patch @@ -183,12 +194,15 @@ func (o *Client) CreateCollector(ctx context.Context, in *Collector) error { return err } -// Update Collector Updates a collector +// UpdateCollector Updates a collector func (o *Client) UpdateCollector(ctx context.Context, in *Collector) error { var Request struct { - Items []Collector `json:"collectors"` + Collectors []Collector `json:"collectors"` } - Request.Items = append(Request.Items, *in) + Request.Collectors = append(Request.Collectors, *in) + r, e := json.Marshal(Request) + log.Println(e) + log.Println(string(r[:])) return o.talkToApstra(ctx, &talkToApstraIn{ method: http.MethodPatch, urlStr: fmt.Sprintf(apiUrlCollectorsByServiceName, in.ServiceName), @@ -218,7 +232,7 @@ func (p1 *CollectorPlatform) Equals(p2 *CollectorPlatform) bool { return false } - m := make(map[CollectorOSVariant]bool, len(p1.OsFamily)) + m := make(map[CollectorOSFamily]bool, len(p1.OsFamily)) for _, v := range p1.OsFamily { m[v] = true } @@ -243,12 +257,27 @@ func (o *Client) DeleteCollector(ctx context.Context, in *Collector) error { return err } + // There are no collectors + if len(cs) == 0 { + return nil + } + + // If there is only one collector, we need to call DELETE + if len(cs) == 1 { + return o.talkToApstra(ctx, &talkToApstraIn{ + method: http.MethodDelete, + urlStr: fmt.Sprintf(apiUrlCollectorsByServiceName, in.ServiceName), + }) + } + + // There is more than one collector, so we need to drop this collector from the list and PUT it back Request.ServiceName = in.ServiceName for _, c := range cs { if !c.Platform.Equals(&in.Platform) { Request.Items = append(Request.Items, c) } } + return o.talkToApstra(ctx, &talkToApstraIn{ method: http.MethodPut, urlStr: fmt.Sprintf(apiUrlCollectorsByServiceName, in.ServiceName), diff --git a/apstra/api_custom_collector_test.go b/apstra/api_custom_collector_test.go index 220ef396..7c2bd2b6 100644 --- a/apstra/api_custom_collector_test.go +++ b/apstra/api_custom_collector_test.go @@ -76,12 +76,12 @@ func TestCollector(t *testing.T) { Platform: CollectorPlatform{ OsType: CollectorOSTypeJunosEvo, OsVersion: CollectorOSVersion22_2r2, - OsFamily: []CollectorOSVariant{CollectorOSVariantACX}, + OsFamily: []CollectorOSFamily{CollectorOSFamilyACX}, Model: "", }, - SourceType: "cli", + SourceType: CollectorSourceTypeCLI, Cli: "cli show interfaces extensive", - Query: Query{ + Query: CollectorQuery{ Accessors: map[string]string{"telemetrykey1": "/interface-information/docsis-information/docsis-media-properties/downstream-buffers-free"}, Keys: map[string]string{"schemakey1": "telemetrykey1"}, Value: "telemetrykey1", @@ -89,7 +89,8 @@ func TestCollector(t *testing.T) { }, RelaxedSchemaValidation: true, } - + c2 := c1 + log.Println("Creating First Collector") err = client.client.CreateCollector(ctx, &c1) require.NoError(t, err) @@ -98,8 +99,9 @@ func TestCollector(t *testing.T) { if len(cs) != 1 { log.Printf("There should be one collector, got %d", len(cs)) } + log.Println("Creating Second Collector") - c1.Platform.OsFamily = []CollectorOSVariant{CollectorOSVariantACX_F, CollectorOSVariantJunos} + c1.Platform.OsFamily = []CollectorOSFamily{CollectorOSFamilyACX_F, CollectorOSFamilyJunos} err = client.client.CreateCollector(ctx, &c1) require.NoError(t, err) cs, err = client.client.GetCollectorsByServiceName(ctx, name) @@ -108,6 +110,7 @@ func TestCollector(t *testing.T) { log.Printf("There should be two collectors, got %d", len(cs)) } + log.Println("Updating Collector") c1.Query.Accessors["telemetrykey1"] = "/interface-information/docsis-information/docsis-media-properties/downstream-buffers-used" err = client.client.UpdateCollector(ctx, &c1) require.NoError(t, err) @@ -125,17 +128,15 @@ func TestCollector(t *testing.T) { log.Printf("There should be one collector, got %d", len(cs)) } - err = client.client.DeleteAllCollectorsInService(ctx, name) + err = client.client.DeleteCollector(ctx, &c2) require.NoError(t, err) - cs, err = client.client.GetCollectorsByServiceName(ctx, name) require.NoError(t, err) if len(cs) != 0 { - log.Println("There should be no collectors, this is a new service") + log.Printf("There should be no collectors, got %d", len(cs)) } - err = client.client.DeleteTelemetryServiceRegistryEntry(ctx, name) + err = client.client.DeleteTelemetryServiceRegistryEntry(ctx, ServiceName) require.NoError(t, err) - } } diff --git a/apstra/enum.go b/apstra/enum.go index 0e136f58..b37c0259 100644 --- a/apstra/enum.go +++ b/apstra/enum.go @@ -218,27 +218,27 @@ var ( CollectorOSTypeJunos, CollectorOSTypeJunosEvo, ) - _ enum = new(CollectorOSVariant) - CollectorOSVariantACX = CollectorOSVariant{Value: "acx"} - CollectorOSVariantACX_F = CollectorOSVariant{Value: "acx-f"} - CollectorOSVariantACX_QFX_7K = CollectorOSVariant{Value: "acx-qfx-7k"} - CollectorOSVariantPTX = CollectorOSVariant{Value: "ptx"} - CollectorOSVariantPTX1K = CollectorOSVariant{Value: "ptx1k"} - CollectorOSVariantQFX_MS_FIXED = CollectorOSVariant{Value: "qfx-ms-fixed"} - CollectorOSVariantJunos = CollectorOSVariant{Value: "junos"} - CollectorOSVariantJunos_EX = CollectorOSVariant{Value: "junos-ex"} - CollectorOSVariantJunos_QFX = CollectorOSVariant{Value: "junos-qfx"} - - CollectorOSVariants = oenum.New( - CollectorOSVariantACX, - CollectorOSVariantACX_F, - CollectorOSVariantACX_QFX_7K, - CollectorOSVariantPTX, - CollectorOSVariantPTX1K, - CollectorOSVariantQFX_MS_FIXED, - CollectorOSVariantJunos, - CollectorOSVariantJunos_EX, - CollectorOSVariantJunos_QFX, + _ enum = new(CollectorOSFamily) + CollectorOSFamilyACX = CollectorOSFamily{Value: "acx"} + CollectorOSFamilyACX_F = CollectorOSFamily{Value: "acx-f"} + CollectorOSFamilyACX_QFX_7K = CollectorOSFamily{Value: "acx-qfx-7k"} + CollectorOSFamilyPTX = CollectorOSFamily{Value: "ptx"} + CollectorOSFamilyPTX1K = CollectorOSFamily{Value: "ptx1k"} + CollectorOSFamilyQFX_MS_FIXED = CollectorOSFamily{Value: "qfx-ms-fixed"} + CollectorOSFamilyJunos = CollectorOSFamily{Value: "junos"} + CollectorOSFamilyJunos_EX = CollectorOSFamily{Value: "junos-ex"} + CollectorOSFamilyJunos_QFX = CollectorOSFamily{Value: "junos-qfx"} + + CollectorOSFamilies = oenum.New( + CollectorOSFamilyACX, + CollectorOSFamilyACX_F, + CollectorOSFamilyACX_QFX_7K, + CollectorOSFamilyPTX, + CollectorOSFamilyPTX1K, + CollectorOSFamilyQFX_MS_FIXED, + CollectorOSFamilyJunos, + CollectorOSFamilyJunos_EX, + CollectorOSFamilyJunos_QFX, ) CollectorOSVersion22_2r2 = CollectorOSVersion{Value: "22.2r2"} @@ -247,6 +247,10 @@ var ( CollectorOSVersion22_2r2, CollectorOSVersion23_2r1, ) + CollectorSourceTypeCLI = CollectorSourceType{Value: "cli"} + CollectorSourceTypes = oenum.New( + CollectorSourceTypeCLI, + ) ) type DeployMode oenum.Member[string] @@ -515,14 +519,27 @@ func (o *CollectorOSVersion) FromString(s string) error { return nil } -type CollectorOSVariant oenum.Member[string] +type CollectorOSFamily oenum.Member[string] + +func (o CollectorOSFamily) String() string { return o.Value } + +func (o *CollectorOSFamily) FromString(s string) error { + t := CollectorOSFamilies.Parse(s) + if t == nil { + return fmt.Errorf("failed to parse CollectorOSFamily %q", s) + } + o.Value = t.Value + return nil +} + +type CollectorSourceType oenum.Member[string] -func (o CollectorOSVariant) String() string { return o.Value } +func (o CollectorSourceType) String() string { return o.Value } -func (o *CollectorOSVariant) FromString(s string) error { - t := CollectorOSVariants.Parse(s) +func (o *CollectorSourceType) FromString(s string) error { + t := CollectorSourceTypes.Parse(s) if t == nil { - return fmt.Errorf("failed to parse CollectorOSVariant %q", s) + return fmt.Errorf("failed to parse CollectorOSFamily %q", s) } o.Value = t.Value return nil From 06780e880a37ee257219430a554156b3d2283101 Mon Sep 17 00:00:00 2001 From: Rajagopalan Subrahmanian Date: Wed, 7 Aug 2024 00:44:00 -0400 Subject: [PATCH 3/7] convert errors to apstra errors where possible --- apstra/api_custom_collector.go | 31 +++++++++++++------------------ 1 file changed, 13 insertions(+), 18 deletions(-) diff --git a/apstra/api_custom_collector.go b/apstra/api_custom_collector.go index d402075c..a1c20ccb 100644 --- a/apstra/api_custom_collector.go +++ b/apstra/api_custom_collector.go @@ -5,7 +5,6 @@ import ( "encoding/json" "errors" "fmt" - "log" "net/http" "strings" ) @@ -123,7 +122,7 @@ func (o *Client) GetAllCollectors(ctx context.Context) ([]Collector, error) { for k := range response.Items { cs, err := o.GetCollectorsByServiceName(ctx, k) if err != nil { - return nil, err + return nil, convertTtaeToAceWherePossible(err) } for _, v := range cs { v.ServiceName = k @@ -186,12 +185,11 @@ func (o *Client) CreateCollector(ctx context.Context, in *Collector) error { } // There are other collectors, so this is a patch - err = o.talkToApstra(ctx, &talkToApstraIn{ + return convertTtaeToAceWherePossible(o.talkToApstra(ctx, &talkToApstraIn{ method: http.MethodPatch, urlStr: fmt.Sprintf(apiUrlCollectorsByServiceName, in.ServiceName), apiInput: &Request, - }) - return err + })) } // UpdateCollector Updates a collector @@ -200,22 +198,19 @@ func (o *Client) UpdateCollector(ctx context.Context, in *Collector) error { Collectors []Collector `json:"collectors"` } Request.Collectors = append(Request.Collectors, *in) - r, e := json.Marshal(Request) - log.Println(e) - log.Println(string(r[:])) - return o.talkToApstra(ctx, &talkToApstraIn{ + return convertTtaeToAceWherePossible(o.talkToApstra(ctx, &talkToApstraIn{ method: http.MethodPatch, urlStr: fmt.Sprintf(apiUrlCollectorsByServiceName, in.ServiceName), apiInput: &Request, - }) + })) } // DeleteAllCollectorsInService deletes all the collectors under a service func (o *Client) DeleteAllCollectorsInService(ctx context.Context, name string) error { - return o.talkToApstra(ctx, &talkToApstraIn{ + return convertTtaeToAceWherePossible(o.talkToApstra(ctx, &talkToApstraIn{ method: http.MethodDelete, urlStr: fmt.Sprintf(apiUrlCollectorsByServiceName, name), - }) + })) } func (p1 *CollectorPlatform) Equals(p2 *CollectorPlatform) bool { @@ -254,7 +249,7 @@ func (o *Client) DeleteCollector(ctx context.Context, in *Collector) error { cs, err := o.GetCollectorsByServiceName(ctx, in.ServiceName) if err != nil { - return err + return convertTtaeToAceWherePossible(err) } // There are no collectors @@ -264,13 +259,13 @@ func (o *Client) DeleteCollector(ctx context.Context, in *Collector) error { // If there is only one collector, we need to call DELETE if len(cs) == 1 { - return o.talkToApstra(ctx, &talkToApstraIn{ + return convertTtaeToAceWherePossible(o.talkToApstra(ctx, &talkToApstraIn{ method: http.MethodDelete, urlStr: fmt.Sprintf(apiUrlCollectorsByServiceName, in.ServiceName), - }) + })) } - // There is more than one collector, so we need to drop this collector from the list and PUT it back + // There is more than one collector, so we need to drop this collector from the list and PUT it backsxxa Request.ServiceName = in.ServiceName for _, c := range cs { if !c.Platform.Equals(&in.Platform) { @@ -278,9 +273,9 @@ func (o *Client) DeleteCollector(ctx context.Context, in *Collector) error { } } - return o.talkToApstra(ctx, &talkToApstraIn{ + return convertTtaeToAceWherePossible(o.talkToApstra(ctx, &talkToApstraIn{ method: http.MethodPut, urlStr: fmt.Sprintf(apiUrlCollectorsByServiceName, in.ServiceName), apiInput: &Request, - }) + })) } From eda533f7470cf4dac75aa37322cc16100d1b9692 Mon Sep 17 00:00:00 2001 From: Rajagopalan Subrahmanian Date: Wed, 7 Aug 2024 21:14:29 -0400 Subject: [PATCH 4/7] change OSVersion to string from enum --- apstra/api_custom_collector.go | 10 +++------- apstra/api_custom_collector_test.go | 2 +- apstra/enum.go | 19 ------------------- 3 files changed, 4 insertions(+), 27 deletions(-) diff --git a/apstra/api_custom_collector.go b/apstra/api_custom_collector.go index a1c20ccb..bf650ab5 100644 --- a/apstra/api_custom_collector.go +++ b/apstra/api_custom_collector.go @@ -16,7 +16,7 @@ const ( type CollectorPlatform struct { OsType CollectorOSType - OsVersion CollectorOSVersion + OsVersion string OsFamily []CollectorOSFamily Model string } @@ -39,11 +39,7 @@ func (o *CollectorPlatform) UnmarshalJSON(data []byte) error { return err } - err = o.OsVersion.FromString(raw.OsVersion) - if err != nil { - return err - } - + o.OsVersion = raw.OsVersion o.Model = raw.Model for _, v := range strings.Split(raw.OsFamily, ",") { @@ -66,7 +62,7 @@ func (o *CollectorPlatform) MarshalJSON() ([]byte, error) { Model string `json:"model"` } raw.OsType = o.OsType.String() - raw.OsVersion = o.OsVersion.String() + raw.OsVersion = o.OsVersion raw.Model = o.Model raw.OsFamily = o.OsFamily[0].String() for _, v := range o.OsFamily[1:] { diff --git a/apstra/api_custom_collector_test.go b/apstra/api_custom_collector_test.go index 7c2bd2b6..f5e5da9e 100644 --- a/apstra/api_custom_collector_test.go +++ b/apstra/api_custom_collector_test.go @@ -75,7 +75,7 @@ func TestCollector(t *testing.T) { ServiceName: name, Platform: CollectorPlatform{ OsType: CollectorOSTypeJunosEvo, - OsVersion: CollectorOSVersion22_2r2, + OsVersion: "22.2r2", OsFamily: []CollectorOSFamily{CollectorOSFamilyACX}, Model: "", }, diff --git a/apstra/enum.go b/apstra/enum.go index b37c0259..eb37b62d 100644 --- a/apstra/enum.go +++ b/apstra/enum.go @@ -241,12 +241,6 @@ var ( CollectorOSFamilyJunos_QFX, ) - CollectorOSVersion22_2r2 = CollectorOSVersion{Value: "22.2r2"} - CollectorOSVersion23_2r1 = CollectorOSVersion{Value: "23.2r1"} - CollectorOSVersions = oenum.New( - CollectorOSVersion22_2r2, - CollectorOSVersion23_2r1, - ) CollectorSourceTypeCLI = CollectorSourceType{Value: "cli"} CollectorSourceTypes = oenum.New( CollectorSourceTypeCLI, @@ -506,19 +500,6 @@ func (o *CollectorOSType) FromString(s string) error { return nil } -type CollectorOSVersion oenum.Member[string] - -func (o CollectorOSVersion) String() string { return o.Value } - -func (o *CollectorOSVersion) FromString(s string) error { - t := CollectorOSVersions.Parse(s) - if t == nil { - return fmt.Errorf("failed to parse CollectorOSVersion %q", s) - } - o.Value = t.Value - return nil -} - type CollectorOSFamily oenum.Member[string] func (o CollectorOSFamily) String() string { return o.Value } From d2c6d97531a9e9527334696fe689115a5d0d7b55 Mon Sep 17 00:00:00 2001 From: Rajagopalan Subrahmanian Date: Wed, 7 Aug 2024 23:57:56 -0400 Subject: [PATCH 5/7] create a service registry level lock. A collector level lock might be overkill --- apstra/api_custom_collector.go | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/apstra/api_custom_collector.go b/apstra/api_custom_collector.go index bf650ab5..5753413c 100644 --- a/apstra/api_custom_collector.go +++ b/apstra/api_custom_collector.go @@ -12,6 +12,7 @@ import ( const ( apiUrlCollectors = "/api/telemetry/collectors" apiUrlCollectorsByServiceName = apiUrlCollectors + apiUrlPathDelim + "%s" + CollectorLock = "collector_lock_%s" ) type CollectorPlatform struct { @@ -167,6 +168,11 @@ func (o *Client) CreateCollector(ctx context.Context, in *Collector) error { } Request.ServiceName = in.ServiceName Request.Items = append(Request.Items, *in) + + lockId := fmt.Sprintf(CollectorLock, in.ServiceName) + o.lock(lockId) + defer o.unlock(lockId) + // This is the first collector for this service name // So we POST err := o.talkToApstra(ctx, &talkToApstraIn{ @@ -243,6 +249,10 @@ func (o *Client) DeleteCollector(ctx context.Context, in *Collector) error { Items []Collector `json:"collectors"` } + lockId := fmt.Sprintf(CollectorLock, in.ServiceName) + o.lock(lockId) + defer o.unlock(lockId) + cs, err := o.GetCollectorsByServiceName(ctx, in.ServiceName) if err != nil { return convertTtaeToAceWherePossible(err) From f891633b442c5227172b780e4afce20ccdfa2e6a Mon Sep 17 00:00:00 2001 From: Rajagopalan Subrahmanian Date: Thu, 8 Aug 2024 06:50:07 -0400 Subject: [PATCH 6/7] fix enum.go --- apstra/enum.go | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/apstra/enum.go b/apstra/enum.go index ee1c4c15..e294b8cd 100644 --- a/apstra/enum.go +++ b/apstra/enum.go @@ -219,7 +219,7 @@ var ( CollectorOSTypeJunosEvo, ) - _ enum = new(CollectorOSFamily) + _ enum = new(CollectorOSFamily) CollectorOSFamilyACX = CollectorOSFamily{Value: "acx"} CollectorOSFamilyACX_F = CollectorOSFamily{Value: "acx-f"} CollectorOSFamilyACX_QFX_7K = CollectorOSFamily{Value: "acx-qfx-7k"} @@ -229,7 +229,7 @@ var ( CollectorOSFamilyJunos = CollectorOSFamily{Value: "junos"} CollectorOSFamilyJunos_EX = CollectorOSFamily{Value: "junos-ex"} CollectorOSFamilyJunos_QFX = CollectorOSFamily{Value: "junos-qfx"} - CollectorOSFamilies = oenum.New( + CollectorOSFamilies = oenum.New( CollectorOSFamilyACX, CollectorOSFamilyACX_F, CollectorOSFamilyACX_QFX_7K, @@ -245,7 +245,8 @@ var ( CollectorSourceTypes = oenum.New( CollectorSourceTypeCLI, ) - _ enum = new(RoutingZoneConstraintMode) + + _ enum = new(RoutingZoneConstraintMode) RoutingZoneConstraintModeNone = RoutingZoneConstraintMode{Value: "none"} RoutingZoneConstraintModeAllow = RoutingZoneConstraintMode{Value: "allow"} RoutingZoneConstraintModeDeny = RoutingZoneConstraintMode{Value: "deny"} @@ -253,6 +254,7 @@ var ( RoutingZoneConstraintModeNone, RoutingZoneConstraintModeAllow, RoutingZoneConstraintModeDeny, + ) ) type DeployMode oenum.Member[string] @@ -533,6 +535,7 @@ func (o *CollectorSourceType) FromString(s string) error { o.Value = t.Value return nil } + type RoutingZoneConstraintMode oenum.Member[string] func (o RoutingZoneConstraintMode) String() string { @@ -543,3 +546,6 @@ func (o *RoutingZoneConstraintMode) FromString(s string) error { t := RoutingZoneConstraintModes.Parse(s) if t == nil { return fmt.Errorf("failed to parse RoutingZoneConstraintMode %q", s) + } + return nil +} From 02b4820a30d0e77908314190a307ce25e8b3475d Mon Sep 17 00:00:00 2001 From: Rajagopalan Subrahmanian Date: Mon, 12 Aug 2024 12:43:59 -0400 Subject: [PATCH 7/7] fixes to gofumpt_check.sh and some format fixes --- .ci/scripts/gofumpt_check.sh | 2 +- apstra/api_custom_collector.go | 1 - apstra/api_custom_collector_test.go | 3 ++- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.ci/scripts/gofumpt_check.sh b/.ci/scripts/gofumpt_check.sh index 8ca7d10c..95d3e418 100755 --- a/.ci/scripts/gofumpt_check.sh +++ b/.ci/scripts/gofumpt_check.sh @@ -26,7 +26,7 @@ then for f in "${needs_update[@]}" do - echo " go run mvdan.cc/gofumpt -w '$file'" + echo " go run mvdan.cc/gofumpt -w '$f'" done echo "" diff --git a/apstra/api_custom_collector.go b/apstra/api_custom_collector.go index 5753413c..9b9447ef 100644 --- a/apstra/api_custom_collector.go +++ b/apstra/api_custom_collector.go @@ -140,7 +140,6 @@ func (o *Client) GetCollectorsByServiceName(ctx context.Context, name string) ([ urlStr: fmt.Sprintf(apiUrlCollectorsByServiceName, name), apiResponse: &Response, }) - if err != nil { err = convertTtaeToAceWherePossible(err) if errors.As(err, &ace) && ace.Type() == ErrNotfound { diff --git a/apstra/api_custom_collector_test.go b/apstra/api_custom_collector_test.go index f5e5da9e..71a1546c 100644 --- a/apstra/api_custom_collector_test.go +++ b/apstra/api_custom_collector_test.go @@ -5,9 +5,10 @@ package apstra import ( "context" - "github.com/stretchr/testify/require" "log" "testing" + + "github.com/stretchr/testify/require" ) func TestCollector(t *testing.T) {