From 1f3ca306c4204e099d40ee4f6255fb79c72267cc Mon Sep 17 00:00:00 2001 From: Allen Ma Date: Tue, 17 Nov 2020 09:52:44 -0500 Subject: [PATCH 1/4] fixes for server --- cmd/mixtool/server.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/cmd/mixtool/server.go b/cmd/mixtool/server.go index 19da711..894679b 100644 --- a/cmd/mixtool/server.go +++ b/cmd/mixtool/server.go @@ -102,7 +102,7 @@ func (p *ruleProvisioner) provision(r io.Reader) (bool, error) { b := bytes.NewBuffer(nil) tr := io.TeeReader(r, b) - f, err := os.Open(p.ruleFile) + f, err := os.OpenFile(p.ruleFile, os.O_RDWR, 0644) if err != nil && !os.IsNotExist(err) { return false, fmt.Errorf("open rule file: %w", err) } @@ -125,6 +125,11 @@ func (p *ruleProvisioner) provision(r io.Reader) (bool, error) { return false, fmt.Errorf("truncate file: %w", err) } + _, err = f.Seek(0, 0) + if err != nil { + return false, fmt.Errorf("seek file %w", err) + } + if _, err := io.Copy(f, b); err != nil { return false, fmt.Errorf("provision rule to file: %w", err) } From bc44a9163b93a18cb970934b6b83b84c8ebbfbea Mon Sep 17 00:00:00 2001 From: Allen Ma Date: Wed, 18 Nov 2020 01:38:15 -0500 Subject: [PATCH 2/4] rewrite provision so that it loads files into memory and does a bytewise comparison --- cmd/mixtool/server.go | 69 ++++++++++++++++--------------------------- 1 file changed, 26 insertions(+), 43 deletions(-) diff --git a/cmd/mixtool/server.go b/cmd/mixtool/server.go index 894679b..d670a04 100644 --- a/cmd/mixtool/server.go +++ b/cmd/mixtool/server.go @@ -15,15 +15,14 @@ package main import ( - "bufio" "bytes" "context" - "errors" "fmt" "io" "io/ioutil" "net/http" "os" + "path/filepath" "github.com/urfave/cli" ) @@ -99,63 +98,47 @@ type ruleProvisioner struct { // to existing, does not provision them. It returns whether Prometheus should // be reloaded and if an error has occurred. func (p *ruleProvisioner) provision(r io.Reader) (bool, error) { - b := bytes.NewBuffer(nil) - tr := io.TeeReader(r, b) - - f, err := os.OpenFile(p.ruleFile, os.O_RDWR, 0644) - if err != nil && !os.IsNotExist(err) { - return false, fmt.Errorf("open rule file: %w", err) - } - if os.IsNotExist(err) { - f, err = os.Create(p.ruleFile) - if err != nil { - return false, fmt.Errorf("create rule file: %w", err) - } + fileData, err := ioutil.ReadFile(p.ruleFile) + if err != nil { + return false, fmt.Errorf("unable to read %w", err) } - equal, err := readersEqual(tr, f) + newData, err := ioutil.ReadAll(r) if err != nil { - return false, fmt.Errorf("compare existing rules with provisioned intention: %w", err) + return false, fmt.Errorf("unable to read %w", err) } - if equal { + + res := bytes.Compare(fileData, newData) + if res == 0 { return false, nil } - if err := f.Truncate(0); err != nil { - return false, fmt.Errorf("truncate file: %w", err) + // create a temp file + tempfile, err := ioutil.TempFile(filepath.Dir(p.ruleFile), "temp-mixtool") + if err != nil { + return false, fmt.Errorf("unable to create temp file %w", err) } - _, err = f.Seek(0, 0) + n, err := tempfile.Write(newData) if err != nil { - return false, fmt.Errorf("seek file %w", err) + return false, fmt.Errorf("error when writing %w", err) } - if _, err := io.Copy(f, b); err != nil { - return false, fmt.Errorf("provision rule to file: %w", err) + if n != len(newData) { + return false, fmt.Errorf("writing error, wrote %v bytes, expected %v", n, len(newData)) } - return true, nil -} + tempfile.Sync() -func readersEqual(r1, r2 io.Reader) (bool, error) { - buf1 := bufio.NewReader(r1) - buf2 := bufio.NewReader(r2) - for { - b1, err1 := buf1.ReadByte() - b2, err2 := buf2.ReadByte() - if err1 != nil && !errors.Is(err1, io.EOF) { - return false, err1 - } - if err2 != nil && !errors.Is(err2, io.EOF) { - return false, err2 - } - if errors.Is(err1, io.EOF) || errors.Is(err2, io.EOF) { - return err1 == err2, nil - } - if b1 != b2 { - return false, nil - } + // delete the original file and rename tempfile to the original file + if err = os.Remove(p.ruleFile); err != nil { + return false, fmt.Errorf("failed to remove old rule file %w", err) } + + if err = os.Rename(tempfile.Name(), p.ruleFile); err != nil { + return false, fmt.Errorf("cannot rename %w", err) + } + return true, nil } type prometheusReloader struct { From 82da88af75c2da3de108d3ab52bc1ad0fefed48b Mon Sep 17 00:00:00 2001 From: Allen Ma Date: Wed, 18 Nov 2020 12:48:04 -0500 Subject: [PATCH 3/4] add reader comparison for server --- cmd/mixtool/server.go | 65 ++++++++++++++++++++++++++++++------------- go.mod | 2 +- go.sum | 4 +++ 3 files changed, 50 insertions(+), 21 deletions(-) diff --git a/cmd/mixtool/server.go b/cmd/mixtool/server.go index d670a04..d043714 100644 --- a/cmd/mixtool/server.go +++ b/cmd/mixtool/server.go @@ -15,8 +15,9 @@ package main import ( - "bytes" + "bufio" "context" + "errors" "fmt" "io" "io/ioutil" @@ -98,45 +99,48 @@ type ruleProvisioner struct { // to existing, does not provision them. It returns whether Prometheus should // be reloaded and if an error has occurred. func (p *ruleProvisioner) provision(r io.Reader) (bool, error) { - fileData, err := ioutil.ReadFile(p.ruleFile) - if err != nil { - return false, fmt.Errorf("unable to read %w", err) - } - newData, err := ioutil.ReadAll(r) if err != nil { - return false, fmt.Errorf("unable to read %w", err) - } - - res := bytes.Compare(fileData, newData) - if res == 0 { - return false, nil + return false, fmt.Errorf("unable to read new rules: %w", err) } - // create a temp file tempfile, err := ioutil.TempFile(filepath.Dir(p.ruleFile), "temp-mixtool") if err != nil { - return false, fmt.Errorf("unable to create temp file %w", err) + return false, fmt.Errorf("unable to create temp file: %w", err) } n, err := tempfile.Write(newData) if err != nil { - return false, fmt.Errorf("error when writing %w", err) + return false, fmt.Errorf("error when writing new rules: %w", err) } if n != len(newData) { - return false, fmt.Errorf("writing error, wrote %v bytes, expected %v", n, len(newData)) + return false, fmt.Errorf("writing error, wrote %d bytes, expected %d", n, len(newData)) } tempfile.Sync() - // delete the original file and rename tempfile to the original file - if err = os.Remove(p.ruleFile); err != nil { - return false, fmt.Errorf("failed to remove old rule file %w", err) + ruleFileReader, err := os.OpenFile(p.ruleFile, os.O_RDWR, 0644) + if err != nil { + return false, fmt.Errorf("unable to read existing rules: %w", err) + } + + newFileReader, err := os.OpenFile(tempfile.Name(), os.O_RDWR, 0644) + if err != nil { + return false, fmt.Errorf("unable to open new rules file: %w", err) + } + + reload, err := readersEqual(newFileReader, ruleFileReader) + if err != nil { + return false, fmt.Errorf("error from readersEqual: %w", err) + } + + if !reload { + return false, nil } if err = os.Rename(tempfile.Name(), p.ruleFile); err != nil { - return false, fmt.Errorf("cannot rename %w", err) + return false, fmt.Errorf("cannot rename rules file: %w", err) } return true, nil } @@ -145,6 +149,27 @@ type prometheusReloader struct { prometheusReloadURL string } +func readersEqual(r1, r2 io.Reader) (bool, error) { + buf1 := bufio.NewReader(r1) + buf2 := bufio.NewReader(r2) + for { + b1, err1 := buf1.ReadByte() + b2, err2 := buf2.ReadByte() + if err1 != nil && !errors.Is(err1, io.EOF) { + return false, err1 + } + if err2 != nil && !errors.Is(err2, io.EOF) { + return false, err2 + } + if errors.Is(err1, io.EOF) || errors.Is(err2, io.EOF) { + return err1 == err2, nil + } + if b1 != b2 { + return false, nil + } + } +} + func (r *prometheusReloader) triggerReload(ctx context.Context) error { req, err := http.NewRequest("POST", r.prometheusReloadURL, nil) if err != nil { diff --git a/go.mod b/go.mod index c9dc2b0..70b8548 100644 --- a/go.mod +++ b/go.mod @@ -11,7 +11,7 @@ require ( github.com/go-macaron/inject v0.0.0-20160627170012-d8a0b8677191 // indirect github.com/go-macaron/session v0.0.0-20170320172209-b8e286a0dba8 // indirect github.com/gobuffalo/packr v1.30.1 - github.com/gobuffalo/packr/v2 v2.8.0 + github.com/gobuffalo/packr/v2 v2.8.1 github.com/google/go-jsonnet v0.16.1-0.20200908152747-b70cbd441a39 github.com/gosimple/slug v1.2.0 // indirect github.com/grafana/grafana v5.2.4+incompatible diff --git a/go.sum b/go.sum index 7dcd3e0..4bff3c9 100644 --- a/go.sum +++ b/go.sum @@ -313,6 +313,8 @@ github.com/gobuffalo/packr/v2 v2.5.1 h1:TFOeY2VoGamPjQLiNDT3mn//ytzk236VMO2j7iHx github.com/gobuffalo/packr/v2 v2.5.1/go.mod h1:8f9c96ITobJlPzI44jj+4tHnEKNt0xXWSVlXRN9X1Iw= github.com/gobuffalo/packr/v2 v2.8.0 h1:IULGd15bQL59ijXLxEvA5wlMxsmx/ZkQv9T282zNVIY= github.com/gobuffalo/packr/v2 v2.8.0/go.mod h1:PDk2k3vGevNE3SwVyVRgQCCXETC9SaONCNSXT1Q8M1g= +github.com/gobuffalo/packr/v2 v2.8.1 h1:tkQpju6i3EtMXJ9uoF5GT6kB+LMTimDWD8Xvbz6zDVA= +github.com/gobuffalo/packr/v2 v2.8.1/go.mod h1:c/PLlOuTU+p3SybaJATW3H6lX/iK7xEz5OeMf+NnJpg= github.com/gobuffalo/syncx v0.0.0-20190224160051-33c29581e754/go.mod h1:HhnNqWY95UYwwW3uSASeV7vtgYkT2t16hJgV3AEPUpw= github.com/gobwas/glob v0.2.3/go.mod h1:d3Ez4x06l9bZtSvzIay5+Yzi0fmZzPgnTbPcKjJAkT8= github.com/gogo/googleapis v1.1.0/go.mod h1:gf4bu3Q80BeJ6H1S1vYPm8/ELATdvryBaNFGgqEef3s= @@ -491,6 +493,8 @@ github.com/karrick/godirwalk v1.10.12/go.mod h1:RoGL9dQei4vP9ilrpETWE8CLOZ1kiN0L github.com/karrick/godirwalk v1.15.3/go.mod h1:j4mkqPuvaLI8mp1DroR3P6ad7cyYd4c1qeJ3RV7ULlk= github.com/karrick/godirwalk v1.15.5 h1:ErdAEFW/cKxQ5+9Gm/hopxB8ki21/di+vyNb9mHnHrA= github.com/karrick/godirwalk v1.15.5/go.mod h1:j4mkqPuvaLI8mp1DroR3P6ad7cyYd4c1qeJ3RV7ULlk= +github.com/karrick/godirwalk v1.15.8 h1:7+rWAZPn9zuRxaIqqT8Ohs2Q2Ac0msBqwRdxNCr2VVs= +github.com/karrick/godirwalk v1.15.8/go.mod h1:j4mkqPuvaLI8mp1DroR3P6ad7cyYd4c1qeJ3RV7ULlk= github.com/kisielk/errcheck v1.1.0/go.mod h1:EZBBE59ingxPouuu3KfxchcWSUPOHkagtvWXihfKN4Q= github.com/kisielk/errcheck v1.2.0/go.mod h1:/BMXB+zMLi60iA8Vv6Ksmxu/1UDYcXs4uQLJ+jE2L00= github.com/kisielk/gotool v1.0.0/go.mod h1:XhKaO+MFFWcvkIS/tQcRk01m1F5IRFswLeQ+oQHNcck= From 9e3fc24d9f38f12497f5b1ba430d0f80db667b83 Mon Sep 17 00:00:00 2001 From: Allen Ma Date: Mon, 23 Nov 2020 20:35:57 -0500 Subject: [PATCH 4/4] change reload boolean --- cmd/mixtool/server.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cmd/mixtool/server.go b/cmd/mixtool/server.go index d043714..45f50a9 100644 --- a/cmd/mixtool/server.go +++ b/cmd/mixtool/server.go @@ -130,12 +130,12 @@ func (p *ruleProvisioner) provision(r io.Reader) (bool, error) { return false, fmt.Errorf("unable to open new rules file: %w", err) } - reload, err := readersEqual(newFileReader, ruleFileReader) + equal, err := readersEqual(newFileReader, ruleFileReader) if err != nil { return false, fmt.Errorf("error from readersEqual: %w", err) } - if !reload { + if equal { return false, nil }