From 33f0e61203d72782e2d10e4ea28c49764cf1a9b4 Mon Sep 17 00:00:00 2001 From: Ralph Schmieder Date: Fri, 7 Jun 2024 19:37:03 +0200 Subject: [PATCH 1/7] Use slog --- apiclient.go | 6 +- auth.go | 4 +- cmd/ctest/main.go | 64 ++- go.mod | 3 +- go.sum | 2 + iface.go | 2 + lab.go | 26 +- system.go | 10 +- vendor/github.com/lmittmann/tint/LICENSE | 21 + vendor/github.com/lmittmann/tint/README.md | 88 ++++ vendor/github.com/lmittmann/tint/buffer.go | 46 ++ vendor/github.com/lmittmann/tint/handler.go | 438 ++++++++++++++++++++ vendor/modules.txt | 3 + 13 files changed, 683 insertions(+), 30 deletions(-) create mode 100644 vendor/github.com/lmittmann/tint/LICENSE create mode 100644 vendor/github.com/lmittmann/tint/README.md create mode 100644 vendor/github.com/lmittmann/tint/buffer.go create mode 100644 vendor/github.com/lmittmann/tint/handler.go diff --git a/apiclient.go b/apiclient.go index 3d335ca..e80bf1d 100644 --- a/apiclient.go +++ b/apiclient.go @@ -6,7 +6,7 @@ import ( "errors" "fmt" "io" - "log" + "log/slog" "net/http" "net/url" "strings" @@ -64,7 +64,7 @@ func (c *Client) doAPI(ctx context.Context, req *http.Request, depth int32) ([]b } if c.state.get() != stateAuthenticated && c.authRequired(req.URL) { - log.Println("needs auth") + slog.Info("needs auth") c.state.set(stateAuthenticating) if err := c.jsonGet(ctx, authokAPI, nil, depth); err != nil { return nil, err @@ -102,7 +102,7 @@ retry: if res.StatusCode == http.StatusUnauthorized { invalid_token := len(c.apiToken) > 0 c.apiToken = "" - log.Println("need to authenticate") + slog.Info("need to authenticate") c.state.set(stateAuthRequired) if !c.userpass.valid() { errmsg := "no credentials provided" diff --git a/auth.go b/auth.go index 530bd72..d359272 100644 --- a/auth.go +++ b/auth.go @@ -6,7 +6,7 @@ import ( "crypto/x509" "encoding/json" "errors" - "log" + "log/slog" "net/http" "net/url" "strconv" @@ -56,7 +56,7 @@ func (c *Client) authenticate(ctx context.Context, userpass userPass, depth int3 if err != nil { return err } - log.Printf("user id %s, is admin: %s", auth.ID, strconv.FormatBool(auth.Admin)) + slog.Info("user auth", "id", auth.ID, "is_admin", strconv.FormatBool(auth.Admin)) c.apiToken = auth.Token return nil } diff --git a/cmd/ctest/main.go b/cmd/ctest/main.go index 271ea3a..8edf106 100644 --- a/cmd/ctest/main.go +++ b/cmd/ctest/main.go @@ -2,23 +2,36 @@ package main import ( "context" + "encoding/json" "errors" - "log" + "fmt" + "log/slog" "os" + "time" + "github.com/lmittmann/tint" cmlclient "github.com/rschmied/gocmlclient" ) func main() { + // set global logger with custom options + slog.SetDefault(slog.New( + tint.NewHandler(os.Stderr, &tint.Options{ + AddSource: true, + Level: slog.LevelDebug, + TimeFormat: time.RFC822, + }), + )) + // address and lab id host, found := os.LookupEnv("CML_HOST") if !found { - log.Println("CML_HOST env var not found!") + slog.Error("CML_HOST env var not found!") return } // labID, found := os.LookupEnv("CML_LABID") // if !found { - // log.Println("CML_LABID env var not found!") + // slog.Error("CML_LABID env var not found!") // return // } // _ = labID @@ -28,11 +41,11 @@ func main() { password, pass_found := os.LookupEnv("CML_PASSWORD") token, token_found := os.LookupEnv("CML_TOKEN") if !(token_found || (user_found && pass_found)) { - log.Println("either CML_TOKEN or CML_USERNAME and CML_PASSWORD env vars must be present!") + slog.Error("either CML_TOKEN or CML_USERNAME and CML_PASSWORD env vars must be present!") return } ctx := context.Background() - client := cmlclient.New(host, true, false) + client := cmlclient.New(host, false, false) // if err := client.Ready(ctx); err != nil { // log.Fatal(err) // } @@ -86,16 +99,49 @@ func main() { // result, err := client.UserGroups(ctx, "cc42bd56-1dc6-445c-b7e7-569b0a8b0c94") err := client.Ready(ctx) if errors.Is(err, cmlclient.ErrSystemNotReady) { - log.Println("it is not ready") + slog.Error("it is not ready") + return } if err != nil && !errors.Is(err, cmlclient.ErrSystemNotReady) { - log.Println(err) + slog.Error("ready", slog.Any("error", err)) + return + } + node := &cmlclient.Node{ + // ID: "28ec08ec-483a-415a-a3ed-625b0d45bef0", + // ID: "8116a609-8b68-4e0f-a196-5225da9f05c0", + ID: "0577f1c4-4907-4c49-a4fd-c6daa61b6e78", + LabID: "2b7435f2-b247-4cc8-8509-6b0d0f593c4c", + } + node, err = client.NodeGet(ctx, node, false) + if err != nil { + slog.Error("nodeget", slog.Any("error", err)) + return + } + + je, err := json.Marshal(node) + if err != nil { + slog.Error("marshal", slog.Any("error", err)) return } + fmt.Println(string(je)) - // je, err := json.Marshal(result) + // lab, err := client.LabGet(ctx, "2b7435f2-b247-4cc8-8509-6b0d0f593c4c", true) // if err != nil { - // log.Println(err) + // slog.Error("get", slog.Any("error", err)) + // return + // } + // + // for _, v := range lab.Nodes { + // if v.Configuration != nil { + // fmt.Printf("[1] %T: %s\n", v.Configuration, *v.Configuration) + // } + // fmt.Printf("[2] %T: %+v\n", v.Configurations, v.Configurations) + // } + // return + // je, err := json.Marshal(lab) + // if err != nil { + // slog.Error("marshal", slog.Any("error", err)) + // return // } // fmt.Println(string(je)) } diff --git a/go.mod b/go.mod index dc29f9a..f7501eb 100644 --- a/go.mod +++ b/go.mod @@ -1,9 +1,10 @@ module github.com/rschmied/gocmlclient -go 1.20 +go 1.21 require ( github.com/Masterminds/semver/v3 v3.2.1 + github.com/lmittmann/tint v1.0.4 github.com/rschmied/mockresponder v1.0.4 github.com/stretchr/testify v1.8.2 golang.org/x/sync v0.7.0 diff --git a/go.sum b/go.sum index 8f3514a..654ae27 100644 --- a/go.sum +++ b/go.sum @@ -3,6 +3,8 @@ github.com/Masterminds/semver/v3 v3.2.1/go.mod h1:qvl/7zhW3nngYb5+80sSMF+FG2BjYr github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= +github.com/lmittmann/tint v1.0.4 h1:LeYihpJ9hyGvE0w+K2okPTGUdVLfng1+nDNVR4vWISc= +github.com/lmittmann/tint v1.0.4/go.mod h1:HIS3gSy7qNwGCj+5oRjAutErFBl4BzdQP6cJZ0NfMwE= github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= github.com/rschmied/mockresponder v1.0.4 h1:VFXa9Y9QJ/5oZFhKoqh9u3HQlbjcBfE9pxI+BanMlEs= diff --git a/iface.go b/iface.go index 4828347..f86a9ba 100644 --- a/iface.go +++ b/iface.go @@ -5,6 +5,7 @@ import ( "context" "encoding/json" "fmt" + "log/slog" "sort" ) @@ -173,6 +174,7 @@ func (c *Client) getInterfacesForNode(ctx context.Context, node *Node) error { interfaceList := InterfaceList{} err := c.jsonGet(ctx, api, &interfaceList, 0) if err != nil { + slog.Info("done", "lab", node.LabID, "node", node.ID, slog.Any("err", err)) return err } diff --git a/lab.go b/lab.go index 42f7b73..7fb0b7a 100644 --- a/lab.go +++ b/lab.go @@ -5,7 +5,7 @@ import ( "context" "encoding/json" "fmt" - "log" + "log/slog" "strings" "golang.org/x/sync/errgroup" @@ -335,9 +335,10 @@ func (c *Client) labFill(ctx context.Context, la *labAlias) (*Lab, error) { g, ctx := errgroup.WithContext(ctx) g.Go(func() error { - defer log.Printf("user done") + defer slog.Debug("user done") la.Owner, err = c.UserGet(ctx, la.OwnerID) if err != nil { + slog.Info("user") return err } return nil @@ -350,7 +351,7 @@ func (c *Client) labFill(ctx context.Context, la *labAlias) (*Lab, error) { ch := make(chan struct{}) g.Go(func() error { defer func() { - log.Printf("nodes/interfaces done") + slog.Debug("nodes/interfaces done") // two sync points, we can run the API endpoints but we need to // wait for the node data to be read until we can add the layer3 // info (1) and the link info (2) @@ -359,11 +360,13 @@ func (c *Client) labFill(ctx context.Context, la *labAlias) (*Lab, error) { }() err := c.getNodesForLab(ctx, lab) if err != nil { + slog.Info("nodes for lab") return err } for _, node := range lab.Nodes { err = c.getInterfacesForNode(ctx, node) if err != nil { + slog.Info("interfaces for node") return err } } @@ -371,12 +374,13 @@ func (c *Client) labFill(ctx context.Context, la *labAlias) (*Lab, error) { }) g.Go(func() error { - defer log.Printf("l3info done") + defer slog.Debug("l3info done") l3info, err := c.getL3Info(ctx, lab.ID) if err != nil { + slog.Info("l3info") return err } - log.Printf("l3info read") + slog.Debug("l3info read") // wait for node data read complete <-ch // map and merge the l3 data... @@ -394,27 +398,29 @@ func (c *Client) labFill(ctx context.Context, la *labAlias) (*Lab, error) { } } } - log.Printf("l3info loop done") + slog.Debug("l3info loop done") return nil }) g.Go(func() error { - defer log.Printf("links done") + defer slog.Debug("links done") idlist, err := c.getLinkIDsForLab(ctx, lab) if err != nil { + slog.Info("links") return err } - log.Printf("linkidlist read") + slog.Debug("links read") // wait for node data read complete <-ch return c.getLinksForLab(ctx, lab, idlist) }) if err := g.Wait(); err != nil { + slog.Info("wait") return nil, err } - log.Printf("wait done") - // lab.filled = true + slog.Debug("wait done") + // lab.filled = trueNodeConfig{} // return c.cacheLab(lab, nil) return lab, nil } diff --git a/system.go b/system.go index f138c76..e792109 100644 --- a/system.go +++ b/system.go @@ -3,7 +3,7 @@ package cmlclient import ( "context" "fmt" - "log" + "log/slog" "regexp" "github.com/Masterminds/semver/v3" @@ -51,9 +51,9 @@ func (c *Client) versionCheck(ctx context.Context, depth int32) error { if m == nil { return versionError(sv.Version) } - log.Printf("controller version: %s", sv.Version) + slog.Info("controller", "version", sv.Version) if len(m[3]) > 0 { - log.Printf("Warning, this is a DEV version %s", sv.Version) + slog.Warn("this is a DEV version", "version", sv.Version) } stem := m[1] v, err := semver.NewVersion(stem) @@ -76,7 +76,7 @@ func (c *Client) Version() string { // Ready returns nil if the system is compatible and ready func (c *Client) Ready(ctx context.Context) error { - // we can safely assume depth 0 as the API endpoint does not - // require authentication + // we can safely assume depth 0 as the API endpoint does not require + // authentication return c.versionCheck(ctx, 0) } diff --git a/vendor/github.com/lmittmann/tint/LICENSE b/vendor/github.com/lmittmann/tint/LICENSE new file mode 100644 index 0000000..3f49589 --- /dev/null +++ b/vendor/github.com/lmittmann/tint/LICENSE @@ -0,0 +1,21 @@ +MIT License + +Copyright (c) 2023 lmittmann + +Permission is hereby granted, free of charge, to any person obtaining a copy +of this software and associated documentation files (the "Software"), to deal +in the Software without restriction, including without limitation the rights +to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +copies of the Software, and to permit persons to whom the Software is +furnished to do so, subject to the following conditions: + +The above copyright notice and this permission notice shall be included in all +copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +SOFTWARE. diff --git a/vendor/github.com/lmittmann/tint/README.md b/vendor/github.com/lmittmann/tint/README.md new file mode 100644 index 0000000..88a77ac --- /dev/null +++ b/vendor/github.com/lmittmann/tint/README.md @@ -0,0 +1,88 @@ +# `tint`: 🌈 **slog.Handler** that writes tinted logs + +[![Go Reference](https://pkg.go.dev/badge/github.com/lmittmann/tint.svg)](https://pkg.go.dev/github.com/lmittmann/tint#section-documentation) +[![Go Report Card](https://goreportcard.com/badge/github.com/lmittmann/tint)](https://goreportcard.com/report/github.com/lmittmann/tint) + + + + + + +
+
+ +Package `tint` implements a zero-dependency [`slog.Handler`](https://pkg.go.dev/log/slog#Handler) +that writes tinted (colorized) logs. Its output format is inspired by the `zerolog.ConsoleWriter` and +[`slog.TextHandler`](https://pkg.go.dev/log/slog#TextHandler). + +The output format can be customized using [`Options`](https://pkg.go.dev/github.com/lmittmann/tint#Options) +which is a drop-in replacement for [`slog.HandlerOptions`](https://pkg.go.dev/log/slog#HandlerOptions). + +``` +go get github.com/lmittmann/tint +``` + +## Usage + +```go +w := os.Stderr + +// create a new logger +logger := slog.New(tint.NewHandler(w, nil)) + +// set global logger with custom options +slog.SetDefault(slog.New( + tint.NewHandler(w, &tint.Options{ + Level: slog.LevelDebug, + TimeFormat: time.Kitchen, + }), +)) +``` + +### Customize Attributes + +`ReplaceAttr` can be used to alter or drop attributes. If set, it is called on +each non-group attribute before it is logged. See [`slog.HandlerOptions`](https://pkg.go.dev/log/slog#HandlerOptions) +for details. + +```go +// create a new logger that doesn't write the time +w := os.Stderr +logger := slog.New( + tint.NewHandler(w, &tint.Options{ + ReplaceAttr: func(groups []string, a slog.Attr) slog.Attr { + if a.Key == slog.TimeKey && len(groups) == 0 { + return slog.Attr{} + } + return a + }, + }), +) +``` + +### Automatically Enable Colors + +Colors are enabled by default and can be disabled using the `Options.NoColor` +attribute. To automatically enable colors based on the terminal capabilities, +use e.g. the [`go-isatty`](https://github.com/mattn/go-isatty) package. + +```go +w := os.Stderr +logger := slog.New( + tint.NewHandler(w, &tint.Options{ + NoColor: !isatty.IsTerminal(w.Fd()), + }), +) +``` + +### Windows Support + +Color support on Windows can be added by using e.g. the +[`go-colorable`](https://github.com/mattn/go-colorable) package. + +```go +w := os.Stderr +logger := slog.New( + tint.NewHandler(colorable.NewColorable(w), nil), +) +``` diff --git a/vendor/github.com/lmittmann/tint/buffer.go b/vendor/github.com/lmittmann/tint/buffer.go new file mode 100644 index 0000000..4d7321a --- /dev/null +++ b/vendor/github.com/lmittmann/tint/buffer.go @@ -0,0 +1,46 @@ +package tint + +import "sync" + +type buffer []byte + +var bufPool = sync.Pool{ + New: func() any { + b := make(buffer, 0, 1024) + return (*buffer)(&b) + }, +} + +func newBuffer() *buffer { + return bufPool.Get().(*buffer) +} + +func (b *buffer) Free() { + // To reduce peak allocation, return only smaller buffers to the pool. + const maxBufferSize = 16 << 10 + if cap(*b) <= maxBufferSize { + *b = (*b)[:0] + bufPool.Put(b) + } +} +func (b *buffer) Write(bytes []byte) (int, error) { + *b = append(*b, bytes...) + return len(bytes), nil +} + +func (b *buffer) WriteByte(char byte) error { + *b = append(*b, char) + return nil +} + +func (b *buffer) WriteString(str string) (int, error) { + *b = append(*b, str...) + return len(str), nil +} + +func (b *buffer) WriteStringIf(ok bool, str string) (int, error) { + if !ok { + return 0, nil + } + return b.WriteString(str) +} diff --git a/vendor/github.com/lmittmann/tint/handler.go b/vendor/github.com/lmittmann/tint/handler.go new file mode 100644 index 0000000..62d153c --- /dev/null +++ b/vendor/github.com/lmittmann/tint/handler.go @@ -0,0 +1,438 @@ +/* +Package tint implements a zero-dependency [slog.Handler] that writes tinted +(colorized) logs. The output format is inspired by the [zerolog.ConsoleWriter] +and [slog.TextHandler]. + +The output format can be customized using [Options], which is a drop-in +replacement for [slog.HandlerOptions]. + +# Customize Attributes + +Options.ReplaceAttr can be used to alter or drop attributes. If set, it is +called on each non-group attribute before it is logged. +See [slog.HandlerOptions] for details. + + w := os.Stderr + logger := slog.New( + tint.NewHandler(w, &tint.Options{ + ReplaceAttr: func(groups []string, a slog.Attr) slog.Attr { + if a.Key == slog.TimeKey && len(groups) == 0 { + return slog.Attr{} + } + return a + }, + }), + ) + +# Automatically Enable Colors + +Colors are enabled by default and can be disabled using the Options.NoColor +attribute. To automatically enable colors based on the terminal capabilities, +use e.g. the [go-isatty] package. + + w := os.Stderr + logger := slog.New( + tint.NewHandler(w, &tint.Options{ + NoColor: !isatty.IsTerminal(w.Fd()), + }), + ) + +# Windows Support + +Color support on Windows can be added by using e.g. the [go-colorable] package. + + w := os.Stderr + logger := slog.New( + tint.NewHandler(colorable.NewColorable(w), nil), + ) + +[zerolog.ConsoleWriter]: https://pkg.go.dev/github.com/rs/zerolog#ConsoleWriter +[go-isatty]: https://pkg.go.dev/github.com/mattn/go-isatty +[go-colorable]: https://pkg.go.dev/github.com/mattn/go-colorable +*/ +package tint + +import ( + "context" + "encoding" + "fmt" + "io" + "log/slog" + "path/filepath" + "runtime" + "strconv" + "sync" + "time" + "unicode" +) + +// ANSI modes +const ( + ansiReset = "\033[0m" + ansiFaint = "\033[2m" + ansiResetFaint = "\033[22m" + ansiBrightRed = "\033[91m" + ansiBrightGreen = "\033[92m" + ansiBrightYellow = "\033[93m" + ansiBrightRedFaint = "\033[91;2m" +) + +const errKey = "err" + +var ( + defaultLevel = slog.LevelInfo + defaultTimeFormat = time.StampMilli +) + +// Options for a slog.Handler that writes tinted logs. A zero Options consists +// entirely of default values. +// +// Options can be used as a drop-in replacement for [slog.HandlerOptions]. +type Options struct { + // Enable source code location (Default: false) + AddSource bool + + // Minimum level to log (Default: slog.LevelInfo) + Level slog.Leveler + + // ReplaceAttr is called to rewrite each non-group attribute before it is logged. + // See https://pkg.go.dev/log/slog#HandlerOptions for details. + ReplaceAttr func(groups []string, attr slog.Attr) slog.Attr + + // Time format (Default: time.StampMilli) + TimeFormat string + + // Disable color (Default: false) + NoColor bool +} + +// NewHandler creates a [slog.Handler] that writes tinted logs to Writer w, +// using the default options. If opts is nil, the default options are used. +func NewHandler(w io.Writer, opts *Options) slog.Handler { + h := &handler{ + w: w, + level: defaultLevel, + timeFormat: defaultTimeFormat, + } + if opts == nil { + return h + } + + h.addSource = opts.AddSource + if opts.Level != nil { + h.level = opts.Level + } + h.replaceAttr = opts.ReplaceAttr + if opts.TimeFormat != "" { + h.timeFormat = opts.TimeFormat + } + h.noColor = opts.NoColor + return h +} + +// handler implements a [slog.Handler]. +type handler struct { + attrsPrefix string + groupPrefix string + groups []string + + mu sync.Mutex + w io.Writer + + addSource bool + level slog.Leveler + replaceAttr func([]string, slog.Attr) slog.Attr + timeFormat string + noColor bool +} + +func (h *handler) clone() *handler { + return &handler{ + attrsPrefix: h.attrsPrefix, + groupPrefix: h.groupPrefix, + groups: h.groups, + w: h.w, + addSource: h.addSource, + level: h.level, + replaceAttr: h.replaceAttr, + timeFormat: h.timeFormat, + noColor: h.noColor, + } +} + +func (h *handler) Enabled(_ context.Context, level slog.Level) bool { + return level >= h.level.Level() +} + +func (h *handler) Handle(_ context.Context, r slog.Record) error { + // get a buffer from the sync pool + buf := newBuffer() + defer buf.Free() + + rep := h.replaceAttr + + // write time + if !r.Time.IsZero() { + val := r.Time.Round(0) // strip monotonic to match Attr behavior + if rep == nil { + h.appendTime(buf, r.Time) + buf.WriteByte(' ') + } else if a := rep(nil /* groups */, slog.Time(slog.TimeKey, val)); a.Key != "" { + if a.Value.Kind() == slog.KindTime { + h.appendTime(buf, a.Value.Time()) + } else { + h.appendValue(buf, a.Value, false) + } + buf.WriteByte(' ') + } + } + + // write level + if rep == nil { + h.appendLevel(buf, r.Level) + buf.WriteByte(' ') + } else if a := rep(nil /* groups */, slog.Any(slog.LevelKey, r.Level)); a.Key != "" { + h.appendValue(buf, a.Value, false) + buf.WriteByte(' ') + } + + // write source + if h.addSource { + fs := runtime.CallersFrames([]uintptr{r.PC}) + f, _ := fs.Next() + if f.File != "" { + src := &slog.Source{ + Function: f.Function, + File: f.File, + Line: f.Line, + } + + if rep == nil { + h.appendSource(buf, src) + buf.WriteByte(' ') + } else if a := rep(nil /* groups */, slog.Any(slog.SourceKey, src)); a.Key != "" { + h.appendValue(buf, a.Value, false) + buf.WriteByte(' ') + } + } + } + + // write message + if rep == nil { + buf.WriteString(r.Message) + buf.WriteByte(' ') + } else if a := rep(nil /* groups */, slog.String(slog.MessageKey, r.Message)); a.Key != "" { + h.appendValue(buf, a.Value, false) + buf.WriteByte(' ') + } + + // write handler attributes + if len(h.attrsPrefix) > 0 { + buf.WriteString(h.attrsPrefix) + } + + // write attributes + r.Attrs(func(attr slog.Attr) bool { + h.appendAttr(buf, attr, h.groupPrefix, h.groups) + return true + }) + + if len(*buf) == 0 { + return nil + } + (*buf)[len(*buf)-1] = '\n' // replace last space with newline + + h.mu.Lock() + defer h.mu.Unlock() + + _, err := h.w.Write(*buf) + return err +} + +func (h *handler) WithAttrs(attrs []slog.Attr) slog.Handler { + if len(attrs) == 0 { + return h + } + h2 := h.clone() + + buf := newBuffer() + defer buf.Free() + + // write attributes to buffer + for _, attr := range attrs { + h.appendAttr(buf, attr, h.groupPrefix, h.groups) + } + h2.attrsPrefix = h.attrsPrefix + string(*buf) + return h2 +} + +func (h *handler) WithGroup(name string) slog.Handler { + if name == "" { + return h + } + h2 := h.clone() + h2.groupPrefix += name + "." + h2.groups = append(h2.groups, name) + return h2 +} + +func (h *handler) appendTime(buf *buffer, t time.Time) { + buf.WriteStringIf(!h.noColor, ansiFaint) + *buf = t.AppendFormat(*buf, h.timeFormat) + buf.WriteStringIf(!h.noColor, ansiReset) +} + +func (h *handler) appendLevel(buf *buffer, level slog.Level) { + switch { + case level < slog.LevelInfo: + buf.WriteString("DBG") + appendLevelDelta(buf, level-slog.LevelDebug) + case level < slog.LevelWarn: + buf.WriteStringIf(!h.noColor, ansiBrightGreen) + buf.WriteString("INF") + appendLevelDelta(buf, level-slog.LevelInfo) + buf.WriteStringIf(!h.noColor, ansiReset) + case level < slog.LevelError: + buf.WriteStringIf(!h.noColor, ansiBrightYellow) + buf.WriteString("WRN") + appendLevelDelta(buf, level-slog.LevelWarn) + buf.WriteStringIf(!h.noColor, ansiReset) + default: + buf.WriteStringIf(!h.noColor, ansiBrightRed) + buf.WriteString("ERR") + appendLevelDelta(buf, level-slog.LevelError) + buf.WriteStringIf(!h.noColor, ansiReset) + } +} + +func appendLevelDelta(buf *buffer, delta slog.Level) { + if delta == 0 { + return + } else if delta > 0 { + buf.WriteByte('+') + } + *buf = strconv.AppendInt(*buf, int64(delta), 10) +} + +func (h *handler) appendSource(buf *buffer, src *slog.Source) { + dir, file := filepath.Split(src.File) + + buf.WriteStringIf(!h.noColor, ansiFaint) + buf.WriteString(filepath.Join(filepath.Base(dir), file)) + buf.WriteByte(':') + buf.WriteString(strconv.Itoa(src.Line)) + buf.WriteStringIf(!h.noColor, ansiReset) +} + +func (h *handler) appendAttr(buf *buffer, attr slog.Attr, groupsPrefix string, groups []string) { + attr.Value = attr.Value.Resolve() + if rep := h.replaceAttr; rep != nil && attr.Value.Kind() != slog.KindGroup { + attr = rep(groups, attr) + attr.Value = attr.Value.Resolve() + } + + if attr.Equal(slog.Attr{}) { + return + } + + if attr.Value.Kind() == slog.KindGroup { + if attr.Key != "" { + groupsPrefix += attr.Key + "." + groups = append(groups, attr.Key) + } + for _, groupAttr := range attr.Value.Group() { + h.appendAttr(buf, groupAttr, groupsPrefix, groups) + } + } else if err, ok := attr.Value.Any().(tintError); ok { + // append tintError + h.appendTintError(buf, err, groupsPrefix) + buf.WriteByte(' ') + } else { + h.appendKey(buf, attr.Key, groupsPrefix) + h.appendValue(buf, attr.Value, true) + buf.WriteByte(' ') + } +} + +func (h *handler) appendKey(buf *buffer, key, groups string) { + buf.WriteStringIf(!h.noColor, ansiFaint) + appendString(buf, groups+key, true) + buf.WriteByte('=') + buf.WriteStringIf(!h.noColor, ansiReset) +} + +func (h *handler) appendValue(buf *buffer, v slog.Value, quote bool) { + switch v.Kind() { + case slog.KindString: + appendString(buf, v.String(), quote) + case slog.KindInt64: + *buf = strconv.AppendInt(*buf, v.Int64(), 10) + case slog.KindUint64: + *buf = strconv.AppendUint(*buf, v.Uint64(), 10) + case slog.KindFloat64: + *buf = strconv.AppendFloat(*buf, v.Float64(), 'g', -1, 64) + case slog.KindBool: + *buf = strconv.AppendBool(*buf, v.Bool()) + case slog.KindDuration: + appendString(buf, v.Duration().String(), quote) + case slog.KindTime: + appendString(buf, v.Time().String(), quote) + case slog.KindAny: + switch cv := v.Any().(type) { + case slog.Level: + h.appendLevel(buf, cv) + case encoding.TextMarshaler: + data, err := cv.MarshalText() + if err != nil { + break + } + appendString(buf, string(data), quote) + case *slog.Source: + h.appendSource(buf, cv) + default: + appendString(buf, fmt.Sprintf("%+v", v.Any()), quote) + } + } +} + +func (h *handler) appendTintError(buf *buffer, err error, groupsPrefix string) { + buf.WriteStringIf(!h.noColor, ansiBrightRedFaint) + appendString(buf, groupsPrefix+errKey, true) + buf.WriteByte('=') + buf.WriteStringIf(!h.noColor, ansiResetFaint) + appendString(buf, err.Error(), true) + buf.WriteStringIf(!h.noColor, ansiReset) +} + +func appendString(buf *buffer, s string, quote bool) { + if quote && needsQuoting(s) { + *buf = strconv.AppendQuote(*buf, s) + } else { + buf.WriteString(s) + } +} + +func needsQuoting(s string) bool { + if len(s) == 0 { + return true + } + for _, r := range s { + if unicode.IsSpace(r) || r == '"' || r == '=' || !unicode.IsPrint(r) { + return true + } + } + return false +} + +type tintError struct{ error } + +// Err returns a tinted (colorized) [slog.Attr] that will be written in red color +// by the [tint.Handler]. When used with any other [slog.Handler], it behaves as +// +// slog.Any("err", err) +func Err(err error) slog.Attr { + if err != nil { + err = tintError{err} + } + return slog.Any(errKey, err) +} diff --git a/vendor/modules.txt b/vendor/modules.txt index 0c9740a..dbddfec 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -4,6 +4,9 @@ github.com/Masterminds/semver/v3 # github.com/davecgh/go-spew v1.1.1 ## explicit github.com/davecgh/go-spew/spew +# github.com/lmittmann/tint v1.0.4 +## explicit; go 1.21 +github.com/lmittmann/tint # github.com/pmezard/go-difflib v1.0.0 ## explicit github.com/pmezard/go-difflib/difflib From 33843d06514a0be7452c838b6015e8ec5c03eefc Mon Sep 17 00:00:00 2001 From: Ralph Schmieder Date: Sat, 8 Jun 2024 16:27:02 +0200 Subject: [PATCH 2/7] add named configs --- .gitignore | 2 + auth.go | 12 ++-- cmd/ctest/main.go | 44 ++++++------- cml.go | 6 +- iface.go | 13 ++-- lab.go | 15 ++--- link.go | 40 +++++------- node.go | 157 ++++++++++++++++++++++++++++++++++++++-------- system.go | 15 +++-- system_test.go | 27 ++++---- 10 files changed, 217 insertions(+), 114 deletions(-) diff --git a/.gitignore b/.gitignore index b8abd57..fe0daba 100644 --- a/.gitignore +++ b/.gitignore @@ -1,3 +1,5 @@ coverage.out *.code-workspace +.envrc +.tool-versions diff --git a/auth.go b/auth.go index d359272..152289b 100644 --- a/auth.go +++ b/auth.go @@ -36,8 +36,8 @@ func (up userPass) valid() bool { return len(up.Username) > 0 && len(up.Password) > 0 } -// technically, authokAPI requires auth, but it's used specifically -// to test whether auth is OK, so it will take a different path +// technically, authokAPI requires auth, but it's used specifically to test +// whether auth is OK, so it will take a different path func (c *Client) authRequired(api *url.URL) bool { url := api.String() return !(strings.HasSuffix(url, authAPI) || @@ -62,10 +62,10 @@ func (c *Client) authenticate(ctx context.Context, userpass userPass, depth int3 } // SetToken sets a specific API token to be used. A token takes precedence over -// a username/password. However, if the token expires, the username/password are -// used to authorize the client again. An error is raised if no token and no -// username/password are provided or if the token expires when no username/password -// are set. +// a username/password. However, if the token expires, the username/password +// are used to authorize the client again. An error is raised if no token and +// no username/password are provided or if the token expires when no +// username/password are set. func (c *Client) SetToken(token string) { c.apiToken = token } diff --git a/cmd/ctest/main.go b/cmd/ctest/main.go index 8edf106..671e566 100644 --- a/cmd/ctest/main.go +++ b/cmd/ctest/main.go @@ -17,7 +17,7 @@ func main() { // set global logger with custom options slog.SetDefault(slog.New( tint.NewHandler(os.Stderr, &tint.Options{ - AddSource: true, + AddSource: true, Level: slog.LevelDebug, TimeFormat: time.RFC822, }), @@ -106,7 +106,7 @@ func main() { slog.Error("ready", slog.Any("error", err)) return } - node := &cmlclient.Node{ + /* node := &cmlclient.Node{ // ID: "28ec08ec-483a-415a-a3ed-625b0d45bef0", // ID: "8116a609-8b68-4e0f-a196-5225da9f05c0", ID: "0577f1c4-4907-4c49-a4fd-c6daa61b6e78", @@ -123,25 +123,25 @@ func main() { slog.Error("marshal", slog.Any("error", err)) return } - fmt.Println(string(je)) + fmt.Println(string(je)) */ - // lab, err := client.LabGet(ctx, "2b7435f2-b247-4cc8-8509-6b0d0f593c4c", true) - // if err != nil { - // slog.Error("get", slog.Any("error", err)) - // return - // } - // - // for _, v := range lab.Nodes { - // if v.Configuration != nil { - // fmt.Printf("[1] %T: %s\n", v.Configuration, *v.Configuration) - // } - // fmt.Printf("[2] %T: %+v\n", v.Configurations, v.Configurations) - // } - // return - // je, err := json.Marshal(lab) - // if err != nil { - // slog.Error("marshal", slog.Any("error", err)) - // return - // } - // fmt.Println(string(je)) + lab, err := client.LabGet(ctx, "2b7435f2-b247-4cc8-8509-6b0d0f593c4c", true) + if err != nil { + slog.Error("get", slog.Any("error", err)) + return + } + + for _, v := range lab.Nodes { + if v.Configuration != nil { + fmt.Printf("[1] %T: %s\n", v.Configuration, *v.Configuration) + } + fmt.Printf("[2] %T: %+v\n", v.Configurations, v.Configurations) + } + return + je, err := json.Marshal(lab) + if err != nil { + slog.Error("marshal", slog.Any("error", err)) + return + } + fmt.Println(string(je)) } diff --git a/cml.go b/cml.go index 2cffac4..fee5253 100644 --- a/cml.go +++ b/cml.go @@ -21,11 +21,12 @@ type Client struct { mu sync.RWMutex labCache map[string]*Lab useCache bool + namedConfigs bool version string } -// New returns a new CML client instance. The host must be a valid URL including -// scheme (https://). +// New returns a new CML client instance. The host must be a valid URL +// including scheme (https://). func New(host string, insecure, useCache bool) *Client { tr := http.DefaultTransport.(*http.Transport) tr.TLSClientConfig = &tls.Config{ @@ -46,5 +47,6 @@ func New(host string, insecure, useCache bool) *Client { state: newState(), labCache: make(map[string]*Lab), useCache: useCache, + namedConfigs: false, } } diff --git a/iface.go b/iface.go index f86a9ba..49ec138 100644 --- a/iface.go +++ b/iface.go @@ -259,12 +259,13 @@ func (c *Client) InterfaceCreate(ctx context.Context, labID, nodeID string, slot return nil, err } - // This is quite awkward, not even sure if it's a good REST design practice: - // "Returns a JSON object that identifies the interface that was created. In - // the case of bulk interface creation, returns a JSON array of such - // objects." <-- from the API documentation - // A list is returned when slot is defined, even if it's just creating - // one interface + // This is quite awkward, not even sure if it's a good REST design + // practice: "Returns a JSON object that identifies the interface that was + // created. In the case of bulk interface creation, returns a JSON array of + // such objects." <-- from the API documentation + // + // A list is returned when slot is defined, even if it's just creating one + // interface api := fmt.Sprintf("labs/%s/interfaces", labID) if slotPtr == nil { diff --git a/lab.go b/lab.go index 7fb0b7a..b2501b0 100644 --- a/lab.go +++ b/lab.go @@ -80,9 +80,6 @@ type Lab struct { Nodes NodeMap `json:"nodes"` Links linkList `json:"links"` Groups LabGroupList `json:"groups"` - - // private - // filled bool } // CanBeWiped returns `true` when all nodes in the lab are wiped. @@ -118,8 +115,8 @@ func (l *Lab) Booted() bool { return true } -// NodeByLabel returns the node of a lab identified by its `label“ or an -// error if not found. +// NodeByLabel returns the node of a lab identified by its `label“ or an error +// if not found. func (l *Lab) NodeByLabel(ctx context.Context, label string) (*Node, error) { for _, node := range l.Nodes { if node.Label == label { @@ -311,8 +308,9 @@ func (c *Client) LabGetByTitle(ctx context.Context, title string, deep bool) (*L } // LabGet returns the lab identified by `id` (a UUIDv4). If `deep` is provided, -// then the nodes, their interfaces and links are also fetched from the controller. -// Also, with `deep`, the L3 IP address info is fetched for the given lab. +// then the nodes, their interfaces and links are also fetched from the +// controller. Also, with `deep`, the L3 IP address info is fetched for the +// given lab. func (c *Client) LabGet(ctx context.Context, id string, deep bool) (*Lab, error) { if lab, ok := c.getCachedLab(id, deep); ok { return lab, nil @@ -388,7 +386,6 @@ func (c *Client) labFill(ctx context.Context, la *labAlias) (*Lab, error) { if node, found := lab.Nodes[nid]; found { for mac, l3i := range l3data.Interfaces { for _, iface := range node.Interfaces { - // if iface, found := node.Interfaces[l3i.ID]; found { if iface.MACaddress == mac { iface.IP4 = l3i.IP4 iface.IP6 = l3i.IP6 @@ -420,7 +417,5 @@ func (c *Client) labFill(ctx context.Context, la *labAlias) (*Lab, error) { return nil, err } slog.Debug("wait done") - // lab.filled = trueNodeConfig{} - // return c.cacheLab(lab, nil) return lab, nil } diff --git a/link.go b/link.go index a24ddfd..04f3293 100644 --- a/link.go +++ b/link.go @@ -81,9 +81,9 @@ func (c *Client) getLinksForLab(ctx context.Context, lab *Lab, linkIDlist IDlist return nil } -// LinkGet returns the link data for the given `labID` and `linkID`. If `deep` is -// set to `true` then bot interface and node data for the given link are also -// fetched from the controller. +// LinkGet returns the link data for the given `labID` and `linkID`. If `deep` +// is set to `true` then bot interface and node data for the given link are +// also fetched from the controller. func (c *Client) LinkGet(ctx context.Context, labID, linkID string, deep bool) (*Link, error) { api := fmt.Sprintf("labs/%s/links/%s", labID, linkID) link := &Link{} @@ -96,7 +96,6 @@ func (c *Client) LinkGet(ctx context.Context, labID, linkID string, deep bool) ( if deep { var err error - // ifaceA, ifaceB *Interface ifaceA := &Interface{ ID: link.SrcID, @@ -136,16 +135,17 @@ func (c *Client) LinkGet(ctx context.Context, labID, linkID string, deep bool) ( return link, err } -// LinkCreate creates a link based on the the data passed in `link`. Required fields -// are the `LabID` and either a pair of interfaces `SrcID` / `DstID` or a pair of -// nodes `SrcNode` / `DstNode`. With nodes it's also possible to provide specific -// slots in `SrcSlot` / `DstSlot` where the link should be created. -// If one or both of the provided slots aren't available, then new interfaces will -// be craeted. If interface creation fails or the provided Interface IDs can't be -// found, the API returns an error, otherwise the returned Link variable has the -// updated link data. -// Node: -1 for a slot means: use next free slot. Specific slots run from 0 to the -// maximum slot number -1 per the node definition of the node type. +// LinkCreate creates a link based on the data passed in `link`. Required +// fields are the `LabID` and either a pair of interfaces `SrcID` / `DstID` or +// a pair of nodes `SrcNode` / `DstNode`. With nodes it's also possible to +// provide specific slots in `SrcSlot` / `DstSlot` where the link should be +// created. +// If one or both of the provided slots aren't available, then new interfaces +// will be created. If interface creation fails or the provided Interface IDs +// can't be found, the API returns an error, otherwise the returned Link +// variable has the updated link data. +// Node: -1 for a slot means: use next free slot. Specific slots run from 0 to +// the maximum slot number -1 per the node definition of the node type. func (c *Client) LinkCreate(ctx context.Context, link *Link) (*Link, error) { api := fmt.Sprintf("labs/%s/links", link.LabID) @@ -157,24 +157,12 @@ func (c *Client) LinkCreate(ctx context.Context, link *Link) (*Link, error) { if len(link.SrcNode) > 0 && len(link.DstNode) > 0 { nodeA = &Node{LabID: link.LabID, ID: link.SrcNode} - // if c.useCache { - // nodeA, err = c.NodeGet(ctx, nodeA, false) - // if err != nil { - // return nil, err - // } - // } err = c.getInterfacesForNode(ctx, nodeA) if err != nil { return nil, err } nodeB = &Node{LabID: link.LabID, ID: link.DstNode} - // if c.useCache { - // nodeB, err = c.NodeGet(ctx, nodeB, false) - // if err != nil { - // return nil, err - // } - // } err = c.getInterfacesForNode(ctx, nodeB) if err != nil { return nil, err diff --git a/node.go b/node.go index 7572888..be5a480 100644 --- a/node.go +++ b/node.go @@ -4,7 +4,9 @@ import ( "bytes" "context" "encoding/json" + "errors" "fmt" + "log/slog" "sort" ) @@ -58,6 +60,11 @@ const ( NodeStateBooted = "BOOTED" ) +type NodeConfig struct { + Name string `json:"name"` + Content string `json:"content"` +} + type SerialDevice struct { ConsoleKey string `json:"console_key"` DeviceNumber int `json:"device_number"` @@ -73,6 +80,7 @@ type Node struct { NodeDefinition string `json:"node_definition"` ImageDefinition string `json:"image_definition"` Configuration *string `json:"configuration"` + Configurations []NodeConfig `json:"-"` CPUs int `json:"cpus"` CPUlimit int `json:"cpu_limit"` RAM int `json:"ram"` @@ -85,8 +93,7 @@ type Node struct { SerialDevices []SerialDevice `json:"serial_devices"` ComputeID string `json:"compute_id"` - // not exported, needed for internal linking - lab *Lab + // Configurations is not exported, it's overloaded within the API } type nodePatchPostAlias struct { @@ -96,7 +103,7 @@ type nodePatchPostAlias struct { HideLinks bool `json:"hide_links"` NodeDefinition string `json:"node_definition,omitempty"` ImageDefinition string `json:"image_definition,omitempty"` - Configuration *string `json:"configuration,omitempty"` + Configuration any `json:"configuration,omitempty"` CPUs int `json:"cpus,omitempty"` CPUlimit int `json:"cpu_limit,omitempty"` RAM int `json:"ram,omitempty"` @@ -114,9 +121,9 @@ func newNodeAlias(node *Node, update bool) nodePatchPostAlias { npp.HideLinks = node.HideLinks npp.Tags = node.Tags - // node tags can't be null, either the tag has to be omitted or it has - // to be an empty list. but since we can't use "omitempty" we need to - // ensure it's an empty list if no tags are provided. + // node tags can't be null, either the tag has to be omitted or it has to + // be an empty list. But since we can't use "omitempty" we need to ensure + // it's an empty list if no tags are provided. if node.Tags == nil { npp.Tags = []string{} } @@ -153,6 +160,67 @@ func (nmap NodeMap) MarshalJSON() ([]byte, error) { return json.Marshal(nodeList) } +func (n *Node) UnmarshalJSON(data []byte) error { + if string(data) == "null" || string(data) == `""` { + return nil + } + + type nodeAlias Node + + var tmpNode struct { + nodeAlias + Configs any `json:"configuration"` + } + + // Unmarshal the JSON into the tmpNode struct. + if err := json.Unmarshal(data, &tmpNode); err != nil { + return err + } + + na := tmpNode.nodeAlias + + switch thing := tmpNode.Configs.(type) { + case nil: + na.Configuration = nil + case string: + na.Configuration = &thing + case []any: + b, err := json.Marshal(thing) + if err != nil { + return err + } + err = json.Unmarshal(b, &na.Configurations) + if err != nil { + return err + } + if len(na.Configurations) == 1 { + config := na.Configurations[0].Content + na.Configuration = &config + na.Configurations = nil + } + default: + return fmt.Errorf("unexpected type: %T", thing) + } + *n = (Node)(na) + + return nil +} + +func (node *Node) MarshalJSON() ([]byte, error) { + type nodeAlias Node + if len(node.Configurations) > 1 { + node.Configuration = nil + return json.Marshal(&struct { + *nodeAlias + NamedConfig []NodeConfig `json:"configuration"` + }{ + (*nodeAlias)(node), + node.Configurations, + }) + } + return json.Marshal((*nodeAlias)(node)) +} + func (c *Client) updateCachedNode(existingNode, node *Node) *Node { // only copy fields which can be updated existingNode.Label = node.Label @@ -164,6 +232,7 @@ func (c *Client) updateCachedNode(existingNode, node *Node) *Node { // these can be changed but only when the node VM doesn't exist if node.State == NodeStateDefined { existingNode.Configuration = node.Configuration + existingNode.Configurations = node.Configurations existingNode.CPUs = node.CPUs existingNode.CPUlimit = node.CPUlimit existingNode.RAM = node.RAM @@ -238,6 +307,10 @@ func (c *Client) deleteCachedNode(node *Node, err error) error { func (c *Client) getNodesForLab(ctx context.Context, lab *Lab) error { api := fmt.Sprintf("labs/%s/nodes?data=true", lab.ID) + if c.namedConfigs { + api += "&operational=true&exclude_configurations=false" + } + nodes := &nodeList{} err := c.jsonGet(ctx, api, nodes, 0) if err != nil { @@ -255,19 +328,11 @@ func (c *Client) getNodesForLab(ctx context.Context, lab *Lab) error { return nil } -// NodeSetConfig sets a configuration for the specified node. At least the `ID` of -// the node and the `labID` must be provided in `node`. The `node` instance will -// be updated with the current values for the node as provided by the controller. -func (c *Client) NodeSetConfig(ctx context.Context, node *Node, configuration string) error { +func (c *Client) nodeSetConfigData(ctx context.Context, node *Node, data any) error { api := fmt.Sprintf("labs/%s/nodes/%s", node.LabID, node.ID) - type nodeConfig struct { - Configuration string `json:"configuration"` - } - buf := &bytes.Buffer{} - nodeCfg := nodeConfig{Configuration: configuration} - err := json.NewEncoder(buf).Encode(nodeCfg) + err := json.NewEncoder(buf).Encode(data) if err != nil { return err } @@ -282,12 +347,43 @@ func (c *Client) NodeSetConfig(ctx context.Context, node *Node, configuration st return err } +// NodeSetConfig sets a configuration for the specified node. At least the `ID` +// of the node and the `labID` must be provided in `node`. The `node` instance +// will be updated with the current values for the node as provided by the +// controller. +func (c *Client) NodeSetConfig(ctx context.Context, node *Node, configuration string) error { + nodeCfg := struct { + Configuration string `json:"configuration"` + }{configuration} + return c.nodeSetConfigData(ctx, node, nodeCfg) +} + +// NodeSetNamedConfigs sets a list of named configurations for the specified +// node. At least the `ID` of the node and the `labID` must be provided in +// `node`. +func (c *Client) NodeSetNamedConfigs(ctx context.Context, node *Node, configs []NodeConfig) error { + nodeCfg := struct { + NamedConfigs []NodeConfig `json:"configuration"` + }{configs} + return c.nodeSetConfigData(ctx, node, nodeCfg) +} + // NodeUpdate updates the node specified by data in `node` (e.g. ID and LabID) -// with the other data provided. It returns the udpated node. +// with the other data provided. It returns the updated node. func (c *Client) NodeUpdate(ctx context.Context, node *Node) (*Node, error) { api := fmt.Sprintf("labs/%s/nodes/%s", node.LabID, node.ID) postAlias := newNodeAlias(node, true) + + if len(node.Configurations) > 0 { + if !c.namedConfigs { + err := errors.New("named configs are not supported by the controller!") + slog.Error("update", slog.Any("error", err)) + return node, err + } + postAlias.Configuration = node.Configurations + } + buf := &bytes.Buffer{} err := json.NewEncoder(buf).Encode(postAlias) if err != nil { @@ -370,7 +466,7 @@ func (c *Client) NodeCreate(ctx context.Context, node *Node) (*Node, error) { if err != nil { // for consistency, remove the created node that can't be updated // this assumes that the error was because of the provided data and - // not because of e.g. a conncectivity issue between the initial create + // not because of e.g. a connectivity issue between the initial create // and the attempted removal. node.ID = newNode.ID c.NodeDestroy(ctx, node) @@ -392,15 +488,24 @@ func (c *Client) NodeGet(ctx context.Context, node *Node, nocache bool) (*Node, } } - newNode := &Node{} + /* SIMPLE-5052 -- results are different for simplified=true vs false + for the inherited values. In the simplified case, all values are always + null. */ + + var err error + newNode := Node{} api := fmt.Sprintf("labs/%s/nodes/%s", node.LabID, node.ID) - err := c.jsonGet(ctx, api, newNode, 0) - // SIMPLE-5052 -- results are different for simplified=true vs false - // for the inherited values. in the simplified case, all values are - // always null. - // There's yet another modified "operational" which also influences - // the returned values - return c.cacheNode(newNode, err) + if c.namedConfigs { + api += "?operational=true&exclude_configurations=false" + } + err = c.jsonGet(ctx, api, &newNode, 0) + // for backwards compatibility, store single config in the old field name + // and reset the multi-config field. + if len(newNode.Configurations) == 1 { + newNode.Configuration = &newNode.Configurations[0].Content + newNode.Configurations = nil + } + return c.cacheNode(&newNode, err) } // NodeDestroy deletes the node from the controller. diff --git a/system.go b/system.go index e792109..dc6ba8d 100644 --- a/system.go +++ b/system.go @@ -21,7 +21,10 @@ type systemVersion struct { Ready bool `json:"ready"` } -const versionConstraint = ">=2.4.0,<3.0.0" +const ( + versionConstraint = ">=2.4.0,<3.0.0" + namedConfigsConstraint = ">=2.7.0" +) func versionError(got string) error { return fmt.Errorf( @@ -41,10 +44,7 @@ func (c *Client) versionCheck(ctx context.Context, depth int32) error { return ErrSystemNotReady } - constraint, err := semver.NewConstraint(versionConstraint) - if err != nil { - panic("unparsable semver version constant") - } + constraint, _ := semver.NewConstraint(versionConstraint) re := regexp.MustCompile(`^(\d\.\d\.\d)((-dev0)?\+build.*)?$`) m := re.FindStringSubmatch(sv.Version) @@ -65,6 +65,11 @@ func (c *Client) versionCheck(ctx context.Context, depth int32) error { if !ok { return versionError(sv.Version) } + constraint, _ = semver.NewConstraint(namedConfigsConstraint) + if ok = constraint.Check(v); ok { + slog.Info("named configs supported") + c.namedConfigs = true + } c.version = sv.Version return nil } diff --git a/system_test.go b/system_test.go index 80f2c58..ebd53fe 100644 --- a/system_test.go +++ b/system_test.go @@ -14,18 +14,20 @@ func TestClient_VersionCheck(t *testing.T) { c.state.set(stateAuthenticated) tests := []struct { - name string - wantJSON string - wantErr bool + name string + wantJSON string + wantErr bool + canNamedCfg bool }{ - {"too old", `{"version": "2.1.0","ready": true}`, true}, - {"garbage", `{"version": "garbage","ready": true}`, true}, - {"too new", `{"version": "2.35.0","ready": true}`, true}, - {"perfect", `{"version": "2.4.0","ready": true}`, false}, - {"actual", `{"version": "2.4.0+build.1","ready": true}`, false}, - {"newer", `{"version": "2.4.1","ready": true}`, false}, - {"dev", `{"version": "2.5.0-dev0+build.3.2f7875762","ready": true}`, false}, - {"v2.5.0", `{"version": "2.5.0+build.5","ready": true}`, false}, + {"too old", `{"version": "2.1.0","ready": true}`, true, false}, + {"garbage", `{"version": "garbage","ready": true}`, true, false}, + {"too new", `{"version": "2.35.0","ready": true}`, true, false}, + {"perfect", `{"version": "2.4.0","ready": true}`, false, false}, + {"actual", `{"version": "2.4.0+build.1","ready": true}`, false, false}, + {"newer", `{"version": "2.4.1","ready": true}`, false, false}, + {"dev", `{"version": "2.5.0-dev0+build.3.2f7875762","ready": true}`, false, false}, + {"v2.5.0", `{"version": "2.5.0+build.5","ready": true}`, false, false}, + {"v2.7.0", `{"version": "2.7.0+build.8","ready": true}`, false, true}, } for _, tt := range tests { mrClient.SetData(mr.MockRespList{{Data: []byte(tt.wantJSON)}}) @@ -33,6 +35,9 @@ func TestClient_VersionCheck(t *testing.T) { if err := c.versionCheck(ctx, 0); (err != nil) != tt.wantErr { t.Errorf("Client.VersionCheck() error = %v, wantErr %v", err, tt.wantErr) } + if tt.canNamedCfg != c.namedConfigs { + t.Errorf("Client.VersionCheck() canNamedConfigs is = %t, want %t", c.namedConfigs, tt.canNamedCfg) + } }) if !mrClient.Empty() { t.Error("not all data in mock client consumed") From 2df1c14ce4b5397588504875bf9ce9ace2b208d2 Mon Sep 17 00:00:00 2001 From: Ralph Schmieder Date: Thu, 13 Jun 2024 13:17:40 +0200 Subject: [PATCH 3/7] Implement some 2.7 changes - making a somewhat bigger version bump due to some bigger changes - removed all the caching logic / code. It didn't really work well due to races. In addition, TF doesn't really keep a connection / client over multiple resource calls so the caching was somewhat limited even it would have properly worked (which it did not). - named configurations (added with CML 2.7) - added some tests for the named configs --- CHANGELOG.md | 41 ++++------ apiclient_test.go | 6 +- auth_test.go | 6 +- cmd/ctest/main.go | 2 +- cml.go | 10 +-- error.go | 5 +- iface.go | 116 +---------------------------- iface_test.go | 84 --------------------- lab.go | 68 +---------------- lab_test.go | 154 +++++++++++--------------------------- link.go | 18 +---- link_test.go | 105 +++++++++++++++++--------- node.go | 186 +++++++++++++++------------------------------- node_test.go | 127 +++++++++++++++++++++---------- system.go | 11 ++- system_test.go | 18 +++-- 16 files changed, 316 insertions(+), 641 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f359504..866a81b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,13 @@ Lists the changes in the gocmlclient package. +## Version 0.1.0 + +- making a somewhat bigger version bump due to some bigger changes +- removed all the caching logic / code. It didn't really work well due to races. In addition, TF doesn't really keep a connection / client over multiple resource calls so the caching was somewhat limited even it would have properly worked (which it did not). +- named configurations (added with CML 2.7) +- added some tests for the named configs + ## Version 0.0.23 - added LinkDestroy() method @@ -34,9 +41,7 @@ fix header and connection error ## Version 0.0.17 - added cache control headers to requests -- return ErrSystemNotReady for Connection refused and 502, also always - reset the client's compatibility error property when versionCheck is called - so that it always queries the backend. +- return ErrSystemNotReady for Connection refused and 502, also always reset the client's compatibility error property when versionCheck is called so that it always queries the backend. - bumped semver to 3.2.1 ## Version 0.0.16 @@ -49,43 +54,32 @@ fix header and connection error ## Version 0.0.15 -- made node configuration a pointer to differentiate between - "no configuration" (null), "empty configuration" and "specific - configuration". With a null configuration, the default configuration - from the node definition will be inserted if there is one +- made node configuration a pointer to differentiate between "no configuration" (null), "empty configuration" and "specific configuration". With a null configuration, the default configuration from the node definition will be inserted if there is one - added Version var/func, moved NewClient() to New() - bump go to 1.19 and vendor deps ## Version 0.0.12 -- Realized that the empty tags removal from 0.0.11 caused a regression. - node tags are always returned/set even when there's no tags... in that - case, the empty list is returned or needs to be provided. See 0.0.3 comment. +- Realized that the empty tags removal from 0.0.11 caused a regression. node tags are always returned/set even when there's no tags... in that case, the empty list is returned or needs to be provided. See 0.0.3 comment. - Test coverage improvement ## Version 0.0.8 to 0.0.11 - Added most of the doc string for exported functions. - reversed the sorting of images for the image definitions. -- sort image definitions by their ID. Lists have the newest (highest version) - image as the first element. +- sort image definitions by their ID. Lists have the newest (highest version) image as the first element. - updated dependencies. -- have InterfaceCreate accept a slot value (not a pointer). A negative slot - indicates "don't specify a slot", this was previously indicated by nil. +- have InterfaceCreate accept a slot value (not a pointer). A negative slot indicates "don't specify a slot", this was previously indicated by nil. - added more values to the ImageDefinition and Nodedefinition structs. - added a link unit test. - more node attributes can be updated when a node is DEFINED_ON_CORE -- NodeCreate removes a node now when the 2nd API call fails. The 2nd call is - needed to update certain attributes which are not accepted in the actual - create API (POST). +- NodeCreate removes a node now when the 2nd API call fails. The 2nd call is needed to update certain attributes which are not accepted in the actual create API (POST). - move the upper version for the version constraint from <2.6.0 to <3.0.0. - omit empty tags on update. ## Version 0.0.5 to 0.0.7 -- refactored the code so that interfaces are read in one go ("data=true"). This - without this, only a list of interface IDs is returned by the API. With this, - the API returns a list of complete interface object. +- refactored the code so that interfaces are read in one go ("data=true"). This without this, only a list of interface IDs is returned by the API. With this, the API returns a list of complete interface object. - Implement the same approach for nodes (0.0.6). - updated dependencies. - Due to the data=true option, restrict the code to only work with 2.4.0 and later. @@ -97,12 +91,9 @@ fix header and connection error ## Version 0.0.3 -- Fixed node tag list update. To delete all tags from a node, an empty tag list - must be serialized in the `PATCH` JSON. This was prevented by having - `omitempty` in the struct. Fixed +- Fixed node tag list update. To delete all tags from a node, an empty tag list must be serialized in the `PATCH` JSON. This was prevented by having `omitempty` in the struct. Fixed - Also moved the `ctest` cmd fro the terraform provider repo to the code base. ## Versions prior to 0.0.3 -Nothing in particular to be noteworthy -- just huge chunks of initial code -refactoring. +Nothing in particular to be noteworthy -- just huge chunks of initial code refactoring. diff --git a/apiclient_test.go b/apiclient_test.go index a516d32..8069443 100644 --- a/apiclient_test.go +++ b/apiclient_test.go @@ -8,8 +8,6 @@ import ( "github.com/stretchr/testify/assert" ) -const useCache bool = false - type testClient struct { client *Client mr *mr.MockResponder @@ -17,7 +15,7 @@ type testClient struct { } func newTestAPIclient() testClient { - c := New("https://controller", true, useCache) + c := New("https://controller", true) mrClient, ctx := mr.NewMockResponder() c.httpClient = mrClient c.SetUsernamePassword("user", "pass") @@ -31,7 +29,7 @@ func newAuthedTestAPIclient() testClient { } func TestClient_methoderror(t *testing.T) { - c := New("", true, useCache) + c := New("", true) err := c.jsonReq(context.Background(), "ü", "###", nil, nil, 0) assert.Error(t, err) } diff --git a/auth_test.go b/auth_test.go index dfbc4c9..cb7b44e 100644 --- a/auth_test.go +++ b/auth_test.go @@ -151,20 +151,20 @@ func TestClient_token_auth(t *testing.T) { } func TestClient_SetToken(t *testing.T) { - c := New("https://bla.bla", true, useCache) + c := New("https://bla.bla", true) c.SetToken("qwe") assert.Equal(t, "qwe", c.apiToken) } func TestClient_SetUsernamePassword(t *testing.T) { - c := New("https://bla.bla", true, useCache) + c := New("https://bla.bla", true) c.SetUsernamePassword("user", "pass") assert.Equal(t, "user", c.userpass.Username) assert.Equal(t, "pass", c.userpass.Password) } func TestClient_SetCACert(t *testing.T) { - c := New("https://bla.bla", true, useCache) + c := New("https://bla.bla", true) err := c.SetCACert([]byte("crapdata")) assert.EqualError(t, err, "failed to parse root certificate") testCA := "testdata/ca.pem" diff --git a/cmd/ctest/main.go b/cmd/ctest/main.go index 671e566..2535b43 100644 --- a/cmd/ctest/main.go +++ b/cmd/ctest/main.go @@ -45,7 +45,7 @@ func main() { return } ctx := context.Background() - client := cmlclient.New(host, false, false) + client := cmlclient.New(host, false) // if err := client.Ready(ctx); err != nil { // log.Fatal(err) // } diff --git a/cml.go b/cml.go index fee5253..daa2efc 100644 --- a/cml.go +++ b/cml.go @@ -19,15 +19,13 @@ type Client struct { compatibilityErr error state *apiClientState mu sync.RWMutex - labCache map[string]*Lab - useCache bool - namedConfigs bool + useNamedConfigs bool version string } // New returns a new CML client instance. The host must be a valid URL // including scheme (https://). -func New(host string, insecure, useCache bool) *Client { +func New(host string, insecure bool) *Client { tr := http.DefaultTransport.(*http.Transport) tr.TLSClientConfig = &tls.Config{ InsecureSkipVerify: insecure, @@ -45,8 +43,6 @@ func New(host string, insecure, useCache bool) *Client { }, compatibilityErr: nil, state: newState(), - labCache: make(map[string]*Lab), - useCache: useCache, - namedConfigs: false, + useNamedConfigs: false, } } diff --git a/error.go b/error.go index 778fced..d3c0fad 100644 --- a/error.go +++ b/error.go @@ -3,6 +3,7 @@ package cmlclient import "errors" var ( - ErrSystemNotReady = errors.New("system not ready") - ErrElementNotFound = errors.New("element not found") + ErrSystemNotReady = errors.New("system not ready") + ErrElementNotFound = errors.New("element not found") + ErrNoNamedConfigSupport = errors.New("backend does not support named configs") ) diff --git a/iface.go b/iface.go index 49ec138..b3f54f9 100644 --- a/iface.go +++ b/iface.go @@ -69,104 +69,6 @@ func (iface Interface) IsPhysical() bool { return iface.Type == IfaceTypePhysical } -func (c *Client) updateCachedIface(existingIface, _ *Interface) *Interface { - // this is a no-op at this point, we don't allow updating interfaces - return existingIface -} - -func (c *Client) cacheIface(iface *Interface, err error) (*Interface, error) { - if !c.useCache || err != nil { - return iface, err - } - - c.mu.RLock() - lab, ok := c.labCache[iface.LabID] - c.mu.RUnlock() - if !ok { - return iface, err - } - - c.mu.RLock() - node, ok := lab.Nodes[iface.Node] - c.mu.RUnlock() - if !ok { - return iface, err - } - c.mu.RLock() - interfaces := node.Interfaces - c.mu.RUnlock() - for _, nodeIface := range interfaces { - if nodeIface.ID == iface.ID { - return c.updateCachedIface(nodeIface, iface), nil - } - } - - iface.node = node // internal linking - c.mu.Lock() - node.Interfaces = append(node.Interfaces, iface) - c.mu.Unlock() - return iface, nil -} - -func (c *Client) getCachedIface(iface *Interface) (*Interface, bool) { - if !c.useCache { - return nil, false - } - c.mu.RLock() - defer c.mu.RUnlock() - lab, ok := c.labCache[iface.LabID] - if !ok { - return nil, false - } - - node, ok := lab.Nodes[iface.Node] - if !ok { - return iface, false - } - - for _, nodeIface := range node.Interfaces { - if nodeIface != nil && nodeIface.ID == iface.ID { - if nodeIface.node == nil { - nodeIface.node = node - } - return nodeIface, true - } - } - - return iface, false -} - -func (c *Client) deleteCachedIface(iface *Interface, err error) error { - if !c.useCache || err != nil { - return err - } - - c.mu.RLock() - lab, ok := c.labCache[iface.LabID] - c.mu.RUnlock() - if !ok { - return err - } - - c.mu.RLock() - node, ok := lab.Nodes[iface.Node] - c.mu.RUnlock() - if !ok { - return err - } - - c.mu.Lock() - newList := InterfaceList{} - for _, nodeIface := range node.Interfaces { - if nodeIface.ID != iface.ID { - newList = append(newList, nodeIface) - } - } - node.Interfaces = newList - c.mu.Unlock() - return nil -} - func (c *Client) getInterfacesForNode(ctx context.Context, node *Node) error { // with the data=true option, we get not only the list of IDs but the // interfaces themselves as well! @@ -182,9 +84,6 @@ func (c *Client) getInterfacesForNode(ctx context.Context, node *Node) error { sort.Slice(interfaceList, func(i, j int) bool { return interfaceList[i].Slot < interfaceList[j].Slot }) - for _, iface := range interfaceList { - c.cacheIface(iface, nil) - } c.mu.Lock() node.Interfaces = interfaceList c.mu.Unlock() @@ -226,18 +125,14 @@ func (c *Client) getInterfacesForNode(ctx context.Context, node *Node) error { // InterfaceGet returns the interface identified by its `ID` (iface.ID). func (c *Client) InterfaceGet(ctx context.Context, iface *Interface) (*Interface, error) { - if iface, ok := c.getCachedIface(iface); ok { - return iface, nil - } - api := fmt.Sprintf("labs/%s/interfaces/%s", iface.LabID, iface.ID) err := c.jsonGet(ctx, api, iface, 0) - return c.cacheIface(iface, err) + return iface, err } // InterfaceCreate creates an interface in the given lab and node. If the slot -// is >= 0, the request creates all unallocated slots up to and including -// that slot. Conversely, if the slot is < 0 (e.g. -1), the next free slot is used. +// is >= 0, the request creates all unallocated slots up to and including that +// slot. Conversely, if the slot is < 0 (e.g. -1), the next free slot is used. func (c *Client) InterfaceCreate(ctx context.Context, labID, nodeID string, slot int) (*Interface, error) { var slotPtr *int @@ -274,7 +169,7 @@ func (c *Client) InterfaceCreate(ctx context.Context, labID, nodeID string, slot if err != nil { return nil, err } - return c.cacheIface(&result, err) + return &result, err } // this is when a slot has been provided; the API provides now a list of @@ -286,8 +181,5 @@ func (c *Client) InterfaceCreate(ctx context.Context, labID, nodeID string, slot } lastIface := &result[len(result)-1] - for _, li := range result { - c.cacheIface(&li, nil) - } return lastIface, nil } diff --git a/iface_test.go b/iface_test.go index a19735e..526bced 100644 --- a/iface_test.go +++ b/iface_test.go @@ -1,7 +1,6 @@ package cmlclient import ( - "errors" "math/rand" "sync" "testing" @@ -43,7 +42,6 @@ func TestClient_IfaceRuns(t *testing.T) { func TestClient_IfaceWithSlots(t *testing.T) { tc := newAuthedTestAPIclient() - tc.client.useCache = true ifaceList := []byte(`[{ "id": "n2i0", @@ -79,87 +77,8 @@ func TestClient_IfaceWithSlots(t *testing.T) { } } -func TestClient_IfaceDelete(t *testing.T) { - tc := newAuthedTestAPIclient() - tc.client.useCache = true - - tests := []struct { - name string - lab Lab - node Node - ifaceList InterfaceList - preErr error - want bool - }{ - { - "error before", - Lab{ - ID: "different", - Nodes: make(NodeMap), - }, - Node{}, - InterfaceList{}, - errors.New("some error"), - false, - }, - { - "nolab", - Lab{ - ID: "different", - Nodes: make(NodeMap), - }, - Node{}, - InterfaceList{}, - nil, - false, - }, - { - "nolab", - Lab{ - ID: "lab1", - Nodes: make(NodeMap), - }, - Node{ID: "node2"}, - InterfaceList{}, - nil, - false, - }, - { - "good", - Lab{ - ID: "lab1", - Nodes: make(NodeMap), - }, - Node{ID: "node1"}, - InterfaceList{ - &Interface{ID: "iface0"}, - &Interface{ID: "iface1"}, - &Interface{ID: "iface2"}, - &Interface{ID: "iface3"}, - }, - nil, - false, - }, - } - - iface := &Interface{ID: "iface2", LabID: "lab1", Node: "node1"} - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - tt.node.Interfaces = tt.ifaceList - tt.lab.Nodes[tt.node.ID] = &tt.node - tc.client.labCache[tt.lab.ID] = &tt.lab - err := tc.client.deleteCachedIface(iface, tt.preErr) - assert.Equal(t, tt.preErr, err) - if err == nil && tt.name == "good" { - assert.Len(t, tt.node.Interfaces, 3) - } - }) - } -} - func Test_Race(t *testing.T) { tc := newAuthedTestAPIclient() - tc.client.useCache = true iface0 := []byte(`{ "id": "iface0", @@ -236,9 +155,6 @@ func Test_Race(t *testing.T) { } node := Node{ID: "node1", LabID: lab.ID} lab.Nodes[node.ID] = &node - tc.client.labCache[lab.ID] = &lab - // rand.Seed is not needed anymore from 1.20 onward - // rand.Seed(time.Now().UnixNano()) for i := 0; i < 50; i++ { tc.mr.Reset() diff --git a/lab.go b/lab.go index b2501b0..61466d1 100644 --- a/lab.go +++ b/lab.go @@ -131,57 +131,6 @@ type LabImport struct { Warnings []string `json:"warnings"` } -func (c *Client) updateCachedLab(existingLab, updatedLab *Lab) *Lab { - // only copy fields which can be updated - c.mu.Lock() - existingLab.Title = updatedLab.Title - existingLab.Description = updatedLab.Description - existingLab.Nodes = updatedLab.Nodes - existingLab.State = updatedLab.State - c.mu.Unlock() - return existingLab -} - -func (c *Client) cacheLab(lab *Lab, err error) (*Lab, error) { - if !c.useCache || err != nil { - return lab, err - } - - c.mu.RLock() - existingLab, ok := c.labCache[lab.ID] - c.mu.RUnlock() - if ok { - return c.updateCachedLab(existingLab, lab), nil - } - - lab.Nodes = make(NodeMap) - c.mu.Lock() - c.labCache[lab.ID] = lab - c.mu.Unlock() - return lab, nil -} - -func (c *Client) getCachedLab(id string, deep bool) (*Lab, bool) { - // no caching when reading deep - if !c.useCache || deep { - return nil, false - } - c.mu.RLock() - lab, ok := c.labCache[id] - c.mu.RUnlock() - return lab, ok -} - -func (c *Client) deleteCachedLab(id string, err error) error { - if !c.useCache || err != nil { - return err - } - c.mu.Lock() - delete(c.labCache, id) - c.mu.Unlock() - return nil -} - // LabCreate creates a new lab on the controller. func (c *Client) LabCreate(ctx context.Context, lab Lab) (*Lab, error) { // TODO: inconsistent attributes lab_title vs title, ... @@ -232,8 +181,7 @@ func (c *Client) LabUpdate(ctx context.Context, lab Lab) (*Lab, error) { } la.Owner = &User{ID: la.OwnerID} - la.Nodes = make(NodeMap) - return c.cacheLab(&la.Lab, nil) + return &la.Lab, nil } // LabImport imports a lab topology into the controller. This is expected to be @@ -281,7 +229,7 @@ func (c *Client) LabWipe(ctx context.Context, id string) error { // LabDestroy deletes the lab identified by the `id` (a UUIDv4). func (c *Client) LabDestroy(ctx context.Context, id string) error { - return c.deleteCachedLab(id, c.jsonDelete(ctx, fmt.Sprintf("labs/%s", id), 0)) + return c.jsonDelete(ctx, fmt.Sprintf("labs/%s", id), 0) } // LabGetByTitle returns the lab identified by its `title`. For the use of @@ -312,9 +260,6 @@ func (c *Client) LabGetByTitle(ctx context.Context, title string, deep bool) (*L // controller. Also, with `deep`, the L3 IP address info is fetched for the // given lab. func (c *Client) LabGet(ctx context.Context, id string, deep bool) (*Lab, error) { - if lab, ok := c.getCachedLab(id, deep); ok { - return lab, nil - } api := fmt.Sprintf("labs/%s", id) la := &labAlias{} err := c.jsonGet(ctx, api, la, 0) @@ -323,7 +268,7 @@ func (c *Client) LabGet(ctx context.Context, id string, deep bool) (*Lab, error) } if !deep { la.Owner = &User{ID: la.OwnerID} - return c.cacheLab(&la.Lab, nil) + return &la.Lab, nil } return c.labFill(ctx, la) } @@ -336,14 +281,12 @@ func (c *Client) labFill(ctx context.Context, la *labAlias) (*Lab, error) { defer slog.Debug("user done") la.Owner, err = c.UserGet(ctx, la.OwnerID) if err != nil { - slog.Info("user") return err } return nil }) lab := &la.Lab - lab, _ = c.cacheLab(lab, nil) // need to ensure that this block finishes before the others run ch := make(chan struct{}) @@ -358,13 +301,11 @@ func (c *Client) labFill(ctx context.Context, la *labAlias) (*Lab, error) { }() err := c.getNodesForLab(ctx, lab) if err != nil { - slog.Info("nodes for lab") return err } for _, node := range lab.Nodes { err = c.getInterfacesForNode(ctx, node) if err != nil { - slog.Info("interfaces for node") return err } } @@ -375,7 +316,6 @@ func (c *Client) labFill(ctx context.Context, la *labAlias) (*Lab, error) { defer slog.Debug("l3info done") l3info, err := c.getL3Info(ctx, lab.ID) if err != nil { - slog.Info("l3info") return err } slog.Debug("l3info read") @@ -403,7 +343,6 @@ func (c *Client) labFill(ctx context.Context, la *labAlias) (*Lab, error) { defer slog.Debug("links done") idlist, err := c.getLinkIDsForLab(ctx, lab) if err != nil { - slog.Info("links") return err } slog.Debug("links read") @@ -413,7 +352,6 @@ func (c *Client) labFill(ctx context.Context, la *labAlias) (*Lab, error) { }) if err := g.Wait(); err != nil { - slog.Info("wait") return nil, err } slog.Debug("wait done") diff --git a/lab_test.go b/lab_test.go index bb0e388..dbbd576 100644 --- a/lab_test.go +++ b/lab_test.go @@ -494,21 +494,18 @@ func TestClient_StartStopWipeDestroy(t *testing.T) { tc.client.LabDestroy, } - for _, useCache := range []bool{true, false} { - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - tc.client.useCache = useCache - tc.mr.SetData(tt.data) - for _, f := range funcs { - err := f(tc.ctx, "bla") - if tt.want { - assert.Error(t, err) - } else { - assert.NoError(t, err) - } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + tc.mr.SetData(tt.data) + for _, f := range funcs { + err := f(tc.ctx, "bla") + if tt.want { + assert.Error(t, err) + } else { + assert.NoError(t, err) } - }) - } + } + }) } } @@ -642,7 +639,6 @@ func TestClient_LabCreate(t *testing.T) { func TestClient_LabUpdate(t *testing.T) { tc := newAuthedTestAPIclient() - tc.client.useCache = true data := mr.MockRespList{ mr.MockResp{ @@ -661,6 +657,22 @@ func TestClient_LabUpdate(t *testing.T) { "groups": [] }`), }, + mr.MockResp{ + Data: []byte(`{ + "state": "DEFINED_ON_CORE", + "created": "2022-10-14T10:05:07+00:00", + "modified": "2022-10-14T10:05:07+00:00", + "lab_title": "Lab at Mon 17:27 PM", + "lab_description": "string", + "lab_notes": "string", + "owner": "00000000-0000-4000-a000-000000000000", + "owner_username": "admin", + "node_count": 0, + "link_count": 0, + "id": "lab99", + "groups": [] + }`), + }, } data2 := mr.MockRespList{ mr.MockResp{ @@ -679,6 +691,22 @@ func TestClient_LabUpdate(t *testing.T) { "groups": [] }`), }, + mr.MockResp{ + Data: []byte(`{ + "state": "DEFINED_ON_CORE", + "created": "2022-10-14T10:05:07+00:00", + "modified": "2022-10-14T12:05:07+00:00", + "lab_title": "new title", + "lab_description": "string", + "lab_notes": "string", + "owner": "00000000-0000-4000-a000-000000000000", + "owner_username": "admin", + "node_count": 0, + "link_count": 0, + "id": "lab99", + "groups": [] + }`), + }, } tests := []struct { @@ -692,9 +720,6 @@ func TestClient_LabUpdate(t *testing.T) { {"bad", Lab{}, mr.MockRespList{mr.MockResp{Code: 405}}, true}, } - lab := &Lab{ID: "lab99"} - tc.client.labCache["lab99"] = lab - for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { tc.mr.SetData(tt.data) @@ -711,96 +736,3 @@ func TestClient_LabUpdate(t *testing.T) { }) } } - -func TestClient_CompleteCache(t *testing.T) { - tc := newAuthedTestAPIclient() - tc.client.useCache = true - - ifacen1i2 := []byte(`{ - "id": "n1i2", - "lab_id": "lab1", - "node": "node1", - "label": "eth1", - "slot": 1, - "type": "physical", - "mac_address": "52:54:00:0c:e0:69", - "is_connected": true, - "state": "STARTED" - }`) - ifacen2i2 := []byte(`{ - "id": "n2i2", - "lab_id": "lab1", - "node": "node2", - "label": "eth1", - "slot": 1, - "type": "physical", - "mac_address": "52:54:00:0c:e0:70", - "is_connected": true, - "state": "STOPPED" - }`) - link2n1n2 := []byte(`{ - "id": "link2", - "interface_a": "n1i2", - "interface_b": "n2i2", - "lab_id": "lab1", - "label": "alpine-0-eth1<->alpine-1-eth1", - "link_capture_key": "", - "node_a": "node1", - "node_b": "node2", - "state": "DEFINED_ON_CORE" - }`) - - data := mr.MockRespList{ - mr.MockResp{Data: demoLab, URL: `/labs/lab1$`}, - mr.MockResp{Data: links, URL: `/links$`}, - mr.MockResp{Data: lab_layer3, URL: `layer3_addresses$`}, - mr.MockResp{Data: ownerUser, URL: `/users/.+$`}, - mr.MockResp{Data: nodes, URL: `/nodes\?data=true$`}, - mr.MockResp{Data: ifacesn1, URL: `/node1/interfaces\?data=true$`}, - mr.MockResp{Data: ifacesn2, URL: `/node2/interfaces\?data=true$`}, - mr.MockResp{Data: linkn1n2, URL: `/links/link1$`}, - - mr.MockResp{Data: ifacesn1, URL: `/node1/interfaces\?data=true$`}, - mr.MockResp{Data: ifacesn2, URL: `/node2/interfaces\?data=true$`}, - - // 2 new interfaces, one new link, followed by a get - mr.MockResp{Data: ifacen1i2, URL: `/interfaces$`}, - mr.MockResp{Data: ifacen2i2, URL: `/interfaces$`}, - mr.MockResp{Data: link2n1n2, URL: `/links$`}, - mr.MockResp{Data: link2n1n2, URL: `/links/link2$`}, - } - - tests := []struct { - name string - lab Lab - data mr.MockRespList - want bool - }{ - {"good", Lab{}, data, false}, - // {"bad", Lab{}, mr.MockRespList{mr.MockResp{Code: 405}}, true}, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - tc.mr.SetData(tt.data) - _, err := tc.client.LabGet(tc.ctx, "lab1", true) - assert.NoError(t, err) - link := &Link{ - LabID: "lab1", - SrcNode: "node1", - DstNode: "node2", - SrcSlot: -1, - DstSlot: -1, - } - link, err = tc.client.LinkCreate(tc.ctx, link) - if tt.want { - assert.Error(t, err) - } else { - if assert.NoError(t, err) { - assert.Equal(t, "link2", link.ID) - } - } - assert.True(t, tc.mr.Empty()) - }) - } -} diff --git a/link.go b/link.go index 04f3293..1deaa91 100644 --- a/link.go +++ b/link.go @@ -107,7 +107,7 @@ func (c *Client) LinkGet(ctx context.Context, labID, linkID string, deep bool) ( if err != nil { return nil, err } - ifaceA.node, err = c.NodeGet(ctx, ifaceA.node, false) + ifaceA.node, err = c.NodeGet(ctx, ifaceA.node) if err != nil { return nil, err } @@ -122,7 +122,7 @@ func (c *Client) LinkGet(ctx context.Context, labID, linkID string, deep bool) ( if err != nil { return nil, err } - ifaceB.node, err = c.NodeGet(ctx, ifaceB.node, false) + ifaceB.node, err = c.NodeGet(ctx, ifaceB.node) if err != nil { return nil, err } @@ -169,19 +169,7 @@ func (c *Client) LinkCreate(ctx context.Context, link *Link) (*Link, error) { } matches := func(slot int, iface *Interface) bool { - if !iface.IsPhysical() { - return false - } - if slot >= 0 { - if iface.Slot == slot && !iface.IsConnected { - return true - } - } else { - if !iface.IsConnected { - return true - } - } - return false + return iface.IsPhysical() && !iface.IsConnected && (slot < 0 || iface.Slot == slot) } for _, iface := range nodeA.Interfaces { diff --git a/link_test.go b/link_test.go index 45be699..bf66d4b 100644 --- a/link_test.go +++ b/link_test.go @@ -104,23 +104,16 @@ func TestClient_GetLink(t *testing.T) { false, }, } - for _, useCache := range []bool{false, true} { - for _, tt := range tests { - tc.mr.SetData(tt.responses) - tc.client.useCache = useCache - t.Run(tt.name, func(t *testing.T) { - lab, err := tc.client.LinkGet(tc.ctx, "qweaa", "link1", tt.deep) - assert.NoError(t, err) - assert.NotNil(t, lab) - // if tt.deep { - // assert.Len(t, lab.Links, 1) - // assert.Len(t, lab.Nodes, 2) - if !(useCache || tc.mr.Empty()) { - t.Errorf("not all data in mock client consumed: %v", useCache) - } - // } - }) - } + for _, tt := range tests { + tc.mr.SetData(tt.responses) + t.Run(tt.name, func(t *testing.T) { + link, err := tc.client.LinkGet(tc.ctx, "qweaa", "link1", tt.deep) + assert.NoError(t, err) + assert.NotNil(t, link) + if !tc.mr.Empty() { + t.Error("not all data in mock client consumed") + } + }) } } @@ -229,6 +222,30 @@ func TestClient_CreateLink(t *testing.T) { "state": "DEFINED_ON_CORE" }`) + newiface0 := []byte(`{ + "id": "iface0", + "lab_id": "lab1", + "node": "node1", + "label": "eth0", + "slot": 0, + "type": "physical", + "mac_address": "52:54:00:0c:e0:70", + "is_connected": false, + "state": "STOPPED" + }`) + + newiface1 := []byte(`{ + "id": "iface0", + "lab_id": "lab1", + "node": "node1", + "label": "eth0", + "slot": 0, + "type": "physical", + "mac_address": "52:54:00:0c:e0:70", + "is_connected": false, + "state": "STOPPED" + }`) + mockdata := mr.MockRespList{ mr.MockResp{Data: ifaceList1, URL: `node1/interfaces\?data=true$`}, mr.MockResp{Data: ifaceList2, URL: `node2/interfaces\?data=true$`}, @@ -240,6 +257,19 @@ func TestClient_CreateLink(t *testing.T) { mr.MockResp{Data: node1, URL: `/labs/lab1/nodes/node2$`}, } + mockdata2 := mr.MockRespList{ + mr.MockResp{Data: []byte(`[]`), URL: `node1/interfaces\?data=true$`}, + mr.MockResp{Data: []byte(`[]`), URL: `node2/interfaces\?data=true$`}, + mr.MockResp{Data: newiface0, URL: `/labs/lab1/interfaces$`}, + mr.MockResp{Data: newiface1, URL: `/labs/lab1/interfaces$`}, + mr.MockResp{Data: linkn1n2, URL: `/links$`}, + mr.MockResp{Data: linkn1n2, URL: `/links/link1$`}, + mr.MockResp{Data: iface1, URL: `/interfaces/iface1$`}, + mr.MockResp{Data: iface6, URL: `/interfaces/iface6$`}, + mr.MockResp{Data: node1, URL: `/labs/lab1/nodes/node1$`}, + mr.MockResp{Data: node1, URL: `/labs/lab1/nodes/node2$`}, + } + tests := []struct { name string link *Link @@ -257,7 +287,7 @@ func TestClient_CreateLink(t *testing.T) { mockdata, }, { - "link_no_slots", + "link_no_slots_1", &Link{ LabID: "lab1", SrcNode: "node1", @@ -267,24 +297,28 @@ func TestClient_CreateLink(t *testing.T) { }, mockdata, }, + { + "link_no_slots_2", + &Link{ + LabID: "lab1", + SrcNode: "node1", + DstNode: "node2", + SrcSlot: -1, + DstSlot: -1, + }, + mockdata2, + }, } - for _, useCache := range []bool{false, true} { - for _, tt := range tests { - tc.mr.SetData(tt.responses) - tc.client.useCache = useCache - t.Run(tt.name, func(t *testing.T) { - lab, err := tc.client.LinkCreate(tc.ctx, tt.link) - assert.NoError(t, err) - assert.NotNil(t, lab) - // if tt.deep { - // assert.Len(t, lab.Links, 1) - // assert.Len(t, lab.Nodes, 2) - if !(useCache || tc.mr.Empty()) { - t.Errorf("not all data in mock client consumed: %v", useCache) - } - // } - }) - } + for _, tt := range tests { + tc.mr.SetData(tt.responses) + t.Run(tt.name, func(t *testing.T) { + lab, err := tc.client.LinkCreate(tc.ctx, tt.link) + assert.NoError(t, err) + assert.NotNil(t, lab) + if !tc.mr.Empty() { + t.Error("not all data in mock client consumed") + } + }) } } @@ -295,7 +329,6 @@ func TestClient_DestroyLink(t *testing.T) { mr.MockResp{Code: 200}, }) - tc.client.useCache = false err := tc.client.LinkDestroy(tc.ctx, &Link{LabID: "lab1", ID: "link1"}) assert.NoError(t, err) } diff --git a/node.go b/node.go index be5a480..f12c72a 100644 --- a/node.go +++ b/node.go @@ -4,7 +4,6 @@ import ( "bytes" "context" "encoding/json" - "errors" "fmt" "log/slog" "sort" @@ -93,23 +92,24 @@ type Node struct { SerialDevices []SerialDevice `json:"serial_devices"` ComputeID string `json:"compute_id"` - // Configurations is not exported, it's overloaded within the API + // Configurations is not exported, it's overloaded within the API } type nodePatchPostAlias struct { - Label string `json:"label,omitempty"` - X int `json:"x"` - Y int `json:"y"` - HideLinks bool `json:"hide_links"` - NodeDefinition string `json:"node_definition,omitempty"` - ImageDefinition string `json:"image_definition,omitempty"` - Configuration any `json:"configuration,omitempty"` - CPUs int `json:"cpus,omitempty"` - CPUlimit int `json:"cpu_limit,omitempty"` - RAM int `json:"ram,omitempty"` - DataVolume int `json:"data_volume,omitempty"` - BootDiskSize int `json:"boot_disk_size,omitempty"` - Tags []string `json:"tags"` + Label string `json:"label,omitempty"` + X int `json:"x"` + Y int `json:"y"` + HideLinks bool `json:"hide_links"` + NodeDefinition string `json:"node_definition,omitempty"` + ImageDefinition string `json:"image_definition,omitempty"` + Configuration *string `json:"configuration,omitempty"` + Configurations []NodeConfig `json:"-"` + CPUs int `json:"cpus,omitempty"` + CPUlimit int `json:"cpu_limit,omitempty"` + RAM int `json:"ram,omitempty"` + DataVolume int `json:"data_volume,omitempty"` + BootDiskSize int `json:"boot_disk_size,omitempty"` + Tags []string `json:"tags"` } func newNodeAlias(node *Node, update bool) nodePatchPostAlias { @@ -131,6 +131,8 @@ func newNodeAlias(node *Node, update bool) nodePatchPostAlias { // these can be changed but only when the node VM doesn't exist if node.State == NodeStateDefined { npp.Configuration = node.Configuration + npp.Configurations = make([]NodeConfig, len(node.Configurations)) + copy(npp.Configurations, node.Configurations) npp.CPUs = node.CPUs npp.CPUlimit = node.CPUlimit npp.RAM = node.RAM @@ -144,6 +146,8 @@ func newNodeAlias(node *Node, update bool) nodePatchPostAlias { npp.NodeDefinition = node.NodeDefinition } + // slog.Warn("NODE", slog.Any("node", node), slog.Any("npp", npp)) + return npp } @@ -193,11 +197,6 @@ func (n *Node) UnmarshalJSON(data []byte) error { if err != nil { return err } - if len(na.Configurations) == 1 { - config := na.Configurations[0].Content - na.Configuration = &config - na.Configurations = nil - } default: return fmt.Errorf("unexpected type: %T", thing) } @@ -207,107 +206,59 @@ func (n *Node) UnmarshalJSON(data []byte) error { } func (node *Node) MarshalJSON() ([]byte, error) { - type nodeAlias Node - if len(node.Configurations) > 1 { + type alias Node + if len(node.Configurations) > 0 { node.Configuration = nil return json.Marshal(&struct { - *nodeAlias + *alias NamedConfig []NodeConfig `json:"configuration"` }{ - (*nodeAlias)(node), + (*alias)(node), node.Configurations, }) } - return json.Marshal((*nodeAlias)(node)) + return json.Marshal((*alias)(node)) } -func (c *Client) updateCachedNode(existingNode, node *Node) *Node { - // only copy fields which can be updated - existingNode.Label = node.Label - existingNode.X = node.X - existingNode.Y = node.Y - existingNode.Tags = node.Tags - existingNode.HideLinks = node.HideLinks - - // these can be changed but only when the node VM doesn't exist - if node.State == NodeStateDefined { - existingNode.Configuration = node.Configuration - existingNode.Configurations = node.Configurations - existingNode.CPUs = node.CPUs - existingNode.CPUlimit = node.CPUlimit - existingNode.RAM = node.RAM - existingNode.DataVolume = node.DataVolume - existingNode.BootDiskSize = node.BootDiskSize - existingNode.ImageDefinition = node.ImageDefinition +func (node Node) SameConfig(other Node) bool { + if node.Configuration != nil && other.Configuration != nil && *other.Configuration != *node.Configuration { + return false } - // these can never be updated: - // - existingNode.NodeDefinition - return existingNode -} -func (c *Client) cacheNode(node *Node, err error) (*Node, error) { - if !c.useCache || err != nil { - return node, err + if len(node.Configurations) != len(other.Configurations) { + return false } - c.mu.RLock() - lab, ok := c.labCache[node.LabID] - c.mu.RUnlock() - if !ok { - return node, err - } - - c.mu.RLock() - existingNode, ok := lab.Nodes[node.ID] - c.mu.RUnlock() - if ok { - return c.updateCachedNode(existingNode, node), nil - } - - if lab.Nodes != nil { - c.mu.Lock() - lab.Nodes[node.ID] = node - c.mu.Unlock() - } - return node, nil -} - -func (c *Client) getCachedNode(lab_id, id string) (*Node, bool) { - if !c.useCache { - return nil, false - } - c.mu.RLock() - lab, ok := c.labCache[lab_id] - c.mu.RUnlock() - if !ok { - return nil, false + for idx, cfg := range node.Configurations { + if cfg.Name != other.Configurations[idx].Name { + return false + } + if cfg.Content != other.Configurations[idx].Content { + return false + } } - node, ok := lab.Nodes[id] - return node, ok + return true } -func (c *Client) deleteCachedNode(node *Node, err error) error { - if !c.useCache || err != nil { - return err - } - - c.mu.RLock() - lab, ok := c.labCache[node.LabID] - c.mu.RUnlock() - if !ok { - return err +func (node nodePatchPostAlias) MarshalJSON() ([]byte, error) { + type alias nodePatchPostAlias + if len(node.Configurations) > 0 { + node.Configuration = nil + return json.Marshal(&struct { + alias + NamedConfig []NodeConfig `json:"configuration"` + }{ + (alias)(node), + node.Configurations, + }) } - - c.mu.Lock() - delete(lab.Nodes, node.ID) - c.mu.Unlock() - return nil + return json.Marshal((alias)(node)) } func (c *Client) getNodesForLab(ctx context.Context, lab *Lab) error { api := fmt.Sprintf("labs/%s/nodes?data=true", lab.ID) - if c.namedConfigs { + if c.useNamedConfigs { api += "&operational=true&exclude_configurations=false" } @@ -343,7 +294,7 @@ func (c *Client) nodeSetConfigData(ctx context.Context, node *Node, data any) er if err != nil { return err } - _, err = c.cacheNode(c.NodeGet(ctx, node, true)) + _, err = c.NodeGet(ctx, node) return err } @@ -375,15 +326,6 @@ func (c *Client) NodeUpdate(ctx context.Context, node *Node) (*Node, error) { postAlias := newNodeAlias(node, true) - if len(node.Configurations) > 0 { - if !c.namedConfigs { - err := errors.New("named configs are not supported by the controller!") - slog.Error("update", slog.Any("error", err)) - return node, err - } - postAlias.Configuration = node.Configurations - } - buf := &bytes.Buffer{} err := json.NewEncoder(buf).Encode(postAlias) if err != nil { @@ -396,7 +338,7 @@ func (c *Client) NodeUpdate(ctx context.Context, node *Node) (*Node, error) { if err != nil { return nil, err } - return c.cacheNode(c.NodeGet(ctx, node, true)) + return c.NodeGet(ctx, node) } // NodeStart starts the given node. @@ -477,17 +419,11 @@ func (c *Client) NodeCreate(ctx context.Context, node *Node) (*Node, error) { node.Interfaces = InterfaceList{} // fetch the node again, with all data - return c.cacheNode(c.NodeGet(ctx, node, true)) + return c.NodeGet(ctx, node) } // NodeGet returns the node identified by its `ID` and `LabID` in the provided node. -func (c *Client) NodeGet(ctx context.Context, node *Node, nocache bool) (*Node, error) { - if !nocache { - if node, ok := c.getCachedNode(node.LabID, node.ID); ok { - return node, nil - } - } - +func (c *Client) NodeGet(ctx context.Context, node *Node) (*Node, error) { /* SIMPLE-5052 -- results are different for simplified=true vs false for the inherited values. In the simplified case, all values are always null. */ @@ -495,23 +431,19 @@ func (c *Client) NodeGet(ctx context.Context, node *Node, nocache bool) (*Node, var err error newNode := Node{} api := fmt.Sprintf("labs/%s/nodes/%s", node.LabID, node.ID) - if c.namedConfigs { + // slog.Warn("using named ####", slog.Any("client", c)) + if c.useNamedConfigs { + slog.Warn("using named configs") api += "?operational=true&exclude_configurations=false" } err = c.jsonGet(ctx, api, &newNode, 0) - // for backwards compatibility, store single config in the old field name - // and reset the multi-config field. - if len(newNode.Configurations) == 1 { - newNode.Configuration = &newNode.Configurations[0].Content - newNode.Configurations = nil - } - return c.cacheNode(&newNode, err) + return &newNode, err } // NodeDestroy deletes the node from the controller. func (c *Client) NodeDestroy(ctx context.Context, node *Node) error { api := fmt.Sprintf("labs/%s/nodes/%s", node.LabID, node.ID) - return c.deleteCachedNode(node, c.jsonDelete(ctx, api, 0)) + return c.jsonDelete(ctx, api, 0) } // NodeWipe removes all runtime data from a node on the controller/compute. diff --git a/node_test.go b/node_test.go index 50b438f..d68b374 100644 --- a/node_test.go +++ b/node_test.go @@ -14,6 +14,7 @@ var ( "id": "node1", "lab_id": "lab1", "label": "alpine-0", + "configuration": "helloo", "node_definition": "alpine", "state": "STARTED", "tags": [ "tag1", "tag2" ] @@ -26,11 +27,29 @@ var ( "node_definition": "alpine", "state": "STOPPED" }`) + + node1_namedConfigs = []byte(`{ + "id": "node1", + "lab_id": "lab1", + "label": "alpine-0", + "node_definition": "alpine", + "configuration": [ + { + "name": "config", + "content": "hostname node1" + } + ], + "state": "STARTED", + "tags": [ "tag1", "tag2" ] + }`) ) -func TestClient_NodeMapMarschalJSON(t *testing.T) { +func TestClient_NodeMapMarshalJSON(t *testing.T) { nm := NodeMap{ - "zzz": &Node{ID: "zzz"}, + "zzz": &Node{ID: "zzz", Configurations: []NodeConfig{ + {Name: "main", Content: "bla"}, + {Name: "second", Content: "blabla"}, + }}, "aaa": &Node{ID: "aaa"}, } b, err := nm.MarshalJSON() @@ -165,29 +184,21 @@ func TestClient_NodeUpdate(t *testing.T) { }, } - // node99 := &Node{LabID: "lab99", ID: "node99"} - lab := &Lab{ID: "lab99", Nodes: make(NodeMap)} - // lab.Nodes["node99"] = node99 - tc.client.labCache["lab99"] = lab - - for _, useCache := range []bool{true, false} { - tc.client.useCache = useCache - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - tc.mr.SetData(tt.responses) - node := Node{ - LabID: "lab99", ID: "node99", X: 100, Y: 100, - Tags: []string{"newtag"}, - } - resultNode, err := tc.client.NodeUpdate(tc.ctx, &node) - _ = resultNode - if (err != nil) != tt.wantErr { - t.Errorf("Client.NodeUpdate() error = %v, wantErr %v", err, tt.wantErr) - return - } - assert.True(t, tc.mr.Empty()) - }) - } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + tc.mr.SetData(tt.responses) + node := Node{ + LabID: "lab99", ID: "node99", X: 100, Y: 100, + Tags: []string{"newtag"}, + } + resultNode, err := tc.client.NodeUpdate(tc.ctx, &node) + _ = resultNode + if (err != nil) != tt.wantErr { + t.Errorf("Client.NodeUpdate() error = %v, wantErr %v", err, tt.wantErr) + return + } + assert.True(t, tc.mr.Empty()) + }) } } @@ -213,22 +224,56 @@ func TestClient_NodeFuncs(t *testing.T) { node99 := &Node{LabID: "lab99", ID: "node99"} lab := &Lab{ID: "lab99", Nodes: make(NodeMap)} lab.Nodes["node99"] = node99 - tc.client.labCache["lab99"] = lab - - for _, useCache := range []bool{true, false} { - tc.client.useCache = useCache - for tfname, tf := range funcs { - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - tc.mr.SetData(tt.responses) - err := tf(tc.ctx, &Node{ID: "node99", LabID: "lab99"}) - if (err != nil) != tt.wantErr { - t.Errorf("%s error = %v, wantErr %v", tfname, err, tt.wantErr) - return - } - assert.True(t, tc.mr.Empty()) - }) - } + for tfname, tf := range funcs { + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + tc.mr.SetData(tt.responses) + err := tf(tc.ctx, &Node{ID: "node99", LabID: "lab99"}) + if (err != nil) != tt.wantErr { + t.Errorf("%s error = %v, wantErr %v", tfname, err, tt.wantErr) + return + } + assert.True(t, tc.mr.Empty()) + }) } } } + +func TestClient_NodeSetNamedConfigs(t *testing.T) { + tc := newAuthedTestAPIclient() + tc.client.useNamedConfigs = true + + dataWithUser := mr.MockRespList{ + mr.MockResp{Data: []byte("\"node1\""), URL: `/labs/lab1/nodes/node1$`}, + mr.MockResp{Data: node1_namedConfigs, URL: `/labs/lab1/nodes/node1.*exclude_configurations=false$`}, + } + + tests := []struct { + name string + configuration []NodeConfig + responses mr.MockRespList + wantErr bool + }{ + { + "good", + []NodeConfig{ + {Name: "Main", Content: "hostname bla"}, + }, + dataWithUser, + false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + tc.mr.SetData(tt.responses) + node := Node{LabID: "lab1", ID: "node1"} + err := tc.client.NodeSetNamedConfigs(tc.ctx, &node, tt.configuration) + if (err != nil) != tt.wantErr { + t.Errorf("Client.NodeSetNamedConfig() error = %v, wantErr %v", err, tt.wantErr) + return + } + assert.True(t, tc.mr.Empty()) + }) + } +} diff --git a/system.go b/system.go index dc6ba8d..73f3f80 100644 --- a/system.go +++ b/system.go @@ -65,10 +65,13 @@ func (c *Client) versionCheck(ctx context.Context, depth int32) error { if !ok { return versionError(sv.Version) } + + // unset useNamedConfig if necessary constraint, _ = semver.NewConstraint(namedConfigsConstraint) if ok = constraint.Check(v); ok { slog.Info("named configs supported") - c.namedConfigs = true + } else { + c.useNamedConfigs = false } c.version = sv.Version return nil @@ -79,6 +82,12 @@ func (c *Client) Version() string { return c.version } +// Turns on the use of named configs (only with 2.7.0 and newer) +func (c *Client) UseNamedConfigs() { + slog.Info("USE named configs") + c.useNamedConfigs = true +} + // Ready returns nil if the system is compatible and ready func (c *Client) Ready(ctx context.Context) error { // we can safely assume depth 0 as the API endpoint does not require diff --git a/system_test.go b/system_test.go index ebd53fe..5605a7c 100644 --- a/system_test.go +++ b/system_test.go @@ -8,9 +8,10 @@ import ( ) func TestClient_VersionCheck(t *testing.T) { - c := New("https://bla.bla", true, useCache) + c := New("https://bla.bla", true) mrClient, ctx := mr.NewMockResponder() c.httpClient = mrClient + c.useNamedConfigs = true c.state.set(stateAuthenticated) tests := []struct { @@ -19,9 +20,11 @@ func TestClient_VersionCheck(t *testing.T) { wantErr bool canNamedCfg bool }{ - {"too old", `{"version": "2.1.0","ready": true}`, true, false}, - {"garbage", `{"version": "garbage","ready": true}`, true, false}, - {"too new", `{"version": "2.35.0","ready": true}`, true, false}, + // these three yield an error, useNamedConfigs is untouched + {"too old", `{"version": "2.1.0","ready": true}`, true, true}, + {"garbage", `{"version": "garbage","ready": true}`, true, true}, + {"too new", `{"version": "2.35.0","ready": true}`, true, true}, + // the rest will reset useNamedConfigs, if needed {"perfect", `{"version": "2.4.0","ready": true}`, false, false}, {"actual", `{"version": "2.4.0+build.1","ready": true}`, false, false}, {"newer", `{"version": "2.4.1","ready": true}`, false, false}, @@ -32,11 +35,12 @@ func TestClient_VersionCheck(t *testing.T) { for _, tt := range tests { mrClient.SetData(mr.MockRespList{{Data: []byte(tt.wantJSON)}}) t.Run(tt.name, func(t *testing.T) { + c.UseNamedConfigs() if err := c.versionCheck(ctx, 0); (err != nil) != tt.wantErr { t.Errorf("Client.VersionCheck() error = %v, wantErr %v", err, tt.wantErr) } - if tt.canNamedCfg != c.namedConfigs { - t.Errorf("Client.VersionCheck() canNamedConfigs is = %t, want %t", c.namedConfigs, tt.canNamedCfg) + if tt.canNamedCfg != c.useNamedConfigs { + t.Errorf("Client.VersionCheck() useNamedConfigs is = %t, want %t", c.useNamedConfigs, tt.canNamedCfg) } }) if !mrClient.Empty() { @@ -46,7 +50,7 @@ func TestClient_VersionCheck(t *testing.T) { } func TestClient_NotReady(t *testing.T) { - c := New("https://bla.bla", true, useCache) + c := New("https://bla.bla", true) mrClient, ctx := mr.NewMockResponder() c.httpClient = mrClient c.state.set(stateAuthenticated) From 6f0c72776b1bce2347de4f279f1e05560dce9d83 Mon Sep 17 00:00:00 2001 From: Ralph Schmieder Date: Thu, 13 Jun 2024 13:23:12 +0200 Subject: [PATCH 4/7] Add slog change to CHANGELOG --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 866a81b..e8f41c7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ Lists the changes in the gocmlclient package. ## Version 0.1.0 - making a somewhat bigger version bump due to some bigger changes +- moved logging to log/slog - removed all the caching logic / code. It didn't really work well due to races. In addition, TF doesn't really keep a connection / client over multiple resource calls so the caching was somewhat limited even it would have properly worked (which it did not). - named configurations (added with CML 2.7) - added some tests for the named configs From a934564a579406f1f3172be334a56ebc8e471096 Mon Sep 17 00:00:00 2001 From: Ralph Schmieder Date: Thu, 13 Jun 2024 13:47:15 +0200 Subject: [PATCH 5/7] Updated GH actions and Go version --- .github/workflows/go.yml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/go.yml b/.github/workflows/go.yml index a4e7c00..dd3aa74 100644 --- a/.github/workflows/go.yml +++ b/.github/workflows/go.yml @@ -11,12 +11,12 @@ jobs: build: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 - name: Set up Go - uses: actions/setup-go@v3 + uses: actions/setup-go@v5 with: - go-version: 1.18 + go-version: 1.21 - name: Build run: go build -v ./... From b34d3b3a2dbaf98d7fb373a9e057fd7fa4b69091 Mon Sep 17 00:00:00 2001 From: Ralph Schmieder Date: Thu, 13 Jun 2024 15:48:14 +0200 Subject: [PATCH 6/7] Better node unmarshalling tests --- node_test.go | 113 +++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 113 insertions(+) diff --git a/node_test.go b/node_test.go index d68b374..f459207 100644 --- a/node_test.go +++ b/node_test.go @@ -277,3 +277,116 @@ func TestClient_NodeSetNamedConfigs(t *testing.T) { }) } } + +func TestNode_SameConfig(t *testing.T) { + one1 := "one" + one2 := "one" + two := "two" + + type fields struct { + Configuration *string + Configurations []NodeConfig + } + type args struct { + other Node + } + tests := []struct { + name string + fields fields + other Node + want bool + }{ + {"test", fields{Configuration: nil}, Node{Configuration: nil}, true}, + {"test", fields{Configuration: &one1}, Node{Configuration: &one2}, true}, + {"test", fields{Configuration: &one1}, Node{Configuration: &two}, false}, + {"test", fields{ + Configurations: []NodeConfig{ + { + Name: "bla", + Content: "this", + }, + }, + }, + Node{Configurations: []NodeConfig{}}, + false, + }, + {"test", fields{ + Configurations: []NodeConfig{ + { + Name: "bla", + Content: "this", + }, + }, + }, + Node{Configurations: []NodeConfig{ + { + Name: "bla", + Content: "somethingelse", + }, + }, + }, + false, + }, + {"test", fields{ + Configurations: []NodeConfig{ + { + Name: "bla", + Content: "this", + }, + }, + }, + Node{Configurations: []NodeConfig{ + { + Name: "something", + Content: "this", + }, + }, + }, + false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + node := Node{ + Configuration: tt.fields.Configuration, + Configurations: tt.fields.Configurations, + } + if got := node.SameConfig(tt.other); got != tt.want { + t.Errorf("Node.SameConfig() = %v, want %v", got, tt.want) + } + }) + } +} + +func TestNode_UnmarshalJSON(t *testing.T) { + tests := []struct { + name string + data []byte + wantErr bool + }{ + {"nil-config", []byte(`null`), false}, + {"no-config", []byte(`{"id": "bla"}`), false}, + {"ok-config", []byte(`{"id": "bla", "configuration": "hostname bla"}`), false}, + {"ok-named-config", []byte(`{"id": "bla", "configuration": [{"name": "name", "content": "content"}]}`), false}, + {"invalid-named-config", []byte(`{"id": "bla", "configuration": 10}`), true}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var n Node + var err error + if err = json.Unmarshal(tt.data, &n); (err != nil) != tt.wantErr { + t.Errorf("Node.UnmarshalJSON() error = %v, wantErr %v", err, tt.wantErr) + } + }) + } + + // direct call with error + t.Run("manual error", func(t *testing.T) { + var bla Node + err := bla.UnmarshalJSON([]byte(`error`)) + if err == nil { + t.Errorf("Node.UnmarshalJSON() expected error, got nil") + } + }) + +} From 8547e89f7f1bab36077bf6eef5113cb3b3b2962a Mon Sep 17 00:00:00 2001 From: Ralph Schmieder Date: Thu, 13 Jun 2024 15:56:08 +0200 Subject: [PATCH 7/7] More cleanup --- iface.go | 2 -- node.go | 9 +++------ 2 files changed, 3 insertions(+), 8 deletions(-) diff --git a/iface.go b/iface.go index b3f54f9..b2e6747 100644 --- a/iface.go +++ b/iface.go @@ -5,7 +5,6 @@ import ( "context" "encoding/json" "fmt" - "log/slog" "sort" ) @@ -76,7 +75,6 @@ func (c *Client) getInterfacesForNode(ctx context.Context, node *Node) error { interfaceList := InterfaceList{} err := c.jsonGet(ctx, api, &interfaceList, 0) if err != nil { - slog.Info("done", "lab", node.LabID, "node", node.ID, slog.Any("err", err)) return err } diff --git a/node.go b/node.go index f12c72a..eade499 100644 --- a/node.go +++ b/node.go @@ -5,7 +5,6 @@ import ( "context" "encoding/json" "fmt" - "log/slog" "sort" ) @@ -424,16 +423,14 @@ func (c *Client) NodeCreate(ctx context.Context, node *Node) (*Node, error) { // NodeGet returns the node identified by its `ID` and `LabID` in the provided node. func (c *Client) NodeGet(ctx context.Context, node *Node) (*Node, error) { - /* SIMPLE-5052 -- results are different for simplified=true vs false - for the inherited values. In the simplified case, all values are always - null. */ + // SIMPLE-5052 -- results are different for simplified=true vs false for + // the inherited values. In the simplified case, all values are always + // null. var err error newNode := Node{} api := fmt.Sprintf("labs/%s/nodes/%s", node.LabID, node.ID) - // slog.Warn("using named ####", slog.Any("client", c)) if c.useNamedConfigs { - slog.Warn("using named configs") api += "?operational=true&exclude_configurations=false" } err = c.jsonGet(ctx, api, &newNode, 0)