From 2e08a7e1c73558c0494aab0ef4be10a660568e79 Mon Sep 17 00:00:00 2001 From: "R.B. Boyer" <4903+rboyer@users.noreply.github.com> Date: Fri, 19 Jan 2024 16:31:49 -0600 Subject: [PATCH] v2: prevent use of the v2 experiments in secondary datacenters for now (#20299) Ultimately we will have to rectify wan federation with v2 catalog adjacent experiments, but for now blanket prevent usage of the resource-apis, v2dns, and v2tenancy experiments in secondary datacenters. --- .changelog/20299.txt | 3 ++ agent/config/builder.go | 15 +++++++- agent/config/builder_test.go | 71 ++++++++++++++++++++++++++++++++++++ agent/consul/server.go | 22 ++++++++--- 4 files changed, 104 insertions(+), 7 deletions(-) create mode 100644 .changelog/20299.txt diff --git a/.changelog/20299.txt b/.changelog/20299.txt new file mode 100644 index 0000000000..b1c1538e67 --- /dev/null +++ b/.changelog/20299.txt @@ -0,0 +1,3 @@ +```release-note:feature +v2: prevent use of the v2 experiments in secondary datacenters for now +``` diff --git a/agent/config/builder.go b/agent/config/builder.go index 307f22320b..dbb90f2ebf 100644 --- a/agent/config/builder.go +++ b/agent/config/builder.go @@ -316,8 +316,10 @@ func formatFromFileExtension(name string) string { type byName []os.FileInfo -func (a byName) Len() int { return len(a) } -func (a byName) Swap(i, j int) { a[i], a[j] = a[j], a[i] } +func (a byName) Len() int { return len(a) } + +func (a byName) Swap(i, j int) { a[i], a[j] = a[j], a[i] } + func (a byName) Less(i, j int) bool { return a[i].Name() < a[j].Name() } // build constructs the runtime configuration from the config sources @@ -1145,6 +1147,15 @@ func (b *builder) build() (rt RuntimeConfig, err error) { } } + // For now, disallow usage of several v2 experiments in secondary datacenters. + if rt.ServerMode && rt.PrimaryDatacenter != rt.Datacenter { + for _, name := range rt.Experiments { + if !consul.IsExperimentAllowedOnSecondaries(name) { + return RuntimeConfig{}, fmt.Errorf("`experiments` cannot include `%s` for servers in secondary datacenters", name) + } + } + } + if rt.UIConfig.MetricsProvider == "prometheus" { // Handle defaulting for the built-in version of prometheus. if len(rt.UIConfig.MetricsProxy.PathAllowlist) == 0 { diff --git a/agent/config/builder_test.go b/agent/config/builder_test.go index 2d7a16f9b1..9a3ed53743 100644 --- a/agent/config/builder_test.go +++ b/agent/config/builder_test.go @@ -576,6 +576,77 @@ func TestBuidler_hostMetricsWithCloud(t *testing.T) { require.True(t, cfg.Telemetry.EnableHostMetrics) } +func TestBuilder_CheckExperimentsInSecondaryDatacenters(t *testing.T) { + + type testcase struct { + hcl string + expectErr bool + } + + run := func(t *testing.T, tc testcase) { + // using dev mode skips the need for a data dir + devMode := true + builderOpts := LoadOpts{ + DevMode: &devMode, + Overrides: []Source{ + FileSource{ + Name: "overrides", + Format: "hcl", + Data: tc.hcl, + }, + }, + } + _, err := Load(builderOpts) + if tc.expectErr { + require.Error(t, err) + require.Contains(t, err.Error(), "`experiments` cannot include") + } else { + require.NoError(t, err) + } + } + + const ( + primary = `server = true primary_datacenter = "dc1" datacenter = "dc1" ` + secondary = `server = true primary_datacenter = "dc1" datacenter = "dc2" ` + ) + + cases := map[string]testcase{ + "primary server no experiments": { + hcl: primary + `experiments = []`, + }, + "primary server v2catalog": { + hcl: primary + `experiments = ["resource-apis"]`, + }, + "primary server v2dns": { + hcl: primary + `experiments = ["v2dns"]`, + }, + "primary server v2tenancy": { + hcl: primary + `experiments = ["v2tenancy"]`, + }, + "secondary server no experiments": { + hcl: secondary + `experiments = []`, + }, + "secondary server v2catalog": { + hcl: secondary + `experiments = ["resource-apis"]`, + expectErr: true, + }, + "secondary server v2dns": { + hcl: secondary + `experiments = ["v2dns"]`, + expectErr: true, + }, + "secondary server v2tenancy": { + hcl: secondary + `experiments = ["v2tenancy"]`, + expectErr: true, + }, + } + + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + run(t, tc) + }) + } +} + func TestBuilder_WarnCloudConfigWithResourceApis(t *testing.T) { tests := []struct { name string diff --git a/agent/consul/server.go b/agent/consul/server.go index 6517b15992..471e2b2c30 100644 --- a/agent/consul/server.go +++ b/agent/consul/server.go @@ -21,11 +21,6 @@ import ( "github.com/armon/go-metrics" "github.com/fullstorydev/grpchan/inprocgrpc" - "go.etcd.io/bbolt" - "golang.org/x/time/rate" - "google.golang.org/grpc" - - "github.com/hashicorp/consul-net-rpc/net/rpc" "github.com/hashicorp/go-connlimit" "github.com/hashicorp/go-hclog" "github.com/hashicorp/go-memdb" @@ -36,7 +31,11 @@ import ( walmetrics "github.com/hashicorp/raft-wal/metrics" "github.com/hashicorp/raft-wal/verifier" "github.com/hashicorp/serf/serf" + "go.etcd.io/bbolt" + "golang.org/x/time/rate" + "google.golang.org/grpc" + "github.com/hashicorp/consul-net-rpc/net/rpc" "github.com/hashicorp/consul/acl" "github.com/hashicorp/consul/agent/blockingquery" "github.com/hashicorp/consul/agent/connect" @@ -136,6 +135,19 @@ const ( HCPAllowV2ResourceAPIs = "hcp-v2-resource-apis" ) +// IsExperimentAllowedOnSecondaries returns true if an experiment is currently +// disallowed for wan federated secondary datacenters. +// +// Likely these will all be short lived exclusions. +func IsExperimentAllowedOnSecondaries(name string) bool { + switch name { + case CatalogResourceExperimentName, V2DNSExperimentName, V2TenancyExperimentName: + return false + default: + return true + } +} + const ( aclPolicyReplicationRoutineName = "ACL policy replication" aclRoleReplicationRoutineName = "ACL role replication"