Skip to content

Commit

Permalink
Fix parsing int 0 as duration in yaml
Browse files Browse the repository at this point in the history
After upgrading to gopkg.in/yaml.v3 in [#2468], @pstibrany noticed that
int 0 is no longer a valid time.Duration value.

I've sent a PR to fix that: go-yaml/yaml#876 and
cherry-picked the change into the branch we're using in Mimir as the
replacement.

[#2468]: #2468

Signed-off-by: Oleg Zaytsev <mail@olegzaytsev.com>
  • Loading branch information
colega committed Jul 20, 2022
1 parent 96a9824 commit 89578db
Show file tree
Hide file tree
Showing 5 changed files with 45 additions and 6 deletions.
6 changes: 4 additions & 2 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -255,5 +255,7 @@ replace github.com/vimeo/galaxycache => github.com/thanos-community/galaxycache
// grpc v1.46.0 removed "WithBalancerName()" API, still in use by weaveworks/commons.
replace google.golang.org/grpc => google.golang.org/grpc v1.45.0

// gopkg.in/yaml.v3 + https://github.com/go-yaml/yaml/pull/691
replace gopkg.in/yaml.v3 => github.com/colega/go-yaml-yaml v0.0.0-20220720070545-aaba007ebc22
// gopkg.in/yaml.v3
// + https://github.com/go-yaml/yaml/pull/691
// + https://github.com/go-yaml/yaml/pull/876
replace gopkg.in/yaml.v3 => github.com/colega/go-yaml-yaml v0.0.0-20220720105220-255a8d16d094
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -281,8 +281,8 @@ github.com/cockroachdb/apd v1.1.0/go.mod h1:8Sl8LxpKi29FqWXR16WEFZRNSz3SoPzUzeMe
github.com/cockroachdb/cockroach-go v0.0.0-20181001143604-e0a95dfd547c/go.mod h1:XGLbWH/ujMcbPbhZq52Nv6UrCghb1yGn//133kEsvDk=
github.com/cockroachdb/datadriven v0.0.0-20190809214429-80d97fb3cbaa/go.mod h1:zn76sxSg3SzpJ0PPJaLDCu+Bu0Lg3sKTORVIj19EIF8=
github.com/codahale/hdrhistogram v0.0.0-20161010025455-3a0bb77429bd/go.mod h1:sE/e/2PUdi/liOCUjSTXgM1o87ZssimdTWN964YiIeI=
github.com/colega/go-yaml-yaml v0.0.0-20220720070545-aaba007ebc22 h1:uVG5v+c6ndz9seCorYjQmlVlPbh3OMcMWJzAJZWdM/g=
github.com/colega/go-yaml-yaml v0.0.0-20220720070545-aaba007ebc22/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
github.com/colega/go-yaml-yaml v0.0.0-20220720105220-255a8d16d094 h1:FpZSn61BWXbtyH68+uSv416veEswX1M2HRyQfdHnOyQ=
github.com/colega/go-yaml-yaml v0.0.0-20220720105220-255a8d16d094/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
github.com/containerd/containerd v1.2.7/go.mod h1:bC6axHOhabU15QhwfG7w5PipXdVtMXFTttgp+kVtyUA=
github.com/coreos/etcd v3.3.25+incompatible h1:0GQEw6h3YnuOVdtwygkIfJ+Omx0tZ8/QkVyXI4LkbeY=
github.com/coreos/etcd v3.3.25+incompatible/go.mod h1:uF7uidLiAD3TWHmW31ZFd/JWoc32PjwdhPthX9715RE=
Expand Down
34 changes: 34 additions & 0 deletions integration/single_binary_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@
package integration

import (
"io/ioutil"
"path/filepath"
"strings"
"testing"

"github.com/grafana/e2e"
Expand Down Expand Up @@ -58,3 +61,34 @@ func TestMimirShouldStartInSingleBinaryModeWithAllMemcachedConfigured(t *testing
mimir := e2emimir.NewSingleBinary("mimir-1", e2e.MergeFlags(DefaultSingleBinaryFlags(), flags))
require.NoError(t, s.StartAndWaitReady(mimir))
}

// TestMimirCanParseIntZeroAsZeroDuration checks that integer 0 can be used as zero duration in the yaml configuration.
// When parsing config using gopkg.in/yaml.v3 this means that is should include this change: https://github.com/go-yaml/yaml/pull/876
// It is written as an acceptance test to ensure that software that vendors this (i.e., GEM) will also run this test.
func TestMimirCanParseIntZeroAsZeroDuration(t *testing.T) {
s, err := e2e.NewScenario(networkName)
require.NoError(t, err)
defer s.Close()

const singleProcessConfigFile = "docs/configurations/single-process-config-blocks.yaml"

// Use an example single process config file.
config, err := ioutil.ReadFile(filepath.Join(getMimirProjectDir(), singleProcessConfigFile))
require.NoError(t, err, "unable to read config file")

// Ensure that there's `server:` to replace in the config, otherwise we're testing nothing.
require.Containsf(t, string(config), "server:", "Config file %s doesn't contain a 'server:' section anymore.")
// Add a 'graceful_shutdown_timeout: 0' after 'server:'.
config = []byte(strings.ReplaceAll(string(config), "server:", "server:\n graceful_shutdown_timeout: 0"))
// Write the config and use it.
require.NoError(t, writeFileToSharedDir(s, mimirConfigFile, config))

// Use filesystem as storage backend to run this test faster without minio.
flags := map[string]string{
"-common.storage.backend": "filesystem",
"-common.storage.filesystem.dir": "./bucket",
}

mimir := e2emimir.NewSingleBinary("mimir-1", flags, e2emimir.WithPorts(9009, 9095), e2emimir.WithConfigFile(mimirConfigFile))
require.NoError(t, s.StartAndWaitReady(mimir))
}
3 changes: 3 additions & 0 deletions vendor/gopkg.in/yaml.v3/decode.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions vendor/modules.txt

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit 89578db

Please sign in to comment.