From 3006ff6e32ab374a14ada4e3e3afaac69caae377 Mon Sep 17 00:00:00 2001 From: Shane Unruh <87081771+shunr-hpe@users.noreply.github.com> Date: Tue, 4 Jan 2022 12:28:25 -0700 Subject: [PATCH] Fixed the slow rest api, get boot parameters (#42) Made the following improvements when getting boot parameters by name, nid, or mac. - Changed to first look up the name directly in the etcd data. In most cases the request is going to be for an x name and so this check will be the fastest way to satisfy the request. - Changed to use the values it gets in its initial getting of etcd data. The previous implementation was only using the names, and then re-getting the value on a per name case. - Changed to only get the kernel and initrd data once per http request. - Added an "ID to SMComponent" map for the cached state manager data. The previous implementation was looping through this data for each boot parameter entry. Jira: CASMHMS-4540 --- .version | 2 +- CHANGELOG.md | 6 ++ cmd/boot-script-service/boot_data.go | 81 ++++++++++++++++++-- cmd/boot-script-service/default_api.go | 102 ++++++++++++++++--------- cmd/boot-script-service/sm.go | 38 +++++++-- 5 files changed, 180 insertions(+), 49 deletions(-) diff --git a/.version b/.version index feaae22..850e742 100644 --- a/.version +++ b/.version @@ -1 +1 @@ -1.13.0 +1.14.0 diff --git a/CHANGELOG.md b/CHANGELOG.md index c40e4cd..ac92cce 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,12 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [1.14.0] - 2021-12-22 + +### Changed + +- CASMHMS-4540 Improved the performance of getting the bootparamers by name, nid, and mac. + ## [1.13.0] - 2021-10-26 ### Added diff --git a/cmd/boot-script-service/boot_data.go b/cmd/boot-script-service/boot_data.go index c280c89..5b8cd85 100644 --- a/cmd/boot-script-service/boot_data.go +++ b/cmd/boot-script-service/boot_data.go @@ -32,10 +32,6 @@ package main import ( "encoding/json" "fmt" - base "github.com/Cray-HPE/hms-base" - "github.com/Cray-HPE/hms-bss/pkg/bssTypes" - hmetcd "github.com/Cray-HPE/hms-hmetcd" - jsonpatch "github.com/evanphx/json-patch" "hash/fnv" "log" "net/http" @@ -44,6 +40,11 @@ import ( "strings" "sync" "time" + + base "github.com/Cray-HPE/hms-base" + "github.com/Cray-HPE/hms-bss/pkg/bssTypes" + hmetcd "github.com/Cray-HPE/hms-hmetcd" + jsonpatch "github.com/evanphx/json-patch" ) const ( @@ -656,7 +657,7 @@ func getAccessesForPrefix(prefix string) (accesses []bssTypes.EndpointAccess, er } newAccess := bssTypes.EndpointAccess{ - Name: name, + Name: name, Endpoint: bssTypes.EndpointType(endpoint), LastEpoch: lastEpoch, } @@ -720,6 +721,18 @@ func getTags() ([]hmetcd.Kvi_KV, error) { return kvstore.GetRange(paramsPfx+keyMin, paramsPfx+keyMax) } +func GetNamesAndValues() map[string]string { + kvl, err := getTags() + m := make(map[string]string) + if err == nil { + for _, x := range kvl { + name := extractParamName(x) + m[name] = x.Value + } + } + return m +} + func GetNames() (ret []string) { kvl, err := getTags() if err == nil { @@ -730,6 +743,16 @@ func GetNames() (ret []string) { return ret } +func LookupBootData(name string) (BootData, error) { + var bd BootData + bds, err := lookupHost(name) + if err != nil { + return bd, err + } + bd = bdConvert(bds) + return bd, nil +} + func lookupHost(name string) (BootDataStore, error) { key := paramsPfx + name val, exists, err := kvstore.Get(key) @@ -787,6 +810,34 @@ func lookup(name, altName, role, defaultTag string) BootData { return bd } +func bdConvertUsingImageCache(bds BootDataStore, kernelImages map[string]ImageData, initrdImages map[string]ImageData) (ret BootData) { + ret.Params = bds.Params + ret.CloudInit = bds.CloudInit + if bds.Kernel != "" { + if value, ok := kernelImages[bds.Kernel]; ok { + ret.Kernel = value + } else { + imdata, err := getImage(bds.Kernel, "") + if err == nil { + ret.Kernel = imdata + kernelImages[bds.Kernel] = imdata + } + } + } + if bds.Initrd != "" { + if value, ok := initrdImages[bds.Initrd]; ok { + ret.Initrd = value + } else { + imdata, err := getImage(bds.Initrd, "") + if err == nil { + ret.Initrd = imdata + initrdImages[bds.Initrd] = imdata + } + } + } + return ret +} + func bdConvert(bds BootDataStore) (ret BootData) { ret.Params = bds.Params ret.CloudInit = bds.CloudInit @@ -819,6 +870,26 @@ func LookupGlobalData() (BootData, error) { return LookupByRole(GlobalTag) } +func LookupComponentByName(name string) SMComponent { + comp, _ := FindSMCompByNameInCache(name) + return comp +} + +func ToBootData(value string, kernelImages map[string]ImageData, initrdImages map[string]ImageData) (BootData, error) { + var bds BootDataStore + err := json.Unmarshal([]byte(value), &bds) + var bd BootData + if err != nil { + msg := fmt.Sprintf("Error parsing %s: %v", value, err) + herr := base.NewHMSError("Storage", msg) + herr.AddProblem(base.NewProblemDetailsStatus(msg, http.StatusNotFound)) + err = herr + } else { + bd = bdConvertUsingImageCache(bds, kernelImages, initrdImages) + } + return bd, err +} + func LookupByName(name string) (BootData, SMComponent) { comp_name := name comp, ok := FindSMCompByName(name) diff --git a/cmd/boot-script-service/default_api.go b/cmd/boot-script-service/default_api.go index fd01c04..54cc242 100644 --- a/cmd/boot-script-service/default_api.go +++ b/cmd/boot-script-service/default_api.go @@ -42,9 +42,6 @@ import ( "crypto/tls" "encoding/json" "fmt" - base "github.com/Cray-HPE/hms-base" - "github.com/Cray-HPE/hms-bss/pkg/bssTypes" - hms_s3 "github.com/Cray-HPE/hms-s3" "io/ioutil" "log" "net/http" @@ -53,6 +50,10 @@ import ( "strconv" "strings" "time" + + base "github.com/Cray-HPE/hms-base" + "github.com/Cray-HPE/hms-bss/pkg/bssTypes" + hms_s3 "github.com/Cray-HPE/hms-s3" ) const ( @@ -239,44 +240,72 @@ func BootparametersGet(w http.ResponseWriter, r *http.Request) { } } } - names := GetNames() - for _, name := range names { - bd, smc := LookupByName(name) - debugf("Found %s: %v | %v\n", name, bd, smc) - var bp bssTypes.BootParams - ok := false - for _, v := range args.Hosts { - if v == smc.ID || v == smc.Fqdn || v == name { - ok = true - break - } + var unfoundHosts []string + for _, v := range args.Hosts { + bd, err := LookupBootData(v) + if err == nil { + var bp bssTypes.BootParams + bp.Hosts = append(bp.Hosts, v) + bp.Params = bd.Params + bp.Kernel = bd.Kernel.Path + bp.Initrd = bd.Initrd.Path + bp.CloudInit = bd.CloudInit + results = append(results, bp) + } else { + unfoundHosts = append(unfoundHosts, v) } - if !ok { - Outer: - for _, v := range args.Macs { - for _, m := range smc.Mac { - if strings.EqualFold(v, m) { - ok = true - break Outer - } - } + } + args.Hosts = unfoundHosts + + if len(args.Hosts) > 0 || len(args.Macs) > 0 || len(args.Nids) > 0 { + + nameValues := GetNamesAndValues() + + kernelImages := make(map[string]ImageData) + initrdImages := make(map[string]ImageData) + for name, value := range nameValues { + smc := LookupComponentByName(name) + bd, parseErr := ToBootData(value, kernelImages, initrdImages) + if parseErr != nil { + log.Printf("Failed to parse etcd value for %s: %v\n", name, parseErr) } - } - if !ok { - for _, v := range args.Nids { - if nid, err := smc.NID.Int64(); err == nil && int64(v) == nid { + + debugf("Found %s: %v | %v\n", name, bd, smc) + var bp bssTypes.BootParams + ok := false + for _, v := range args.Hosts { + if v == smc.ID || v == smc.Fqdn || v == name { ok = true break } } - } - if ok { - bp.Hosts = append(bp.Hosts, name) - bp.Params = bd.Params - bp.Kernel = bd.Kernel.Path - bp.Initrd = bd.Initrd.Path - bp.CloudInit = bd.CloudInit - results = append(results, bp) + if !ok { + Outer: + for _, v := range args.Macs { + for _, m := range smc.Mac { + if strings.EqualFold(v, m) { + ok = true + break Outer + } + } + } + } + if !ok { + for _, v := range args.Nids { + if nid, err := smc.NID.Int64(); err == nil && int64(v) == nid { + ok = true + break + } + } + } + if ok { + bp.Hosts = append(bp.Hosts, name) + bp.Params = bd.Params + bp.Kernel = bd.Kernel.Path + bp.Initrd = bd.Initrd.Path + bp.CloudInit = bd.CloudInit + results = append(results, bp) + } } } if results == nil { @@ -313,7 +342,6 @@ func BootparametersGet(w http.ResponseWriter, r *http.Request) { } return } - debugf("Retreived names: %v", names) w.Header().Set("Content-Type", "application/json; charset=UTF-8") w.WriteHeader(http.StatusOK) err = json.NewEncoder(w).Encode(results) @@ -512,7 +540,7 @@ func buildBootScript(bd BootData, sp scriptParams, chain, descr string) (string, // Check for special boot parameters. params = checkParam(params, "xname=", sp.xname) params = checkParam(params, "nid=", sp.nid) - // Inject the cloud init address info into the kernal params. If the target + // Inject the cloud init address info into the kernel params. If the target // image does not have cloud-init enabled this wont hurt anything. // If it does, it tells it to come back to us for the cloud-init meta-data params = checkParam(params, "ds=", fmt.Sprintf("nocloud-net;s=%s/", advertiseAddress)) diff --git a/cmd/boot-script-service/sm.go b/cmd/boot-script-service/sm.go index 274b115..93c5c22 100644 --- a/cmd/boot-script-service/sm.go +++ b/cmd/boot-script-service/sm.go @@ -34,9 +34,6 @@ import ( "crypto/tls" "encoding/json" "fmt" - base "github.com/Cray-HPE/hms-base" - "github.com/Cray-HPE/hms-smd/pkg/redfish" - "github.com/Cray-HPE/hms-smd/pkg/sm" "io/ioutil" "log" "net" @@ -46,6 +43,10 @@ import ( "strings" "sync" "time" + + base "github.com/Cray-HPE/hms-base" + rf "github.com/Cray-HPE/hms-smd/pkg/redfish" + "github.com/Cray-HPE/hms-smd/pkg/sm" ) const badMAC = "not available" @@ -67,11 +68,20 @@ var ( smMutex sync.Mutex smData *SMData smClient *http.Client + smDataMap map[string]SMComponent smBaseURL string smJSONFile string smTimeStamp int64 ) +func makeSmMap(state *SMData) map[string]SMComponent { + m := make(map[string]SMComponent) + for _, v := range state.Components { + m[v.ID] = v + } + return m +} + func SmOpen(base, options string) error { u, err := url.Parse(base) if err != nil { @@ -91,6 +101,7 @@ func SmOpen(base, options string) error { debugf("Internal data conversion failure: %v", err) } smData = &comps + smDataMap = makeSmMap(smData) return nil } if u.Scheme == "file" { @@ -352,7 +363,7 @@ func getStateInfo() (ret *SMData) { return ret } -func protectedGetState(ts int64) *SMData { +func protectedGetState(ts int64) (*SMData, map[string]SMComponent) { smMutex.Lock() defer smMutex.Unlock() if ts < 0 || ts > smTimeStamp || smData == nil { @@ -364,17 +375,24 @@ func protectedGetState(ts int64) *SMData { newSMData := getStateInfo() if newSMData != nil { smData = newSMData + smDataMap = makeSmMap(smData) } } - return smData + return smData, smDataMap } func getState() *SMData { + data, _ := protectedGetState(0) + return data +} + +func getStateAndMap() (*SMData, map[string]SMComponent) { return protectedGetState(0) } func refreshState(ts int64) *SMData { - return protectedGetState(ts) + data, _ := protectedGetState(ts) + return data } func FindSMCompByMAC(mac string) (SMComponent, bool) { @@ -391,6 +409,14 @@ func FindSMCompByMAC(mac string) (SMComponent, bool) { return SMComponent{}, false } +func FindSMCompByNameInCache(host string) (SMComponent, bool) { + _, stateMap := getStateAndMap() + if v, ok := stateMap[host]; ok { + return v, true + } + return SMComponent{}, false +} + func FindSMCompByName(host string) (SMComponent, bool) { debugf("Searching SM data for %s\n", host) state := getState()