From a17a509e91a621d2e065c523eca775f5abedcbcf Mon Sep 17 00:00:00 2001 From: Patryk Strusiewicz-Surmacki Date: Wed, 28 Feb 2024 17:31:11 +0100 Subject: [PATCH 01/35] Added monitoring endpoint Signed-off-by: Patryk Strusiewicz-Surmacki --- cmd/frr-exporter/main.go | 6 ++ pkg/frr/cli.go | 12 ++-- pkg/monitoring/endpoint.go | 115 +++++++++++++++++++++++++++++++++++++ 3 files changed, 127 insertions(+), 6 deletions(-) create mode 100644 pkg/monitoring/endpoint.go diff --git a/cmd/frr-exporter/main.go b/cmd/frr-exporter/main.go index c180fdfc..3cb8fb78 100644 --- a/cmd/frr-exporter/main.go +++ b/cmd/frr-exporter/main.go @@ -46,6 +46,12 @@ func main() { } reg.MustRegister(collector) + endpoint := monitoring.NewEndpoint() + + http.HandleFunc("/show/route", endpoint.ShowRoute) + http.HandleFunc("/show/bgp", endpoint.ShowBGP) + http.HandleFunc("/show/evpn", endpoint.ShowEVPN) + // Expose the registered metrics via HTTP. http.Handle("/metrics", promhttp.HandlerFor( reg, diff --git a/pkg/frr/cli.go b/pkg/frr/cli.go index c1f2c314..9f48d34e 100644 --- a/pkg/frr/cli.go +++ b/pkg/frr/cli.go @@ -30,7 +30,7 @@ func getVRFInfo(vrf string) (name string, isMulti bool) { return vrf, false } -func (frr *Cli) executeWithJSON(args []string) []byte { +func (frr *Cli) ExecuteWithJSON(args []string) []byte { // Ensure JSON is always appended args = append(args, "json") return frr.execute(args) @@ -51,7 +51,7 @@ func (frr *Cli) execute(args []string) []byte { func (frr *Cli) ShowEVPNVNIDetail() (EVPNVniDetail, error) { evpnInfo := EVPNVniDetail{} - data := frr.executeWithJSON([]string{ + data := frr.ExecuteWithJSON([]string{ "show", "evpn", "vni", @@ -66,7 +66,7 @@ func (frr *Cli) ShowEVPNVNIDetail() (EVPNVniDetail, error) { func (frr *Cli) ShowBGPSummary(vrf string) (BGPVrfSummary, error) { vrfName, multiVRF := getVRFInfo(vrf) - data := frr.executeWithJSON([]string{ + data := frr.ExecuteWithJSON([]string{ "show", "bgp", "vrf", @@ -93,7 +93,7 @@ func (frr *Cli) ShowBGPSummary(vrf string) (BGPVrfSummary, error) { func (frr *Cli) showVRFVnis() (VrfVni, error) { vrfInfo := VrfVni{} - vrfVniData := frr.executeWithJSON([]string{ + vrfVniData := frr.ExecuteWithJSON([]string{ "show", "vrf", "vni", @@ -204,7 +204,7 @@ func (frr *Cli) ShowVRFs(vrfName string) (VrfVni, error) { } func (frr *Cli) getDualStackRouteSummaries(vrf string) (routeSummariesV4, routeSummariesV6 RouteSummaries, err error) { - dataV4 := frr.executeWithJSON([]string{ + dataV4 := frr.ExecuteWithJSON([]string{ "show", "ip", "route", @@ -212,7 +212,7 @@ func (frr *Cli) getDualStackRouteSummaries(vrf string) (routeSummariesV4, routeS vrf, "summary", }) - dataV6 := frr.executeWithJSON([]string{ + dataV6 := frr.ExecuteWithJSON([]string{ "show", "ipv6", "route", diff --git a/pkg/monitoring/endpoint.go b/pkg/monitoring/endpoint.go new file mode 100644 index 00000000..978efedb --- /dev/null +++ b/pkg/monitoring/endpoint.go @@ -0,0 +1,115 @@ +package monitoring + +import ( + "net/http" + "strings" + + "github.com/telekom/das-schiff-network-operator/pkg/frr" +) + +type endpoint struct { + cli *frr.Cli +} + +func NewEndpoint() *endpoint { + return &endpoint{cli: frr.NewCli()} +} + +func writeResponse(data *[]byte, w http.ResponseWriter) { + // if len(*data) <= 0 { + // http.Error(w, "failed to get data:", http.StatusInternalServerError) + // return + // } + + _, err := w.Write(*data) + if err != nil { + http.Error(w, "failed to write response: "+err.Error(), http.StatusInternalServerError) + } +} + +func (e *endpoint) ShowRoute(w http.ResponseWriter, r *http.Request) { + vrf := r.URL.Query().Get("vrf") + if vrf == "" { + vrf = "all" + } + + protocol := r.URL.Query().Get("protocol") + if protocol == "" { + protocol = "ip" + } + + data := e.cli.ExecuteWithJSON([]string{ + "show", + protocol, + "route", + "vrf", + vrf, + }) + + writeResponse(&data, w) +} + +func (e *endpoint) ShowBGP(w http.ResponseWriter, r *http.Request) { + vrf := r.URL.Query().Get("vrf") + if vrf == "" { + vrf = "all" + } + + data := []byte{} + + requestType := r.URL.Query().Get("type") + if strings.ToLower(requestType) == "summary" { + data = e.cli.ExecuteWithJSON([]string{ + "show", + "bgp", + "vrf", + vrf, + "summary", + }) + } else { + protocol := r.URL.Query().Get("protocol") + if protocol == "" { + protocol = "ipv4" + } + + data = e.cli.ExecuteWithJSON([]string{ + "show", + "bgp", + "vrf", + vrf, + protocol, + "unicast", + }) + + } + + writeResponse(&data, w) +} + +func (e *endpoint) ShowEVPN(w http.ResponseWriter, r *http.Request) { + data := []byte{} + requestType := r.URL.Query().Get("type") + if requestType == "" { + data = e.cli.ExecuteWithJSON([]string{ + "show", + "evpn", + "vni", + }) + } else { + vrf := r.URL.Query().Get("vrf") + if vrf == "" { + vrf = "all" + } + + data = e.cli.ExecuteWithJSON([]string{ + "show", + "evpn", + requestType, + "vni", + vrf, + }) + + } + + writeResponse(&data, w) +} From 3b873c00cc46042f7f39130185b07cf5b49601a5 Mon Sep 17 00:00:00 2001 From: Patryk Strusiewicz-Surmacki Date: Wed, 28 Feb 2024 18:22:06 +0100 Subject: [PATCH 02/35] Fixed linter issues Signed-off-by: Patryk Strusiewicz-Surmacki --- pkg/monitoring/endpoint.go | 32 +++++++++++++++----------------- 1 file changed, 15 insertions(+), 17 deletions(-) diff --git a/pkg/monitoring/endpoint.go b/pkg/monitoring/endpoint.go index 978efedb..e8a2b1af 100644 --- a/pkg/monitoring/endpoint.go +++ b/pkg/monitoring/endpoint.go @@ -7,30 +7,30 @@ import ( "github.com/telekom/das-schiff-network-operator/pkg/frr" ) -type endpoint struct { +const ( + vrfAll = "all" +) + +type Endpoint struct { cli *frr.Cli } -func NewEndpoint() *endpoint { - return &endpoint{cli: frr.NewCli()} +func NewEndpoint() *Endpoint { + return &Endpoint{cli: frr.NewCli()} } func writeResponse(data *[]byte, w http.ResponseWriter) { - // if len(*data) <= 0 { - // http.Error(w, "failed to get data:", http.StatusInternalServerError) - // return - // } - _, err := w.Write(*data) if err != nil { http.Error(w, "failed to write response: "+err.Error(), http.StatusInternalServerError) + return } } -func (e *endpoint) ShowRoute(w http.ResponseWriter, r *http.Request) { +func (e *Endpoint) ShowRoute(w http.ResponseWriter, r *http.Request) { vrf := r.URL.Query().Get("vrf") if vrf == "" { - vrf = "all" + vrf = vrfAll } protocol := r.URL.Query().Get("protocol") @@ -49,16 +49,16 @@ func (e *endpoint) ShowRoute(w http.ResponseWriter, r *http.Request) { writeResponse(&data, w) } -func (e *endpoint) ShowBGP(w http.ResponseWriter, r *http.Request) { +func (e *Endpoint) ShowBGP(w http.ResponseWriter, r *http.Request) { vrf := r.URL.Query().Get("vrf") if vrf == "" { - vrf = "all" + vrf = vrfAll } data := []byte{} requestType := r.URL.Query().Get("type") - if strings.ToLower(requestType) == "summary" { + if strings.EqualFold(requestType, "summary") { data = e.cli.ExecuteWithJSON([]string{ "show", "bgp", @@ -80,13 +80,12 @@ func (e *endpoint) ShowBGP(w http.ResponseWriter, r *http.Request) { protocol, "unicast", }) - } writeResponse(&data, w) } -func (e *endpoint) ShowEVPN(w http.ResponseWriter, r *http.Request) { +func (e *Endpoint) ShowEVPN(w http.ResponseWriter, r *http.Request) { data := []byte{} requestType := r.URL.Query().Get("type") if requestType == "" { @@ -98,7 +97,7 @@ func (e *endpoint) ShowEVPN(w http.ResponseWriter, r *http.Request) { } else { vrf := r.URL.Query().Get("vrf") if vrf == "" { - vrf = "all" + vrf = vrfAll } data = e.cli.ExecuteWithJSON([]string{ @@ -108,7 +107,6 @@ func (e *endpoint) ShowEVPN(w http.ResponseWriter, r *http.Request) { "vni", vrf, }) - } writeResponse(&data, w) From 561170b73661bba2d1e3615f87966bf549b6cf38 Mon Sep 17 00:00:00 2001 From: Patryk Strusiewicz-Surmacki Date: Wed, 6 Mar 2024 17:37:59 +0100 Subject: [PATCH 03/35] Fixed issues from CR and added k8s client Signed-off-by: Patryk Strusiewicz-Surmacki --- cmd/frr-exporter/main.go | 5 +- pkg/monitoring/endpoint.go | 128 +++++++++++++++++++++++++++++-------- 2 files changed, 106 insertions(+), 27 deletions(-) diff --git a/cmd/frr-exporter/main.go b/cmd/frr-exporter/main.go index 3cb8fb78..fd1a04ad 100644 --- a/cmd/frr-exporter/main.go +++ b/cmd/frr-exporter/main.go @@ -46,7 +46,10 @@ func main() { } reg.MustRegister(collector) - endpoint := monitoring.NewEndpoint() + endpoint, err := monitoring.NewEndpoint() + if err != nil { + log.Fatal(fmt.Errorf("failed to create monitoring endpoint %w", err)) + } http.HandleFunc("/show/route", endpoint.ShowRoute) http.HandleFunc("/show/bgp", endpoint.ShowBGP) diff --git a/pkg/monitoring/endpoint.go b/pkg/monitoring/endpoint.go index e8a2b1af..06da1023 100644 --- a/pkg/monitoring/endpoint.go +++ b/pkg/monitoring/endpoint.go @@ -1,50 +1,70 @@ package monitoring import ( + "fmt" + "net" "net/http" - "strings" + "strconv" "github.com/telekom/das-schiff-network-operator/pkg/frr" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" ) const ( - vrfAll = "all" + all = "all" + protocolIP = "ip" + protocolIPv4 = "ipv4" + protocolIPv6 = "ipv6" ) type Endpoint struct { cli *frr.Cli + c client.Client } -func NewEndpoint() *Endpoint { - return &Endpoint{cli: frr.NewCli()} -} - -func writeResponse(data *[]byte, w http.ResponseWriter) { - _, err := w.Write(*data) +func NewEndpoint() (*Endpoint, error) { + clientConfig := ctrl.GetConfigOrDie() + c, err := client.New(clientConfig, client.Options{}) if err != nil { - http.Error(w, "failed to write response: "+err.Error(), http.StatusInternalServerError) - return + return nil, fmt.Errorf("error creating controller-runtime client: %w", err) } + return &Endpoint{cli: frr.NewCli(), c: c}, nil } func (e *Endpoint) ShowRoute(w http.ResponseWriter, r *http.Request) { vrf := r.URL.Query().Get("vrf") if vrf == "" { - vrf = vrfAll + vrf = all } protocol := r.URL.Query().Get("protocol") if protocol == "" { - protocol = "ip" + protocol = protocolIP + } else if protocol != protocolIP && protocol != protocolIPv6 { + http.Error(w, fmt.Sprintf("protocol '%s' is not supported", protocol), http.StatusBadRequest) + return } - data := e.cli.ExecuteWithJSON([]string{ + command := []string{ "show", protocol, "route", "vrf", vrf, - }) + } + + if err := setInput(r, &command); err != nil { + http.Error(w, err.Error(), http.StatusBadRequest) + return + } + + if err := setLongerPrefixes(r, &command); err != nil { + http.Error(w, err.Error(), http.StatusBadRequest) + return + } + + data := e.cli.ExecuteWithJSON(command) writeResponse(&data, w) } @@ -52,13 +72,14 @@ func (e *Endpoint) ShowRoute(w http.ResponseWriter, r *http.Request) { func (e *Endpoint) ShowBGP(w http.ResponseWriter, r *http.Request) { vrf := r.URL.Query().Get("vrf") if vrf == "" { - vrf = vrfAll + vrf = all } data := []byte{} requestType := r.URL.Query().Get("type") - if strings.EqualFold(requestType, "summary") { + switch requestType { + case "summary": data = e.cli.ExecuteWithJSON([]string{ "show", "bgp", @@ -66,20 +87,38 @@ func (e *Endpoint) ShowBGP(w http.ResponseWriter, r *http.Request) { vrf, "summary", }) - } else { + case "": protocol := r.URL.Query().Get("protocol") if protocol == "" { - protocol = "ipv4" + protocol = protocolIPv4 + } else if protocol != protocolIPv4 && protocol != protocolIPv6 { + http.Error(w, fmt.Sprintf("protocol '%s' is not supported", protocol), http.StatusBadRequest) + return } - data = e.cli.ExecuteWithJSON([]string{ + command := []string{ "show", "bgp", "vrf", vrf, protocol, "unicast", - }) + } + + if err := setInput(r, &command); err != nil { + http.Error(w, err.Error(), http.StatusBadRequest) + return + } + + if err := setLongerPrefixes(r, &command); err != nil { + http.Error(w, err.Error(), http.StatusBadRequest) + return + } + + data = e.cli.ExecuteWithJSON(command) + default: + http.Error(w, fmt.Sprintf("request of type '%s' is not supported", requestType), http.StatusBadRequest) + return } writeResponse(&data, w) @@ -88,16 +127,17 @@ func (e *Endpoint) ShowBGP(w http.ResponseWriter, r *http.Request) { func (e *Endpoint) ShowEVPN(w http.ResponseWriter, r *http.Request) { data := []byte{} requestType := r.URL.Query().Get("type") - if requestType == "" { + switch requestType { + case "": data = e.cli.ExecuteWithJSON([]string{ "show", "evpn", "vni", }) - } else { - vrf := r.URL.Query().Get("vrf") - if vrf == "" { - vrf = vrfAll + case "rmac", "mac", "next-hops": + vni := r.URL.Query().Get("vni") + if vni == "" { + vni = all } data = e.cli.ExecuteWithJSON([]string{ @@ -105,9 +145,45 @@ func (e *Endpoint) ShowEVPN(w http.ResponseWriter, r *http.Request) { "evpn", requestType, "vni", - vrf, + vni, }) + default: + http.Error(w, fmt.Sprintf("request of type '%s' is not supported", requestType), http.StatusBadRequest) + return } writeResponse(&data, w) } + +func writeResponse(data *[]byte, w http.ResponseWriter) { + _, err := w.Write(*data) + if err != nil { + http.Error(w, "failed to write response: "+err.Error(), http.StatusInternalServerError) + return + } +} + +func setLongerPrefixes(r *http.Request, command *[]string) error { + longerPrefixes := r.URL.Query().Get("longer_prefixes") + if longerPrefixes != "" { + useLongerPrefixes, err := strconv.ParseBool(longerPrefixes) + if err != nil { + return fmt.Errorf("longer_prefixes value '%s' is not valid: %w", longerPrefixes, err) + } + if useLongerPrefixes { + *command = append(*command, "longer-prefixes") + } + } + return nil +} + +func setInput(r *http.Request, command *[]string) error { + input := r.URL.Query().Get("input") + if input != "" { + if _, _, err := net.ParseCIDR(input); err != nil { + return fmt.Errorf("input '%s' is not valid: %w", input, err) + } + *command = append(*command, input) + } + return nil +} From 2c1b70e4462296e8bcb043789efb10466dcc35a9 Mon Sep 17 00:00:00 2001 From: Patryk Strusiewicz-Surmacki Date: Wed, 6 Mar 2024 19:32:14 +0100 Subject: [PATCH 04/35] Added initial global endpoint Signed-off-by: Patryk Strusiewicz-Surmacki --- cmd/frr-exporter/main.go | 1 + pkg/monitoring/endpoint.go | 52 +++++++++++++++++++++++++++++++++++++- 2 files changed, 52 insertions(+), 1 deletion(-) diff --git a/cmd/frr-exporter/main.go b/cmd/frr-exporter/main.go index fd1a04ad..2b23229f 100644 --- a/cmd/frr-exporter/main.go +++ b/cmd/frr-exporter/main.go @@ -54,6 +54,7 @@ func main() { http.HandleFunc("/show/route", endpoint.ShowRoute) http.HandleFunc("/show/bgp", endpoint.ShowBGP) http.HandleFunc("/show/evpn", endpoint.ShowEVPN) + http.HandleFunc("/all/show/route", endpoint.ShowAllRoute) // Expose the registered metrics via HTTP. http.Handle("/metrics", promhttp.HandlerFor( diff --git a/pkg/monitoring/endpoint.go b/pkg/monitoring/endpoint.go index 06da1023..6628b423 100644 --- a/pkg/monitoring/endpoint.go +++ b/pkg/monitoring/endpoint.go @@ -1,12 +1,17 @@ package monitoring import ( + "context" "fmt" + "log" "net" "net/http" + "os" "strconv" "github.com/telekom/das-schiff-network-operator/pkg/frr" + "github.com/telekom/das-schiff-network-operator/pkg/healthcheck" + corev1 "k8s.io/api/core/v1" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" ) @@ -66,7 +71,52 @@ func (e *Endpoint) ShowRoute(w http.ResponseWriter, r *http.Request) { data := e.cli.ExecuteWithJSON(command) - writeResponse(&data, w) + nodename := os.Getenv(healthcheck.NodenameEnv) + nodeField := fmt.Sprintf("%s:\n", nodename) + + result := []byte{} + + if nodename != "" { + result = append(result, []byte(nodeField)...) + } + + result = append(result, data...) + + writeResponse(&result, w) +} + +func (e *Endpoint) ShowAllRoute(w http.ResponseWriter, r *http.Request) { + ctx := context.TODO() + pods := &corev1.PodList{} + matchLabels := &client.MatchingLabels{ + "app.kubernetes.io/component": "worker", + } + + err := e.c.List(ctx, pods, matchLabels) + if err != nil { + http.Error(w, fmt.Sprintf("error listing pods: %s", err.Error()), http.StatusInternalServerError) + return + } + + if len(pods.Items) == 0 { + http.Error(w, "error listing pods: no pods found", http.StatusInternalServerError) + return + } + + for _, pod := range pods.Items { + fmt.Println(pod.Status.PodIP) + url := fmt.Sprintf("http://%s/show/route?%s", pod.Status.PodIP, r.URL.RawQuery) + fmt.Println(url) + resp, err := http.Get(url) + if err != nil { + log.Fatalln(err) + } + + data := []byte{} + resp.Body.Read(data) + resp.Body.Close() + writeResponse(&data, w) + } } func (e *Endpoint) ShowBGP(w http.ResponseWriter, r *http.Request) { From f9e1d1124b8f9712446354b47f98779d91a35e72 Mon Sep 17 00:00:00 2001 From: Patryk Strusiewicz-Surmacki Date: Thu, 7 Mar 2024 18:20:37 +0100 Subject: [PATCH 05/35] Added global endpoint Signed-off-by: Patryk Strusiewicz-Surmacki --- cmd/frr-exporter/main.go | 5 +- config/rbac/role.yaml | 6 ++ pkg/monitoring/endpoint.go | 148 ++++++++++++++++++++++++------------- 3 files changed, 103 insertions(+), 56 deletions(-) diff --git a/cmd/frr-exporter/main.go b/cmd/frr-exporter/main.go index 2b23229f..d27a42a0 100644 --- a/cmd/frr-exporter/main.go +++ b/cmd/frr-exporter/main.go @@ -51,10 +51,7 @@ func main() { log.Fatal(fmt.Errorf("failed to create monitoring endpoint %w", err)) } - http.HandleFunc("/show/route", endpoint.ShowRoute) - http.HandleFunc("/show/bgp", endpoint.ShowBGP) - http.HandleFunc("/show/evpn", endpoint.ShowEVPN) - http.HandleFunc("/all/show/route", endpoint.ShowAllRoute) + endpoint.SetHandlers() // Expose the registered metrics via HTTP. http.Handle("/metrics", promhttp.HandlerFor( diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml index 4521ca1c..13181350 100644 --- a/config/rbac/role.yaml +++ b/config/rbac/role.yaml @@ -13,6 +13,12 @@ rules: - list - update - watch +- apiGroups: + - "" + resources: + - pods + verbs: + - list - apiGroups: - network.schiff.telekom.de resources: diff --git a/pkg/monitoring/endpoint.go b/pkg/monitoring/endpoint.go index 6628b423..cd77b63b 100644 --- a/pkg/monitoring/endpoint.go +++ b/pkg/monitoring/endpoint.go @@ -3,11 +3,12 @@ package monitoring import ( "context" "fmt" - "log" + "io" "net" "net/http" "os" "strconv" + "strings" "github.com/telekom/das-schiff-network-operator/pkg/frr" "github.com/telekom/das-schiff-network-operator/pkg/healthcheck" @@ -28,6 +29,7 @@ type Endpoint struct { c client.Client } +// NewEndpoint creates new endpoint object. func NewEndpoint() (*Endpoint, error) { clientConfig := ctrl.GetConfigOrDie() c, err := client.New(clientConfig, client.Options{}) @@ -37,6 +39,18 @@ func NewEndpoint() (*Endpoint, error) { return &Endpoint{cli: frr.NewCli(), c: c}, nil } +// SetHandlers configures HTTP handlers. +func (e *Endpoint) SetHandlers() { + http.HandleFunc("/show/route", e.ShowRoute) + http.HandleFunc("/show/bgp", e.ShowBGP) + http.HandleFunc("/show/evpn", e.ShowEVPN) + http.HandleFunc("/all/show/route", e.PassRequest) + http.HandleFunc("/all/show/bgp", e.PassRequest) + http.HandleFunc("/all/show/evpn", e.PassRequest) +} + +// ShowRoute returns result of show ip/ipv6 route command. +// show ip/ipv6 route (vrf ) (longer-prefixes) func (e *Endpoint) ShowRoute(w http.ResponseWriter, r *http.Request) { vrf := r.URL.Query().Get("vrf") if vrf == "" { @@ -71,62 +85,20 @@ func (e *Endpoint) ShowRoute(w http.ResponseWriter, r *http.Request) { data := e.cli.ExecuteWithJSON(command) - nodename := os.Getenv(healthcheck.NodenameEnv) - nodeField := fmt.Sprintf("%s:\n", nodename) - - result := []byte{} - - if nodename != "" { - result = append(result, []byte(nodeField)...) - } - - result = append(result, data...) - - writeResponse(&result, w) -} - -func (e *Endpoint) ShowAllRoute(w http.ResponseWriter, r *http.Request) { - ctx := context.TODO() - pods := &corev1.PodList{} - matchLabels := &client.MatchingLabels{ - "app.kubernetes.io/component": "worker", - } - - err := e.c.List(ctx, pods, matchLabels) - if err != nil { - http.Error(w, fmt.Sprintf("error listing pods: %s", err.Error()), http.StatusInternalServerError) - return - } - - if len(pods.Items) == 0 { - http.Error(w, "error listing pods: no pods found", http.StatusInternalServerError) - return - } - - for _, pod := range pods.Items { - fmt.Println(pod.Status.PodIP) - url := fmt.Sprintf("http://%s/show/route?%s", pod.Status.PodIP, r.URL.RawQuery) - fmt.Println(url) - resp, err := http.Get(url) - if err != nil { - log.Fatalln(err) - } - - data := []byte{} - resp.Body.Read(data) - resp.Body.Close() - writeResponse(&data, w) - } + result := addNodename(&data) + writeResponse(result, w) } +// ShowBGP returns a result of show bgp command. +// show bgp (vrf ) ipv4/ipv6 unicast (longer-prefixes) +// show bgp vrf summary func (e *Endpoint) ShowBGP(w http.ResponseWriter, r *http.Request) { vrf := r.URL.Query().Get("vrf") if vrf == "" { vrf = all } - data := []byte{} - + var data []byte requestType := r.URL.Query().Get("type") switch requestType { case "summary": @@ -171,11 +143,17 @@ func (e *Endpoint) ShowBGP(w http.ResponseWriter, r *http.Request) { return } - writeResponse(&data, w) + result := addNodename(&data) + writeResponse(result, w) } +// ShowEVPN returns result of show evpn command. +// show evpn vni json +// show evpn rmac vni +// show evpn mac vni +// show evpn next-hops vni json func (e *Endpoint) ShowEVPN(w http.ResponseWriter, r *http.Request) { - data := []byte{} + var data []byte requestType := r.URL.Query().Get("type") switch requestType { case "": @@ -202,7 +180,61 @@ func (e *Endpoint) ShowEVPN(w http.ResponseWriter, r *http.Request) { return } - writeResponse(&data, w) + result := addNodename(&data) + writeResponse(result, w) +} + +//+kubebuilder:rbac:groups=core,resources=pods,verbs=list + +// PassRequest - when called, will pass the request to all nodes and return their respones. +func (e *Endpoint) PassRequest(w http.ResponseWriter, r *http.Request) { + ctx := context.TODO() + pods := &corev1.PodList{} + matchLabels := &client.MatchingLabels{ + "app.kubernetes.io/component": "worker", + } + + err := e.c.List(ctx, pods, matchLabels) + if err != nil { + http.Error(w, fmt.Sprintf("error listing pods: %s", err.Error()), http.StatusInternalServerError) + return + } + + if len(pods.Items) == 0 { + http.Error(w, "error listing pods: no pods found", http.StatusInternalServerError) + return + } + + query := strings.ReplaceAll(r.URL.String(), "all/", "") + + s := strings.Split(r.Host, ":") + port := "" + if len(s) > 1 { + port = s[1] + } + + buffer := []byte{} + + for _, pod := range pods.Items { + url := fmt.Sprintf("http://%s:%s%s", pod.Status.PodIP, port, query) + resp, err := http.Get(url) + if err != nil { + http.Error(w, fmt.Sprintf("error getting data from %s: %s", pod.Status.PodIP, err.Error()), http.StatusInternalServerError) + return + } + + data, err := io.ReadAll(resp.Body) + if err != nil { + http.Error(w, fmt.Sprintf("error reading repsonse from %s: %s", pod.Status.PodIP, err.Error()), http.StatusInternalServerError) + return + } + + buffer = append(buffer, data...) + buffer = append(buffer, '\n') + resp.Body.Close() + } + + writeResponse(&buffer, w) } func writeResponse(data *[]byte, w http.ResponseWriter) { @@ -237,3 +269,15 @@ func setInput(r *http.Request, command *[]string) error { } return nil } + +func addNodename(data *[]byte) *[]byte { + nodename := os.Getenv(healthcheck.NodenameEnv) + nodeField := fmt.Sprintf("%s:\n", nodename) + + result := []byte{} + if nodename != "" { + result = append(result, []byte(nodeField)...) + } + result = append(result, *data...) + return &result +} From 38920ffb4338c1b693eae197061c5208bc3b1026 Mon Sep 17 00:00:00 2001 From: Patryk Strusiewicz-Surmacki Date: Fri, 8 Mar 2024 14:22:20 +0100 Subject: [PATCH 06/35] Added asynchronous GET calls Signed-off-by: Patryk Strusiewicz-Surmacki --- Makefile | 19 +++++++++++++++ pkg/monitoring/endpoint.go | 50 ++++++++++++++++++++++++++++---------- 2 files changed, 56 insertions(+), 13 deletions(-) diff --git a/Makefile b/Makefile index 06cb252c..0f2fd6aa 100644 --- a/Makefile +++ b/Makefile @@ -1,6 +1,8 @@ # Image URL to use all building/pushing image targets IMG ?= ghcr.io/telekom/das-schiff-network-operator:latest +# Sidecar image URL to use all building/pushing image targets +SIDECAR_IMG ?= ghcr.io/telekom/frr-exporter:latest # ENVTEST_K8S_VERSION refers to the version of kubebuilder assets to be downloaded by envtest binary. ENVTEST_K8S_VERSION = 1.25 @@ -81,10 +83,19 @@ run: manifests generate fmt vet ## Run a controller from your host. docker-build: test ## Build docker image with the manager. docker build -t ${IMG} . +.PHONY: docker-build-sidecar +docker-build-sidecar: test ## Build docker image with the manager. + docker build -t ${SIDECAR_IMG} -f frr-exporter.Dockerfile . + .PHONY: docker-push docker-push: ## Push docker image with the manager. docker push ${IMG} +.PHONY: docker-push-sidecar +docker-push-sidecar: ## Push docker image with the manager. + docker push ${SIDECAR_IMG} + + ##@ Release RELEASE_DIR ?= out @@ -108,10 +119,18 @@ endif install: manifests kustomize ## Install CRDs into the K8s cluster specified in ~/.kube/config. $(KUSTOMIZE) build config/crd | kubectl apply -f - +.PHONY: install-certs +install-certs: manifests kustomize ## Install certs + $(KUSTOMIZE) build config/certmanager | kubectl apply -f - + .PHONY: uninstall uninstall: manifests kustomize ## Uninstall CRDs from the K8s cluster specified in ~/.kube/config. Call with ignore-not-found=true to ignore resource not found errors during deletion. $(KUSTOMIZE) build config/crd | kubectl delete --ignore-not-found=$(ignore-not-found) -f - +.PHONY: uninstall-certs +uninstall-certs: manifests kustomize ## Uninstall certs + $(KUSTOMIZE) build config/certmanager | kubectl delete --ignore-not-found=$(ignore-not-found) -f - + .PHONY: deploy deploy: manifests kustomize ## Deploy controller to the K8s cluster specified in ~/.kube/config. cd config/manager && $(KUSTOMIZE) edit set image controller=${IMG} diff --git a/pkg/monitoring/endpoint.go b/pkg/monitoring/endpoint.go index cd77b63b..66b402e3 100644 --- a/pkg/monitoring/endpoint.go +++ b/pkg/monitoring/endpoint.go @@ -9,6 +9,7 @@ import ( "os" "strconv" "strings" + "sync" "github.com/telekom/das-schiff-network-operator/pkg/frr" "github.com/telekom/das-schiff-network-operator/pkg/healthcheck" @@ -215,23 +216,46 @@ func (e *Endpoint) PassRequest(w http.ResponseWriter, r *http.Request) { buffer := []byte{} + var wg sync.WaitGroup + results := make(chan []byte, len(pods.Items)) + errors := make(chan error, len(pods.Items)) + for _, pod := range pods.Items { - url := fmt.Sprintf("http://%s:%s%s", pod.Status.PodIP, port, query) - resp, err := http.Get(url) - if err != nil { - http.Error(w, fmt.Sprintf("error getting data from %s: %s", pod.Status.PodIP, err.Error()), http.StatusInternalServerError) - return - } + wg.Add(1) + go func(pod corev1.Pod) { + defer wg.Done() + + url := fmt.Sprintf("http://%s:%s%s", pod.Status.PodIP, port, query) + resp, err := http.Get(url) + if err != nil { + errors <- fmt.Errorf("error getting data from %s: %s", pod.Status.PodIP, err.Error()) + return + } + + data, err := io.ReadAll(resp.Body) + defer resp.Body.Close() + if err != nil { + fmt.Println(err.Error()) + errors <- fmt.Errorf("error reading repsonse from %s: %s", pod.Status.PodIP, err.Error()) + return + } + + results <- data + }(pod) + } - data, err := io.ReadAll(resp.Body) - if err != nil { - http.Error(w, fmt.Sprintf("error reading repsonse from %s: %s", pod.Status.PodIP, err.Error()), http.StatusInternalServerError) - return - } + wg.Wait() + close(results) + close(errors) + + for err := range errors { + http.Error(w, err.Error(), http.StatusInternalServerError) + return + } - buffer = append(buffer, data...) + for result := range results { + buffer = append(buffer, result...) buffer = append(buffer, '\n') - resp.Body.Close() } writeResponse(&buffer, w) From aafc408a060217fa1454406490f1971c35fc5424 Mon Sep 17 00:00:00 2001 From: Patryk Strusiewicz-Surmacki Date: Fri, 8 Mar 2024 15:16:29 +0100 Subject: [PATCH 07/35] Fixed linter issues Signed-off-by: Patryk Strusiewicz-Surmacki --- pkg/monitoring/endpoint.go | 63 +++++++++++++++++++------------------- 1 file changed, 31 insertions(+), 32 deletions(-) diff --git a/pkg/monitoring/endpoint.go b/pkg/monitoring/endpoint.go index 66b402e3..43cbadc8 100644 --- a/pkg/monitoring/endpoint.go +++ b/pkg/monitoring/endpoint.go @@ -1,7 +1,6 @@ package monitoring import ( - "context" "fmt" "io" "net" @@ -51,7 +50,7 @@ func (e *Endpoint) SetHandlers() { } // ShowRoute returns result of show ip/ipv6 route command. -// show ip/ipv6 route (vrf ) (longer-prefixes) +// show ip/ipv6 route (vrf ) (longer-prefixes). func (e *Endpoint) ShowRoute(w http.ResponseWriter, r *http.Request) { vrf := r.URL.Query().Get("vrf") if vrf == "" { @@ -91,8 +90,8 @@ func (e *Endpoint) ShowRoute(w http.ResponseWriter, r *http.Request) { } // ShowBGP returns a result of show bgp command. -// show bgp (vrf ) ipv4/ipv6 unicast (longer-prefixes) -// show bgp vrf summary +// show bgp (vrf ) ipv4/ipv6 unicast (longer-prefixes). +// show bgp vrf summary. func (e *Endpoint) ShowBGP(w http.ResponseWriter, r *http.Request) { vrf := r.URL.Query().Get("vrf") if vrf == "" { @@ -149,10 +148,10 @@ func (e *Endpoint) ShowBGP(w http.ResponseWriter, r *http.Request) { } // ShowEVPN returns result of show evpn command. -// show evpn vni json -// show evpn rmac vni -// show evpn mac vni -// show evpn next-hops vni json +// show evpn vni json. +// show evpn rmac vni . +// show evpn mac vni . +// show evpn next-hops vni json. func (e *Endpoint) ShowEVPN(w http.ResponseWriter, r *http.Request) { var data []byte requestType := r.URL.Query().Get("type") @@ -187,15 +186,14 @@ func (e *Endpoint) ShowEVPN(w http.ResponseWriter, r *http.Request) { //+kubebuilder:rbac:groups=core,resources=pods,verbs=list -// PassRequest - when called, will pass the request to all nodes and return their respones. +// PassRequest - when called, will pass the request to all nodes and return their responses. func (e *Endpoint) PassRequest(w http.ResponseWriter, r *http.Request) { - ctx := context.TODO() pods := &corev1.PodList{} matchLabels := &client.MatchingLabels{ "app.kubernetes.io/component": "worker", } - err := e.c.List(ctx, pods, matchLabels) + err := e.c.List(r.Context(), pods, matchLabels) if err != nil { http.Error(w, fmt.Sprintf("error listing pods: %s", err.Error()), http.StatusInternalServerError) return @@ -220,28 +218,9 @@ func (e *Endpoint) PassRequest(w http.ResponseWriter, r *http.Request) { results := make(chan []byte, len(pods.Items)) errors := make(chan error, len(pods.Items)) - for _, pod := range pods.Items { + for i := range pods.Items { wg.Add(1) - go func(pod corev1.Pod) { - defer wg.Done() - - url := fmt.Sprintf("http://%s:%s%s", pod.Status.PodIP, port, query) - resp, err := http.Get(url) - if err != nil { - errors <- fmt.Errorf("error getting data from %s: %s", pod.Status.PodIP, err.Error()) - return - } - - data, err := io.ReadAll(resp.Body) - defer resp.Body.Close() - if err != nil { - fmt.Println(err.Error()) - errors <- fmt.Errorf("error reading repsonse from %s: %s", pod.Status.PodIP, err.Error()) - return - } - - results <- data - }(pod) + go passToPod(&pods.Items[i], port, query, results, errors, &wg) } wg.Wait() @@ -305,3 +284,23 @@ func addNodename(data *[]byte) *[]byte { result = append(result, *data...) return &result } + +func passToPod(pod *corev1.Pod, port, query string, results chan []byte, errors chan error, wg *sync.WaitGroup) { + defer wg.Done() + + url := fmt.Sprintf("http://%s:%s%s", pod.Status.PodIP, port, query) + resp, err := http.Get(url) //nolint + if err != nil { + errors <- fmt.Errorf("error getting data from %s: %w", pod.Status.PodIP, err) + return + } + + data, err := io.ReadAll(resp.Body) + defer resp.Body.Close() + if err != nil { + errors <- fmt.Errorf("error reading response from %s: %w", pod.Status.PodIP, err) + return + } + + results <- data +} From 0a6fcf6351c25ea8becb4d59e8411b41332aa829 Mon Sep 17 00:00:00 2001 From: Patryk Strusiewicz-Surmacki Date: Tue, 12 Mar 2024 11:05:45 +0100 Subject: [PATCH 08/35] Made label selector more specific Signed-off-by: Patryk Strusiewicz-Surmacki --- pkg/monitoring/endpoint.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/monitoring/endpoint.go b/pkg/monitoring/endpoint.go index 43cbadc8..7f03abd1 100644 --- a/pkg/monitoring/endpoint.go +++ b/pkg/monitoring/endpoint.go @@ -190,6 +190,7 @@ func (e *Endpoint) ShowEVPN(w http.ResponseWriter, r *http.Request) { func (e *Endpoint) PassRequest(w http.ResponseWriter, r *http.Request) { pods := &corev1.PodList{} matchLabels := &client.MatchingLabels{ + "app.kubernetes.io/name": "network-operator", "app.kubernetes.io/component": "worker", } From e0d524925610ed8dce234aba6c707cf40260587b Mon Sep 17 00:00:00 2001 From: Patryk Strusiewicz-Surmacki Date: Tue, 12 Mar 2024 14:34:29 +0100 Subject: [PATCH 09/35] Added basic unit tests Signed-off-by: Patryk Strusiewicz-Surmacki --- cmd/frr-exporter/main.go | 10 +- pkg/monitoring/endpoint.go | 18 +-- pkg/monitoring/endpoint_test.go | 220 +++++++++++++++++++++++++++ pkg/monitoring/mock/mock_endpoint.go | 48 ++++++ 4 files changed, 284 insertions(+), 12 deletions(-) create mode 100644 pkg/monitoring/endpoint_test.go create mode 100644 pkg/monitoring/mock/mock_endpoint.go diff --git a/cmd/frr-exporter/main.go b/cmd/frr-exporter/main.go index d27a42a0..99ba9f41 100644 --- a/cmd/frr-exporter/main.go +++ b/cmd/frr-exporter/main.go @@ -10,8 +10,10 @@ import ( "github.com/prometheus/client_golang/prometheus" "github.com/prometheus/client_golang/prometheus/collectors" "github.com/prometheus/client_golang/prometheus/promhttp" + "github.com/telekom/das-schiff-network-operator/pkg/frr" "github.com/telekom/das-schiff-network-operator/pkg/monitoring" ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/log/zap" ) @@ -46,11 +48,15 @@ func main() { } reg.MustRegister(collector) - endpoint, err := monitoring.NewEndpoint() + clientConfig := ctrl.GetConfigOrDie() + c, err := client.New(clientConfig, client.Options{}) if err != nil { - log.Fatal(fmt.Errorf("failed to create monitoring endpoint %w", err)) + log.Fatal(fmt.Errorf("error creating controller-runtime client: %w", err)) } + frrCli := frr.NewCli() + + endpoint := monitoring.NewEndpoint(c, frrCli) endpoint.SetHandlers() // Expose the registered metrics via HTTP. diff --git a/pkg/monitoring/endpoint.go b/pkg/monitoring/endpoint.go index 7f03abd1..a060867f 100644 --- a/pkg/monitoring/endpoint.go +++ b/pkg/monitoring/endpoint.go @@ -10,10 +10,8 @@ import ( "strings" "sync" - "github.com/telekom/das-schiff-network-operator/pkg/frr" "github.com/telekom/das-schiff-network-operator/pkg/healthcheck" corev1 "k8s.io/api/core/v1" - ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" ) @@ -24,19 +22,19 @@ const ( protocolIPv6 = "ipv6" ) +//go:generate mockgen -destination ./mock/mock_endpoint.go . FRRClient +type FRRClient interface { + ExecuteWithJSON(args []string) []byte +} + type Endpoint struct { - cli *frr.Cli + cli FRRClient c client.Client } // NewEndpoint creates new endpoint object. -func NewEndpoint() (*Endpoint, error) { - clientConfig := ctrl.GetConfigOrDie() - c, err := client.New(clientConfig, client.Options{}) - if err != nil { - return nil, fmt.Errorf("error creating controller-runtime client: %w", err) - } - return &Endpoint{cli: frr.NewCli(), c: c}, nil +func NewEndpoint(k8sClient client.Client, frrcli FRRClient) *Endpoint { + return &Endpoint{cli: frrcli, c: k8sClient} } // SetHandlers configures HTTP handlers. diff --git a/pkg/monitoring/endpoint_test.go b/pkg/monitoring/endpoint_test.go new file mode 100644 index 00000000..fd6cab63 --- /dev/null +++ b/pkg/monitoring/endpoint_test.go @@ -0,0 +1,220 @@ +package monitoring + +import ( + "encoding/json" + "net/http" + "net/http/httptest" + "testing" + + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" + mock_monitoring "github.com/telekom/das-schiff-network-operator/pkg/monitoring/mock" + "go.uber.org/mock/gomock" + corev1 "k8s.io/api/core/v1" + "sigs.k8s.io/controller-runtime/pkg/client/fake" +) + +const testHostname = "worker" + +var ( + fakePodsJSON = `{ + "items": [ + { + "apiVersion": "v1", + "kind": "Pod", + "metadata": { + "labels": { + "app.kubernetes.io/component": "worker", + "app.kubernetes.io/name": "network-operator" + }, + "name": "network-operator-worker-1", + "namespace": "kube-system" + }, + "status": { + "hostIP": "172.18.0.3", + "podIP": "172.18.0.3", + "podIPs": [ + { + "ip": "172.18.0.3" + } + ] + } + }, + { + "apiVersion": "v1", + "kind": "Pod", + "metadata": { + "labels": { + "app.kubernetes.io/component": "worker", + "app.kubernetes.io/name": "network-operator" + }, + "name": "network-operator-worker-2", + "namespace": "kube-system" + }, + "status": { + "hostIP": "172.18.0.4", + "podIP": "172.18.0.4", + "podIPs": [ + { + "ip": "172.18.0.4" + } + ] + } + } + ] + }` + fakePods *corev1.PodList + tmpPath string + mockCtrl *gomock.Controller +) + +var _ = BeforeSuite(func() { + fakePods = &corev1.PodList{} + err := json.Unmarshal([]byte(fakePodsJSON), fakePods) + Expect(err).ShouldNot(HaveOccurred()) +}) + +func TestHealthCheck(t *testing.T) { + RegisterFailHandler(Fail) + tmpPath = t.TempDir() + mockCtrl = gomock.NewController(t) + defer mockCtrl.Finish() + RunSpecs(t, + "HealthCheck Suite") +} + +var _ = Describe("Endpoint", func() { + fcm := mock_monitoring.NewMockFRRClient(mockCtrl) + c := fake.NewClientBuilder().Build() + e := NewEndpoint(c, fcm) + e.SetHandlers() + + Context("ShowRoute() should", func() { + It("return no error", func() { + req := httptest.NewRequest(http.MethodGet, "/show/route", nil) + res := httptest.NewRecorder() + fcm.EXPECT().ExecuteWithJSON(gomock.Any()).Return([]byte{}) + e.ShowRoute(res, req) + Expect(res.Code).To(Equal(http.StatusOK)) + }) + It("return error if protocol is invalid", func() { + req := httptest.NewRequest(http.MethodGet, "/show/route?protocol=ipv42", nil) + res := httptest.NewRecorder() + e.ShowRoute(res, req) + Expect(res.Code).To(Equal(http.StatusBadRequest)) + }) + It("return error if input CIDR is invalid", func() { + req := httptest.NewRequest(http.MethodGet, "/show/route?protocol=ipv6&input=192.168.1.1/42", nil) + res := httptest.NewRecorder() + e.ShowRoute(res, req) + Expect(res.Code).To(Equal(http.StatusBadRequest)) + }) + It("return error if longer_prefixes value is invalid", func() { + req := httptest.NewRequest(http.MethodGet, "/show/route?protocol=ipv6&input=192.168.1.1/32&longer_prefixes=notABool", nil) + res := httptest.NewRecorder() + e.ShowRoute(res, req) + Expect(res.Code).To(Equal(http.StatusBadRequest)) + }) + It("return no error", func() { + req := httptest.NewRequest(http.MethodGet, "/show/route?protocol=ipv6&input=192.168.1.1/32&longer_prefixes=true", nil) + res := httptest.NewRecorder() + fcm.EXPECT().ExecuteWithJSON(gomock.Any()).Return([]byte{}) + e.ShowRoute(res, req) + Expect(res.Code).To(Equal(http.StatusOK)) + }) + }) + + Context("ShowBGP() should", func() { + It("return no error if type is not specified (default)", func() { + req := httptest.NewRequest(http.MethodGet, "/show/bgp", nil) + res := httptest.NewRecorder() + fcm.EXPECT().ExecuteWithJSON(gomock.Any()).Return([]byte{}) + e.ShowBGP(res, req) + Expect(res.Code).To(Equal(http.StatusOK)) + }) + It("return no error if type is summary", func() { + req := httptest.NewRequest(http.MethodGet, "/show/bgp?type=summary", nil) + res := httptest.NewRecorder() + fcm.EXPECT().ExecuteWithJSON(gomock.Any()).Return([]byte{}) + e.ShowBGP(res, req) + Expect(res.Code).To(Equal(http.StatusOK)) + }) + It("return error if type is invalid", func() { + req := httptest.NewRequest(http.MethodGet, "/show/bgp?type=ivalidType", nil) + res := httptest.NewRecorder() + e.ShowBGP(res, req) + Expect(res.Code).To(Equal(http.StatusBadRequest)) + }) + It("return error if protocol is invalid", func() { + req := httptest.NewRequest(http.MethodGet, "/show/bgp?protocol=ipv42", nil) + res := httptest.NewRecorder() + e.ShowBGP(res, req) + Expect(res.Code).To(Equal(http.StatusBadRequest)) + }) + It("return error if input CIDR is invalid", func() { + req := httptest.NewRequest(http.MethodGet, "/show/bgp?protocol=ipv4&input=192.168.1.1/42", nil) + res := httptest.NewRecorder() + e.ShowBGP(res, req) + Expect(res.Code).To(Equal(http.StatusBadRequest)) + }) + It("return error if longer_prefixes value is invalid", func() { + req := httptest.NewRequest(http.MethodGet, "/show/bgp?protocol=ipv4&input=192.168.1.1/32&longer_prefixes=notABool", nil) + res := httptest.NewRecorder() + e.ShowBGP(res, req) + Expect(res.Code).To(Equal(http.StatusBadRequest)) + }) + }) + + Context("ShowEVPN() should", func() { + It("return no error if type is not specified (default)", func() { + req := httptest.NewRequest(http.MethodGet, "/show/evpn", nil) + res := httptest.NewRecorder() + fcm.EXPECT().ExecuteWithJSON(gomock.Any()).Return([]byte{}) + e.ShowEVPN(res, req) + Expect(res.Code).To(Equal(http.StatusOK)) + }) + It("return no error if type is rmac", func() { + req := httptest.NewRequest(http.MethodGet, "/show/evpn?type=rmac", nil) + res := httptest.NewRecorder() + fcm.EXPECT().ExecuteWithJSON(gomock.Any()).Return([]byte{}) + e.ShowEVPN(res, req) + Expect(res.Code).To(Equal(http.StatusOK)) + }) + It("return no error if type is mac", func() { + req := httptest.NewRequest(http.MethodGet, "/show/evpn?type=mac", nil) + res := httptest.NewRecorder() + fcm.EXPECT().ExecuteWithJSON(gomock.Any()).Return([]byte{}) + e.ShowEVPN(res, req) + Expect(res.Code).To(Equal(http.StatusOK)) + }) + It("return no error if type is next-hops", func() { + req := httptest.NewRequest(http.MethodGet, "/show/evpn?type=next-hops", nil) + res := httptest.NewRecorder() + fcm.EXPECT().ExecuteWithJSON(gomock.Any()).Return([]byte{}) + e.ShowEVPN(res, req) + Expect(res.Code).To(Equal(http.StatusOK)) + }) + It("return error if type is invalid", func() { + req := httptest.NewRequest(http.MethodGet, "/show/evpn?type=invalidType", nil) + res := httptest.NewRecorder() + e.ShowEVPN(res, req) + Expect(res.Code).To(Equal(http.StatusBadRequest)) + }) + }) + Context("PassRequest() should", func() { + It("return error if there are no instances to query", func() { + req := httptest.NewRequest(http.MethodGet, "/all/show/route", nil) + res := httptest.NewRecorder() + e.PassRequest(res, req) + Expect(res.Code).To(Equal(http.StatusInternalServerError)) + }) + It("return error if cannot get data from target pod", func() { + c := fake.NewClientBuilder().WithRuntimeObjects(fakePods).Build() + e := NewEndpoint(c, fcm) + req := httptest.NewRequest(http.MethodGet, "/all/show/route", nil) + res := httptest.NewRecorder() + e.PassRequest(res, req) + Expect(res.Code).To(Equal(http.StatusInternalServerError)) + }) + }) +}) diff --git a/pkg/monitoring/mock/mock_endpoint.go b/pkg/monitoring/mock/mock_endpoint.go new file mode 100644 index 00000000..7b0f4481 --- /dev/null +++ b/pkg/monitoring/mock/mock_endpoint.go @@ -0,0 +1,48 @@ +// Code generated by MockGen. DO NOT EDIT. +// Source: github.com/telekom/das-schiff-network-operator/pkg/monitoring (interfaces: FRRClient) + +// Package mock_monitoring is a generated GoMock package. +package mock_monitoring + +import ( + reflect "reflect" + + gomock "go.uber.org/mock/gomock" +) + +// MockFRRClient is a mock of FRRClient interface. +type MockFRRClient struct { + ctrl *gomock.Controller + recorder *MockFRRClientMockRecorder +} + +// MockFRRClientMockRecorder is the mock recorder for MockFRRClient. +type MockFRRClientMockRecorder struct { + mock *MockFRRClient +} + +// NewMockFRRClient creates a new mock instance. +func NewMockFRRClient(ctrl *gomock.Controller) *MockFRRClient { + mock := &MockFRRClient{ctrl: ctrl} + mock.recorder = &MockFRRClientMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockFRRClient) EXPECT() *MockFRRClientMockRecorder { + return m.recorder +} + +// ExecuteWithJSON mocks base method. +func (m *MockFRRClient) ExecuteWithJSON(arg0 []string) []byte { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "ExecuteWithJSON", arg0) + ret0, _ := ret[0].([]byte) + return ret0 +} + +// ExecuteWithJSON indicates an expected call of ExecuteWithJSON. +func (mr *MockFRRClientMockRecorder) ExecuteWithJSON(arg0 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ExecuteWithJSON", reflect.TypeOf((*MockFRRClient)(nil).ExecuteWithJSON), arg0) +} From 0e9939449118a03b6233e9634f2ce1f5ee394f4e Mon Sep 17 00:00:00 2001 From: Patryk Strusiewicz-Surmacki Date: Tue, 12 Mar 2024 14:35:53 +0100 Subject: [PATCH 10/35] Removed redundant const Signed-off-by: Patryk Strusiewicz-Surmacki --- pkg/monitoring/endpoint_test.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/pkg/monitoring/endpoint_test.go b/pkg/monitoring/endpoint_test.go index fd6cab63..a4b72d58 100644 --- a/pkg/monitoring/endpoint_test.go +++ b/pkg/monitoring/endpoint_test.go @@ -14,8 +14,6 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client/fake" ) -const testHostname = "worker" - var ( fakePodsJSON = `{ "items": [ From 4d4ade2ecf361d0a7a46884d7e3b4a660187c846 Mon Sep 17 00:00:00 2001 From: Patryk Strusiewicz-Surmacki Date: Tue, 12 Mar 2024 14:38:40 +0100 Subject: [PATCH 11/35] Fixed linter issues Signed-off-by: Patryk Strusiewicz-Surmacki --- pkg/monitoring/endpoint_test.go | 40 ++++++++++++++++----------------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/pkg/monitoring/endpoint_test.go b/pkg/monitoring/endpoint_test.go index a4b72d58..a03e0704 100644 --- a/pkg/monitoring/endpoint_test.go +++ b/pkg/monitoring/endpoint_test.go @@ -8,7 +8,7 @@ import ( . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" - mock_monitoring "github.com/telekom/das-schiff-network-operator/pkg/monitoring/mock" + monmock "github.com/telekom/das-schiff-network-operator/pkg/monitoring/mock" "go.uber.org/mock/gomock" corev1 "k8s.io/api/core/v1" "sigs.k8s.io/controller-runtime/pkg/client/fake" @@ -82,39 +82,39 @@ func TestHealthCheck(t *testing.T) { } var _ = Describe("Endpoint", func() { - fcm := mock_monitoring.NewMockFRRClient(mockCtrl) + fcm := monmock.NewMockFRRClient(mockCtrl) c := fake.NewClientBuilder().Build() e := NewEndpoint(c, fcm) e.SetHandlers() Context("ShowRoute() should", func() { It("return no error", func() { - req := httptest.NewRequest(http.MethodGet, "/show/route", nil) + req := httptest.NewRequest(http.MethodGet, "/show/route", http.NoBody) res := httptest.NewRecorder() fcm.EXPECT().ExecuteWithJSON(gomock.Any()).Return([]byte{}) e.ShowRoute(res, req) Expect(res.Code).To(Equal(http.StatusOK)) }) It("return error if protocol is invalid", func() { - req := httptest.NewRequest(http.MethodGet, "/show/route?protocol=ipv42", nil) + req := httptest.NewRequest(http.MethodGet, "/show/route?protocol=ipv42", http.NoBody) res := httptest.NewRecorder() e.ShowRoute(res, req) Expect(res.Code).To(Equal(http.StatusBadRequest)) }) It("return error if input CIDR is invalid", func() { - req := httptest.NewRequest(http.MethodGet, "/show/route?protocol=ipv6&input=192.168.1.1/42", nil) + req := httptest.NewRequest(http.MethodGet, "/show/route?protocol=ipv6&input=192.168.1.1/42", http.NoBody) res := httptest.NewRecorder() e.ShowRoute(res, req) Expect(res.Code).To(Equal(http.StatusBadRequest)) }) It("return error if longer_prefixes value is invalid", func() { - req := httptest.NewRequest(http.MethodGet, "/show/route?protocol=ipv6&input=192.168.1.1/32&longer_prefixes=notABool", nil) + req := httptest.NewRequest(http.MethodGet, "/show/route?protocol=ipv6&input=192.168.1.1/32&longer_prefixes=notABool", http.NoBody) res := httptest.NewRecorder() e.ShowRoute(res, req) Expect(res.Code).To(Equal(http.StatusBadRequest)) }) It("return no error", func() { - req := httptest.NewRequest(http.MethodGet, "/show/route?protocol=ipv6&input=192.168.1.1/32&longer_prefixes=true", nil) + req := httptest.NewRequest(http.MethodGet, "/show/route?protocol=ipv6&input=192.168.1.1/32&longer_prefixes=true", http.NoBody) res := httptest.NewRecorder() fcm.EXPECT().ExecuteWithJSON(gomock.Any()).Return([]byte{}) e.ShowRoute(res, req) @@ -124,39 +124,39 @@ var _ = Describe("Endpoint", func() { Context("ShowBGP() should", func() { It("return no error if type is not specified (default)", func() { - req := httptest.NewRequest(http.MethodGet, "/show/bgp", nil) + req := httptest.NewRequest(http.MethodGet, "/show/bgp", http.NoBody) res := httptest.NewRecorder() fcm.EXPECT().ExecuteWithJSON(gomock.Any()).Return([]byte{}) e.ShowBGP(res, req) Expect(res.Code).To(Equal(http.StatusOK)) }) It("return no error if type is summary", func() { - req := httptest.NewRequest(http.MethodGet, "/show/bgp?type=summary", nil) + req := httptest.NewRequest(http.MethodGet, "/show/bgp?type=summary", http.NoBody) res := httptest.NewRecorder() fcm.EXPECT().ExecuteWithJSON(gomock.Any()).Return([]byte{}) e.ShowBGP(res, req) Expect(res.Code).To(Equal(http.StatusOK)) }) It("return error if type is invalid", func() { - req := httptest.NewRequest(http.MethodGet, "/show/bgp?type=ivalidType", nil) + req := httptest.NewRequest(http.MethodGet, "/show/bgp?type=ivalidType", http.NoBody) res := httptest.NewRecorder() e.ShowBGP(res, req) Expect(res.Code).To(Equal(http.StatusBadRequest)) }) It("return error if protocol is invalid", func() { - req := httptest.NewRequest(http.MethodGet, "/show/bgp?protocol=ipv42", nil) + req := httptest.NewRequest(http.MethodGet, "/show/bgp?protocol=ipv42", http.NoBody) res := httptest.NewRecorder() e.ShowBGP(res, req) Expect(res.Code).To(Equal(http.StatusBadRequest)) }) It("return error if input CIDR is invalid", func() { - req := httptest.NewRequest(http.MethodGet, "/show/bgp?protocol=ipv4&input=192.168.1.1/42", nil) + req := httptest.NewRequest(http.MethodGet, "/show/bgp?protocol=ipv4&input=192.168.1.1/42", http.NoBody) res := httptest.NewRecorder() e.ShowBGP(res, req) Expect(res.Code).To(Equal(http.StatusBadRequest)) }) It("return error if longer_prefixes value is invalid", func() { - req := httptest.NewRequest(http.MethodGet, "/show/bgp?protocol=ipv4&input=192.168.1.1/32&longer_prefixes=notABool", nil) + req := httptest.NewRequest(http.MethodGet, "/show/bgp?protocol=ipv4&input=192.168.1.1/32&longer_prefixes=notABool", http.NoBody) res := httptest.NewRecorder() e.ShowBGP(res, req) Expect(res.Code).To(Equal(http.StatusBadRequest)) @@ -165,35 +165,35 @@ var _ = Describe("Endpoint", func() { Context("ShowEVPN() should", func() { It("return no error if type is not specified (default)", func() { - req := httptest.NewRequest(http.MethodGet, "/show/evpn", nil) + req := httptest.NewRequest(http.MethodGet, "/show/evpn", http.NoBody) res := httptest.NewRecorder() fcm.EXPECT().ExecuteWithJSON(gomock.Any()).Return([]byte{}) e.ShowEVPN(res, req) Expect(res.Code).To(Equal(http.StatusOK)) }) It("return no error if type is rmac", func() { - req := httptest.NewRequest(http.MethodGet, "/show/evpn?type=rmac", nil) + req := httptest.NewRequest(http.MethodGet, "/show/evpn?type=rmac", http.NoBody) res := httptest.NewRecorder() fcm.EXPECT().ExecuteWithJSON(gomock.Any()).Return([]byte{}) e.ShowEVPN(res, req) Expect(res.Code).To(Equal(http.StatusOK)) }) It("return no error if type is mac", func() { - req := httptest.NewRequest(http.MethodGet, "/show/evpn?type=mac", nil) + req := httptest.NewRequest(http.MethodGet, "/show/evpn?type=mac", http.NoBody) res := httptest.NewRecorder() fcm.EXPECT().ExecuteWithJSON(gomock.Any()).Return([]byte{}) e.ShowEVPN(res, req) Expect(res.Code).To(Equal(http.StatusOK)) }) It("return no error if type is next-hops", func() { - req := httptest.NewRequest(http.MethodGet, "/show/evpn?type=next-hops", nil) + req := httptest.NewRequest(http.MethodGet, "/show/evpn?type=next-hops", http.NoBody) res := httptest.NewRecorder() fcm.EXPECT().ExecuteWithJSON(gomock.Any()).Return([]byte{}) e.ShowEVPN(res, req) Expect(res.Code).To(Equal(http.StatusOK)) }) It("return error if type is invalid", func() { - req := httptest.NewRequest(http.MethodGet, "/show/evpn?type=invalidType", nil) + req := httptest.NewRequest(http.MethodGet, "/show/evpn?type=invalidType", http.NoBody) res := httptest.NewRecorder() e.ShowEVPN(res, req) Expect(res.Code).To(Equal(http.StatusBadRequest)) @@ -201,7 +201,7 @@ var _ = Describe("Endpoint", func() { }) Context("PassRequest() should", func() { It("return error if there are no instances to query", func() { - req := httptest.NewRequest(http.MethodGet, "/all/show/route", nil) + req := httptest.NewRequest(http.MethodGet, "/all/show/route", http.NoBody) res := httptest.NewRecorder() e.PassRequest(res, req) Expect(res.Code).To(Equal(http.StatusInternalServerError)) @@ -209,7 +209,7 @@ var _ = Describe("Endpoint", func() { It("return error if cannot get data from target pod", func() { c := fake.NewClientBuilder().WithRuntimeObjects(fakePods).Build() e := NewEndpoint(c, fcm) - req := httptest.NewRequest(http.MethodGet, "/all/show/route", nil) + req := httptest.NewRequest(http.MethodGet, "/all/show/route", http.NoBody) res := httptest.NewRecorder() e.PassRequest(res, req) Expect(res.Code).To(Equal(http.StatusInternalServerError)) From b1ba4dd4fe03d4648831340dda1a4bed8100e0b0 Mon Sep 17 00:00:00 2001 From: Patryk Strusiewicz-Surmacki Date: Tue, 12 Mar 2024 14:41:28 +0100 Subject: [PATCH 12/35] ShowBGP - protocol ip will be accepted as ipv4 Signed-off-by: Patryk Strusiewicz-Surmacki --- pkg/monitoring/endpoint.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/monitoring/endpoint.go b/pkg/monitoring/endpoint.go index a060867f..41f41f01 100644 --- a/pkg/monitoring/endpoint.go +++ b/pkg/monitoring/endpoint.go @@ -109,7 +109,7 @@ func (e *Endpoint) ShowBGP(w http.ResponseWriter, r *http.Request) { }) case "": protocol := r.URL.Query().Get("protocol") - if protocol == "" { + if protocol == "" || protocol == protocolIP { protocol = protocolIPv4 } else if protocol != protocolIPv4 && protocol != protocolIPv6 { http.Error(w, fmt.Sprintf("protocol '%s' is not supported", protocol), http.StatusBadRequest) From dfb59cf6f3d59f158976c13b3f5919d8f7e7f6b2 Mon Sep 17 00:00:00 2001 From: Patryk Strusiewicz-Surmacki Date: Tue, 12 Mar 2024 15:39:08 +0100 Subject: [PATCH 13/35] Fixed json generation Signed-off-by: Patryk Strusiewicz-Surmacki --- pkg/monitoring/endpoint.go | 52 ++++++++++++++++++++++++++++---------- 1 file changed, 39 insertions(+), 13 deletions(-) diff --git a/pkg/monitoring/endpoint.go b/pkg/monitoring/endpoint.go index 41f41f01..7cdfecf9 100644 --- a/pkg/monitoring/endpoint.go +++ b/pkg/monitoring/endpoint.go @@ -1,6 +1,7 @@ package monitoring import ( + "encoding/json" "fmt" "io" "net" @@ -83,7 +84,12 @@ func (e *Endpoint) ShowRoute(w http.ResponseWriter, r *http.Request) { data := e.cli.ExecuteWithJSON(command) - result := addNodename(&data) + result, err := withNodename(&data) + if err != nil { + http.Error(w, fmt.Sprintf("error adding nodename: %s", err.Error()), http.StatusBadRequest) + return + } + writeResponse(result, w) } @@ -141,7 +147,12 @@ func (e *Endpoint) ShowBGP(w http.ResponseWriter, r *http.Request) { return } - result := addNodename(&data) + result, err := withNodename(&data) + if err != nil { + http.Error(w, fmt.Sprintf("error adding nodename: %s", err.Error()), http.StatusBadRequest) + return + } + writeResponse(result, w) } @@ -178,7 +189,12 @@ func (e *Endpoint) ShowEVPN(w http.ResponseWriter, r *http.Request) { return } - result := addNodename(&data) + result, err := withNodename(&data) + if err != nil { + http.Error(w, fmt.Sprintf("error adding nodename: %s", err.Error()), http.StatusBadRequest) + return + } + writeResponse(result, w) } @@ -211,7 +227,7 @@ func (e *Endpoint) PassRequest(w http.ResponseWriter, r *http.Request) { port = s[1] } - buffer := []byte{} + responses := []json.RawMessage{} var wg sync.WaitGroup results := make(chan []byte, len(pods.Items)) @@ -232,11 +248,16 @@ func (e *Endpoint) PassRequest(w http.ResponseWriter, r *http.Request) { } for result := range results { - buffer = append(buffer, result...) - buffer = append(buffer, '\n') + responses = append(responses, json.RawMessage(result)) } - writeResponse(&buffer, w) + jsn, err := json.MarshalIndent(responses, "", "\t") + if err != nil { + http.Error(w, "error marshalling data: "+err.Error(), http.StatusInternalServerError) + return + } + + writeResponse(&jsn, w) } func writeResponse(data *[]byte, w http.ResponseWriter) { @@ -272,16 +293,21 @@ func setInput(r *http.Request, command *[]string) error { return nil } -func addNodename(data *[]byte) *[]byte { +func withNodename(data *[]byte) (*[]byte, error) { + res := map[string]json.RawMessage{} nodename := os.Getenv(healthcheck.NodenameEnv) - nodeField := fmt.Sprintf("%s:\n", nodename) - result := []byte{} + result := data if nodename != "" { - result = append(result, []byte(nodeField)...) + res[nodename] = json.RawMessage(*data) + var err error + *result, err = json.MarshalIndent(res, "", "\t") + if err != nil { + return nil, fmt.Errorf("error marshalling data: %w", err) + } } - result = append(result, *data...) - return &result + + return result, nil } func passToPod(pod *corev1.Pod, port, query string, results chan []byte, errors chan error, wg *sync.WaitGroup) { From 869560eb694dad8b6e990ad5758e8fb8ae1bf322 Mon Sep 17 00:00:00 2001 From: Patryk Strusiewicz-Surmacki Date: Tue, 12 Mar 2024 17:17:16 +0100 Subject: [PATCH 14/35] Service/endpoint based discovery used instead of pod based discovery Signed-off-by: Patryk Strusiewicz-Surmacki --- config/rbac/role.yaml | 14 ++- pkg/monitoring/endpoint.go | 163 +++++++++++++++++++++++--------- pkg/monitoring/endpoint_test.go | 10 +- 3 files changed, 140 insertions(+), 47 deletions(-) diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml index 13181350..f94f6fe3 100644 --- a/config/rbac/role.yaml +++ b/config/rbac/role.yaml @@ -4,6 +4,12 @@ kind: ClusterRole metadata: name: manager-role rules: +- apiGroups: + - "" + resources: + - endpoints + verbs: + - get - apiGroups: - "" resources: @@ -16,7 +22,13 @@ rules: - apiGroups: - "" resources: - - pods + - services + verbs: + - get +- apiGroups: + - discovery.k8s.io + resources: + - endpointslices verbs: - list - apiGroups: diff --git a/pkg/monitoring/endpoint.go b/pkg/monitoring/endpoint.go index 7cdfecf9..6c9c3704 100644 --- a/pkg/monitoring/endpoint.go +++ b/pkg/monitoring/endpoint.go @@ -1,6 +1,7 @@ package monitoring import ( + "context" "encoding/json" "fmt" "io" @@ -13,6 +14,7 @@ import ( "github.com/telekom/das-schiff-network-operator/pkg/healthcheck" corev1 "k8s.io/api/core/v1" + discoveryv1 "k8s.io/api/discovery/v1" "sigs.k8s.io/controller-runtime/pkg/client" ) @@ -21,6 +23,8 @@ const ( protocolIP = "ip" protocolIPv4 = "ipv4" protocolIPv6 = "ipv6" + + defaultNamespace = "kube-system" ) //go:generate mockgen -destination ./mock/mock_endpoint.go . FRRClient @@ -198,66 +202,48 @@ func (e *Endpoint) ShowEVPN(w http.ResponseWriter, r *http.Request) { writeResponse(result, w) } -//+kubebuilder:rbac:groups=core,resources=pods,verbs=list +//+kubebuilder:rbac:groups=core,resources=services,verbs=get +//+kubebuilder:rbac:groups=core,resources=endpoints,verbs=get +//+kubebuilder:rbac:groups=discovery.k8s.io,resources=endpointslices,verbs=list // PassRequest - when called, will pass the request to all nodes and return their responses. func (e *Endpoint) PassRequest(w http.ResponseWriter, r *http.Request) { - pods := &corev1.PodList{} - matchLabels := &client.MatchingLabels{ - "app.kubernetes.io/name": "network-operator", - "app.kubernetes.io/component": "worker", + serviceName := r.URL.Query().Get("service") + if serviceName == "" { + http.Error(w, "error looking for service: service name not provided", http.StatusBadRequest) + return + } + + namespace := r.URL.Query().Get("namespace") + if namespace == "" { + namespace = defaultNamespace } - err := e.c.List(r.Context(), pods, matchLabels) + service := &corev1.Service{} + err := e.c.Get(r.Context(), client.ObjectKey{Name: serviceName, Namespace: namespace}, service) if err != nil { http.Error(w, fmt.Sprintf("error listing pods: %s", err.Error()), http.StatusInternalServerError) return } - if len(pods.Items) == 0 { - http.Error(w, "error listing pods: no pods found", http.StatusInternalServerError) + addr, err := e.getAddresses(r.Context(), service) + if err != nil { + http.Error(w, fmt.Sprintf("error getting addresses: %s", err.Error()), http.StatusInternalServerError) return } - query := strings.ReplaceAll(r.URL.String(), "all/", "") - - s := strings.Split(r.Host, ":") - port := "" - if len(s) > 1 { - port = s[1] - } - - responses := []json.RawMessage{} - - var wg sync.WaitGroup - results := make(chan []byte, len(pods.Items)) - errors := make(chan error, len(pods.Items)) - - for i := range pods.Items { - wg.Add(1) - go passToPod(&pods.Items[i], port, query, results, errors, &wg) - } - - wg.Wait() - close(results) - close(errors) - - for err := range errors { - http.Error(w, err.Error(), http.StatusInternalServerError) + if len(addr) == 0 { + http.Error(w, "error listing addresses: no addresses found", http.StatusInternalServerError) return } - for result := range results { - responses = append(responses, json.RawMessage(result)) - } - - jsn, err := json.MarshalIndent(responses, "", "\t") + response, err := queryEndpoints(r, addr) if err != nil { - http.Error(w, "error marshalling data: "+err.Error(), http.StatusInternalServerError) + http.Error(w, "error querying endpoints: "+err.Error(), http.StatusInternalServerError) return } - writeResponse(&jsn, w) + writeResponse(&response, w) } func writeResponse(data *[]byte, w http.ResponseWriter) { @@ -310,22 +296,111 @@ func withNodename(data *[]byte) (*[]byte, error) { return result, nil } -func passToPod(pod *corev1.Pod, port, query string, results chan []byte, errors chan error, wg *sync.WaitGroup) { +func passRequest(addr, port, query string, results chan []byte, errors chan error, wg *sync.WaitGroup) { defer wg.Done() - url := fmt.Sprintf("http://%s:%s%s", pod.Status.PodIP, port, query) + url := fmt.Sprintf("http://%s:%s%s", addr, port, query) resp, err := http.Get(url) //nolint if err != nil { - errors <- fmt.Errorf("error getting data from %s: %w", pod.Status.PodIP, err) + errors <- fmt.Errorf("error getting data from %s: %w", addr, err) return } data, err := io.ReadAll(resp.Body) defer resp.Body.Close() if err != nil { - errors <- fmt.Errorf("error reading response from %s: %w", pod.Status.PodIP, err) + errors <- fmt.Errorf("error reading response from %s: %w", addr, err) return } results <- data } + +func (e *Endpoint) getEndpointslices(ctx context.Context, svc *corev1.Service) (*discoveryv1.EndpointSlice, error) { + endpoints := &discoveryv1.EndpointSliceList{} + if err := e.c.List(ctx, endpoints); err != nil { + return nil, fmt.Errorf("error listing endpointslices: %w", err) + } + + for i := range endpoints.Items { + for _, or := range endpoints.Items[i].OwnerReferences { + if or.UID == svc.UID { + return &endpoints.Items[i], nil + } + } + } + + return nil, nil +} + +func (e *Endpoint) getEndpoints(ctx context.Context, svc *corev1.Service) (*corev1.Endpoints, error) { + endpoints := &corev1.Endpoints{} + if err := e.c.Get(ctx, client.ObjectKeyFromObject(svc), endpoints); err != nil { + return nil, fmt.Errorf("error getting endpoints: %w", err) + } + return endpoints, nil +} + +func (e *Endpoint) getAddresses(ctx context.Context, svc *corev1.Service) ([]string, error) { + es, err := e.getEndpointslices(ctx, svc) + addresses := []string{} + if err != nil { + return nil, fmt.Errorf("error getting endpointslices: %w", err) + } + if es != nil { + for _, ep := range es.Endpoints { + addresses = append(addresses, ep.Addresses...) + } + return addresses, nil + } + ep, err := e.getEndpoints(ctx, svc) + if err != nil { + return nil, fmt.Errorf("error getting endpoints: %w", err) + } + for _, s := range ep.Subsets { + for _, a := range s.Addresses { + addresses = append(addresses, a.IP) + } + } + return addresses, nil +} + +func queryEndpoints(r *http.Request, addr []string) ([]byte, error) { + query := strings.ReplaceAll(r.URL.String(), "all/", "") + + s := strings.Split(r.Host, ":") + port := "" + if len(s) > 1 { + port = s[1] + } + + responses := []json.RawMessage{} + + var wg sync.WaitGroup + results := make(chan []byte, len(addr)) + errors := make(chan error, len(addr)) + + for i := range addr { + wg.Add(1) + go passRequest(addr[i], port, query, results, errors, &wg) + } + + wg.Wait() + close(results) + close(errors) + + for err := range errors { + return nil, fmt.Errorf("error occurred: %w", err) + } + + for result := range results { + responses = append(responses, json.RawMessage(result)) + } + + jsn, err := json.MarshalIndent(responses, "", "\t") + if err != nil { + return nil, fmt.Errorf("error marshalling data: %w", err) + } + + return jsn, nil +} diff --git a/pkg/monitoring/endpoint_test.go b/pkg/monitoring/endpoint_test.go index a03e0704..e56fc4e8 100644 --- a/pkg/monitoring/endpoint_test.go +++ b/pkg/monitoring/endpoint_test.go @@ -200,16 +200,22 @@ var _ = Describe("Endpoint", func() { }) }) Context("PassRequest() should", func() { - It("return error if there are no instances to query", func() { + It("return error if service name is not provided", func() { req := httptest.NewRequest(http.MethodGet, "/all/show/route", http.NoBody) res := httptest.NewRecorder() e.PassRequest(res, req) + Expect(res.Code).To(Equal(http.StatusBadRequest)) + }) + It("return error if there are no instances to query", func() { + req := httptest.NewRequest(http.MethodGet, "/all/show/route?service=test", http.NoBody) + res := httptest.NewRecorder() + e.PassRequest(res, req) Expect(res.Code).To(Equal(http.StatusInternalServerError)) }) It("return error if cannot get data from target pod", func() { c := fake.NewClientBuilder().WithRuntimeObjects(fakePods).Build() e := NewEndpoint(c, fcm) - req := httptest.NewRequest(http.MethodGet, "/all/show/route", http.NoBody) + req := httptest.NewRequest(http.MethodGet, "/all/show/route?service=test", http.NoBody) res := httptest.NewRecorder() e.PassRequest(res, req) Expect(res.Code).To(Equal(http.StatusInternalServerError)) From 47e7e9ba3241e46e017e029248baf4f8da90daac Mon Sep 17 00:00:00 2001 From: Patryk Strusiewicz-Surmacki Date: Wed, 13 Mar 2024 11:14:32 +0100 Subject: [PATCH 15/35] Fixed typo Signed-off-by: Patryk Strusiewicz-Surmacki --- pkg/monitoring/endpoint.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/monitoring/endpoint.go b/pkg/monitoring/endpoint.go index 6c9c3704..bf4b7e3c 100644 --- a/pkg/monitoring/endpoint.go +++ b/pkg/monitoring/endpoint.go @@ -399,7 +399,7 @@ func queryEndpoints(r *http.Request, addr []string) ([]byte, error) { jsn, err := json.MarshalIndent(responses, "", "\t") if err != nil { - return nil, fmt.Errorf("error marshalling data: %w", err) + return nil, fmt.Errorf("error marshaling data: %w", err) } return jsn, nil From f686f936a1daa4316306da2b849efb44cc62ff0c Mon Sep 17 00:00:00 2001 From: Patryk Strusiewicz-Surmacki Date: Wed, 13 Mar 2024 12:54:18 +0100 Subject: [PATCH 16/35] Updated unit tests Signed-off-by: Patryk Strusiewicz-Surmacki --- pkg/monitoring/endpoint.go | 2 +- pkg/monitoring/endpoint_test.go | 223 +++++++++++++++++++++++++++++--- 2 files changed, 209 insertions(+), 16 deletions(-) diff --git a/pkg/monitoring/endpoint.go b/pkg/monitoring/endpoint.go index bf4b7e3c..72394bf7 100644 --- a/pkg/monitoring/endpoint.go +++ b/pkg/monitoring/endpoint.go @@ -289,7 +289,7 @@ func withNodename(data *[]byte) (*[]byte, error) { var err error *result, err = json.MarshalIndent(res, "", "\t") if err != nil { - return nil, fmt.Errorf("error marshalling data: %w", err) + return nil, fmt.Errorf("error marshaling data: %w", err) } } diff --git a/pkg/monitoring/endpoint_test.go b/pkg/monitoring/endpoint_test.go index e56fc4e8..e4b72934 100644 --- a/pkg/monitoring/endpoint_test.go +++ b/pkg/monitoring/endpoint_test.go @@ -2,15 +2,19 @@ package monitoring import ( "encoding/json" + "io" "net/http" "net/http/httptest" + "os" "testing" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" + "github.com/telekom/das-schiff-network-operator/pkg/healthcheck" monmock "github.com/telekom/das-schiff-network-operator/pkg/monitoring/mock" "go.uber.org/mock/gomock" corev1 "k8s.io/api/core/v1" + discoveryv1 "k8s.io/api/discovery/v1" "sigs.k8s.io/controller-runtime/pkg/client/fake" ) @@ -61,15 +65,150 @@ var ( } ] }` - fakePods *corev1.PodList - tmpPath string - mockCtrl *gomock.Controller + + fakeServicesJSON = `{ + "items": [ + { + "apiVersion": "v1", + "kind": "Service", + "metadata": { + "name": "test-service", + "namespace": "test-namespace", + "uid": "ca97f774-7b91-47fd-a333-5fa7ee87f940" + } + } + ] + }` + + fakeEmptyEndpointSlicesJSON = `{ + "items": [ + { + "addressType": "IPv4", + "apiVersion": "discovery.k8s.io/v1", + "endpoints": [], + "kind": "EndpointSlice", + "metadata": { + "generateName": "test-service-", + "generation": 9, + "labels": { + "endpointslice.kubernetes.io/managed-by": "endpointslice-controller.k8s.io", + "kubernetes.io/service-name": "test-service" + }, + "name": "test-service-fvbrx", + "namespace": "kube-system", + "ownerReferences": [ + { + "apiVersion": "v1", + "blockOwnerDeletion": true, + "controller": true, + "kind": "Service", + "name": "test-service", + "uid": "ca97f774-7b91-47fd-a333-5fa7ee87f940" + } + ], + "resourceVersion": "53518", + "uid": "1f358e69-aefa-4181-b9fb-3e218dac09d5" + } + } + ] + }` + + fakeEndpointSlicesJSON = `{ + "items": [ + { + "addressType": "IPv4", + "apiVersion": "discovery.k8s.io/v1", + "endpoints": [ + { + "addresses": [ + "127.0.0.1" + ] + } + ], + "kind": "EndpointSlice", + "metadata": { + "generateName": "test-service-", + "generation": 9, + "labels": { + "endpointslice.kubernetes.io/managed-by": "endpointslice-controller.k8s.io", + "kubernetes.io/service-name": "test-service" + }, + "name": "test-service-fvbrx", + "namespace": "test-namespace", + "ownerReferences": [ + { + "apiVersion": "v1", + "blockOwnerDeletion": true, + "controller": true, + "kind": "Service", + "name": "test-service", + "uid": "ca97f774-7b91-47fd-a333-5fa7ee87f940" + } + ], + "resourceVersion": "53518", + "uid": "1f358e69-aefa-4181-b9fb-3e218dac09d5" + } + } + ] + }` + + fakeEmptyEndpointsJSON = `{ + "items": [ + { + "apiVersion": "v1", + "kind": "Endpoints", + "metadata": { + "name": "test-service", + "namespace": "test-namespace", + "uid": "c147ff3f-bb3f-4376-b6de-b5638c23968a" + }, + "subsets": [ + { + "addresses": [] + } + ] + } + ] + }` + + fakeEndpointsJSON = `{ + "items": [ + { + "apiVersion": "v1", + "kind": "Endpoints", + "metadata": { + "name": "test-service", + "namespace": "test-namespace", + "uid": "c147ff3f-bb3f-4376-b6de-b5638c23968a" + }, + "subsets": [ + { + "addresses": [ + { + "ip": "127.0.0.1" + } + ] + } + ] + } + ] + }` + + fakePods *corev1.PodList + fakeServices *corev1.ServiceList + fakeEndpointSlices *discoveryv1.EndpointSliceList + tmpPath string + mockCtrl *gomock.Controller ) var _ = BeforeSuite(func() { fakePods = &corev1.PodList{} err := json.Unmarshal([]byte(fakePodsJSON), fakePods) Expect(err).ShouldNot(HaveOccurred()) + fakeServices = &corev1.ServiceList{} + err = json.Unmarshal([]byte(fakeServicesJSON), fakeServices) + Expect(err).ShouldNot(HaveOccurred()) + }) func TestHealthCheck(t *testing.T) { @@ -91,7 +230,7 @@ var _ = Describe("Endpoint", func() { It("return no error", func() { req := httptest.NewRequest(http.MethodGet, "/show/route", http.NoBody) res := httptest.NewRecorder() - fcm.EXPECT().ExecuteWithJSON(gomock.Any()).Return([]byte{}) + fcm.EXPECT().ExecuteWithJSON(gomock.Any()).Return([]byte{'{', '}'}) e.ShowRoute(res, req) Expect(res.Code).To(Equal(http.StatusOK)) }) @@ -116,24 +255,41 @@ var _ = Describe("Endpoint", func() { It("return no error", func() { req := httptest.NewRequest(http.MethodGet, "/show/route?protocol=ipv6&input=192.168.1.1/32&longer_prefixes=true", http.NoBody) res := httptest.NewRecorder() - fcm.EXPECT().ExecuteWithJSON(gomock.Any()).Return([]byte{}) + fcm.EXPECT().ExecuteWithJSON(gomock.Any()).Return([]byte{'{', '}'}) e.ShowRoute(res, req) Expect(res.Code).To(Equal(http.StatusOK)) }) + It("return no error and add node name to the respone if "+healthcheck.NodenameEnv+" env is set", func() { + testNodename := "test-nodename" + err := os.Setenv(healthcheck.NodenameEnv, testNodename) + Expect(err).ToNot(HaveOccurred()) + defer os.Unsetenv(healthcheck.NodenameEnv) + req := httptest.NewRequest(http.MethodGet, "/show/route?protocol=ipv6&input=192.168.1.1/32&longer_prefixes=true", http.NoBody) + resp := httptest.NewRecorder() + fcm.EXPECT().ExecuteWithJSON(gomock.Any()).Return([]byte{'{', '}'}) + e.ShowRoute(resp, req) + Expect(resp.Code).To(Equal(http.StatusOK)) + data, err := io.ReadAll(resp.Body) + Expect(err).ToNot(HaveOccurred()) + m := map[string]json.RawMessage{} + json.Unmarshal(data, &m) + _, exists := m[testNodename] + Expect(exists).To(BeTrue()) + }) }) Context("ShowBGP() should", func() { It("return no error if type is not specified (default)", func() { req := httptest.NewRequest(http.MethodGet, "/show/bgp", http.NoBody) res := httptest.NewRecorder() - fcm.EXPECT().ExecuteWithJSON(gomock.Any()).Return([]byte{}) + fcm.EXPECT().ExecuteWithJSON(gomock.Any()).Return([]byte{'{', '}'}) e.ShowBGP(res, req) Expect(res.Code).To(Equal(http.StatusOK)) }) It("return no error if type is summary", func() { req := httptest.NewRequest(http.MethodGet, "/show/bgp?type=summary", http.NoBody) res := httptest.NewRecorder() - fcm.EXPECT().ExecuteWithJSON(gomock.Any()).Return([]byte{}) + fcm.EXPECT().ExecuteWithJSON(gomock.Any()).Return([]byte{'{', '}'}) e.ShowBGP(res, req) Expect(res.Code).To(Equal(http.StatusOK)) }) @@ -167,28 +323,28 @@ var _ = Describe("Endpoint", func() { It("return no error if type is not specified (default)", func() { req := httptest.NewRequest(http.MethodGet, "/show/evpn", http.NoBody) res := httptest.NewRecorder() - fcm.EXPECT().ExecuteWithJSON(gomock.Any()).Return([]byte{}) + fcm.EXPECT().ExecuteWithJSON(gomock.Any()).Return([]byte{'{', '}'}) e.ShowEVPN(res, req) Expect(res.Code).To(Equal(http.StatusOK)) }) It("return no error if type is rmac", func() { req := httptest.NewRequest(http.MethodGet, "/show/evpn?type=rmac", http.NoBody) res := httptest.NewRecorder() - fcm.EXPECT().ExecuteWithJSON(gomock.Any()).Return([]byte{}) + fcm.EXPECT().ExecuteWithJSON(gomock.Any()).Return([]byte{'{', '}'}) e.ShowEVPN(res, req) Expect(res.Code).To(Equal(http.StatusOK)) }) It("return no error if type is mac", func() { req := httptest.NewRequest(http.MethodGet, "/show/evpn?type=mac", http.NoBody) res := httptest.NewRecorder() - fcm.EXPECT().ExecuteWithJSON(gomock.Any()).Return([]byte{}) + fcm.EXPECT().ExecuteWithJSON(gomock.Any()).Return([]byte{'{', '}'}) e.ShowEVPN(res, req) Expect(res.Code).To(Equal(http.StatusOK)) }) It("return no error if type is next-hops", func() { req := httptest.NewRequest(http.MethodGet, "/show/evpn?type=next-hops", http.NoBody) res := httptest.NewRecorder() - fcm.EXPECT().ExecuteWithJSON(gomock.Any()).Return([]byte{}) + fcm.EXPECT().ExecuteWithJSON(gomock.Any()).Return([]byte{'{', '}'}) e.ShowEVPN(res, req) Expect(res.Code).To(Equal(http.StatusOK)) }) @@ -206,16 +362,53 @@ var _ = Describe("Endpoint", func() { e.PassRequest(res, req) Expect(res.Code).To(Equal(http.StatusBadRequest)) }) + It("return error if namsepace was not provided and service not exist in kube-system namespace", func() { + c := fake.NewClientBuilder().WithRuntimeObjects(fakePods, fakeServices).Build() + e := NewEndpoint(c, fcm) + req := httptest.NewRequest(http.MethodGet, "/all/show/route?service=test-service", http.NoBody) + res := httptest.NewRecorder() + e.PassRequest(res, req) + Expect(res.Code).To(Equal(http.StatusInternalServerError)) + }) It("return error if there are no instances to query", func() { - req := httptest.NewRequest(http.MethodGet, "/all/show/route?service=test", http.NoBody) + fakeEndpointSlices = &discoveryv1.EndpointSliceList{} + err := json.Unmarshal([]byte(fakeEmptyEndpointSlicesJSON), fakeEndpointSlices) + Expect(err).ShouldNot(HaveOccurred()) + + c := fake.NewClientBuilder().WithRuntimeObjects(fakePods, fakeServices, fakeEndpointSlices).Build() + e := NewEndpoint(c, fcm) + req := httptest.NewRequest(http.MethodGet, "/all/show/route?service=test-service&namespace=test-namespace", http.NoBody) + res := httptest.NewRecorder() + e.PassRequest(res, req) + Expect(res.Code).To(Equal(http.StatusInternalServerError)) + }) + It("return error if both endpointslices and legacy endpoints are not available", func() { + c := fake.NewClientBuilder().WithRuntimeObjects(fakePods, fakeServices).Build() + e := NewEndpoint(c, fcm) + req := httptest.NewRequest(http.MethodGet, "/all/show/route?service=test-service&namespace=test-namespace", http.NoBody) + res := httptest.NewRecorder() + e.PassRequest(res, req) + Expect(res.Code).To(Equal(http.StatusInternalServerError)) + }) + It("return error if there are no instances to query and legacy endpoints are used", func() { + fakeEndpoints := &corev1.EndpointsList{} + err := json.Unmarshal([]byte(fakeEmptyEndpointsJSON), fakeEndpoints) + Expect(err).ShouldNot(HaveOccurred()) + + c := fake.NewClientBuilder().WithRuntimeObjects(fakePods, fakeServices, fakeEndpoints).Build() + e := NewEndpoint(c, fcm) + req := httptest.NewRequest(http.MethodGet, "/all/show/route?service=test-service&namespace=test-namespace", http.NoBody) res := httptest.NewRecorder() e.PassRequest(res, req) Expect(res.Code).To(Equal(http.StatusInternalServerError)) }) - It("return error if cannot get data from target pod", func() { - c := fake.NewClientBuilder().WithRuntimeObjects(fakePods).Build() + It("return error if cannot get data from the endpoint", func() { + fakeEndpointSlices = &discoveryv1.EndpointSliceList{} + err := json.Unmarshal([]byte(fakeEndpointSlicesJSON), fakeEndpointSlices) + Expect(err).ShouldNot(HaveOccurred()) + c := fake.NewClientBuilder().WithRuntimeObjects(fakePods, fakeServices, fakeEndpointSlices).Build() e := NewEndpoint(c, fcm) - req := httptest.NewRequest(http.MethodGet, "/all/show/route?service=test", http.NoBody) + req := httptest.NewRequest(http.MethodGet, "/all/show/route?service=test-service&namespace=test-namespace", http.NoBody) res := httptest.NewRecorder() e.PassRequest(res, req) Expect(res.Code).To(Equal(http.StatusInternalServerError)) From 58b5509b11df1f9c2bdef1e1536041ec101c87bc Mon Sep 17 00:00:00 2001 From: Patryk Strusiewicz-Surmacki Date: Wed, 13 Mar 2024 12:56:16 +0100 Subject: [PATCH 17/35] Fixed linter issues Signed-off-by: Patryk Strusiewicz-Surmacki --- pkg/monitoring/endpoint_test.go | 29 +++-------------------------- 1 file changed, 3 insertions(+), 26 deletions(-) diff --git a/pkg/monitoring/endpoint_test.go b/pkg/monitoring/endpoint_test.go index e4b72934..4bae6960 100644 --- a/pkg/monitoring/endpoint_test.go +++ b/pkg/monitoring/endpoint_test.go @@ -171,29 +171,6 @@ var ( ] }` - fakeEndpointsJSON = `{ - "items": [ - { - "apiVersion": "v1", - "kind": "Endpoints", - "metadata": { - "name": "test-service", - "namespace": "test-namespace", - "uid": "c147ff3f-bb3f-4376-b6de-b5638c23968a" - }, - "subsets": [ - { - "addresses": [ - { - "ip": "127.0.0.1" - } - ] - } - ] - } - ] - }` - fakePods *corev1.PodList fakeServices *corev1.ServiceList fakeEndpointSlices *discoveryv1.EndpointSliceList @@ -208,7 +185,6 @@ var _ = BeforeSuite(func() { fakeServices = &corev1.ServiceList{} err = json.Unmarshal([]byte(fakeServicesJSON), fakeServices) Expect(err).ShouldNot(HaveOccurred()) - }) func TestHealthCheck(t *testing.T) { @@ -259,7 +235,7 @@ var _ = Describe("Endpoint", func() { e.ShowRoute(res, req) Expect(res.Code).To(Equal(http.StatusOK)) }) - It("return no error and add node name to the respone if "+healthcheck.NodenameEnv+" env is set", func() { + It("return no error and add node name to the response if "+healthcheck.NodenameEnv+" env is set", func() { testNodename := "test-nodename" err := os.Setenv(healthcheck.NodenameEnv, testNodename) Expect(err).ToNot(HaveOccurred()) @@ -272,7 +248,8 @@ var _ = Describe("Endpoint", func() { data, err := io.ReadAll(resp.Body) Expect(err).ToNot(HaveOccurred()) m := map[string]json.RawMessage{} - json.Unmarshal(data, &m) + err = json.Unmarshal(data, &m) + Expect(err).ToNot(HaveOccurred()) _, exists := m[testNodename] Expect(exists).To(BeTrue()) }) From fdc0e00f63032c98c9ecc514ca2c91d6a5b4afcd Mon Sep 17 00:00:00 2001 From: Patryk Strusiewicz-Surmacki Date: Wed, 13 Mar 2024 16:15:05 +0100 Subject: [PATCH 18/35] Reworked request passing and added more unit tests Signed-off-by: Patryk Strusiewicz-Surmacki --- pkg/monitoring/endpoint.go | 27 +++++++++++++----------- pkg/monitoring/endpoint_test.go | 37 +++++++++++++++++++++++++++++++++ 2 files changed, 52 insertions(+), 12 deletions(-) diff --git a/pkg/monitoring/endpoint.go b/pkg/monitoring/endpoint.go index 72394bf7..dec4c144 100644 --- a/pkg/monitoring/endpoint.go +++ b/pkg/monitoring/endpoint.go @@ -33,8 +33,9 @@ type FRRClient interface { } type Endpoint struct { - cli FRRClient - c client.Client + cli FRRClient + c client.Client + httpC http.Client } // NewEndpoint creates new endpoint object. @@ -296,10 +297,18 @@ func withNodename(data *[]byte) (*[]byte, error) { return result, nil } -func passRequest(addr, port, query string, results chan []byte, errors chan error, wg *sync.WaitGroup) { +func passRequest(r *http.Request, addr, query string, results chan []byte, errors chan error, wg *sync.WaitGroup) { defer wg.Done() - url := fmt.Sprintf("http://%s:%s%s", addr, port, query) + s := strings.Split(r.Host, ":") + port := "" + if len(s) > 1 { + port = s[1] + } + + url := strings.Replace(query, r.Host, addr+":"+port, 1) + + // url := fmt.Sprintf("http://%s:%s%s", addr, port, query) resp, err := http.Get(url) //nolint if err != nil { errors <- fmt.Errorf("error getting data from %s: %w", addr, err) @@ -366,13 +375,7 @@ func (e *Endpoint) getAddresses(ctx context.Context, svc *corev1.Service) ([]str } func queryEndpoints(r *http.Request, addr []string) ([]byte, error) { - query := strings.ReplaceAll(r.URL.String(), "all/", "") - - s := strings.Split(r.Host, ":") - port := "" - if len(s) > 1 { - port = s[1] - } + query := strings.ReplaceAll(r.RequestURI, "all/", "") responses := []json.RawMessage{} @@ -382,7 +385,7 @@ func queryEndpoints(r *http.Request, addr []string) ([]byte, error) { for i := range addr { wg.Add(1) - go passRequest(addr[i], port, query, results, errors, &wg) + go passRequest(r, addr[i], query, results, errors, &wg) } wg.Wait() diff --git a/pkg/monitoring/endpoint_test.go b/pkg/monitoring/endpoint_test.go index 4bae6960..29c05388 100644 --- a/pkg/monitoring/endpoint_test.go +++ b/pkg/monitoring/endpoint_test.go @@ -2,6 +2,7 @@ package monitoring import ( "encoding/json" + "fmt" "io" "net/http" "net/http/httptest" @@ -390,5 +391,41 @@ var _ = Describe("Endpoint", func() { e.PassRequest(res, req) Expect(res.Code).To(Equal(http.StatusInternalServerError)) }) + It("return error if request was properly passed to the endpoint but the response is malformed", func() { + fakeEndpointSlices = &discoveryv1.EndpointSliceList{} + err := json.Unmarshal([]byte(fakeEndpointSlicesJSON), fakeEndpointSlices) + Expect(err).ShouldNot(HaveOccurred()) + + svr := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + fmt.Fprintf(w, "invalidJson") + })) + defer svr.Close() + + c := fake.NewClientBuilder().WithRuntimeObjects(fakePods, fakeServices, fakeEndpointSlices).Build() + e := NewEndpoint(c, fcm) + req := httptest.NewRequest(http.MethodGet, svr.URL+"?service=test-service&namespace=test-namespace", http.NoBody) + res := httptest.NewRecorder() + + e.PassRequest(res, req) + Expect(res.Code).To(Equal(http.StatusInternalServerError)) + }) + It("return no error if request was properly passed to the endpoint", func() { + fakeEndpointSlices = &discoveryv1.EndpointSliceList{} + err := json.Unmarshal([]byte(fakeEndpointSlicesJSON), fakeEndpointSlices) + Expect(err).ShouldNot(HaveOccurred()) + + svr := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + fmt.Fprintf(w, "{}") + })) + defer svr.Close() + + c := fake.NewClientBuilder().WithRuntimeObjects(fakePods, fakeServices, fakeEndpointSlices).Build() + e := NewEndpoint(c, fcm) + req := httptest.NewRequest(http.MethodGet, svr.URL+"?service=test-service&namespace=test-namespace", http.NoBody) + res := httptest.NewRecorder() + + e.PassRequest(res, req) + Expect(res.Code).To(Equal(http.StatusOK)) + }) }) }) From c5080b1d45a65826742a5e11c0ed44dd085e3558 Mon Sep 17 00:00:00 2001 From: Patryk Strusiewicz-Surmacki Date: Wed, 13 Mar 2024 16:15:31 +0100 Subject: [PATCH 19/35] Removed redundant line Signed-off-by: Patryk Strusiewicz-Surmacki --- pkg/monitoring/endpoint.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/pkg/monitoring/endpoint.go b/pkg/monitoring/endpoint.go index dec4c144..28075fa6 100644 --- a/pkg/monitoring/endpoint.go +++ b/pkg/monitoring/endpoint.go @@ -307,8 +307,6 @@ func passRequest(r *http.Request, addr, query string, results chan []byte, error } url := strings.Replace(query, r.Host, addr+":"+port, 1) - - // url := fmt.Sprintf("http://%s:%s%s", addr, port, query) resp, err := http.Get(url) //nolint if err != nil { errors <- fmt.Errorf("error getting data from %s: %w", addr, err) From b499702668683974c9098d691dc7c71314be88ed Mon Sep 17 00:00:00 2001 From: Patryk Strusiewicz-Surmacki Date: Wed, 13 Mar 2024 16:50:06 +0100 Subject: [PATCH 20/35] Fixed URL building Signed-off-by: Patryk Strusiewicz-Surmacki --- pkg/monitoring/endpoint.go | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/pkg/monitoring/endpoint.go b/pkg/monitoring/endpoint.go index 28075fa6..6f5f2fca 100644 --- a/pkg/monitoring/endpoint.go +++ b/pkg/monitoring/endpoint.go @@ -306,7 +306,12 @@ func passRequest(r *http.Request, addr, query string, results chan []byte, error port = s[1] } - url := strings.Replace(query, r.Host, addr+":"+port, 1) + protocol := "http" + if r.TLS != nil { + protocol = "https" + } + + url := fmt.Sprintf("%s://%s:%s%s", protocol, addr, port, query) resp, err := http.Get(url) //nolint if err != nil { errors <- fmt.Errorf("error getting data from %s: %w", addr, err) @@ -373,8 +378,7 @@ func (e *Endpoint) getAddresses(ctx context.Context, svc *corev1.Service) ([]str } func queryEndpoints(r *http.Request, addr []string) ([]byte, error) { - query := strings.ReplaceAll(r.RequestURI, "all/", "") - + query := strings.ReplaceAll(r.URL.RequestURI(), "all/", "") responses := []json.RawMessage{} var wg sync.WaitGroup From 595daa4f8b7e48c01838386852c4513bd28c40ee Mon Sep 17 00:00:00 2001 From: Patryk Strusiewicz-Surmacki Date: Wed, 13 Mar 2024 16:51:52 +0100 Subject: [PATCH 21/35] Removed redundant code Signed-off-by: Patryk Strusiewicz-Surmacki --- pkg/monitoring/endpoint.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/pkg/monitoring/endpoint.go b/pkg/monitoring/endpoint.go index 6f5f2fca..bdb69229 100644 --- a/pkg/monitoring/endpoint.go +++ b/pkg/monitoring/endpoint.go @@ -33,9 +33,8 @@ type FRRClient interface { } type Endpoint struct { - cli FRRClient - c client.Client - httpC http.Client + cli FRRClient + c client.Client } // NewEndpoint creates new endpoint object. From 6861d815234b01952ef3b11c20a8d2c656bee8cf Mon Sep 17 00:00:00 2001 From: Patryk Strusiewicz-Surmacki Date: Wed, 13 Mar 2024 16:55:27 +0100 Subject: [PATCH 22/35] Fixed linter issues Signed-off-by: Patryk Strusiewicz-Surmacki --- pkg/monitoring/endpoint_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/monitoring/endpoint_test.go b/pkg/monitoring/endpoint_test.go index 29c05388..dc191053 100644 --- a/pkg/monitoring/endpoint_test.go +++ b/pkg/monitoring/endpoint_test.go @@ -396,7 +396,7 @@ var _ = Describe("Endpoint", func() { err := json.Unmarshal([]byte(fakeEndpointSlicesJSON), fakeEndpointSlices) Expect(err).ShouldNot(HaveOccurred()) - svr := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + svr := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { fmt.Fprintf(w, "invalidJson") })) defer svr.Close() @@ -414,7 +414,7 @@ var _ = Describe("Endpoint", func() { err := json.Unmarshal([]byte(fakeEndpointSlicesJSON), fakeEndpointSlices) Expect(err).ShouldNot(HaveOccurred()) - svr := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + svr := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { fmt.Fprintf(w, "{}") })) defer svr.Close() From dfe005114a812ce7782994a96713acb7153315e2 Mon Sep 17 00:00:00 2001 From: Patryk Strusiewicz-Surmacki Date: Fri, 15 Mar 2024 11:26:34 +0100 Subject: [PATCH 23/35] Reworked pod discovery Signed-off-by: Patryk Strusiewicz-Surmacki --- config/rbac/role.yaml | 16 +--- pkg/monitoring/endpoint.go | 58 +++-------- pkg/monitoring/endpoint_test.go | 165 +++++--------------------------- 3 files changed, 44 insertions(+), 195 deletions(-) diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml index f94f6fe3..b58d5a05 100644 --- a/config/rbac/role.yaml +++ b/config/rbac/role.yaml @@ -4,12 +4,6 @@ kind: ClusterRole metadata: name: manager-role rules: -- apiGroups: - - "" - resources: - - endpoints - verbs: - - get - apiGroups: - "" resources: @@ -22,15 +16,15 @@ rules: - apiGroups: - "" resources: - - services + - pods verbs: - - get + - list - apiGroups: - - discovery.k8s.io + - "" resources: - - endpointslices + - services verbs: - - list + - get - apiGroups: - network.schiff.telekom.de resources: diff --git a/pkg/monitoring/endpoint.go b/pkg/monitoring/endpoint.go index bdb69229..d4190175 100644 --- a/pkg/monitoring/endpoint.go +++ b/pkg/monitoring/endpoint.go @@ -14,7 +14,7 @@ import ( "github.com/telekom/das-schiff-network-operator/pkg/healthcheck" corev1 "k8s.io/api/core/v1" - discoveryv1 "k8s.io/api/discovery/v1" + "k8s.io/apimachinery/pkg/labels" "sigs.k8s.io/controller-runtime/pkg/client" ) @@ -202,9 +202,8 @@ func (e *Endpoint) ShowEVPN(w http.ResponseWriter, r *http.Request) { writeResponse(result, w) } +//+kubebuilder:rbac:groups=core,resources=pods,verbs=list //+kubebuilder:rbac:groups=core,resources=services,verbs=get -//+kubebuilder:rbac:groups=core,resources=endpoints,verbs=get -//+kubebuilder:rbac:groups=discovery.k8s.io,resources=endpointslices,verbs=list // PassRequest - when called, will pass the request to all nodes and return their responses. func (e *Endpoint) PassRequest(w http.ResponseWriter, r *http.Request) { @@ -327,52 +326,21 @@ func passRequest(r *http.Request, addr, query string, results chan []byte, error results <- data } -func (e *Endpoint) getEndpointslices(ctx context.Context, svc *corev1.Service) (*discoveryv1.EndpointSlice, error) { - endpoints := &discoveryv1.EndpointSliceList{} - if err := e.c.List(ctx, endpoints); err != nil { - return nil, fmt.Errorf("error listing endpointslices: %w", err) - } - - for i := range endpoints.Items { - for _, or := range endpoints.Items[i].OwnerReferences { - if or.UID == svc.UID { - return &endpoints.Items[i], nil - } - } - } - - return nil, nil -} - -func (e *Endpoint) getEndpoints(ctx context.Context, svc *corev1.Service) (*corev1.Endpoints, error) { - endpoints := &corev1.Endpoints{} - if err := e.c.Get(ctx, client.ObjectKeyFromObject(svc), endpoints); err != nil { - return nil, fmt.Errorf("error getting endpoints: %w", err) +func (e *Endpoint) getAddresses(ctx context.Context, svc *corev1.Service) ([]string, error) { + var serviceLabels labels.Set = svc.Spec.Selector + pods := &corev1.PodList{} + if err := e.c.List(ctx, pods, &client.ListOptions{ + LabelSelector: serviceLabels.AsSelector(), + Namespace: svc.Namespace, + }); err != nil { + return nil, fmt.Errorf("error getting pods: %w", err) } - return endpoints, nil -} -func (e *Endpoint) getAddresses(ctx context.Context, svc *corev1.Service) ([]string, error) { - es, err := e.getEndpointslices(ctx, svc) addresses := []string{} - if err != nil { - return nil, fmt.Errorf("error getting endpointslices: %w", err) - } - if es != nil { - for _, ep := range es.Endpoints { - addresses = append(addresses, ep.Addresses...) - } - return addresses, nil - } - ep, err := e.getEndpoints(ctx, svc) - if err != nil { - return nil, fmt.Errorf("error getting endpoints: %w", err) - } - for _, s := range ep.Subsets { - for _, a := range s.Addresses { - addresses = append(addresses, a.IP) - } + for _, pod := range pods.Items { + addresses = append(addresses, pod.Status.PodIP) } + return addresses, nil } diff --git a/pkg/monitoring/endpoint_test.go b/pkg/monitoring/endpoint_test.go index dc191053..9ab409a1 100644 --- a/pkg/monitoring/endpoint_test.go +++ b/pkg/monitoring/endpoint_test.go @@ -15,7 +15,6 @@ import ( monmock "github.com/telekom/das-schiff-network-operator/pkg/monitoring/mock" "go.uber.org/mock/gomock" corev1 "k8s.io/api/core/v1" - discoveryv1 "k8s.io/api/discovery/v1" "sigs.k8s.io/controller-runtime/pkg/client/fake" ) @@ -31,14 +30,14 @@ var ( "app.kubernetes.io/name": "network-operator" }, "name": "network-operator-worker-1", - "namespace": "kube-system" + "namespace": "test-namespace" }, "status": { - "hostIP": "172.18.0.3", - "podIP": "172.18.0.3", + "hostIP": "127.0.0.1", + "podIP": "127.0.0.1", "podIPs": [ { - "ip": "172.18.0.3" + "ip": "127.0.0.1" } ] } @@ -52,14 +51,14 @@ var ( "app.kubernetes.io/name": "network-operator" }, "name": "network-operator-worker-2", - "namespace": "kube-system" + "namespace": "test-namespace" }, "status": { - "hostIP": "172.18.0.4", - "podIP": "172.18.0.4", + "hostIP": "127.0.0.1", + "podIP": "127.0.0.1", "podIPs": [ { - "ip": "172.18.0.4" + "ip": "127.0.0.1" } ] } @@ -77,106 +76,29 @@ var ( "namespace": "test-namespace", "uid": "ca97f774-7b91-47fd-a333-5fa7ee87f940" } - } - ] - }` - - fakeEmptyEndpointSlicesJSON = `{ - "items": [ - { - "addressType": "IPv4", - "apiVersion": "discovery.k8s.io/v1", - "endpoints": [], - "kind": "EndpointSlice", - "metadata": { - "generateName": "test-service-", - "generation": 9, - "labels": { - "endpointslice.kubernetes.io/managed-by": "endpointslice-controller.k8s.io", - "kubernetes.io/service-name": "test-service" - }, - "name": "test-service-fvbrx", - "namespace": "kube-system", - "ownerReferences": [ - { - "apiVersion": "v1", - "blockOwnerDeletion": true, - "controller": true, - "kind": "Service", - "name": "test-service", - "uid": "ca97f774-7b91-47fd-a333-5fa7ee87f940" - } - ], - "resourceVersion": "53518", - "uid": "1f358e69-aefa-4181-b9fb-3e218dac09d5" - } - } - ] - }` - - fakeEndpointSlicesJSON = `{ - "items": [ - { - "addressType": "IPv4", - "apiVersion": "discovery.k8s.io/v1", - "endpoints": [ - { - "addresses": [ - "127.0.0.1" - ] - } - ], - "kind": "EndpointSlice", - "metadata": { - "generateName": "test-service-", - "generation": 9, - "labels": { - "endpointslice.kubernetes.io/managed-by": "endpointslice-controller.k8s.io", - "kubernetes.io/service-name": "test-service" - }, - "name": "test-service-fvbrx", - "namespace": "test-namespace", - "ownerReferences": [ - { - "apiVersion": "v1", - "blockOwnerDeletion": true, - "controller": true, - "kind": "Service", - "name": "test-service", - "uid": "ca97f774-7b91-47fd-a333-5fa7ee87f940" - } - ], - "resourceVersion": "53518", - "uid": "1f358e69-aefa-4181-b9fb-3e218dac09d5" - } - } - ] - }` - - fakeEmptyEndpointsJSON = `{ - "items": [ + + }, { "apiVersion": "v1", - "kind": "Endpoints", + "kind": "Service", "metadata": { - "name": "test-service", + "name": "test-service-no-endpoints", "namespace": "test-namespace", - "uid": "c147ff3f-bb3f-4376-b6de-b5638c23968a" + "uid": "ca97f774-7b91-47fd-a333-5fa7ee87f941" }, - "subsets": [ - { - "addresses": [] + "spec": { + "selector": { + "app.kubernetes.io/component": "bad-selector", + "app.kubernetes.io/name": "bad-selector" } - ] + } } ] }` - fakePods *corev1.PodList - fakeServices *corev1.ServiceList - fakeEndpointSlices *discoveryv1.EndpointSliceList - tmpPath string - mockCtrl *gomock.Controller + fakePods *corev1.PodList + fakeServices *corev1.ServiceList + mockCtrl *gomock.Controller ) var _ = BeforeSuite(func() { @@ -190,7 +112,6 @@ var _ = BeforeSuite(func() { func TestHealthCheck(t *testing.T) { RegisterFailHandler(Fail) - tmpPath = t.TempDir() mockCtrl = gomock.NewController(t) defer mockCtrl.Finish() RunSpecs(t, @@ -340,7 +261,7 @@ var _ = Describe("Endpoint", func() { e.PassRequest(res, req) Expect(res.Code).To(Equal(http.StatusBadRequest)) }) - It("return error if namsepace was not provided and service not exist in kube-system namespace", func() { + It("return error if namespace was not provided and service not exist in kube-system namespace", func() { c := fake.NewClientBuilder().WithRuntimeObjects(fakePods, fakeServices).Build() e := NewEndpoint(c, fcm) req := httptest.NewRequest(http.MethodGet, "/all/show/route?service=test-service", http.NoBody) @@ -349,42 +270,16 @@ var _ = Describe("Endpoint", func() { Expect(res.Code).To(Equal(http.StatusInternalServerError)) }) It("return error if there are no instances to query", func() { - fakeEndpointSlices = &discoveryv1.EndpointSliceList{} - err := json.Unmarshal([]byte(fakeEmptyEndpointSlicesJSON), fakeEndpointSlices) - Expect(err).ShouldNot(HaveOccurred()) - - c := fake.NewClientBuilder().WithRuntimeObjects(fakePods, fakeServices, fakeEndpointSlices).Build() - e := NewEndpoint(c, fcm) - req := httptest.NewRequest(http.MethodGet, "/all/show/route?service=test-service&namespace=test-namespace", http.NoBody) - res := httptest.NewRecorder() - e.PassRequest(res, req) - Expect(res.Code).To(Equal(http.StatusInternalServerError)) - }) - It("return error if both endpointslices and legacy endpoints are not available", func() { c := fake.NewClientBuilder().WithRuntimeObjects(fakePods, fakeServices).Build() e := NewEndpoint(c, fcm) - req := httptest.NewRequest(http.MethodGet, "/all/show/route?service=test-service&namespace=test-namespace", http.NoBody) + req := httptest.NewRequest(http.MethodGet, "/all/show/route?service=test-service-no-endpoints&namespace=test-namespace", http.NoBody) res := httptest.NewRecorder() e.PassRequest(res, req) Expect(res.Code).To(Equal(http.StatusInternalServerError)) }) - It("return error if there are no instances to query and legacy endpoints are used", func() { - fakeEndpoints := &corev1.EndpointsList{} - err := json.Unmarshal([]byte(fakeEmptyEndpointsJSON), fakeEndpoints) - Expect(err).ShouldNot(HaveOccurred()) - c := fake.NewClientBuilder().WithRuntimeObjects(fakePods, fakeServices, fakeEndpoints).Build() - e := NewEndpoint(c, fcm) - req := httptest.NewRequest(http.MethodGet, "/all/show/route?service=test-service&namespace=test-namespace", http.NoBody) - res := httptest.NewRecorder() - e.PassRequest(res, req) - Expect(res.Code).To(Equal(http.StatusInternalServerError)) - }) It("return error if cannot get data from the endpoint", func() { - fakeEndpointSlices = &discoveryv1.EndpointSliceList{} - err := json.Unmarshal([]byte(fakeEndpointSlicesJSON), fakeEndpointSlices) - Expect(err).ShouldNot(HaveOccurred()) - c := fake.NewClientBuilder().WithRuntimeObjects(fakePods, fakeServices, fakeEndpointSlices).Build() + c := fake.NewClientBuilder().WithRuntimeObjects(fakePods, fakeServices).Build() e := NewEndpoint(c, fcm) req := httptest.NewRequest(http.MethodGet, "/all/show/route?service=test-service&namespace=test-namespace", http.NoBody) res := httptest.NewRecorder() @@ -392,16 +287,12 @@ var _ = Describe("Endpoint", func() { Expect(res.Code).To(Equal(http.StatusInternalServerError)) }) It("return error if request was properly passed to the endpoint but the response is malformed", func() { - fakeEndpointSlices = &discoveryv1.EndpointSliceList{} - err := json.Unmarshal([]byte(fakeEndpointSlicesJSON), fakeEndpointSlices) - Expect(err).ShouldNot(HaveOccurred()) - svr := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { fmt.Fprintf(w, "invalidJson") })) defer svr.Close() - c := fake.NewClientBuilder().WithRuntimeObjects(fakePods, fakeServices, fakeEndpointSlices).Build() + c := fake.NewClientBuilder().WithRuntimeObjects(fakePods, fakeServices).Build() e := NewEndpoint(c, fcm) req := httptest.NewRequest(http.MethodGet, svr.URL+"?service=test-service&namespace=test-namespace", http.NoBody) res := httptest.NewRecorder() @@ -410,16 +301,12 @@ var _ = Describe("Endpoint", func() { Expect(res.Code).To(Equal(http.StatusInternalServerError)) }) It("return no error if request was properly passed to the endpoint", func() { - fakeEndpointSlices = &discoveryv1.EndpointSliceList{} - err := json.Unmarshal([]byte(fakeEndpointSlicesJSON), fakeEndpointSlices) - Expect(err).ShouldNot(HaveOccurred()) - svr := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { fmt.Fprintf(w, "{}") })) defer svr.Close() - c := fake.NewClientBuilder().WithRuntimeObjects(fakePods, fakeServices, fakeEndpointSlices).Build() + c := fake.NewClientBuilder().WithRuntimeObjects(fakePods, fakeServices).Build() e := NewEndpoint(c, fcm) req := httptest.NewRequest(http.MethodGet, svr.URL+"?service=test-service&namespace=test-namespace", http.NoBody) res := httptest.NewRecorder() From cb9be8c5c5e862b9029bfd5931ed91143fa88f69 Mon Sep 17 00:00:00 2001 From: Patryk Strusiewicz-Surmacki Date: Fri, 15 Mar 2024 12:31:28 +0100 Subject: [PATCH 24/35] Status service and namespace will be now configured using env variables Signed-off-by: Patryk Strusiewicz-Surmacki --- cmd/frr-exporter/main.go | 13 ++++++++++++- config/manager/manager.yaml | 6 ++++++ config/manager/manager_master.yaml | 6 ++++++ config/manager/service.yaml | 15 +++++++++++++++ pkg/monitoring/endpoint.go | 23 +++++++++-------------- pkg/monitoring/endpoint_test.go | 30 ++++++++---------------------- 6 files changed, 56 insertions(+), 37 deletions(-) create mode 100644 config/manager/service.yaml diff --git a/cmd/frr-exporter/main.go b/cmd/frr-exporter/main.go index 99ba9f41..010099a2 100644 --- a/cmd/frr-exporter/main.go +++ b/cmd/frr-exporter/main.go @@ -5,6 +5,7 @@ import ( "fmt" "log" "net/http" + "os" "time" "github.com/prometheus/client_golang/prometheus" @@ -56,7 +57,17 @@ func main() { frrCli := frr.NewCli() - endpoint := monitoring.NewEndpoint(c, frrCli) + svcName := os.Getenv(monitoring.StatusSvcNameEnv) + if svcName == "" { + log.Fatalf("environment variable %s is not set", monitoring.StatusSvcNameEnv) + } + + svcNamespace := os.Getenv(monitoring.StatusSvcNamespaceEnv) + if svcNamespace == "" { + log.Fatalf("environment variable %s is not set", monitoring.StatusSvcNamespaceEnv) + } + + endpoint := monitoring.NewEndpoint(c, frrCli, svcName, svcNamespace) endpoint.SetHandlers() // Expose the registered metrics via HTTP. diff --git a/config/manager/manager.yaml b/config/manager/manager.yaml index df26ce72..16497def 100644 --- a/config/manager/manager.yaml +++ b/config/manager/manager.yaml @@ -85,6 +85,12 @@ spec: valueFrom: fieldRef: fieldPath: spec.nodeName + - name: STATUS_SVC_NAME + value: network-operator-status + - name: STATUS_SVC_NAMESPACE + valueFrom: + fieldRef: + fieldPath: metadata.namespace image: frr-exporter:latest name: frr-exporter securityContext: diff --git a/config/manager/manager_master.yaml b/config/manager/manager_master.yaml index 58c08c99..cd3a4917 100644 --- a/config/manager/manager_master.yaml +++ b/config/manager/manager_master.yaml @@ -87,6 +87,12 @@ spec: valueFrom: fieldRef: fieldPath: spec.nodeName + - name: STATUS_SVC_NAME + value: network-operator-status + - name: STATUS_SVC_NAMESPACE + valueFrom: + fieldRef: + fieldPath: metadata.namespace image: frr-exporter:latest name: frr-exporter securityContext: diff --git a/config/manager/service.yaml b/config/manager/service.yaml new file mode 100644 index 00000000..849c3a51 --- /dev/null +++ b/config/manager/service.yaml @@ -0,0 +1,15 @@ +apiVersion: v1 +kind: Service +metadata: + name: status + namespace: system +spec: + type: NodePort + selector: + app.kubernetes.io/component: worker + app.kubernetes.io/name: network-operator + ports: + - protocol: TCP + port: 7082 + targetPort: 7082 + nodePort: 30082 \ No newline at end of file diff --git a/pkg/monitoring/endpoint.go b/pkg/monitoring/endpoint.go index d4190175..e5767fa1 100644 --- a/pkg/monitoring/endpoint.go +++ b/pkg/monitoring/endpoint.go @@ -25,6 +25,9 @@ const ( protocolIPv6 = "ipv6" defaultNamespace = "kube-system" + + StatusSvcNameEnv = "STATUS_SVC_NAME" + StatusSvcNamespaceEnv = "STATUS_SVC_NAMESPACE" ) //go:generate mockgen -destination ./mock/mock_endpoint.go . FRRClient @@ -35,11 +38,14 @@ type FRRClient interface { type Endpoint struct { cli FRRClient c client.Client + + statusSvcName string + statusSvcNamespace string } // NewEndpoint creates new endpoint object. -func NewEndpoint(k8sClient client.Client, frrcli FRRClient) *Endpoint { - return &Endpoint{cli: frrcli, c: k8sClient} +func NewEndpoint(k8sClient client.Client, frrcli FRRClient, svcName, svcNamespace string) *Endpoint { + return &Endpoint{cli: frrcli, c: k8sClient, statusSvcName: svcName, statusSvcNamespace: svcNamespace} } // SetHandlers configures HTTP handlers. @@ -207,19 +213,8 @@ func (e *Endpoint) ShowEVPN(w http.ResponseWriter, r *http.Request) { // PassRequest - when called, will pass the request to all nodes and return their responses. func (e *Endpoint) PassRequest(w http.ResponseWriter, r *http.Request) { - serviceName := r.URL.Query().Get("service") - if serviceName == "" { - http.Error(w, "error looking for service: service name not provided", http.StatusBadRequest) - return - } - - namespace := r.URL.Query().Get("namespace") - if namespace == "" { - namespace = defaultNamespace - } - service := &corev1.Service{} - err := e.c.Get(r.Context(), client.ObjectKey{Name: serviceName, Namespace: namespace}, service) + err := e.c.Get(r.Context(), client.ObjectKey{Name: e.statusSvcName, Namespace: e.statusSvcNamespace}, service) if err != nil { http.Error(w, fmt.Sprintf("error listing pods: %s", err.Error()), http.StatusInternalServerError) return diff --git a/pkg/monitoring/endpoint_test.go b/pkg/monitoring/endpoint_test.go index 9ab409a1..2d698845 100644 --- a/pkg/monitoring/endpoint_test.go +++ b/pkg/monitoring/endpoint_test.go @@ -121,7 +121,7 @@ func TestHealthCheck(t *testing.T) { var _ = Describe("Endpoint", func() { fcm := monmock.NewMockFRRClient(mockCtrl) c := fake.NewClientBuilder().Build() - e := NewEndpoint(c, fcm) + e := NewEndpoint(c, fcm, "test-service", "test-namespace") e.SetHandlers() Context("ShowRoute() should", func() { @@ -255,24 +255,10 @@ var _ = Describe("Endpoint", func() { }) }) Context("PassRequest() should", func() { - It("return error if service name is not provided", func() { - req := httptest.NewRequest(http.MethodGet, "/all/show/route", http.NoBody) - res := httptest.NewRecorder() - e.PassRequest(res, req) - Expect(res.Code).To(Equal(http.StatusBadRequest)) - }) - It("return error if namespace was not provided and service not exist in kube-system namespace", func() { - c := fake.NewClientBuilder().WithRuntimeObjects(fakePods, fakeServices).Build() - e := NewEndpoint(c, fcm) - req := httptest.NewRequest(http.MethodGet, "/all/show/route?service=test-service", http.NoBody) - res := httptest.NewRecorder() - e.PassRequest(res, req) - Expect(res.Code).To(Equal(http.StatusInternalServerError)) - }) It("return error if there are no instances to query", func() { c := fake.NewClientBuilder().WithRuntimeObjects(fakePods, fakeServices).Build() - e := NewEndpoint(c, fcm) - req := httptest.NewRequest(http.MethodGet, "/all/show/route?service=test-service-no-endpoints&namespace=test-namespace", http.NoBody) + e := NewEndpoint(c, fcm, "test-service-no-endpoints", "test-namespace") + req := httptest.NewRequest(http.MethodGet, "/all/show/route", http.NoBody) res := httptest.NewRecorder() e.PassRequest(res, req) Expect(res.Code).To(Equal(http.StatusInternalServerError)) @@ -280,8 +266,8 @@ var _ = Describe("Endpoint", func() { It("return error if cannot get data from the endpoint", func() { c := fake.NewClientBuilder().WithRuntimeObjects(fakePods, fakeServices).Build() - e := NewEndpoint(c, fcm) - req := httptest.NewRequest(http.MethodGet, "/all/show/route?service=test-service&namespace=test-namespace", http.NoBody) + e := NewEndpoint(c, fcm, "test-service", "test-namespace") + req := httptest.NewRequest(http.MethodGet, "/all/show/route", http.NoBody) res := httptest.NewRecorder() e.PassRequest(res, req) Expect(res.Code).To(Equal(http.StatusInternalServerError)) @@ -293,8 +279,8 @@ var _ = Describe("Endpoint", func() { defer svr.Close() c := fake.NewClientBuilder().WithRuntimeObjects(fakePods, fakeServices).Build() - e := NewEndpoint(c, fcm) - req := httptest.NewRequest(http.MethodGet, svr.URL+"?service=test-service&namespace=test-namespace", http.NoBody) + e := NewEndpoint(c, fcm, "test-service", "test-namespace") + req := httptest.NewRequest(http.MethodGet, svr.URL, http.NoBody) res := httptest.NewRecorder() e.PassRequest(res, req) @@ -307,7 +293,7 @@ var _ = Describe("Endpoint", func() { defer svr.Close() c := fake.NewClientBuilder().WithRuntimeObjects(fakePods, fakeServices).Build() - e := NewEndpoint(c, fcm) + e := NewEndpoint(c, fcm, "test-service", "test-namespace") req := httptest.NewRequest(http.MethodGet, svr.URL+"?service=test-service&namespace=test-namespace", http.NoBody) res := httptest.NewRecorder() From 9aefc32be511bf01f125640263342029e18f27f9 Mon Sep 17 00:00:00 2001 From: Patryk Strusiewicz-Surmacki Date: Fri, 15 Mar 2024 12:42:23 +0100 Subject: [PATCH 25/35] Fixed linter issues Signed-off-by: Patryk Strusiewicz-Surmacki --- cmd/frr-exporter/main.go | 12 +++--------- pkg/monitoring/endpoint.go | 23 +++++++++++++++++++---- 2 files changed, 22 insertions(+), 13 deletions(-) diff --git a/cmd/frr-exporter/main.go b/cmd/frr-exporter/main.go index 010099a2..51ab1ed9 100644 --- a/cmd/frr-exporter/main.go +++ b/cmd/frr-exporter/main.go @@ -5,7 +5,6 @@ import ( "fmt" "log" "net/http" - "os" "time" "github.com/prometheus/client_golang/prometheus" @@ -57,14 +56,9 @@ func main() { frrCli := frr.NewCli() - svcName := os.Getenv(monitoring.StatusSvcNameEnv) - if svcName == "" { - log.Fatalf("environment variable %s is not set", monitoring.StatusSvcNameEnv) - } - - svcNamespace := os.Getenv(monitoring.StatusSvcNamespaceEnv) - if svcNamespace == "" { - log.Fatalf("environment variable %s is not set", monitoring.StatusSvcNamespaceEnv) + svcName, svcNamespace, err := monitoring.GetStatusServiceConfig() + if err != nil { + log.Fatal(fmt.Errorf("error getting status service info: %w", err)) } endpoint := monitoring.NewEndpoint(c, frrCli, svcName, svcNamespace) diff --git a/pkg/monitoring/endpoint.go b/pkg/monitoring/endpoint.go index e5767fa1..c12336dd 100644 --- a/pkg/monitoring/endpoint.go +++ b/pkg/monitoring/endpoint.go @@ -24,8 +24,6 @@ const ( protocolIPv4 = "ipv4" protocolIPv6 = "ipv6" - defaultNamespace = "kube-system" - StatusSvcNameEnv = "STATUS_SVC_NAME" StatusSvcNamespaceEnv = "STATUS_SVC_NAMESPACE" ) @@ -332,8 +330,8 @@ func (e *Endpoint) getAddresses(ctx context.Context, svc *corev1.Service) ([]str } addresses := []string{} - for _, pod := range pods.Items { - addresses = append(addresses, pod.Status.PodIP) + for i := range pods.Items { + addresses = append(addresses, pods.Items[i].Status.PodIP) } return addresses, nil @@ -371,3 +369,20 @@ func queryEndpoints(r *http.Request, addr []string) ([]byte, error) { return jsn, nil } + +// GetStatusServiceConfig gets status service's name and namespace from the environment. +func GetStatusServiceConfig() (name, namespace string, err error) { + name = os.Getenv(StatusSvcNameEnv) + if name == "" { + err = fmt.Errorf("environment variable %s is not set", StatusSvcNameEnv) + return name, namespace, err + } + + namespace = os.Getenv(StatusSvcNamespaceEnv) + if namespace == "" { + err = fmt.Errorf("environment variable %s is not set", StatusSvcNamespaceEnv) + return name, namespace, err + } + + return name, namespace, err +} From 2ec4a6afe196bb5c1df738bc5e10bb9f1759b1ed Mon Sep 17 00:00:00 2001 From: Patryk Strusiewicz-Surmacki Date: Wed, 10 Apr 2024 12:07:48 +0200 Subject: [PATCH 26/35] CR related changes Signed-off-by: Patryk Strusiewicz-Surmacki --- config/manager/service.yaml | 4 ++-- pkg/monitoring/endpoint.go | 5 +++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/config/manager/service.yaml b/config/manager/service.yaml index 849c3a51..0e63d3bd 100644 --- a/config/manager/service.yaml +++ b/config/manager/service.yaml @@ -4,7 +4,8 @@ metadata: name: status namespace: system spec: - type: NodePort + type: ClusterIP + clusterIP: none selector: app.kubernetes.io/component: worker app.kubernetes.io/name: network-operator @@ -12,4 +13,3 @@ spec: - protocol: TCP port: 7082 targetPort: 7082 - nodePort: 30082 \ No newline at end of file diff --git a/pkg/monitoring/endpoint.go b/pkg/monitoring/endpoint.go index c12336dd..87dab0bd 100644 --- a/pkg/monitoring/endpoint.go +++ b/pkg/monitoring/endpoint.go @@ -206,7 +206,6 @@ func (e *Endpoint) ShowEVPN(w http.ResponseWriter, r *http.Request) { writeResponse(result, w) } -//+kubebuilder:rbac:groups=core,resources=pods,verbs=list //+kubebuilder:rbac:groups=core,resources=services,verbs=get // PassRequest - when called, will pass the request to all nodes and return their responses. @@ -214,7 +213,7 @@ func (e *Endpoint) PassRequest(w http.ResponseWriter, r *http.Request) { service := &corev1.Service{} err := e.c.Get(r.Context(), client.ObjectKey{Name: e.statusSvcName, Namespace: e.statusSvcNamespace}, service) if err != nil { - http.Error(w, fmt.Sprintf("error listing pods: %s", err.Error()), http.StatusInternalServerError) + http.Error(w, fmt.Sprintf("error getting service: %s", err.Error()), http.StatusInternalServerError) return } @@ -319,6 +318,8 @@ func passRequest(r *http.Request, addr, query string, results chan []byte, error results <- data } +//+kubebuilder:rbac:groups=core,resources=pods,verbs=list + func (e *Endpoint) getAddresses(ctx context.Context, svc *corev1.Service) ([]string, error) { var serviceLabels labels.Set = svc.Spec.Selector pods := &corev1.PodList{} From 4ca3c4265ac611c7c4b39688eeafdfb15cc13209 Mon Sep 17 00:00:00 2001 From: Patryk Strusiewicz-Surmacki <137421299+p-strusiewiczsurmacki-mobica@users.noreply.github.com> Date: Thu, 18 Apr 2024 15:19:59 +0200 Subject: [PATCH 27/35] Update pkg/monitoring/endpoint.go Separated passRequest Co-authored-by: Jakob Schrettenbrunner --- pkg/monitoring/endpoint.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/pkg/monitoring/endpoint.go b/pkg/monitoring/endpoint.go index 87dab0bd..7360592d 100644 --- a/pkg/monitoring/endpoint.go +++ b/pkg/monitoring/endpoint.go @@ -348,7 +348,10 @@ func queryEndpoints(r *http.Request, addr []string) ([]byte, error) { for i := range addr { wg.Add(1) - go passRequest(r, addr[i], query, results, errors, &wg) + go func() { + passRequest(r, addr[i], query, results, errors, &wg) + wg.Done() + }() } wg.Wait() From 3c402b22d3df121fdab1c48992b25d820ec1433d Mon Sep 17 00:00:00 2001 From: Patryk Strusiewicz-Surmacki Date: Thu, 18 Apr 2024 13:38:33 +0200 Subject: [PATCH 28/35] Separate HHTTP servers for frr-exporter and monitoring endpoint. Separate docker images and contianers in pod. Signed-off-by: Patryk Strusiewicz-Surmacki --- Makefile | 14 ++++++++ cmd/frr-exporter/main.go | 23 +++---------- cmd/monitoring/main.go | 55 ++++++++++++++++++++++++++++++ config/manager/kustomization.yaml | 3 ++ config/manager/manager.yaml | 30 ++++++++++++++++ config/manager/manager_master.yaml | 30 ++++++++++++++++ monitoring.Dockerfile | 33 ++++++++++++++++++ pkg/monitoring/endpoint.go | 18 +++++----- pkg/monitoring/endpoint_test.go | 2 +- 9 files changed, 180 insertions(+), 28 deletions(-) create mode 100644 cmd/monitoring/main.go create mode 100644 monitoring.Dockerfile diff --git a/Makefile b/Makefile index 0f2fd6aa..9249ccb6 100644 --- a/Makefile +++ b/Makefile @@ -3,6 +3,8 @@ IMG ?= ghcr.io/telekom/das-schiff-network-operator:latest # Sidecar image URL to use all building/pushing image targets SIDECAR_IMG ?= ghcr.io/telekom/frr-exporter:latest +# Monitoring image URL to use all building/pushing image targets +MONITORING_IMG ?= ghcr.io/telekom/monitoring:latest # ENVTEST_K8S_VERSION refers to the version of kubebuilder assets to be downloaded by envtest binary. ENVTEST_K8S_VERSION = 1.25 @@ -75,6 +77,10 @@ build: generate fmt vet ## Build manager binary. sidecar-build: build go build -o bin/frr-exporter cmd/frr-exporter/main.go +.PHONY: build +monitoring-build: build + go build -o bin/monitoring cmd/monitoring/main.go + .PHONY: run run: manifests generate fmt vet ## Run a controller from your host. go run ./cmd/manager/main.go @@ -87,6 +93,10 @@ docker-build: test ## Build docker image with the manager. docker-build-sidecar: test ## Build docker image with the manager. docker build -t ${SIDECAR_IMG} -f frr-exporter.Dockerfile . +.PHONY: docker-build-monitoring +docker-build-monitoring: test ## Build docker image with the manager. + docker build -t ${MONITORING_IMG} -f monitoring.Dockerfile . + .PHONY: docker-push docker-push: ## Push docker image with the manager. docker push ${IMG} @@ -95,6 +105,10 @@ docker-push: ## Push docker image with the manager. docker-push-sidecar: ## Push docker image with the manager. docker push ${SIDECAR_IMG} +.PHONY: docker-push-monitoring +docker-push-monitoring: ## Push docker image with the manager. + docker push ${MONITORING_IMG} + ##@ Release diff --git a/cmd/frr-exporter/main.go b/cmd/frr-exporter/main.go index 51ab1ed9..bc821dfe 100644 --- a/cmd/frr-exporter/main.go +++ b/cmd/frr-exporter/main.go @@ -10,10 +10,8 @@ import ( "github.com/prometheus/client_golang/prometheus" "github.com/prometheus/client_golang/prometheus/collectors" "github.com/prometheus/client_golang/prometheus/promhttp" - "github.com/telekom/das-schiff-network-operator/pkg/frr" "github.com/telekom/das-schiff-network-operator/pkg/monitoring" ctrl "sigs.k8s.io/controller-runtime" - "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/log/zap" ) @@ -48,24 +46,9 @@ func main() { } reg.MustRegister(collector) - clientConfig := ctrl.GetConfigOrDie() - c, err := client.New(clientConfig, client.Options{}) - if err != nil { - log.Fatal(fmt.Errorf("error creating controller-runtime client: %w", err)) - } - - frrCli := frr.NewCli() - - svcName, svcNamespace, err := monitoring.GetStatusServiceConfig() - if err != nil { - log.Fatal(fmt.Errorf("error getting status service info: %w", err)) - } - - endpoint := monitoring.NewEndpoint(c, frrCli, svcName, svcNamespace) - endpoint.SetHandlers() - // Expose the registered metrics via HTTP. - http.Handle("/metrics", promhttp.HandlerFor( + mux := http.NewServeMux() + mux.Handle("/metrics", promhttp.HandlerFor( reg, promhttp.HandlerOpts{ // Opt into OpenMetrics to support exemplars. @@ -73,10 +56,12 @@ func main() { Timeout: time.Minute, }, )) + server := http.Server{ Addr: addr, ReadHeaderTimeout: twenty * time.Second, ReadTimeout: time.Minute, + Handler: mux, } err = server.ListenAndServe() // Run server diff --git a/cmd/monitoring/main.go b/cmd/monitoring/main.go new file mode 100644 index 00000000..46626ba8 --- /dev/null +++ b/cmd/monitoring/main.go @@ -0,0 +1,55 @@ +package main + +import ( + "flag" + "fmt" + "log" + "net/http" + "time" + + "github.com/telekom/das-schiff-network-operator/pkg/frr" + "github.com/telekom/das-schiff-network-operator/pkg/monitoring" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/log/zap" +) + +const ( + twenty = 20 +) + +func main() { + var addr string + flag.StringVar(&addr, "listen-address", ":7083", "The address to listen on for HTTP requests.") + opts := zap.Options{ + Development: true, + } + opts.BindFlags(flag.CommandLine) + flag.Parse() + ctrl.SetLogger(zap.New(zap.UseFlagOptions(&opts))) + + clientConfig := ctrl.GetConfigOrDie() + c, err := client.New(clientConfig, client.Options{}) + if err != nil { + log.Fatal(fmt.Errorf("error creating controller-runtime client: %w", err)) + } + + svcName, svcNamespace, err := monitoring.GetStatusServiceConfig() + if err != nil { + log.Fatal(fmt.Errorf("error getting status service info: %w", err)) + } + + endpoint := monitoring.NewEndpoint(c, frr.NewCli(), svcName, svcNamespace) + + server := http.Server{ + Addr: addr, + ReadHeaderTimeout: twenty * time.Second, + ReadTimeout: time.Minute, + Handler: endpoint.CreateMux(), + } + err = server.ListenAndServe() + // Run server + if err != nil { + log.Fatal(fmt.Errorf("failed to start server: %w", err)) + } +} diff --git a/config/manager/kustomization.yaml b/config/manager/kustomization.yaml index c3344e1c..9790bb26 100644 --- a/config/manager/kustomization.yaml +++ b/config/manager/kustomization.yaml @@ -22,3 +22,6 @@ images: - name: frr-exporter newName: ghcr.io/telekom/frr-exporter newTag: latest +- name: monitoring + newName: ghcr.io/telekom/monitoring + newTag: latest diff --git a/config/manager/manager.yaml b/config/manager/manager.yaml index 16497def..690054c1 100644 --- a/config/manager/manager.yaml +++ b/config/manager/manager.yaml @@ -108,6 +108,36 @@ spec: name: frr-config - mountPath: /var/run/frr name: frr-run + - command: + - /monitoring + env: + - name: NODE_NAME + valueFrom: + fieldRef: + fieldPath: spec.nodeName + - name: STATUS_SVC_NAME + value: network-operator-status + - name: STATUS_SVC_NAMESPACE + valueFrom: + fieldRef: + fieldPath: metadata.namespace + image: monitoring:latest + name: monitoring + securityContext: + privileged: true + runAsUser: 0 + resources: + limits: + cpu: 100m + memory: 64Mi + requests: + cpu: 10m + memory: 64Mi + volumeMounts: + - mountPath: /etc/frr + name: frr-config + - mountPath: /var/run/frr + name: frr-run serviceAccountName: controller-manager terminationGracePeriodSeconds: 10 volumes: diff --git a/config/manager/manager_master.yaml b/config/manager/manager_master.yaml index cd3a4917..4b7df44d 100644 --- a/config/manager/manager_master.yaml +++ b/config/manager/manager_master.yaml @@ -110,6 +110,36 @@ spec: name: frr-config - mountPath: /var/run/frr name: frr-run + - command: + - /monitoring + env: + - name: NODE_NAME + valueFrom: + fieldRef: + fieldPath: spec.nodeName + - name: STATUS_SVC_NAME + value: network-operator-status + - name: STATUS_SVC_NAMESPACE + valueFrom: + fieldRef: + fieldPath: metadata.namespace + image: monitoring:latest + name: monitoring + securityContext: + privileged: true + runAsUser: 0 + resources: + limits: + cpu: 100m + memory: 64Mi + requests: + cpu: 10m + memory: 64Mi + volumeMounts: + - mountPath: /etc/frr + name: frr-config + - mountPath: /var/run/frr + name: frr-run serviceAccountName: controller-manager terminationGracePeriodSeconds: 10 volumes: diff --git a/monitoring.Dockerfile b/monitoring.Dockerfile new file mode 100644 index 00000000..18c196fb --- /dev/null +++ b/monitoring.Dockerfile @@ -0,0 +1,33 @@ +ARG FRR_VERSION="9.1.0" +ARG REGISTRY="quay.io" +# Build the manager binary +FROM docker.io/library/golang:1.21-alpine as builder + +WORKDIR /workspace +# Copy the Go Modules manifests +COPY go.mod go.mod +COPY go.sum go.sum +# cache deps before building and copying source so that we don't need to re-download as much +# and so that source changes don't invalidate our downloaded layer +RUN go mod download + +# Copy the go source +COPY cmd/monitoring/main.go main.go +COPY pkg/ pkg/ + +# Build +RUN CGO_ENABLED=0 GOOS=linux GOARCH=amd64 go build -a -o monitoring main.go + + +FROM ${REGISTRY}/frrouting/frr:${FRR_VERSION} + +RUN apk add --no-cache frr + +WORKDIR / +COPY --from=builder /workspace/monitoring . +## Needs to run as root +## vtysh is required to have extended rights +## to be able to connect to vty sockets on the host +# USER 65532:65532 + +ENTRYPOINT ["/monitoring"] diff --git a/pkg/monitoring/endpoint.go b/pkg/monitoring/endpoint.go index 7360592d..ac926350 100644 --- a/pkg/monitoring/endpoint.go +++ b/pkg/monitoring/endpoint.go @@ -46,14 +46,16 @@ func NewEndpoint(k8sClient client.Client, frrcli FRRClient, svcName, svcNamespac return &Endpoint{cli: frrcli, c: k8sClient, statusSvcName: svcName, statusSvcNamespace: svcNamespace} } -// SetHandlers configures HTTP handlers. -func (e *Endpoint) SetHandlers() { - http.HandleFunc("/show/route", e.ShowRoute) - http.HandleFunc("/show/bgp", e.ShowBGP) - http.HandleFunc("/show/evpn", e.ShowEVPN) - http.HandleFunc("/all/show/route", e.PassRequest) - http.HandleFunc("/all/show/bgp", e.PassRequest) - http.HandleFunc("/all/show/evpn", e.PassRequest) +// CreateMux configures HTTP handlers. +func (e *Endpoint) CreateMux() *http.ServeMux { + sm := http.NewServeMux() + sm.HandleFunc("/show/route", e.ShowRoute) + sm.HandleFunc("/show/bgp", e.ShowBGP) + sm.HandleFunc("/show/evpn", e.ShowEVPN) + sm.HandleFunc("/all/show/route", e.PassRequest) + sm.HandleFunc("/all/show/bgp", e.PassRequest) + sm.HandleFunc("/all/show/evpn", e.PassRequest) + return sm } // ShowRoute returns result of show ip/ipv6 route command. diff --git a/pkg/monitoring/endpoint_test.go b/pkg/monitoring/endpoint_test.go index 2d698845..601a9ef5 100644 --- a/pkg/monitoring/endpoint_test.go +++ b/pkg/monitoring/endpoint_test.go @@ -122,7 +122,7 @@ var _ = Describe("Endpoint", func() { fcm := monmock.NewMockFRRClient(mockCtrl) c := fake.NewClientBuilder().Build() e := NewEndpoint(c, fcm, "test-service", "test-namespace") - e.SetHandlers() + e.CreateMux() Context("ShowRoute() should", func() { It("return no error", func() { From d5f5df6d2e21c85b04c59c34cf6518d17be7c3cd Mon Sep 17 00:00:00 2001 From: Patryk Strusiewicz-Surmacki Date: Thu, 18 Apr 2024 13:41:15 +0200 Subject: [PATCH 29/35] Ranamed PassRequest function to QueryAll Signed-off-by: Patryk Strusiewicz-Surmacki --- pkg/monitoring/endpoint.go | 10 +++++----- pkg/monitoring/endpoint_test.go | 8 ++++---- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/pkg/monitoring/endpoint.go b/pkg/monitoring/endpoint.go index ac926350..4ab17336 100644 --- a/pkg/monitoring/endpoint.go +++ b/pkg/monitoring/endpoint.go @@ -52,9 +52,9 @@ func (e *Endpoint) CreateMux() *http.ServeMux { sm.HandleFunc("/show/route", e.ShowRoute) sm.HandleFunc("/show/bgp", e.ShowBGP) sm.HandleFunc("/show/evpn", e.ShowEVPN) - sm.HandleFunc("/all/show/route", e.PassRequest) - sm.HandleFunc("/all/show/bgp", e.PassRequest) - sm.HandleFunc("/all/show/evpn", e.PassRequest) + sm.HandleFunc("/all/show/route", e.QueryAll) + sm.HandleFunc("/all/show/bgp", e.QueryAll) + sm.HandleFunc("/all/show/evpn", e.QueryAll) return sm } @@ -210,8 +210,8 @@ func (e *Endpoint) ShowEVPN(w http.ResponseWriter, r *http.Request) { //+kubebuilder:rbac:groups=core,resources=services,verbs=get -// PassRequest - when called, will pass the request to all nodes and return their responses. -func (e *Endpoint) PassRequest(w http.ResponseWriter, r *http.Request) { +// QueryAll - when called, will pass the request to all nodes and return their responses. +func (e *Endpoint) QueryAll(w http.ResponseWriter, r *http.Request) { service := &corev1.Service{} err := e.c.Get(r.Context(), client.ObjectKey{Name: e.statusSvcName, Namespace: e.statusSvcNamespace}, service) if err != nil { diff --git a/pkg/monitoring/endpoint_test.go b/pkg/monitoring/endpoint_test.go index 601a9ef5..252ab645 100644 --- a/pkg/monitoring/endpoint_test.go +++ b/pkg/monitoring/endpoint_test.go @@ -260,7 +260,7 @@ var _ = Describe("Endpoint", func() { e := NewEndpoint(c, fcm, "test-service-no-endpoints", "test-namespace") req := httptest.NewRequest(http.MethodGet, "/all/show/route", http.NoBody) res := httptest.NewRecorder() - e.PassRequest(res, req) + e.QueryAll(res, req) Expect(res.Code).To(Equal(http.StatusInternalServerError)) }) @@ -269,7 +269,7 @@ var _ = Describe("Endpoint", func() { e := NewEndpoint(c, fcm, "test-service", "test-namespace") req := httptest.NewRequest(http.MethodGet, "/all/show/route", http.NoBody) res := httptest.NewRecorder() - e.PassRequest(res, req) + e.QueryAll(res, req) Expect(res.Code).To(Equal(http.StatusInternalServerError)) }) It("return error if request was properly passed to the endpoint but the response is malformed", func() { @@ -283,7 +283,7 @@ var _ = Describe("Endpoint", func() { req := httptest.NewRequest(http.MethodGet, svr.URL, http.NoBody) res := httptest.NewRecorder() - e.PassRequest(res, req) + e.QueryAll(res, req) Expect(res.Code).To(Equal(http.StatusInternalServerError)) }) It("return no error if request was properly passed to the endpoint", func() { @@ -297,7 +297,7 @@ var _ = Describe("Endpoint", func() { req := httptest.NewRequest(http.MethodGet, svr.URL+"?service=test-service&namespace=test-namespace", http.NoBody) res := httptest.NewRecorder() - e.PassRequest(res, req) + e.QueryAll(res, req) Expect(res.Code).To(Equal(http.StatusOK)) }) }) From 0744b4c3940703cc8ca9321e0dc43ffe3bc28b87 Mon Sep 17 00:00:00 2001 From: Patryk Strusiewicz-Surmacki Date: Thu, 18 Apr 2024 14:26:19 +0200 Subject: [PATCH 30/35] Separated passRequest go routine as suggested in the CR Signed-off-by: Patryk Strusiewicz-Surmacki --- cmd/monitoring/main.go | 8 +++ pkg/monitoring/endpoint.go | 106 +++++++++++++++++++++++++++---------- 2 files changed, 85 insertions(+), 29 deletions(-) diff --git a/cmd/monitoring/main.go b/cmd/monitoring/main.go index 46626ba8..0c614200 100644 --- a/cmd/monitoring/main.go +++ b/cmd/monitoring/main.go @@ -19,6 +19,7 @@ const ( ) func main() { + setupLog := ctrl.Log.WithName("setup") var addr string flag.StringVar(&addr, "listen-address", ":7083", "The address to listen on for HTTP requests.") opts := zap.Options{ @@ -34,10 +35,13 @@ func main() { log.Fatal(fmt.Errorf("error creating controller-runtime client: %w", err)) } + setupLog.Info("loaded kubernetes config") + svcName, svcNamespace, err := monitoring.GetStatusServiceConfig() if err != nil { log.Fatal(fmt.Errorf("error getting status service info: %w", err)) } + setupLog.Info("loaded status service config") endpoint := monitoring.NewEndpoint(c, frr.NewCli(), svcName, svcNamespace) @@ -47,6 +51,10 @@ func main() { ReadTimeout: time.Minute, Handler: endpoint.CreateMux(), } + + setupLog.Info("created server, starting...", "Addr", server.Addr, + "ReadHeaderTimeout", server.ReadHeaderTimeout, "ReadTimeout", server.ReadTimeout) + err = server.ListenAndServe() // Run server if err != nil { diff --git a/pkg/monitoring/endpoint.go b/pkg/monitoring/endpoint.go index 4ab17336..97a6efd2 100644 --- a/pkg/monitoring/endpoint.go +++ b/pkg/monitoring/endpoint.go @@ -12,10 +12,12 @@ import ( "strings" "sync" + "github.com/go-logr/logr" "github.com/telekom/das-schiff-network-operator/pkg/healthcheck" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/labels" "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/log" ) const ( @@ -39,11 +41,18 @@ type Endpoint struct { statusSvcName string statusSvcNamespace string + logr.Logger } // NewEndpoint creates new endpoint object. func NewEndpoint(k8sClient client.Client, frrcli FRRClient, svcName, svcNamespace string) *Endpoint { - return &Endpoint{cli: frrcli, c: k8sClient, statusSvcName: svcName, statusSvcNamespace: svcNamespace} + return &Endpoint{ + cli: frrcli, + c: k8sClient, + statusSvcName: svcName, + statusSvcNamespace: svcNamespace, + Logger: log.Log.WithName("monitoring"), + } } // CreateMux configures HTTP handlers. @@ -55,12 +64,15 @@ func (e *Endpoint) CreateMux() *http.ServeMux { sm.HandleFunc("/all/show/route", e.QueryAll) sm.HandleFunc("/all/show/bgp", e.QueryAll) sm.HandleFunc("/all/show/evpn", e.QueryAll) + e.Logger.Info("created ServeMux") return sm } // ShowRoute returns result of show ip/ipv6 route command. // show ip/ipv6 route (vrf ) (longer-prefixes). func (e *Endpoint) ShowRoute(w http.ResponseWriter, r *http.Request) { + e.Logger.Info("got ShowRoute request", "request", r) + vrf := r.URL.Query().Get("vrf") if vrf == "" { vrf = all @@ -70,6 +82,7 @@ func (e *Endpoint) ShowRoute(w http.ResponseWriter, r *http.Request) { if protocol == "" { protocol = protocolIP } else if protocol != protocolIP && protocol != protocolIPv6 { + e.Logger.Error(fmt.Errorf("protocol '%s' is not supported", protocol), "protocol not supported") http.Error(w, fmt.Sprintf("protocol '%s' is not supported", protocol), http.StatusBadRequest) return } @@ -83,30 +96,36 @@ func (e *Endpoint) ShowRoute(w http.ResponseWriter, r *http.Request) { } if err := setInput(r, &command); err != nil { + e.Logger.Error(err, "unable to set input") http.Error(w, err.Error(), http.StatusBadRequest) return } if err := setLongerPrefixes(r, &command); err != nil { + e.Logger.Error(err, "unable to set longer prefixes") http.Error(w, err.Error(), http.StatusBadRequest) return } + e.Logger.Info("command to be executed", "command", command) + data := e.cli.ExecuteWithJSON(command) result, err := withNodename(&data) if err != nil { + e.Logger.Error(err, "unable to add nodename") http.Error(w, fmt.Sprintf("error adding nodename: %s", err.Error()), http.StatusBadRequest) return } - writeResponse(result, w) + e.writeResponse(result, w) } // ShowBGP returns a result of show bgp command. // show bgp (vrf ) ipv4/ipv6 unicast (longer-prefixes). // show bgp vrf summary. func (e *Endpoint) ShowBGP(w http.ResponseWriter, r *http.Request) { + e.Logger.Info("got ShowBGP request", "request", r) vrf := r.URL.Query().Get("vrf") if vrf == "" { vrf = all @@ -116,22 +135,24 @@ func (e *Endpoint) ShowBGP(w http.ResponseWriter, r *http.Request) { requestType := r.URL.Query().Get("type") switch requestType { case "summary": - data = e.cli.ExecuteWithJSON([]string{ + command := []string{ "show", "bgp", "vrf", vrf, "summary", - }) + } + e.Logger.Info("command to be executed", "command", command) + data = e.cli.ExecuteWithJSON(command) case "": protocol := r.URL.Query().Get("protocol") if protocol == "" || protocol == protocolIP { protocol = protocolIPv4 } else if protocol != protocolIPv4 && protocol != protocolIPv6 { + e.Logger.Error(fmt.Errorf("protocol '%s' is not supported", protocol), "protocol not supported") http.Error(w, fmt.Sprintf("protocol '%s' is not supported", protocol), http.StatusBadRequest) return } - command := []string{ "show", "bgp", @@ -142,28 +163,33 @@ func (e *Endpoint) ShowBGP(w http.ResponseWriter, r *http.Request) { } if err := setInput(r, &command); err != nil { + e.Logger.Error(err, "unable to set input") http.Error(w, err.Error(), http.StatusBadRequest) return } if err := setLongerPrefixes(r, &command); err != nil { + e.Logger.Error(err, "unable to set longer prefixes") http.Error(w, err.Error(), http.StatusBadRequest) return } + e.Logger.Info("command to be executed", "command", command) data = e.cli.ExecuteWithJSON(command) default: + e.Logger.Error(fmt.Errorf("request of type '%s' is not supported", requestType), "request type not supported") http.Error(w, fmt.Sprintf("request of type '%s' is not supported", requestType), http.StatusBadRequest) return } result, err := withNodename(&data) if err != nil { + e.Logger.Error(err, "error adding nodename") http.Error(w, fmt.Sprintf("error adding nodename: %s", err.Error()), http.StatusBadRequest) return } - writeResponse(result, w) + e.writeResponse(result, w) } // ShowEVPN returns result of show evpn command. @@ -172,79 +198,99 @@ func (e *Endpoint) ShowBGP(w http.ResponseWriter, r *http.Request) { // show evpn mac vni . // show evpn next-hops vni json. func (e *Endpoint) ShowEVPN(w http.ResponseWriter, r *http.Request) { + e.Logger.Info("got ShowEVPN request", "request", r) var data []byte requestType := r.URL.Query().Get("type") switch requestType { case "": - data = e.cli.ExecuteWithJSON([]string{ + command := []string{ "show", "evpn", "vni", - }) + } + e.Logger.Info("command to be executed", "command", command) + data = e.cli.ExecuteWithJSON(command) case "rmac", "mac", "next-hops": vni := r.URL.Query().Get("vni") if vni == "" { vni = all } - data = e.cli.ExecuteWithJSON([]string{ + command := []string{ "show", "evpn", requestType, "vni", vni, - }) + } + e.Logger.Info("command to be executed", "command", command) + data = e.cli.ExecuteWithJSON(command) default: + e.Logger.Error(fmt.Errorf("request of type '%s' is not supported", requestType), "request type not supported") http.Error(w, fmt.Sprintf("request of type '%s' is not supported", requestType), http.StatusBadRequest) return } result, err := withNodename(&data) if err != nil { + e.Logger.Error(err, "error adding nodename") http.Error(w, fmt.Sprintf("error adding nodename: %s", err.Error()), http.StatusBadRequest) return } - writeResponse(result, w) + e.writeResponse(result, w) } //+kubebuilder:rbac:groups=core,resources=services,verbs=get // QueryAll - when called, will pass the request to all nodes and return their responses. func (e *Endpoint) QueryAll(w http.ResponseWriter, r *http.Request) { + e.Logger.Info("got QueryAll request", "request", r) service := &corev1.Service{} err := e.c.Get(r.Context(), client.ObjectKey{Name: e.statusSvcName, Namespace: e.statusSvcNamespace}, service) if err != nil { + e.Logger.Error(err, "error getting service") http.Error(w, fmt.Sprintf("error getting service: %s", err.Error()), http.StatusInternalServerError) return } addr, err := e.getAddresses(r.Context(), service) if err != nil { + e.Logger.Error(err, "error getting addresses") http.Error(w, fmt.Sprintf("error getting addresses: %s", err.Error()), http.StatusInternalServerError) return } if len(addr) == 0 { + e.Logger.Error(fmt.Errorf("addr slice length: %d", len(addr)), "error listing addresses: no addresses found") http.Error(w, "error listing addresses: no addresses found", http.StatusInternalServerError) return } - response, err := queryEndpoints(r, addr) - if err != nil { - http.Error(w, "error querying endpoints: "+err.Error(), http.StatusInternalServerError) + e.Logger.Info("will querry endpoints", "endpoints", addr) + response, errs := queryEndpoints(r, addr) + if len(errs) > 0 { + for _, err := range errs { + e.Logger.Error(err, "error querying endpoint") + } + if len(errs) == 1 { + http.Error(w, fmt.Sprintf("error querying endpoints - %s", errs[0].Error()), http.StatusInternalServerError) + return + } + http.Error(w, "multiple errors occurred while querying endpoints - please check logs for the details", http.StatusInternalServerError) return } - writeResponse(&response, w) + e.writeResponse(&response, w) } -func writeResponse(data *[]byte, w http.ResponseWriter) { +func (e *Endpoint) writeResponse(data *[]byte, w http.ResponseWriter) { _, err := w.Write(*data) if err != nil { http.Error(w, "failed to write response: "+err.Error(), http.StatusInternalServerError) return } + e.Logger.Info("response written", "data", *data) } func setLongerPrefixes(r *http.Request, command *[]string) error { @@ -289,9 +335,7 @@ func withNodename(data *[]byte) (*[]byte, error) { return result, nil } -func passRequest(r *http.Request, addr, query string, results chan []byte, errors chan error, wg *sync.WaitGroup) { - defer wg.Done() - +func passRequest(r *http.Request, addr, query string, results chan []byte, errors chan error) { s := strings.Split(r.Host, ":") port := "" if len(s) > 1 { @@ -340,28 +384,32 @@ func (e *Endpoint) getAddresses(ctx context.Context, svc *corev1.Service) ([]str return addresses, nil } -func queryEndpoints(r *http.Request, addr []string) ([]byte, error) { +func queryEndpoints(r *http.Request, addr []string) ([]byte, []error) { query := strings.ReplaceAll(r.URL.RequestURI(), "all/", "") responses := []json.RawMessage{} var wg sync.WaitGroup results := make(chan []byte, len(addr)) - errors := make(chan error, len(addr)) + requestErrors := make(chan error, len(addr)) for i := range addr { wg.Add(1) - go func() { - passRequest(r, addr[i], query, results, errors, &wg) - wg.Done() - }() + go func(i int) { + defer wg.Done() + passRequest(r, addr[i], query, results, requestErrors) + }(i) } wg.Wait() close(results) - close(errors) + close(requestErrors) - for err := range errors { - return nil, fmt.Errorf("error occurred: %w", err) + if len(requestErrors) > 0 { + err := []error{} + for e := range requestErrors { + err = append(err, e) + } + return nil, err } for result := range results { @@ -370,7 +418,7 @@ func queryEndpoints(r *http.Request, addr []string) ([]byte, error) { jsn, err := json.MarshalIndent(responses, "", "\t") if err != nil { - return nil, fmt.Errorf("error marshaling data: %w", err) + return nil, []error{fmt.Errorf("error marshaling data: %w", err)} } return jsn, nil From 77d718231040c652333dd5a041d81b825ef3eb14 Mon Sep 17 00:00:00 2001 From: Patryk Strusiewicz-Surmacki Date: Thu, 18 Apr 2024 15:49:58 +0200 Subject: [PATCH 31/35] Sanitized inputs Signed-off-by: Patryk Strusiewicz-Surmacki --- pkg/monitoring/endpoint.go | 95 +++++++++++++++++++++++--------------- 1 file changed, 58 insertions(+), 37 deletions(-) diff --git a/pkg/monitoring/endpoint.go b/pkg/monitoring/endpoint.go index 97a6efd2..37bd4e7b 100644 --- a/pkg/monitoring/endpoint.go +++ b/pkg/monitoring/endpoint.go @@ -8,6 +8,7 @@ import ( "net" "net/http" "os" + "regexp" "strconv" "strings" "sync" @@ -30,6 +31,8 @@ const ( StatusSvcNamespaceEnv = "STATUS_SVC_NAMESPACE" ) +var validation = regexp.MustCompile("^[a-zA-Z0-9_]") + //go:generate mockgen -destination ./mock/mock_endpoint.go . FRRClient type FRRClient interface { ExecuteWithJSON(args []string) []byte @@ -78,6 +81,12 @@ func (e *Endpoint) ShowRoute(w http.ResponseWriter, r *http.Request) { vrf = all } + if !validation.MatchString(vrf) { + e.Logger.Error(fmt.Errorf("invalid VRF value"), "error validating value") + http.Error(w, "invalid VRF value", http.StatusBadRequest) + return + } + protocol := r.URL.Query().Get("protocol") if protocol == "" { protocol = protocolIP @@ -131,29 +140,52 @@ func (e *Endpoint) ShowBGP(w http.ResponseWriter, r *http.Request) { vrf = all } - var data []byte + if !validation.MatchString(vrf) { + e.Logger.Error(fmt.Errorf("invalid VRF value"), "error validating value") + http.Error(w, "invalid VRF value", http.StatusBadRequest) + return + } + + command, err := prepareBGPCommand(r, vrf) + if err != nil { + e.Logger.Error(err, "error preparing ShowBGP command") + http.Error(w, "error preparing ShowBGP command: "+err.Error(), http.StatusBadRequest) + return + } + + e.Logger.Info("command to be executed", "command", command) + data := e.cli.ExecuteWithJSON(command) + + result, err := withNodename(&data) + if err != nil { + e.Logger.Error(err, "error adding nodename") + http.Error(w, fmt.Sprintf("error adding nodename: %s", err.Error()), http.StatusBadRequest) + return + } + + e.writeResponse(result, w) +} + +func prepareBGPCommand(r *http.Request, vrf string) ([]string, error) { + var command []string requestType := r.URL.Query().Get("type") switch requestType { case "summary": - command := []string{ + command = []string{ "show", "bgp", "vrf", vrf, "summary", } - e.Logger.Info("command to be executed", "command", command) - data = e.cli.ExecuteWithJSON(command) case "": protocol := r.URL.Query().Get("protocol") if protocol == "" || protocol == protocolIP { protocol = protocolIPv4 } else if protocol != protocolIPv4 && protocol != protocolIPv6 { - e.Logger.Error(fmt.Errorf("protocol '%s' is not supported", protocol), "protocol not supported") - http.Error(w, fmt.Sprintf("protocol '%s' is not supported", protocol), http.StatusBadRequest) - return + return nil, fmt.Errorf("protocol %s is not supported", protocol) } - command := []string{ + command = []string{ "show", "bgp", "vrf", @@ -163,33 +195,17 @@ func (e *Endpoint) ShowBGP(w http.ResponseWriter, r *http.Request) { } if err := setInput(r, &command); err != nil { - e.Logger.Error(err, "unable to set input") - http.Error(w, err.Error(), http.StatusBadRequest) - return + return nil, fmt.Errorf("unable to set input: %w", err) } if err := setLongerPrefixes(r, &command); err != nil { - e.Logger.Error(err, "unable to set longer prefixes") - http.Error(w, err.Error(), http.StatusBadRequest) - return + return nil, fmt.Errorf("unable to set longer prefixes: %w", err) } - - e.Logger.Info("command to be executed", "command", command) - data = e.cli.ExecuteWithJSON(command) default: - e.Logger.Error(fmt.Errorf("request of type '%s' is not supported", requestType), "request type not supported") - http.Error(w, fmt.Sprintf("request of type '%s' is not supported", requestType), http.StatusBadRequest) - return - } - - result, err := withNodename(&data) - if err != nil { - e.Logger.Error(err, "error adding nodename") - http.Error(w, fmt.Sprintf("error adding nodename: %s", err.Error()), http.StatusBadRequest) - return + return nil, fmt.Errorf("request of type '%s' is not supported", requestType) } - e.writeResponse(result, w) + return command, nil } // ShowEVPN returns result of show evpn command. @@ -199,38 +215,43 @@ func (e *Endpoint) ShowBGP(w http.ResponseWriter, r *http.Request) { // show evpn next-hops vni json. func (e *Endpoint) ShowEVPN(w http.ResponseWriter, r *http.Request) { e.Logger.Info("got ShowEVPN request", "request", r) - var data []byte + var command []string requestType := r.URL.Query().Get("type") switch requestType { case "": - command := []string{ + command = []string{ "show", "evpn", "vni", } - e.Logger.Info("command to be executed", "command", command) - data = e.cli.ExecuteWithJSON(command) case "rmac", "mac", "next-hops": vni := r.URL.Query().Get("vni") if vni == "" { vni = all } - command := []string{ + if !validation.MatchString(vni) { + e.Logger.Error(fmt.Errorf("invalid VNI value"), "error validating value") + http.Error(w, "invalid VNI value", http.StatusBadRequest) + return + } + + command = []string{ "show", "evpn", requestType, "vni", vni, } - e.Logger.Info("command to be executed", "command", command) - data = e.cli.ExecuteWithJSON(command) default: e.Logger.Error(fmt.Errorf("request of type '%s' is not supported", requestType), "request type not supported") http.Error(w, fmt.Sprintf("request of type '%s' is not supported", requestType), http.StatusBadRequest) return } + e.Logger.Info("command to be executed", "command", command) + data := e.cli.ExecuteWithJSON(command) + result, err := withNodename(&data) if err != nil { e.Logger.Error(err, "error adding nodename") @@ -298,7 +319,7 @@ func setLongerPrefixes(r *http.Request, command *[]string) error { if longerPrefixes != "" { useLongerPrefixes, err := strconv.ParseBool(longerPrefixes) if err != nil { - return fmt.Errorf("longer_prefixes value '%s' is not valid: %w", longerPrefixes, err) + return fmt.Errorf("longer_prefixes value is not valid: %w", err) } if useLongerPrefixes { *command = append(*command, "longer-prefixes") @@ -311,7 +332,7 @@ func setInput(r *http.Request, command *[]string) error { input := r.URL.Query().Get("input") if input != "" { if _, _, err := net.ParseCIDR(input); err != nil { - return fmt.Errorf("input '%s' is not valid: %w", input, err) + return fmt.Errorf("input value is not valid: %w", err) } *command = append(*command, input) } From 0448608d08256150b10eecb6500bafc470128b27 Mon Sep 17 00:00:00 2001 From: Patryk Strusiewicz-Surmacki Date: Thu, 18 Apr 2024 16:55:10 +0200 Subject: [PATCH 32/35] Fixed logging and other minor issues Signed-off-by: Patryk Strusiewicz-Surmacki --- Makefile | 2 ++ config/manager/kustomization.yaml | 1 + config/manager/service.yaml | 6 +++--- pkg/monitoring/endpoint.go | 20 ++++++++++---------- 4 files changed, 16 insertions(+), 13 deletions(-) diff --git a/Makefile b/Makefile index 9249ccb6..49af1d46 100644 --- a/Makefile +++ b/Makefile @@ -148,6 +148,8 @@ uninstall-certs: manifests kustomize ## Uninstall certs .PHONY: deploy deploy: manifests kustomize ## Deploy controller to the K8s cluster specified in ~/.kube/config. cd config/manager && $(KUSTOMIZE) edit set image controller=${IMG} + cd config/manager && $(KUSTOMIZE) edit set image frr-exporter=${SIDECAR_IMG} + cd config/manager && $(KUSTOMIZE) edit set image monitoring=${MONITORING_IMG} $(KUSTOMIZE) build config/default | kubectl apply -f - .PHONY: undeploy diff --git a/config/manager/kustomization.yaml b/config/manager/kustomization.yaml index 9790bb26..a0fa3d1e 100644 --- a/config/manager/kustomization.yaml +++ b/config/manager/kustomization.yaml @@ -1,6 +1,7 @@ resources: - manager.yaml - manager_master.yaml +- service.yaml # - namespace.yaml generatorOptions: diff --git a/config/manager/service.yaml b/config/manager/service.yaml index 0e63d3bd..f85b7cad 100644 --- a/config/manager/service.yaml +++ b/config/manager/service.yaml @@ -5,11 +5,11 @@ metadata: namespace: system spec: type: ClusterIP - clusterIP: none + clusterIP: None selector: app.kubernetes.io/component: worker app.kubernetes.io/name: network-operator ports: - protocol: TCP - port: 7082 - targetPort: 7082 + port: 7083 + targetPort: 7083 diff --git a/pkg/monitoring/endpoint.go b/pkg/monitoring/endpoint.go index 37bd4e7b..39d1d830 100644 --- a/pkg/monitoring/endpoint.go +++ b/pkg/monitoring/endpoint.go @@ -74,7 +74,7 @@ func (e *Endpoint) CreateMux() *http.ServeMux { // ShowRoute returns result of show ip/ipv6 route command. // show ip/ipv6 route (vrf ) (longer-prefixes). func (e *Endpoint) ShowRoute(w http.ResponseWriter, r *http.Request) { - e.Logger.Info("got ShowRoute request", "request", r) + e.Logger.Info("got ShowRoute request") vrf := r.URL.Query().Get("vrf") if vrf == "" { @@ -127,14 +127,14 @@ func (e *Endpoint) ShowRoute(w http.ResponseWriter, r *http.Request) { return } - e.writeResponse(result, w) + e.writeResponse(result, w, "ShowRoute") } // ShowBGP returns a result of show bgp command. // show bgp (vrf ) ipv4/ipv6 unicast (longer-prefixes). // show bgp vrf summary. func (e *Endpoint) ShowBGP(w http.ResponseWriter, r *http.Request) { - e.Logger.Info("got ShowBGP request", "request", r) + e.Logger.Info("got ShowBGP request") vrf := r.URL.Query().Get("vrf") if vrf == "" { vrf = all @@ -163,7 +163,7 @@ func (e *Endpoint) ShowBGP(w http.ResponseWriter, r *http.Request) { return } - e.writeResponse(result, w) + e.writeResponse(result, w, "ShowBGP") } func prepareBGPCommand(r *http.Request, vrf string) ([]string, error) { @@ -214,7 +214,7 @@ func prepareBGPCommand(r *http.Request, vrf string) ([]string, error) { // show evpn mac vni . // show evpn next-hops vni json. func (e *Endpoint) ShowEVPN(w http.ResponseWriter, r *http.Request) { - e.Logger.Info("got ShowEVPN request", "request", r) + e.Logger.Info("got ShowEVPN request") var command []string requestType := r.URL.Query().Get("type") switch requestType { @@ -259,14 +259,14 @@ func (e *Endpoint) ShowEVPN(w http.ResponseWriter, r *http.Request) { return } - e.writeResponse(result, w) + e.writeResponse(result, w, "ShowEVPN") } //+kubebuilder:rbac:groups=core,resources=services,verbs=get // QueryAll - when called, will pass the request to all nodes and return their responses. func (e *Endpoint) QueryAll(w http.ResponseWriter, r *http.Request) { - e.Logger.Info("got QueryAll request", "request", r) + e.Logger.Info("got QueryAll request") service := &corev1.Service{} err := e.c.Get(r.Context(), client.ObjectKey{Name: e.statusSvcName, Namespace: e.statusSvcNamespace}, service) if err != nil { @@ -302,16 +302,16 @@ func (e *Endpoint) QueryAll(w http.ResponseWriter, r *http.Request) { return } - e.writeResponse(&response, w) + e.writeResponse(&response, w, "QueryAll") } -func (e *Endpoint) writeResponse(data *[]byte, w http.ResponseWriter) { +func (e *Endpoint) writeResponse(data *[]byte, w http.ResponseWriter, requestType string) { _, err := w.Write(*data) if err != nil { http.Error(w, "failed to write response: "+err.Error(), http.StatusInternalServerError) return } - e.Logger.Info("response written", "data", *data) + e.Logger.Info("response written", "type", requestType) } func setLongerPrefixes(r *http.Request, command *[]string) error { From 71cb8625786290b92675d404661cfc0aa722a327 Mon Sep 17 00:00:00 2001 From: Patryk Strusiewicz-Surmacki Date: Fri, 19 Apr 2024 19:13:37 +0200 Subject: [PATCH 33/35] Joined monitoring endpoint HTTP server and frr collector HTTP server together Signed-off-by: Patryk Strusiewicz-Surmacki --- Makefile | 17 +------ cmd/frr-exporter/main.go | 74 +++++++++++++++++++++++++----- cmd/monitoring/main.go | 63 ------------------------- config/manager/kustomization.yaml | 3 -- config/manager/manager.yaml | 30 ------------ config/manager/manager_master.yaml | 30 ------------ monitoring.Dockerfile | 33 ------------- 7 files changed, 64 insertions(+), 186 deletions(-) delete mode 100644 cmd/monitoring/main.go delete mode 100644 monitoring.Dockerfile diff --git a/Makefile b/Makefile index 49af1d46..061715fe 100644 --- a/Makefile +++ b/Makefile @@ -3,8 +3,6 @@ IMG ?= ghcr.io/telekom/das-schiff-network-operator:latest # Sidecar image URL to use all building/pushing image targets SIDECAR_IMG ?= ghcr.io/telekom/frr-exporter:latest -# Monitoring image URL to use all building/pushing image targets -MONITORING_IMG ?= ghcr.io/telekom/monitoring:latest # ENVTEST_K8S_VERSION refers to the version of kubebuilder assets to be downloaded by envtest binary. ENVTEST_K8S_VERSION = 1.25 @@ -73,14 +71,10 @@ test: manifests generate fmt vet envtest ## Run tests. build: generate fmt vet ## Build manager binary. go build -o bin/manager cmd/manager/main.go -.PHONY: build +.PHONY: sidecar-build sidecar-build: build go build -o bin/frr-exporter cmd/frr-exporter/main.go -.PHONY: build -monitoring-build: build - go build -o bin/monitoring cmd/monitoring/main.go - .PHONY: run run: manifests generate fmt vet ## Run a controller from your host. go run ./cmd/manager/main.go @@ -93,10 +87,6 @@ docker-build: test ## Build docker image with the manager. docker-build-sidecar: test ## Build docker image with the manager. docker build -t ${SIDECAR_IMG} -f frr-exporter.Dockerfile . -.PHONY: docker-build-monitoring -docker-build-monitoring: test ## Build docker image with the manager. - docker build -t ${MONITORING_IMG} -f monitoring.Dockerfile . - .PHONY: docker-push docker-push: ## Push docker image with the manager. docker push ${IMG} @@ -105,10 +95,6 @@ docker-push: ## Push docker image with the manager. docker-push-sidecar: ## Push docker image with the manager. docker push ${SIDECAR_IMG} -.PHONY: docker-push-monitoring -docker-push-monitoring: ## Push docker image with the manager. - docker push ${MONITORING_IMG} - ##@ Release @@ -149,7 +135,6 @@ uninstall-certs: manifests kustomize ## Uninstall certs deploy: manifests kustomize ## Deploy controller to the K8s cluster specified in ~/.kube/config. cd config/manager && $(KUSTOMIZE) edit set image controller=${IMG} cd config/manager && $(KUSTOMIZE) edit set image frr-exporter=${SIDECAR_IMG} - cd config/manager && $(KUSTOMIZE) edit set image monitoring=${MONITORING_IMG} $(KUSTOMIZE) build config/default | kubectl apply -f - .PHONY: undeploy diff --git a/cmd/frr-exporter/main.go b/cmd/frr-exporter/main.go index bc821dfe..4a36a769 100644 --- a/cmd/frr-exporter/main.go +++ b/cmd/frr-exporter/main.go @@ -10,8 +10,10 @@ import ( "github.com/prometheus/client_golang/prometheus" "github.com/prometheus/client_golang/prometheus/collectors" "github.com/prometheus/client_golang/prometheus/promhttp" + "github.com/telekom/das-schiff-network-operator/pkg/frr" "github.com/telekom/das-schiff-network-operator/pkg/monitoring" ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/log/zap" ) @@ -19,6 +21,10 @@ const ( twenty = 20 ) +var ( + setupLog = ctrl.Log.WithName("setup") +) + func main() { var addr string flag.StringVar(&addr, "listen-address", ":7082", "The address to listen on for HTTP requests.") @@ -29,6 +35,42 @@ func main() { flag.Parse() ctrl.SetLogger(zap.New(zap.UseFlagOptions(&opts))) + // Setup a new registry. + reg, err := setupPrometheusRegistry() + if err != nil { + log.Fatal(fmt.Errorf("prometheus registry setup error: %w", err)) + } + + setupLog.Info("configured Prometheus registry") + + endpoint, err := setupMonitoringEndpoint() + if err != nil { + log.Fatal(fmt.Errorf("error configuring monitoring endpoint: %w", err)) + } + + setupLog.Info("configured monitoring endpoint") + + // Expose the registered metrics and monitoring endpoint via HTTP. + mux := setupMux(reg, endpoint) + + server := http.Server{ + Addr: addr, + ReadHeaderTimeout: twenty * time.Second, + ReadTimeout: time.Minute, + Handler: mux, + } + + setupLog.Info("created server, starting...", "Addr", server.Addr, + "ReadHeaderTimeout", server.ReadHeaderTimeout, "ReadTimeout", server.ReadTimeout) + + // Run server + err = server.ListenAndServe() + if err != nil { + log.Fatal(fmt.Errorf("failed to start server: %w", err)) + } +} + +func setupPrometheusRegistry() (*prometheus.Registry, error) { // Create a new registry. reg := prometheus.NewRegistry() @@ -42,12 +84,15 @@ func main() { "bpf": false, }) if err != nil { - log.Fatal(fmt.Errorf("failed to create collector %w", err)) + return nil, fmt.Errorf("failed to create collector %w", err) } reg.MustRegister(collector) - // Expose the registered metrics via HTTP. - mux := http.NewServeMux() + return reg, nil +} + +func setupMux(reg *prometheus.Registry, e *monitoring.Endpoint) *http.ServeMux { + mux := e.CreateMux() mux.Handle("/metrics", promhttp.HandlerFor( reg, promhttp.HandlerOpts{ @@ -56,16 +101,23 @@ func main() { Timeout: time.Minute, }, )) + return mux +} - server := http.Server{ - Addr: addr, - ReadHeaderTimeout: twenty * time.Second, - ReadTimeout: time.Minute, - Handler: mux, +func setupMonitoringEndpoint() (*monitoring.Endpoint, error) { + clientConfig := ctrl.GetConfigOrDie() + c, err := client.New(clientConfig, client.Options{}) + if err != nil { + return nil, fmt.Errorf("error creating controller-runtime client: %w", err) } - err = server.ListenAndServe() - // Run server + + setupLog.Info("loaded kubernetes config") + + svcName, svcNamespace, err := monitoring.GetStatusServiceConfig() if err != nil { - log.Fatal(fmt.Errorf("failed to start server: %w", err)) + return nil, fmt.Errorf("error getting status service info: %w", err) } + setupLog.Info("loaded status service config") + + return monitoring.NewEndpoint(c, frr.NewCli(), svcName, svcNamespace), nil } diff --git a/cmd/monitoring/main.go b/cmd/monitoring/main.go deleted file mode 100644 index 0c614200..00000000 --- a/cmd/monitoring/main.go +++ /dev/null @@ -1,63 +0,0 @@ -package main - -import ( - "flag" - "fmt" - "log" - "net/http" - "time" - - "github.com/telekom/das-schiff-network-operator/pkg/frr" - "github.com/telekom/das-schiff-network-operator/pkg/monitoring" - ctrl "sigs.k8s.io/controller-runtime" - "sigs.k8s.io/controller-runtime/pkg/client" - "sigs.k8s.io/controller-runtime/pkg/log/zap" -) - -const ( - twenty = 20 -) - -func main() { - setupLog := ctrl.Log.WithName("setup") - var addr string - flag.StringVar(&addr, "listen-address", ":7083", "The address to listen on for HTTP requests.") - opts := zap.Options{ - Development: true, - } - opts.BindFlags(flag.CommandLine) - flag.Parse() - ctrl.SetLogger(zap.New(zap.UseFlagOptions(&opts))) - - clientConfig := ctrl.GetConfigOrDie() - c, err := client.New(clientConfig, client.Options{}) - if err != nil { - log.Fatal(fmt.Errorf("error creating controller-runtime client: %w", err)) - } - - setupLog.Info("loaded kubernetes config") - - svcName, svcNamespace, err := monitoring.GetStatusServiceConfig() - if err != nil { - log.Fatal(fmt.Errorf("error getting status service info: %w", err)) - } - setupLog.Info("loaded status service config") - - endpoint := monitoring.NewEndpoint(c, frr.NewCli(), svcName, svcNamespace) - - server := http.Server{ - Addr: addr, - ReadHeaderTimeout: twenty * time.Second, - ReadTimeout: time.Minute, - Handler: endpoint.CreateMux(), - } - - setupLog.Info("created server, starting...", "Addr", server.Addr, - "ReadHeaderTimeout", server.ReadHeaderTimeout, "ReadTimeout", server.ReadTimeout) - - err = server.ListenAndServe() - // Run server - if err != nil { - log.Fatal(fmt.Errorf("failed to start server: %w", err)) - } -} diff --git a/config/manager/kustomization.yaml b/config/manager/kustomization.yaml index a0fa3d1e..e30e1a4f 100644 --- a/config/manager/kustomization.yaml +++ b/config/manager/kustomization.yaml @@ -23,6 +23,3 @@ images: - name: frr-exporter newName: ghcr.io/telekom/frr-exporter newTag: latest -- name: monitoring - newName: ghcr.io/telekom/monitoring - newTag: latest diff --git a/config/manager/manager.yaml b/config/manager/manager.yaml index 690054c1..16497def 100644 --- a/config/manager/manager.yaml +++ b/config/manager/manager.yaml @@ -108,36 +108,6 @@ spec: name: frr-config - mountPath: /var/run/frr name: frr-run - - command: - - /monitoring - env: - - name: NODE_NAME - valueFrom: - fieldRef: - fieldPath: spec.nodeName - - name: STATUS_SVC_NAME - value: network-operator-status - - name: STATUS_SVC_NAMESPACE - valueFrom: - fieldRef: - fieldPath: metadata.namespace - image: monitoring:latest - name: monitoring - securityContext: - privileged: true - runAsUser: 0 - resources: - limits: - cpu: 100m - memory: 64Mi - requests: - cpu: 10m - memory: 64Mi - volumeMounts: - - mountPath: /etc/frr - name: frr-config - - mountPath: /var/run/frr - name: frr-run serviceAccountName: controller-manager terminationGracePeriodSeconds: 10 volumes: diff --git a/config/manager/manager_master.yaml b/config/manager/manager_master.yaml index 4b7df44d..cd3a4917 100644 --- a/config/manager/manager_master.yaml +++ b/config/manager/manager_master.yaml @@ -110,36 +110,6 @@ spec: name: frr-config - mountPath: /var/run/frr name: frr-run - - command: - - /monitoring - env: - - name: NODE_NAME - valueFrom: - fieldRef: - fieldPath: spec.nodeName - - name: STATUS_SVC_NAME - value: network-operator-status - - name: STATUS_SVC_NAMESPACE - valueFrom: - fieldRef: - fieldPath: metadata.namespace - image: monitoring:latest - name: monitoring - securityContext: - privileged: true - runAsUser: 0 - resources: - limits: - cpu: 100m - memory: 64Mi - requests: - cpu: 10m - memory: 64Mi - volumeMounts: - - mountPath: /etc/frr - name: frr-config - - mountPath: /var/run/frr - name: frr-run serviceAccountName: controller-manager terminationGracePeriodSeconds: 10 volumes: diff --git a/monitoring.Dockerfile b/monitoring.Dockerfile deleted file mode 100644 index 18c196fb..00000000 --- a/monitoring.Dockerfile +++ /dev/null @@ -1,33 +0,0 @@ -ARG FRR_VERSION="9.1.0" -ARG REGISTRY="quay.io" -# Build the manager binary -FROM docker.io/library/golang:1.21-alpine as builder - -WORKDIR /workspace -# Copy the Go Modules manifests -COPY go.mod go.mod -COPY go.sum go.sum -# cache deps before building and copying source so that we don't need to re-download as much -# and so that source changes don't invalidate our downloaded layer -RUN go mod download - -# Copy the go source -COPY cmd/monitoring/main.go main.go -COPY pkg/ pkg/ - -# Build -RUN CGO_ENABLED=0 GOOS=linux GOARCH=amd64 go build -a -o monitoring main.go - - -FROM ${REGISTRY}/frrouting/frr:${FRR_VERSION} - -RUN apk add --no-cache frr - -WORKDIR / -COPY --from=builder /workspace/monitoring . -## Needs to run as root -## vtysh is required to have extended rights -## to be able to connect to vty sockets on the host -# USER 65532:65532 - -ENTRYPOINT ["/monitoring"] From 0ba752b0ad498317a52064e522aaa1c125bbf2db Mon Sep 17 00:00:00 2001 From: Patryk Strusiewicz-Surmacki Date: Tue, 30 Apr 2024 12:47:10 +0200 Subject: [PATCH 34/35] Fixed validation Signed-off-by: Patryk Strusiewicz-Surmacki --- pkg/monitoring/endpoint.go | 31 ++++++++--- pkg/monitoring/endpoint_test.go | 98 ++++++++++++++++++++++++--------- 2 files changed, 96 insertions(+), 33 deletions(-) diff --git a/pkg/monitoring/endpoint.go b/pkg/monitoring/endpoint.go index 39d1d830..507b8ae4 100644 --- a/pkg/monitoring/endpoint.go +++ b/pkg/monitoring/endpoint.go @@ -31,7 +31,10 @@ const ( StatusSvcNamespaceEnv = "STATUS_SVC_NAMESPACE" ) -var validation = regexp.MustCompile("^[a-zA-Z0-9_]") +var ( + validVRF = regexp.MustCompile("^[a-zA-Z0-9-_.]+$") + validVNI = regexp.MustCompile("^[0-9]{0,8}$") +) //go:generate mockgen -destination ./mock/mock_endpoint.go . FRRClient type FRRClient interface { @@ -81,7 +84,7 @@ func (e *Endpoint) ShowRoute(w http.ResponseWriter, r *http.Request) { vrf = all } - if !validation.MatchString(vrf) { + if !validVRF.MatchString(vrf) { e.Logger.Error(fmt.Errorf("invalid VRF value"), "error validating value") http.Error(w, "invalid VRF value", http.StatusBadRequest) return @@ -140,7 +143,7 @@ func (e *Endpoint) ShowBGP(w http.ResponseWriter, r *http.Request) { vrf = all } - if !validation.MatchString(vrf) { + if !validVRF.MatchString(vrf) { e.Logger.Error(fmt.Errorf("invalid VRF value"), "error validating value") http.Error(w, "invalid VRF value", http.StatusBadRequest) return @@ -228,10 +231,8 @@ func (e *Endpoint) ShowEVPN(w http.ResponseWriter, r *http.Request) { vni := r.URL.Query().Get("vni") if vni == "" { vni = all - } - - if !validation.MatchString(vni) { - e.Logger.Error(fmt.Errorf("invalid VNI value"), "error validating value") + } else if err := validateVNI(vni); err != nil { + e.Logger.Error(fmt.Errorf("invalid VNI value: %w", err), "error validating value") http.Error(w, "invalid VNI value", http.StatusBadRequest) return } @@ -262,6 +263,22 @@ func (e *Endpoint) ShowEVPN(w http.ResponseWriter, r *http.Request) { e.writeResponse(result, w, "ShowEVPN") } +func validateVNI(vni string) error { + if !validVNI.MatchString(vni) { + return fmt.Errorf("VNI does not match regular expression") + } + value, err := strconv.Atoi(vni) + if err != nil { + return fmt.Errorf("VNI cannot be paresd to int: %w", err) + } + + if uint(value) > uint(1<<24) { + return fmt.Errorf("VNI is not a valid 24-bit number") + } + + return nil +} + //+kubebuilder:rbac:groups=core,resources=services,verbs=get // QueryAll - when called, will pass the request to all nodes and return their responses. diff --git a/pkg/monitoring/endpoint_test.go b/pkg/monitoring/endpoint_test.go index 252ab645..13e163c2 100644 --- a/pkg/monitoring/endpoint_test.go +++ b/pkg/monitoring/endpoint_test.go @@ -18,6 +18,11 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client/fake" ) +const ( + testSvcName = "svcName" + testSvcNamespace = "svcNamespace" +) + var ( fakePodsJSON = `{ "items": [ @@ -114,8 +119,10 @@ func TestHealthCheck(t *testing.T) { RegisterFailHandler(Fail) mockCtrl = gomock.NewController(t) defer mockCtrl.Finish() + t.Setenv(StatusSvcNameEnv, testSvcName) + t.Setenv(StatusSvcNamespaceEnv, testSvcNamespace) RunSpecs(t, - "HealthCheck Suite") + "Endpoint Suite") } var _ = Describe("Endpoint", func() { @@ -124,40 +131,46 @@ var _ = Describe("Endpoint", func() { e := NewEndpoint(c, fcm, "test-service", "test-namespace") e.CreateMux() - Context("ShowRoute() should", func() { - It("return no error", func() { + Context("ShowRoute()", func() { + It("returns no error", func() { req := httptest.NewRequest(http.MethodGet, "/show/route", http.NoBody) res := httptest.NewRecorder() fcm.EXPECT().ExecuteWithJSON(gomock.Any()).Return([]byte{'{', '}'}) e.ShowRoute(res, req) Expect(res.Code).To(Equal(http.StatusOK)) }) - It("return error if protocol is invalid", func() { + It("returns error if protocol is invalid", func() { req := httptest.NewRequest(http.MethodGet, "/show/route?protocol=ipv42", http.NoBody) res := httptest.NewRecorder() e.ShowRoute(res, req) Expect(res.Code).To(Equal(http.StatusBadRequest)) }) - It("return error if input CIDR is invalid", func() { + It("returns error if input CIDR is invalid", func() { req := httptest.NewRequest(http.MethodGet, "/show/route?protocol=ipv6&input=192.168.1.1/42", http.NoBody) res := httptest.NewRecorder() e.ShowRoute(res, req) Expect(res.Code).To(Equal(http.StatusBadRequest)) }) - It("return error if longer_prefixes value is invalid", func() { + It("returns error if longer_prefixes value is invalid", func() { req := httptest.NewRequest(http.MethodGet, "/show/route?protocol=ipv6&input=192.168.1.1/32&longer_prefixes=notABool", http.NoBody) res := httptest.NewRecorder() e.ShowRoute(res, req) Expect(res.Code).To(Equal(http.StatusBadRequest)) }) - It("return no error", func() { + It("returns error if VRF is invalid", func() { + req := httptest.NewRequest(http.MethodGet, "/show/route?protocol=ipv6&input=192.168.1.1/32&longer_prefixes=true&vrf=invalid$vrf", http.NoBody) + res := httptest.NewRecorder() + e.ShowRoute(res, req) + Expect(res.Code).To(Equal(http.StatusBadRequest)) + }) + It("returns no error", func() { req := httptest.NewRequest(http.MethodGet, "/show/route?protocol=ipv6&input=192.168.1.1/32&longer_prefixes=true", http.NoBody) res := httptest.NewRecorder() fcm.EXPECT().ExecuteWithJSON(gomock.Any()).Return([]byte{'{', '}'}) e.ShowRoute(res, req) Expect(res.Code).To(Equal(http.StatusOK)) }) - It("return no error and add node name to the response if "+healthcheck.NodenameEnv+" env is set", func() { + It("returns no error and add node name to the response if "+healthcheck.NodenameEnv+" env is set", func() { testNodename := "test-nodename" err := os.Setenv(healthcheck.NodenameEnv, testNodename) Expect(err).ToNot(HaveOccurred()) @@ -177,85 +190,110 @@ var _ = Describe("Endpoint", func() { }) }) - Context("ShowBGP() should", func() { - It("return no error if type is not specified (default)", func() { + Context("ShowBGP()", func() { + It("returns no error if type is not specified (default)", func() { req := httptest.NewRequest(http.MethodGet, "/show/bgp", http.NoBody) res := httptest.NewRecorder() fcm.EXPECT().ExecuteWithJSON(gomock.Any()).Return([]byte{'{', '}'}) e.ShowBGP(res, req) Expect(res.Code).To(Equal(http.StatusOK)) }) - It("return no error if type is summary", func() { + It("returns no error if type is summary", func() { req := httptest.NewRequest(http.MethodGet, "/show/bgp?type=summary", http.NoBody) res := httptest.NewRecorder() fcm.EXPECT().ExecuteWithJSON(gomock.Any()).Return([]byte{'{', '}'}) e.ShowBGP(res, req) Expect(res.Code).To(Equal(http.StatusOK)) }) - It("return error if type is invalid", func() { + It("returns error if type is invalid", func() { req := httptest.NewRequest(http.MethodGet, "/show/bgp?type=ivalidType", http.NoBody) res := httptest.NewRecorder() e.ShowBGP(res, req) Expect(res.Code).To(Equal(http.StatusBadRequest)) }) - It("return error if protocol is invalid", func() { + It("returns error if protocol is invalid", func() { req := httptest.NewRequest(http.MethodGet, "/show/bgp?protocol=ipv42", http.NoBody) res := httptest.NewRecorder() e.ShowBGP(res, req) Expect(res.Code).To(Equal(http.StatusBadRequest)) }) - It("return error if input CIDR is invalid", func() { + It("returns error if input CIDR is invalid", func() { req := httptest.NewRequest(http.MethodGet, "/show/bgp?protocol=ipv4&input=192.168.1.1/42", http.NoBody) res := httptest.NewRecorder() e.ShowBGP(res, req) Expect(res.Code).To(Equal(http.StatusBadRequest)) }) - It("return error if longer_prefixes value is invalid", func() { + It("returns error if longer_prefixes value is invalid", func() { req := httptest.NewRequest(http.MethodGet, "/show/bgp?protocol=ipv4&input=192.168.1.1/32&longer_prefixes=notABool", http.NoBody) res := httptest.NewRecorder() e.ShowBGP(res, req) Expect(res.Code).To(Equal(http.StatusBadRequest)) }) + It("returns error if VRF is invalid", func() { + req := httptest.NewRequest(http.MethodGet, "/show/bgp?vrf=invalid$VRF", http.NoBody) + res := httptest.NewRecorder() + e.ShowBGP(res, req) + Expect(res.Code).To(Equal(http.StatusBadRequest)) + }) }) - Context("ShowEVPN() should", func() { - It("return no error if type is not specified (default)", func() { + Context("ShowEVPN()", func() { + It("returns no error if type is not specified (default)", func() { req := httptest.NewRequest(http.MethodGet, "/show/evpn", http.NoBody) res := httptest.NewRecorder() fcm.EXPECT().ExecuteWithJSON(gomock.Any()).Return([]byte{'{', '}'}) e.ShowEVPN(res, req) Expect(res.Code).To(Equal(http.StatusOK)) }) - It("return no error if type is rmac", func() { + It("returns no error if type is rmac", func() { req := httptest.NewRequest(http.MethodGet, "/show/evpn?type=rmac", http.NoBody) res := httptest.NewRecorder() fcm.EXPECT().ExecuteWithJSON(gomock.Any()).Return([]byte{'{', '}'}) e.ShowEVPN(res, req) Expect(res.Code).To(Equal(http.StatusOK)) }) - It("return no error if type is mac", func() { + It("returns no error if type is mac", func() { req := httptest.NewRequest(http.MethodGet, "/show/evpn?type=mac", http.NoBody) res := httptest.NewRecorder() fcm.EXPECT().ExecuteWithJSON(gomock.Any()).Return([]byte{'{', '}'}) e.ShowEVPN(res, req) Expect(res.Code).To(Equal(http.StatusOK)) }) - It("return no error if type is next-hops", func() { + It("returns no error if type is next-hops", func() { req := httptest.NewRequest(http.MethodGet, "/show/evpn?type=next-hops", http.NoBody) res := httptest.NewRecorder() fcm.EXPECT().ExecuteWithJSON(gomock.Any()).Return([]byte{'{', '}'}) e.ShowEVPN(res, req) Expect(res.Code).To(Equal(http.StatusOK)) }) - It("return error if type is invalid", func() { + It("returns error if type is invalid", func() { req := httptest.NewRequest(http.MethodGet, "/show/evpn?type=invalidType", http.NoBody) res := httptest.NewRecorder() e.ShowEVPN(res, req) Expect(res.Code).To(Equal(http.StatusBadRequest)) }) + It("returns error if VNI is invalid", func() { + req := httptest.NewRequest(http.MethodGet, "/show/evpn?type=rmac&vni=invalidVNI", http.NoBody) + res := httptest.NewRecorder() + e.ShowEVPN(res, req) + Expect(res.Code).To(Equal(http.StatusBadRequest)) + }) + It("returns error if VNI value is bigger than 24bit uint", func() { + req := httptest.NewRequest(http.MethodGet, "/show/evpn?type=rmac&vni=96777215", http.NoBody) + res := httptest.NewRecorder() + e.ShowEVPN(res, req) + Expect(res.Code).To(Equal(http.StatusBadRequest)) + }) + It("returns error no error if VNI is valid", func() { + req := httptest.NewRequest(http.MethodGet, "/show/evpn?type=rmac&vni=42", http.NoBody) + res := httptest.NewRecorder() + fcm.EXPECT().ExecuteWithJSON(gomock.Any()).Return([]byte{'{', '}'}) + e.ShowEVPN(res, req) + Expect(res.Code).To(Equal(http.StatusOK)) + }) }) - Context("PassRequest() should", func() { - It("return error if there are no instances to query", func() { + Context("PassRequest()", func() { + It("returns error if there are no instances to query", func() { c := fake.NewClientBuilder().WithRuntimeObjects(fakePods, fakeServices).Build() e := NewEndpoint(c, fcm, "test-service-no-endpoints", "test-namespace") req := httptest.NewRequest(http.MethodGet, "/all/show/route", http.NoBody) @@ -264,7 +302,7 @@ var _ = Describe("Endpoint", func() { Expect(res.Code).To(Equal(http.StatusInternalServerError)) }) - It("return error if cannot get data from the endpoint", func() { + It("returns error if cannot get data from the endpoint", func() { c := fake.NewClientBuilder().WithRuntimeObjects(fakePods, fakeServices).Build() e := NewEndpoint(c, fcm, "test-service", "test-namespace") req := httptest.NewRequest(http.MethodGet, "/all/show/route", http.NoBody) @@ -272,7 +310,7 @@ var _ = Describe("Endpoint", func() { e.QueryAll(res, req) Expect(res.Code).To(Equal(http.StatusInternalServerError)) }) - It("return error if request was properly passed to the endpoint but the response is malformed", func() { + It("returns error if request was properly passed to the endpoint but the response is malformed", func() { svr := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { fmt.Fprintf(w, "invalidJson") })) @@ -286,7 +324,7 @@ var _ = Describe("Endpoint", func() { e.QueryAll(res, req) Expect(res.Code).To(Equal(http.StatusInternalServerError)) }) - It("return no error if request was properly passed to the endpoint", func() { + It("returns no error if request was properly passed to the endpoint", func() { svr := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { fmt.Fprintf(w, "{}") })) @@ -301,4 +339,12 @@ var _ = Describe("Endpoint", func() { Expect(res.Code).To(Equal(http.StatusOK)) }) }) + Context("GetStatusServiceConfig()", func() { + It("returns no error if envs are set", func() { + name, namespace, err := GetStatusServiceConfig() + Expect(err).ToNot(HaveOccurred()) + Expect(name).To(Equal(testSvcName)) + Expect(namespace).To(Equal(testSvcNamespace)) + }) + }) }) From 1827b804395fb0eabba3c88cefd632635c4781b5 Mon Sep 17 00:00:00 2001 From: Patryk Strusiewicz-Surmacki Date: Tue, 30 Apr 2024 12:52:34 +0200 Subject: [PATCH 35/35] Fixed linter issue Signed-off-by: Patryk Strusiewicz-Surmacki --- pkg/monitoring/endpoint.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/pkg/monitoring/endpoint.go b/pkg/monitoring/endpoint.go index 507b8ae4..23efebe4 100644 --- a/pkg/monitoring/endpoint.go +++ b/pkg/monitoring/endpoint.go @@ -29,6 +29,8 @@ const ( StatusSvcNameEnv = "STATUS_SVC_NAME" StatusSvcNamespaceEnv = "STATUS_SVC_NAMESPACE" + + vniBitLength = 24 ) var ( @@ -272,7 +274,7 @@ func validateVNI(vni string) error { return fmt.Errorf("VNI cannot be paresd to int: %w", err) } - if uint(value) > uint(1<<24) { + if uint(value) > uint(1<