internal/config/dynconfig: move dynamic config to its own package
This lets us use our log package, and internal.Experiment.
Change-Id: I48772c2f5d254cfa57c5b791535b9dad9bd3be8d
Reviewed-on: https://go-review.googlesource.com/c/pkgsite/+/256518
Trust: Jonathan Amsterdam <jba@google.com>
Run-TryBot: Jonathan Amsterdam <jba@google.com>
TryBot-Result: kokoro <noreply+kokoro@google.com>
Reviewed-by: Julie Qiu <julie@golang.org>
diff --git a/cmd/internal/cmdconfig/cmdconfig.go b/cmd/internal/cmdconfig/cmdconfig.go
index 1315813..f6f3acf 100644
--- a/cmd/internal/cmdconfig/cmdconfig.go
+++ b/cmd/internal/cmdconfig/cmdconfig.go
@@ -13,6 +13,7 @@
"cloud.google.com/go/errorreporting"
"golang.org/x/pkgsite/internal"
"golang.org/x/pkgsite/internal/config"
+ "golang.org/x/pkgsite/internal/config/dynconfig"
"golang.org/x/pkgsite/internal/log"
"golang.org/x/pkgsite/internal/middleware"
)
@@ -52,19 +53,11 @@
// Ignore getter, use dynamic config.
log.Infof(ctx, "using dynamic config for experiments")
getter = func(ctx context.Context) ([]*internal.Experiment, error) {
- var exps []*internal.Experiment
- dc, err := cfg.ReadDynamic(ctx)
+ dc, err := dynconfig.Read(ctx, cfg.Bucket, cfg.DynamicObject)
if err != nil {
return nil, err
}
- for _, cexp := range dc.Experiments {
- exps = append(exps, &internal.Experiment{
- Name: cexp.Name,
- Description: cexp.Description,
- Rollout: cexp.Rollout,
- })
- }
- return exps, nil
+ return dc.Experiments, nil
}
}
e, err := middleware.NewExperimenter(ctx, 1*time.Minute, getter, reportingClient)
diff --git a/internal/config/config.go b/internal/config/config.go
index 687acda..6ea2452 100644
--- a/internal/config/config.go
+++ b/internal/config/config.go
@@ -169,8 +169,10 @@
// In case of invalid/empty value, all logs will be printed.
LogLevel string
- // GCS bucket and object for dynamic config.
- bucket, dynamicObject string
+ // GCS Bucket for overrides and dynamic config.
+ Bucket string
+ // GCS object name for dynamic config.
+ DynamicObject string
}
// AppVersionLabel returns the version label for the current instance. This is
@@ -398,23 +400,17 @@
SuccsToGreen: GetEnvInt("GO_DISCOVERY_TEEPROXY_SUCCS_TO_GREEN", 20),
},
LogLevel: os.Getenv("GO_DISCOVERY_LOG_LEVEL"),
- bucket: os.Getenv("GO_DISCOVERY_CONFIG_BUCKET"),
- dynamicObject: os.Getenv("GO_DISCOVERY_CONFIG_DYNAMIC"),
+ Bucket: os.Getenv("GO_DISCOVERY_CONFIG_BUCKET"),
+ DynamicObject: os.Getenv("GO_DISCOVERY_CONFIG_DYNAMIC"),
}
// Check that the dynamic config is readable, but don't do anything with it.
- if cfg.bucket == "" {
+ if cfg.Bucket == "" {
return nil, errors.New("GO_DISCOVERY_CONFIG_BUCKET must be set")
}
- if cfg.dynamicObject == "" {
+ if cfg.DynamicObject == "" {
return nil, errors.New("GO_DISCOVERY_CONFIG_DYNAMIC must be set")
}
- if _, err := cfg.ReadDynamic(ctx); err != nil {
- // This should be an error, but initially, to test permissions in the prod environment,
- // just log it.
- log.Printf("ERROR: %v", err)
- }
-
if cfg.OnGCP() {
// Zone is not available in the environment but can be queried via the metadata API.
zone, err := gceMetadata(ctx, "instance/zone")
@@ -479,11 +475,11 @@
// to re-deploy. (Otherwise, do not use it.)
overrideObj := os.Getenv("GO_DISCOVERY_CONFIG_OVERRIDE")
if overrideObj != "" {
- overrideBytes, err := readOverrideFile(ctx, cfg.bucket, overrideObj)
+ overrideBytes, err := readOverrideFile(ctx, cfg.Bucket, overrideObj)
if err != nil {
log.Print(err)
} else {
- log.Printf("processing overrides from gs://%s/%s", cfg.bucket, overrideObj)
+ log.Printf("processing overrides from gs://%s/%s", cfg.Bucket, overrideObj)
processOverrides(cfg, overrideBytes)
}
}
diff --git a/internal/config/dynamic.go b/internal/config/dynamic.go
deleted file mode 100644
index cd46296..0000000
--- a/internal/config/dynamic.go
+++ /dev/null
@@ -1,71 +0,0 @@
-// Copyright 2020 The Go Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style
-// license that can be found in the LICENSE file.
-
-package config
-
-import (
- "context"
- "io/ioutil"
- "log"
-
- "cloud.google.com/go/storage"
- "github.com/ghodss/yaml"
- "golang.org/x/pkgsite/internal/derrors"
-)
-
-// DynamicConfig holds configuration that can change over the lifetime of the
-// process. It is loaded from a GCS file or other external source.
-type DynamicConfig struct {
- // Fields can be added at any time, but removing or changing a field
- // requires careful coordination with the config file contents.
-
- Experiments []Experiment
-}
-
-// An experiment, as dynamically configured.
-// Intended to match internal.Experiment, but we can't use
-// that directly due to an import cycle.
-type Experiment struct {
- Name string
- Rollout uint
- Description string
-}
-
-// ReadDynamic reads the dynamic configuration.
-func (c *Config) ReadDynamic(ctx context.Context) (*DynamicConfig, error) {
- return ReadDynamic(ctx, c.bucket, c.dynamicObject)
-}
-
-// ReadDynamic reads dynamic configuration from the given GCS bucket and object.
-func ReadDynamic(ctx context.Context, bucket, object string) (_ *DynamicConfig, err error) {
- defer derrors.Wrap(&err, "ReadDynamic(%q, %q)", bucket, object)
-
- log.Printf("reading experiments from gs://%s/%s", bucket, object)
- client, err := storage.NewClient(ctx)
- if err != nil {
- return nil, err
- }
- defer client.Close()
- r, err := client.Bucket(bucket).Object(object).NewReader(ctx)
- if err != nil {
- return nil, err
- }
- defer r.Close()
- data, err := ioutil.ReadAll(r)
- if err != nil {
- return nil, err
- }
- return ParseDynamic(data)
-}
-
-// ParseDynamic parses yamlData as a YAML description of DynamicConfig.
-func ParseDynamic(yamlData []byte) (_ *DynamicConfig, err error) {
- defer derrors.Wrap(&err, "ParseDynamic(data)")
-
- var dc DynamicConfig
- if err := yaml.Unmarshal(yamlData, &dc); err != nil {
- return nil, err
- }
- return &dc, nil
-}
diff --git a/internal/config/dynconfig/dynconfig.go b/internal/config/dynconfig/dynconfig.go
new file mode 100644
index 0000000..7a4815a
--- /dev/null
+++ b/internal/config/dynconfig/dynconfig.go
@@ -0,0 +1,61 @@
+// Copyright 2020 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+// Package dynconfig supports dynamic configuration for pkgsite services.
+// Dynamic configuration is read from a file and can change over the lifetime of
+// the process.
+package dynconfig
+
+import (
+ "context"
+ "io/ioutil"
+
+ "cloud.google.com/go/storage"
+ "github.com/ghodss/yaml"
+ "golang.org/x/pkgsite/internal"
+ "golang.org/x/pkgsite/internal/derrors"
+ "golang.org/x/pkgsite/internal/log"
+)
+
+// DynamicConfig holds configuration that can change over the lifetime of the
+// process. It is loaded from a GCS file or other external source.
+type DynamicConfig struct {
+ // Fields can be added at any time, but removing or changing a field
+ // requires careful coordination with the config file contents.
+
+ Experiments []*internal.Experiment
+}
+
+// Read reads dynamic configuration from the given GCS bucket and object.
+func Read(ctx context.Context, bucket, object string) (_ *DynamicConfig, err error) {
+ defer derrors.Wrap(&err, "dynconfig.Read(%q, %q)", bucket, object)
+
+ log.Infof(ctx, "reading dynamic config from gs://%s/%s", bucket, object)
+ client, err := storage.NewClient(ctx)
+ if err != nil {
+ return nil, err
+ }
+ defer client.Close()
+ r, err := client.Bucket(bucket).Object(object).NewReader(ctx)
+ if err != nil {
+ return nil, err
+ }
+ defer r.Close()
+ data, err := ioutil.ReadAll(r)
+ if err != nil {
+ return nil, err
+ }
+ return Parse(data)
+}
+
+// Parse parses yamlData as a YAML description of DynamicConfig.
+func Parse(yamlData []byte) (_ *DynamicConfig, err error) {
+ defer derrors.Wrap(&err, "dynconfig.Parse(data)")
+
+ var dc DynamicConfig
+ if err := yaml.Unmarshal(yamlData, &dc); err != nil {
+ return nil, err
+ }
+ return &dc, nil
+}
diff --git a/internal/experiment.go b/internal/experiment.go
index 99d39bc..1ef1a0d 100644
--- a/internal/experiment.go
+++ b/internal/experiment.go
@@ -24,6 +24,10 @@
// Experiment holds data associated with an experimental feature for frontend
// or worker.
type Experiment struct {
+ // This struct is used to decode dynamic config (see
+ // internal/config/dynconfig). Make sure that changes to this struct are
+ // coordinated with the deployment of config files.
+
// Name is the name of the feature.
Name string
diff --git a/internal/middleware/experiment.go b/internal/middleware/experiment.go
index 393c4f1..a977952 100644
--- a/internal/middleware/experiment.go
+++ b/internal/middleware/experiment.go
@@ -48,6 +48,7 @@
reporter: rep,
pollEvery: pollEvery,
}
+ // If we can't load the initial state, then fail.
if err := e.loadNextSnapshot(ctx); err != nil {
return nil, err
}
@@ -109,6 +110,8 @@
case <-ticker.C:
ctx2, cancel := context.WithTimeout(ctx, e.pollEvery)
if err := e.loadNextSnapshot(ctx2); err != nil {
+ // We already have a snapshot to fall back on, so log and report
+ // the error, but don't fail.
log.Error(ctx, err)
if e.reporter != nil {
e.reporter.Report(errorreporting.Entry{
